Re: [PATCH 11/14] KVM: selftests: Disable "gnu-variable-sized-type-not-at-end" warning
On Mon, Dec 12, 2022, Nick Desaulniers wrote: > > diff --git a/tools/testing/selftests/kvm/Makefile > > b/tools/testing/selftests/kvm/Makefile > > index 2487db21b177..9cff99a1cb2e 100644 > > --- a/tools/testing/selftests/kvm/Makefile > > +++ b/tools/testing/selftests/kvm/Makefile > > @@ -196,6 +196,7 @@ else > > LINUX_TOOL_ARCH_INCLUDE = $(top_srcdir)/tools/arch/$(ARCH)/include > > endif > > CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \ > > + -Wno-gnu-variable-sized-type-not-at-end \ > > This is a clang-specific warning. This will need to be wrapped in a > cc-option check. Not that I'm against guarding this code, but I don't think cc-option() will do anything in this case. AFAICT, gcc stopped treating unknown "-Wno" flags as unconditional errors starting with gcc-4.4, and the kernel's min supported version is 5.1. gcc-4.4 through gcc-9.5 all print a mild warning if there's a different error, but otherwise silently ignore the uknown "-Wno". cc1: warning: unrecognized command line option '-Wno-gnu-variable-sized-type-not-at-end' gcc-10.1 is even friendlier and notes that the unknown flag may have been related to the error. cc1: note: unrecognized command-line option '-Wno-gnu-variable-sized-type-not-at-end' may have been intended to silence earlier diagnostics Because cc-option() doesn't have errors in its probing code, it will return "true" on gcc for literally any "-Wno-*" input that gcc deems syntacially valid, e.g. gcc barfs on depends on $(cc-option,-Wno-) depends on $(cc-option,-Wno) but happily succeeds with depends on $(cc-option,-Wno-lol-gcc) Various man pages suggest -Wunknown-warnings is a thing, but no gcc version supported by godbolt recognizes it. So unless I'm missing something, trying to detect lack of support will be non-trivial, and the worst case scenario is that users of older gcc version will see a potentially confusing warning when the build fails. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 01/37] KVM: x86/mmu: Store the address space ID directly in kvm_mmu_page_role
On Mon, Dec 12, 2022, Paolo Bonzini wrote: > On 12/12/22 18:39, Sean Christopherson wrote: > > My preference would be to leave .smm in x86's page role > > What about defining a byte of arch_role and a macro to build it? I think David already carved out a big chunk for arch role bits, my objection was purely to making as_id a generic 8-bit role. > > Unless I'm missing something, e.g. a need to map GPAs differently for > > SMM vs. non-SMM, SMM could have been implemented with a simple flag > > in a memslot to mark the memslot as SMM-only. > > Unfortunately not, because there can be another region (for example video > RAM at 0Ah) underneath SMRAM. Ugh, it's even a very explicitly documented "feature". When compatible SMM space is enabled, SMM-mode CBO accesses to this range route to physical system DRAM at 00_000A_0h – 00_000B_h. Non-SMM mode CBO accesses to this range are considered to be to the Video Buffer Area as described above. PCI Express* and DMI originated cycles to SMM space are not supported and are considered to be to the Video Buffer Area. I also forgot KVM supports SMBASE relocation :-( > In fact, KVM_MEM_X86_SMRAM was the first idea. It was uglier than multiple > address spaces > (https://lore.kernel.org/lkml/1431084034-8425-1-git-send-email-pbonz...@redhat.com). > In retrospect there were probably ways to mix the best of the two designs, > but it wasn't generic enough. > > > And separate address spaces become truly nasty if the CPU can access > > multiple > > protected regions, e.g. if the CPU can access type X and type Y at the same > > time, > > then there would need to be memslots for "regular", X, Y, and X+Y. > > Without a usecase that's hard to say. It's just as possible that there > would be a natural hierarchy of levels. Ah, true. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 11/14] KVM: selftests: Disable "gnu-variable-sized-type-not-at-end" warning
On Mon, Dec 12, 2022 at 4:17 PM Sean Christopherson wrote: > > Disable gnu-variable-sized-type-not-at-end so that tests and libraries > can create overlays of variable sized arrays at the end of structs when > using a fixed number of entries, e.g. to get/set a single MSR. > > It's possible to fudge around the warning, e.g. by defining a custom > struct that hardcodes the number of entries, but that is a burden for > both developers and readers of the code. > > lib/x86_64/processor.c:664:19: warning: field 'header' with variable sized > type 'struct kvm_msrs' > not at the end of a struct or class is a GNU extension > [-Wgnu-variable-sized-type-not-at-end] > struct kvm_msrs header; > ^ > lib/x86_64/processor.c:772:19: warning: field 'header' with variable sized > type 'struct kvm_msrs' > not at the end of a struct or class is a GNU extension > [-Wgnu-variable-sized-type-not-at-end] > struct kvm_msrs header; > ^ > lib/x86_64/processor.c:787:19: warning: field 'header' with variable sized > type 'struct kvm_msrs' > not at the end of a struct or class is a GNU extension > [-Wgnu-variable-sized-type-not-at-end] > struct kvm_msrs header; > ^ > 3 warnings generated. > > x86_64/hyperv_tlb_flush.c:54:18: warning: field 'hv_vp_set' with variable > sized type 'struct hv_vpset' > not at the end of a struct or class is a GNU extension > [-Wgnu-variable-sized-type-not-at-end] > struct hv_vpset hv_vp_set; > ^ > 1 warning generated. > > x86_64/xen_shinfo_test.c:137:25: warning: field 'info' with variable sized > type 'struct kvm_irq_routing' > not at the end of a struct or class is a GNU extension > [-Wgnu-variable-sized-type-not-at-end] > struct kvm_irq_routing info; >^ > 1 warning generated. > > Signed-off-by: Sean Christopherson > --- > tools/testing/selftests/kvm/Makefile | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/testing/selftests/kvm/Makefile > b/tools/testing/selftests/kvm/Makefile > index 2487db21b177..9cff99a1cb2e 100644 > --- a/tools/testing/selftests/kvm/Makefile > +++ b/tools/testing/selftests/kvm/Makefile > @@ -196,6 +196,7 @@ else > LINUX_TOOL_ARCH_INCLUDE = $(top_srcdir)/tools/arch/$(ARCH)/include > endif > CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \ > + -Wno-gnu-variable-sized-type-not-at-end \ This is a clang-specific warning. This will need to be wrapped in a cc-option check. tools/build/Build.include seems to redefine that make macro, so be sure to test it first. > -fno-builtin-memcmp -fno-builtin-memcpy -fno-builtin-memset \ > -fno-stack-protector -fno-PIE -I$(LINUX_TOOL_INCLUDE) \ > -I$(LINUX_TOOL_ARCH_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude \ > -- > 2.39.0.rc1.256.g54fd8350bd-goog > > -- Thanks, ~Nick Desaulniers ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 01/37] KVM: x86/mmu: Store the address space ID directly in kvm_mmu_page_role
On Mon, Dec 12, 2022 at 11:50:29PM +0100, Paolo Bonzini wrote: > On 12/12/22 18:39, Sean Christopherson wrote: > > > The notion of address spaces is already existing architecture-neutral > > > concept in KVM (e.g. see uses of KVM_ADDRESS_SPACE_NUM in > > > virt/kvm/kvm_main.c), although SMM is the only use-case I'm aware of. > > > > Yes, SMM is currently the only use-case. > > It's possible that in the future Hyper-V VTLs will also have per-level > protections. It wouldn't use as_id, but it would likely be recorded in the > upper byte of the role. > > I'm not sure if Microsoft intends to port those to ARM as well. > > > My preference would be to leave .smm in x86's page role > > What about defining a byte of arch_role and a macro to build it? Both would work. I went with as_id in the common role since that's how it's encoded in kvm_memory_slot and because, not matter what, the TDP MMU still has to handle multiple address spaces. i.e. Even if we hide SMM away in the role, the TDP MMU still has to access it with some wrapper e.g. kvm_mmu_page_as_id() (that would just return 0 outside of x86). From that perspective, just having as_id directly in the common role seemed like the cleanest option. The only way to truly shield the TDP MMU from SMM would be to disallow it. e.g. Disable the TDP MMU if defined(CONFIG_KVM_SMM), or something similar. But I don't know enough about how KVM SMM support is used to say if that's even worth entertaining. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 01/37] KVM: x86/mmu: Store the address space ID directly in kvm_mmu_page_role
On Mon, Dec 12, 2022 at 06:17:36PM +, Oliver Upton wrote: > On Mon, Dec 12, 2022 at 05:39:38PM +, Sean Christopherson wrote: > > On Fri, Dec 09, 2022, David Matlack wrote: > > > On Fri, Dec 9, 2022 at 9:25 AM Oliver Upton > > > wrote: > > My preference would be to leave .smm in x86's page role. IMO, defining > > multiple > > address spaces to support SMM emulation was a mistake that should be > > contained to > > SMM, i.e. should never be used for any other feature. And with > > CONFIG_KVM_SMM, > > even x86 can opt out. > > +1 > > I don't think something is architecture-neutral by virtue of it existing > in virt/kvm/*. Put another way, just because something exists in virt/kvm/* doesn't mean it is used (or will be useful) to more than one architecture. Totally agree. In this case, there never turned out to be any other usecases for memslot address spaces. As for role.arch.smm vs role.as_id, I'll post my response on the other thread with Paolo. Juggling these threads is hard. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 02/37] KVM: MMU: Move struct kvm_mmu_page_role into common code
On Mon, Dec 12, 2022 at 3:11 PM Paolo Bonzini wrote:
>
> On 12/8/22 20:38, David Matlack wrote:
> > +/*
> > + * kvm_mmu_page_role tracks the properties of a shadow page (where shadow
> > page
> > + * also includes TDP pages) to determine whether or not a page can be used
> > in
> > + * the given MMU context.
> > + */
> > +union kvm_mmu_page_role {
> > + u32 word;
> > + struct {
> > + struct {
> > + /* The address space ID mapped by the page. */
> > + u16 as_id:8;
> > +
> > + /* The level of the page in the page table hierarchy.
> > */
> > + u16 level:4;
> > +
> > + /* Whether the page is invalid, i.e. pending
> > destruction. */
> > + u16 invalid:1;
> > + };
> > +
> > + /* Architecture-specific properties. */
> > + struct kvm_mmu_page_role_arch arch;
> > + };
> > +};
> > +
>
> Have you considered adding a tdp_mmu:1 field to the arch-independent
> part? I think that as long as _that_ field is the same, there's no need
> to have any overlap between TDP MMU and shadow MMU roles.
>
> I'm not even sure if the x86 TDP MMU needs _any_ other role bit. It
> needs of course the above three, and it also needs "direct" but it is
> used exactly to mean "is this a TDP MMU page". So we could have
>
> union {
> struct {
> u32 tdp_mmu:1;
> u32 invalid:1;
> u32 :6;
> u32 level:8;
> u32 arch:8;
> u32 :8;
> } tdp;
> /* the first field must be "u32 tdp_mmu:1;" */
> struct kvm_mmu_page_role_arch shadow;
We could but then that prevents having common fields between the
Shadow MMU and TDP MMU. For example, make_spte() and
make_huge_page_split_spte() use sp->role.level regardless of TDP or
Shadow MMU, and is_obsolete_sp() uses sp->role.invalid. Plus then you
need the `arch:8` byte for SMM.
It's possible to make it work, but I don't see what the benefit would be.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 20/37] KVM: x86/mmu: Abstract away computing the max mapping level
On Mon, Dec 12, 2022, David Matlack wrote: > On Mon, Dec 12, 2022 at 11:32 AM Ben Gardon wrote: > > > > On Thu, Dec 8, 2022 at 11:39 AM David Matlack wrote: > > > > > > Abstract away kvm_mmu_max_mapping_level(), which is an x86-specific > > > function for computing the max level that a given GFN can be mapped in > > > KVM's page tables. This will be used in a future commit to enable moving > > > the TDP MMU to common code. > > > > > > Provide a default implementation for non-x86 architectures that just > > > returns the max level. This will result in more zapping than necessary > > > when disabling dirty logging (i.e. less than optimal performance) but no > > > correctness issues. > > > > Apologies if you already implemented it in a later patch in this > > series, but would it not at least be possible to port > > host_pfn_mapping_level to common code and check that? > > I'm assuming, though I could be wrong, that all archs map GFNs with at > > most a host page table granularity mapping. > > I suppose that doesn't strictly need to be included in this series, > > but it would be worth addressing in the commit description. > > It's not implemented later in this series, but I agree it's something > we should do. In fact, it's worth doing regardless of this series as a > way to share more code across architectures (e.g. KVM/ARM has it's own > version in arch/arm64/kvm/mmu.c:get_user_mapping_size()). Ya, ARM converted to walking the host user page tables largely in response to x86's conversion. After x86 switched, ARM was left holding the bag that was PageTransCompoundMap(). On a related topic, I'm guessing all the comments in transparent_hugepage_adjust() about the code working _only_ for THP are stale. Unless ARM support for HugeTLB works differently, walking host user page tables should Just Work for all hugepage types. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 13/14] KVM: selftests: Use wildcards to find targets and test source files
Use $(wildcard ...) to find the test targets (.sh files) and test source files (.c and .S) instead of manually adding files/targets for every architecture, which is a maintenance burden and error prone, e.g. RISC-V supports RSEQ but doesn't build the test for reasons unknown. To deal with common tests that are only supported on a subset of architectures, add a dummy macro TEST_UNSUPPORTED() that tests can use to declare the test as unsupported for a given architecture, and filter out unsupported tests for the target architecture via grep. As a bonus, explicitly naming unsupported architectures will also force developers to opt-out of architectures (or opt-out of tests when adding a new arch). Signed-off-by: Sean Christopherson --- tools/testing/selftests/kvm/Makefile | 128 +- .../selftests/kvm/access_tracking_perf_test.c | 3 + .../selftests/kvm/dirty_log_perf_test.c | 3 + .../selftests/kvm/hardware_disable_test.c | 4 + .../testing/selftests/kvm/include/test_util.h | 11 ++ .../selftests/kvm/max_guest_memory_test.c | 4 + .../kvm/memslot_modification_stress_test.c| 3 + .../testing/selftests/kvm/memslot_perf_test.c | 3 + tools/testing/selftests/kvm/rseq_test.c | 2 + tools/testing/selftests/kvm/steal_time.c | 3 + .../kvm/system_counter_offset_test.c | 4 + 11 files changed, 46 insertions(+), 122 deletions(-) diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile index a9930e9197da..76382850a28f 100644 --- a/tools/testing/selftests/kvm/Makefile +++ b/tools/testing/selftests/kvm/Makefile @@ -19,130 +19,14 @@ else $(error Unknown architecture '$(ARCH)') endif -# Non-compiled test targets -TEST_PROGS_x86_64 += x86_64/nx_huge_pages_test.sh +TESTS_SRC := $(shell grep -L TEST_UNSUPPORTED\($(ARCH_DIR)\) *.c) +TESTS_SRC += $(wildcard $(ARCH_DIR)/*.c) -# Compiled test targets -TEST_GEN_PROGS_x86_64 = x86_64/cpuid_test -TEST_GEN_PROGS_x86_64 += x86_64/cr4_cpuid_sync_test -TEST_GEN_PROGS_x86_64 += x86_64/get_msr_index_features -TEST_GEN_PROGS_x86_64 += x86_64/exit_on_emulation_failure_test -TEST_GEN_PROGS_x86_64 += x86_64/fix_hypercall_test -TEST_GEN_PROGS_x86_64 += x86_64/hyperv_clock -TEST_GEN_PROGS_x86_64 += x86_64/hyperv_cpuid -TEST_GEN_PROGS_x86_64 += x86_64/hyperv_evmcs -TEST_GEN_PROGS_x86_64 += x86_64/hyperv_features -TEST_GEN_PROGS_x86_64 += x86_64/hyperv_ipi -TEST_GEN_PROGS_x86_64 += x86_64/hyperv_svm_test -TEST_GEN_PROGS_x86_64 += x86_64/hyperv_tlb_flush -TEST_GEN_PROGS_x86_64 += x86_64/kvm_clock_test -TEST_GEN_PROGS_x86_64 += x86_64/kvm_pv_test -TEST_GEN_PROGS_x86_64 += x86_64/mmio_warning_test -TEST_GEN_PROGS_x86_64 += x86_64/monitor_mwait_test -TEST_GEN_PROGS_x86_64 += x86_64/nested_exceptions_test -TEST_GEN_PROGS_x86_64 += x86_64/platform_info_test -TEST_GEN_PROGS_x86_64 += x86_64/pmu_event_filter_test -TEST_GEN_PROGS_x86_64 += x86_64/set_boot_cpu_id -TEST_GEN_PROGS_x86_64 += x86_64/set_sregs_test -TEST_GEN_PROGS_x86_64 += x86_64/smaller_maxphyaddr_emulation_test -TEST_GEN_PROGS_x86_64 += x86_64/smm_test -TEST_GEN_PROGS_x86_64 += x86_64/state_test -TEST_GEN_PROGS_x86_64 += x86_64/vmx_preemption_timer_test -TEST_GEN_PROGS_x86_64 += x86_64/svm_vmcall_test -TEST_GEN_PROGS_x86_64 += x86_64/svm_int_ctl_test -TEST_GEN_PROGS_x86_64 += x86_64/svm_nested_shutdown_test -TEST_GEN_PROGS_x86_64 += x86_64/svm_nested_soft_inject_test -TEST_GEN_PROGS_x86_64 += x86_64/tsc_scaling_sync -TEST_GEN_PROGS_x86_64 += x86_64/sync_regs_test -TEST_GEN_PROGS_x86_64 += x86_64/ucna_injection_test -TEST_GEN_PROGS_x86_64 += x86_64/userspace_io_test -TEST_GEN_PROGS_x86_64 += x86_64/userspace_msr_exit_test -TEST_GEN_PROGS_x86_64 += x86_64/vmx_apic_access_test -TEST_GEN_PROGS_x86_64 += x86_64/vmx_close_while_nested_test -TEST_GEN_PROGS_x86_64 += x86_64/vmx_dirty_log_test -TEST_GEN_PROGS_x86_64 += x86_64/vmx_exception_with_invalid_guest_state -TEST_GEN_PROGS_x86_64 += x86_64/vmx_msrs_test -TEST_GEN_PROGS_x86_64 += x86_64/vmx_invalid_nested_guest_state -TEST_GEN_PROGS_x86_64 += x86_64/vmx_set_nested_state_test -TEST_GEN_PROGS_x86_64 += x86_64/vmx_tsc_adjust_test -TEST_GEN_PROGS_x86_64 += x86_64/vmx_nested_tsc_scaling_test -TEST_GEN_PROGS_x86_64 += x86_64/xapic_ipi_test -TEST_GEN_PROGS_x86_64 += x86_64/xapic_state_test -TEST_GEN_PROGS_x86_64 += x86_64/xss_msr_test -TEST_GEN_PROGS_x86_64 += x86_64/debug_regs -TEST_GEN_PROGS_x86_64 += x86_64/tsc_msrs_test -TEST_GEN_PROGS_x86_64 += x86_64/vmx_pmu_caps_test -TEST_GEN_PROGS_x86_64 += x86_64/xen_shinfo_test -TEST_GEN_PROGS_x86_64 += x86_64/xen_vmcall_test -TEST_GEN_PROGS_x86_64 += x86_64/sev_migrate_tests -TEST_GEN_PROGS_x86_64 += x86_64/amx_test -TEST_GEN_PROGS_x86_64 += x86_64/max_vcpuid_cap_test -TEST_GEN_PROGS_x86_64 += x86_64/triple_fault_event_test -TEST_GEN_PROGS_x86_64 += access_tracking_perf_test -TEST_GEN_PROGS_x86_64 += demand_paging_test -TEST_GEN_PROGS_x86_64 += dirty_log_test -TEST_GEN_PROGS_x86_64 += dirty_log_perf_test -TEST_GEN_PROGS_x86_64 += hardw
[PATCH 14/14] KVM: selftests: Enable RSEQ test for RISC-V
Enable the RSEQ test for RISC-V, which according to HAVE_RSEQ is supported by the kernel and thus should be tested. The RSEQ test was added shortly before RISC-V selftests support landed, i.e. was likely overlooked during merging. Note, the RSEQ test currently doesn't compile with clang due to an issue in the base RSEQ test code. Given that clang is constantly broken for KVM selftests, enable the RSEQ test and deal with its broken clang state in a separate commit/series. In file included from rseq_test.c:23: In file included from ./../rseq/rseq.c:33: In file included from ../rseq/rseq.h:97: ../rseq/rseq-riscv.h:657:17: error: invalid input constraint 'er' in asm [off] "er" (off), ^ Signed-off-by: Sean Christopherson --- tools/testing/selftests/kvm/rseq_test.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c index 34c3df9b4e81..3045fdf9bdf5 100644 --- a/tools/testing/selftests/kvm/rseq_test.c +++ b/tools/testing/selftests/kvm/rseq_test.c @@ -22,8 +22,6 @@ #include "../rseq/rseq.c" -TEST_UNSUPPORTED(riscv); - /* * Any bug related to task migration is likely to be timing-dependent; perform * a large number of migrations to reduce the odds of a false negative. -- 2.39.0.rc1.256.g54fd8350bd-goog ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 12/14] KVM: selftests: Use wildcards to find library source files
Use $(wildcard ...) to find the library source files instead of manually
defining the inputs, which is a maintenance burden and error prone.
Signed-off-by: Sean Christopherson
---
tools/testing/selftests/kvm/Makefile | 45
1 file changed, 5 insertions(+), 40 deletions(-)
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index 9cff99a1cb2e..a9930e9197da 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -19,44 +19,6 @@ else
$(error Unknown architecture '$(ARCH)')
endif
-LIBKVM += lib/assert.c
-LIBKVM += lib/elf.c
-LIBKVM += lib/guest_modes.c
-LIBKVM += lib/io.c
-LIBKVM += lib/kvm_util.c
-LIBKVM += lib/memstress.c
-LIBKVM += lib/rbtree.c
-LIBKVM += lib/sparsebit.c
-LIBKVM += lib/test_util.c
-LIBKVM += lib/ucall_common.c
-LIBKVM += lib/userfaultfd_util.c
-
-LIBKVM_STRING += lib/string_override.c
-
-LIBKVM_x86_64 += lib/x86_64/apic.c
-LIBKVM_x86_64 += lib/x86_64/handlers.S
-LIBKVM_x86_64 += lib/x86_64/hyperv.c
-LIBKVM_x86_64 += lib/x86_64/memstress.c
-LIBKVM_x86_64 += lib/x86_64/processor.c
-LIBKVM_x86_64 += lib/x86_64/svm.c
-LIBKVM_x86_64 += lib/x86_64/ucall.c
-LIBKVM_x86_64 += lib/x86_64/vmx.c
-
-LIBKVM_aarch64 += lib/aarch64/gic.c
-LIBKVM_aarch64 += lib/aarch64/gic_v3.c
-LIBKVM_aarch64 += lib/aarch64/handlers.S
-LIBKVM_aarch64 += lib/aarch64/processor.c
-LIBKVM_aarch64 += lib/aarch64/spinlock.c
-LIBKVM_aarch64 += lib/aarch64/ucall.c
-LIBKVM_aarch64 += lib/aarch64/vgic.c
-
-LIBKVM_s390x += lib/s390x/diag318_test_handler.c
-LIBKVM_s390x += lib/s390x/processor.c
-LIBKVM_s390x += lib/s390x/ucall.c
-
-LIBKVM_riscv += lib/riscv/processor.c
-LIBKVM_riscv += lib/riscv/ucall.c
-
# Non-compiled test targets
TEST_PROGS_x86_64 += x86_64/nx_huge_pages_test.sh
@@ -213,10 +175,13 @@ pgste-option = $(call try-run, echo 'int main(void) {
return 0; }' | \
LDLIBS += -ldl
LDFLAGS += -pthread $(no-pie-option) $(pgste-option)
-LIBKVM_C := $(filter %.c,$(LIBKVM))
-LIBKVM_S := $(filter %.S,$(LIBKVM))
+LIBKVM_C := $(filter-out lib/string_override.c,$(wildcard lib/*.c))
+LIBKVM_C += $(wildcard lib/$(ARCH_DIR)/*.c)
+LIBKVM_S := $(wildcard lib/*.S)
+LIBKVM_S += $(wildcard lib/$(ARCH_DIR)/*.S)
LIBKVM_C_OBJ := $(patsubst %.c, $(OUTPUT)/%.o, $(LIBKVM_C))
LIBKVM_S_OBJ := $(patsubst %.S, $(OUTPUT)/%.o, $(LIBKVM_S))
+LIBKVM_STRING := lib/string_override.c
LIBKVM_STRING_OBJ := $(patsubst %.c, $(OUTPUT)/%.o, $(LIBKVM_STRING))
LIBKVM_OBJS = $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ) $(LIBKVM_STRING_OBJ)
--
2.39.0.rc1.256.g54fd8350bd-goog
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 11/14] KVM: selftests: Disable "gnu-variable-sized-type-not-at-end" warning
Disable gnu-variable-sized-type-not-at-end so that tests and libraries can create overlays of variable sized arrays at the end of structs when using a fixed number of entries, e.g. to get/set a single MSR. It's possible to fudge around the warning, e.g. by defining a custom struct that hardcodes the number of entries, but that is a burden for both developers and readers of the code. lib/x86_64/processor.c:664:19: warning: field 'header' with variable sized type 'struct kvm_msrs' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end] struct kvm_msrs header; ^ lib/x86_64/processor.c:772:19: warning: field 'header' with variable sized type 'struct kvm_msrs' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end] struct kvm_msrs header; ^ lib/x86_64/processor.c:787:19: warning: field 'header' with variable sized type 'struct kvm_msrs' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end] struct kvm_msrs header; ^ 3 warnings generated. x86_64/hyperv_tlb_flush.c:54:18: warning: field 'hv_vp_set' with variable sized type 'struct hv_vpset' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end] struct hv_vpset hv_vp_set; ^ 1 warning generated. x86_64/xen_shinfo_test.c:137:25: warning: field 'info' with variable sized type 'struct kvm_irq_routing' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end] struct kvm_irq_routing info; ^ 1 warning generated. Signed-off-by: Sean Christopherson --- tools/testing/selftests/kvm/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile index 2487db21b177..9cff99a1cb2e 100644 --- a/tools/testing/selftests/kvm/Makefile +++ b/tools/testing/selftests/kvm/Makefile @@ -196,6 +196,7 @@ else LINUX_TOOL_ARCH_INCLUDE = $(top_srcdir)/tools/arch/$(ARCH)/include endif CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \ + -Wno-gnu-variable-sized-type-not-at-end \ -fno-builtin-memcmp -fno-builtin-memcpy -fno-builtin-memset \ -fno-stack-protector -fno-PIE -I$(LINUX_TOOL_INCLUDE) \ -I$(LINUX_TOOL_ARCH_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude \ -- 2.39.0.rc1.256.g54fd8350bd-goog ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 10/14] KVM: selftests: Include lib.mk before consuming $(CC)
Include lib.mk before consuming $(CC) and document that lib.mk overwrites
$(CC) unless make was invoked with -e or $(CC) was specified after make
(which apparently makes the environment override the Makefile?!?!).
Including lib.mk after using it for probing, e.g. for -no-pie, can lead
to weirdness.
Signed-off-by: Sean Christopherson
---
tools/testing/selftests/kvm/Makefile | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index 6594ed51eeea..2487db21b177 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -182,6 +182,11 @@ TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(ARCH_DIR))
TEST_GEN_PROGS_EXTENDED += $(TEST_GEN_PROGS_EXTENDED_$(ARCH_DIR))
LIBKVM += $(LIBKVM_$(ARCH_DIR))
+# lib.mak defines $(OUTPUT), prepends $(OUTPUT)/ to $(TEST_GEN_PROGS), and most
+# importantly defines, i.e. overwrites, $(CC) (unless `make -e` or `make CC=`,
+# which causes the environment variable to override the makefile).
+include ../lib.mk
+
INSTALL_HDR_PATH = $(top_srcdir)/usr
LINUX_HDR_PATH = $(INSTALL_HDR_PATH)/include/
LINUX_TOOL_INCLUDE = $(top_srcdir)/tools/include
@@ -207,10 +212,6 @@ pgste-option = $(call try-run, echo 'int main(void) {
return 0; }' | \
LDLIBS += -ldl
LDFLAGS += -pthread $(no-pie-option) $(pgste-option)
-# After inclusion, $(OUTPUT) is defined and
-# $(TEST_GEN_PROGS) starts with $(OUTPUT)/
-include ../lib.mk
-
LIBKVM_C := $(filter %.c,$(LIBKVM))
LIBKVM_S := $(filter %.S,$(LIBKVM))
LIBKVM_C_OBJ := $(patsubst %.c, $(OUTPUT)/%.o, $(LIBKVM_C))
--
2.39.0.rc1.256.g54fd8350bd-goog
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 06/14] KVM: selftests: Rename UNAME_M to ARCH_DIR, fill explicitly for x86
Rename UNAME_M to ARCH_DIR and explicitly set it directly for x86. At this point, the name of the arch directory really doesn't have anything to do with `uname -m`, and UNAME_M is unnecessarily confusing given that its purpose is purely to identify the arch specific directory. Signed-off-by: Sean Christopherson --- tools/testing/selftests/kvm/Makefile | 49 +--- 1 file changed, 15 insertions(+), 34 deletions(-) diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile index 947676983da1..d761a77c3a80 100644 --- a/tools/testing/selftests/kvm/Makefile +++ b/tools/testing/selftests/kvm/Makefile @@ -7,35 +7,16 @@ top_srcdir = ../../../.. include $(top_srcdir)/scripts/subarch.include ARCH?= $(SUBARCH) -# For cross-builds to work, UNAME_M has to map to ARCH and arch specific -# directories and targets in this Makefile. "uname -m" doesn't map to -# arch specific sub-directory names. -# -# UNAME_M variable to used to run the compiles pointing to the right arch -# directories and build the right targets for these supported architectures. -# -# TEST_GEN_PROGS and LIBKVM are set using UNAME_M variable. -# LINUX_TOOL_ARCH_INCLUDE is set using ARCH variable. -# -# x86_64 targets are named to include x86_64 as a suffix and directories -# for includes are in x86_64 sub-directory. s390x and aarch64 follow the -# same convention. "uname -m" doesn't result in the correct mapping for -# s390x and aarch64. -# -# No change necessary for x86_64 -UNAME_M := $(shell uname -m) - -# Set UNAME_M for arm64 compile/install to work -ifeq ($(ARCH),arm64) - UNAME_M := aarch64 -endif -# Set UNAME_M s390x compile/install to work -ifeq ($(ARCH),s390) - UNAME_M := s390x -endif -# Set UNAME_M riscv compile/install to work -ifeq ($(ARCH),riscv) - UNAME_M := riscv +ifeq ($(ARCH),x86) + ARCH_DIR := x86_64 +else ifeq ($(ARCH),arm64) + ARCH_DIR := aarch64 +else ifeq ($(ARCH),s390) + ARCH_DIR := s390x +else ifeq ($(ARCH),riscv) + ARCH_DIR := riscv +else +$(error Unknown architecture '$(ARCH)') endif LIBKVM += lib/assert.c @@ -196,10 +177,10 @@ TEST_GEN_PROGS_riscv += kvm_page_table_test TEST_GEN_PROGS_riscv += set_memory_region_test TEST_GEN_PROGS_riscv += kvm_binary_stats_test -TEST_PROGS += $(TEST_PROGS_$(UNAME_M)) -TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M)) -TEST_GEN_PROGS_EXTENDED += $(TEST_GEN_PROGS_EXTENDED_$(UNAME_M)) -LIBKVM += $(LIBKVM_$(UNAME_M)) +TEST_PROGS += $(TEST_PROGS_$(ARCH_DIR)) +TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(ARCH_DIR)) +TEST_GEN_PROGS_EXTENDED += $(TEST_GEN_PROGS_EXTENDED_$(ARCH_DIR)) +LIBKVM += $(LIBKVM_$(ARCH_DIR)) INSTALL_HDR_PATH = $(top_srcdir)/usr LINUX_HDR_PATH = $(INSTALL_HDR_PATH)/include/ @@ -212,7 +193,7 @@ endif CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \ -fno-stack-protector -fno-PIE -I$(LINUX_TOOL_INCLUDE) \ -I$(LINUX_TOOL_ARCH_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude \ - -I$(https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 09/14] KVM: selftests: Explicitly disable builtins for mem*() overrides
Explicitly disable the compiler's builtin memcmp(), memcpy(), and
memset(). Because only lib/string_override.c is built with -ffreestanding,
the compiler reserves the right to do what it wants and can try to link the
non-freestanding code to its own crud.
/usr/bin/x86_64-linux-gnu-ld: /lib/x86_64-linux-gnu/libc.a(memcmp.o): in
function `memcmp_ifunc':
(.text+0x0): multiple definition of `memcmp';
tools/testing/selftests/kvm/lib/string_override.o:
tools/testing/selftests/kvm/lib/string_override.c:15: first defined here
clang: error: linker command failed with exit code 1 (use -v to see
invocation)
Fixes: 6b6f71484bf4 ("KVM: selftests: Implement memcmp(), memcpy(), and
memset() for guest use")
Reported-by: Aaron Lewis
Reported-by: Raghavendra Rao Ananta
Signed-off-by: Sean Christopherson
---
tools/testing/selftests/kvm/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index a6050dcc381a..6594ed51eeea 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -191,6 +191,7 @@ else
LINUX_TOOL_ARCH_INCLUDE = $(top_srcdir)/tools/arch/$(ARCH)/include
endif
CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \
+ -fno-builtin-memcmp -fno-builtin-memcpy -fno-builtin-memset \
-fno-stack-protector -fno-PIE -I$(LINUX_TOOL_INCLUDE) \
-I$(LINUX_TOOL_ARCH_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude \
-I$(https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 05/14] KVM: selftests: Fix a typo in x86-64's kvm_get_cpu_address_width()
Fix a == vs. = typo in kvm_get_cpu_address_width() that results in
@pa_bits being left unset if the CPU doesn't support enumerating its
MAX_PHY_ADDR. Flagged by clang's unusued-value warning.
lib/x86_64/processor.c:1034:51: warning: expression result unused
[-Wunused-value]
*pa_bits == kvm_cpu_has(X86_FEATURE_PAE) ? 36 : 32;
Fixes: 3bd396353d18 ("KVM: selftests: Add X86_FEATURE_PAE and use it calc
"fallback" MAXPHYADDR")
Signed-off-by: Sean Christopherson
---
tools/testing/selftests/kvm/lib/x86_64/processor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c
b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index c4d368d56cfe..acfa1d01e7df 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -1031,7 +1031,7 @@ bool is_amd_cpu(void)
void kvm_get_cpu_address_width(unsigned int *pa_bits, unsigned int *va_bits)
{
if (!kvm_cpu_has_p(X86_PROPERTY_MAX_PHY_ADDR)) {
- *pa_bits == kvm_cpu_has(X86_FEATURE_PAE) ? 36 : 32;
+ *pa_bits = kvm_cpu_has(X86_FEATURE_PAE) ? 36 : 32;
*va_bits = 32;
} else {
*pa_bits = kvm_cpu_property(X86_PROPERTY_MAX_PHY_ADDR);
--
2.39.0.rc1.256.g54fd8350bd-goog
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 04/14] KVM: selftests: Use pattern matching in .gitignore
Use pattern matching to exclude everything except .c, .h, .S, and .sh files from Git. Manually adding every test target has an absurd maintenance cost, is comically error prone, and leads to bikeshedding over whether or not the targets should be listed in alphabetical order. Deliberately do not include the one-off assets, e.g. config, settings, .gitignore itself, etc as Git doesn't ignore files that are already in the repository. Adding the one-off assets won't prevent mistakes where developers forget to --force add files that don't match the "allowed". Signed-off-by: Sean Christopherson --- tools/testing/selftests/kvm/.gitignore | 91 ++ 1 file changed, 6 insertions(+), 85 deletions(-) diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore index 6ce8c488d62e..6d9381d60172 100644 --- a/tools/testing/selftests/kvm/.gitignore +++ b/tools/testing/selftests/kvm/.gitignore @@ -1,86 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-only -/aarch64/aarch32_id_regs -/aarch64/arch_timer -/aarch64/debug-exceptions -/aarch64/get-reg-list -/aarch64/hypercalls -/aarch64/page_fault_test -/aarch64/psci_test -/aarch64/vcpu_width_config -/aarch64/vgic_init -/aarch64/vgic_irq -/s390x/memop -/s390x/resets -/s390x/sync_regs_test -/s390x/tprot -/x86_64/amx_test -/x86_64/cpuid_test -/x86_64/cr4_cpuid_sync_test -/x86_64/debug_regs -/x86_64/exit_on_emulation_failure_test -/x86_64/fix_hypercall_test -/x86_64/get_msr_index_features -/x86_64/kvm_clock_test -/x86_64/kvm_pv_test -/x86_64/hyperv_clock -/x86_64/hyperv_cpuid -/x86_64/hyperv_evmcs -/x86_64/hyperv_features -/x86_64/hyperv_ipi -/x86_64/hyperv_svm_test -/x86_64/hyperv_tlb_flush -/x86_64/max_vcpuid_cap_test -/x86_64/mmio_warning_test -/x86_64/monitor_mwait_test -/x86_64/nested_exceptions_test -/x86_64/nx_huge_pages_test -/x86_64/platform_info_test -/x86_64/pmu_event_filter_test -/x86_64/set_boot_cpu_id -/x86_64/set_sregs_test -/x86_64/sev_migrate_tests -/x86_64/smaller_maxphyaddr_emulation_test -/x86_64/smm_test -/x86_64/state_test -/x86_64/svm_vmcall_test -/x86_64/svm_int_ctl_test -/x86_64/svm_nested_soft_inject_test -/x86_64/svm_nested_shutdown_test -/x86_64/sync_regs_test -/x86_64/tsc_msrs_test -/x86_64/tsc_scaling_sync -/x86_64/ucna_injection_test -/x86_64/userspace_io_test -/x86_64/userspace_msr_exit_test -/x86_64/vmx_apic_access_test -/x86_64/vmx_close_while_nested_test -/x86_64/vmx_dirty_log_test -/x86_64/vmx_exception_with_invalid_guest_state -/x86_64/vmx_invalid_nested_guest_state -/x86_64/vmx_msrs_test -/x86_64/vmx_preemption_timer_test -/x86_64/vmx_set_nested_state_test -/x86_64/vmx_tsc_adjust_test -/x86_64/vmx_nested_tsc_scaling_test -/x86_64/xapic_ipi_test -/x86_64/xapic_state_test -/x86_64/xen_shinfo_test -/x86_64/xen_vmcall_test -/x86_64/xss_msr_test -/x86_64/vmx_pmu_caps_test -/x86_64/triple_fault_event_test -/access_tracking_perf_test -/demand_paging_test -/dirty_log_test -/dirty_log_perf_test -/hardware_disable_test -/kvm_create_max_vcpus -/kvm_page_table_test -/max_guest_memory_test -/memslot_modification_stress_test -/memslot_perf_test -/rseq_test -/set_memory_region_test -/steal_time -/kvm_binary_stats_test -/system_counter_offset_test +* +!/**/ +!*.c +!*.h +!*.S +!*.sh -- 2.39.0.rc1.256.g54fd8350bd-goog ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 08/14] KVM: selftests: Probe -no-pie with actual CFLAGS used to compile
Probe -no-pie with the actual set of CFLAGS used to compile the tests,
clang whines about -no-pie being unused if the tests are compiled with
-static.
clang: warning: argument unused during compilation: '-no-pie'
[-Wunused-command-line-argument]
Signed-off-by: Sean Christopherson
---
tools/testing/selftests/kvm/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index c22c3002492d..a6050dcc381a 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -197,7 +197,7 @@ CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g
-std=gnu99 \
$(KHDR_INCLUDES)
no-pie-option := $(call try-run, echo 'int main(void) { return 0; }' | \
-$(CC) -Werror -no-pie -x c - -o "$$TMP", -no-pie)
+$(CC) -Werror $(CFLAGS) -no-pie -x c - -o "$$TMP", -no-pie)
# On s390, build the testcases KVM-enabled
pgste-option = $(call try-run, echo 'int main(void) { return 0; }' | \
--
2.39.0.rc1.256.g54fd8350bd-goog
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 07/14] KVM: selftests: Use proper function prototypes in probing code
Make the main() functions in the probing code proper prototypes so that compiling the probing code with more strict flags won't generate false negatives. :1:5: error: function declaration isn’t a prototype [-Werror=strict-prototypes] Signed-off-by: Sean Christopherson --- tools/testing/selftests/kvm/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile index d761a77c3a80..c22c3002492d 100644 --- a/tools/testing/selftests/kvm/Makefile +++ b/tools/testing/selftests/kvm/Makefile @@ -196,11 +196,11 @@ CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \ -I$(https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 03/14] KVM: selftests: Fix divide-by-zero bug in memslot_perf_test
Check that the number of pages per slot is non-zero in get_max_slots()
prior to computing the remaining number of pages. clang generates code
that uses an actual DIV for calculating the remaining, which causes a #DE
if the total number of pages is less than the number of slots.
traps: memslot_perf_te[97611] trap divide error ip:4030c4 sp:7ffd18ae58f0
error:0 in memslot_perf_test[401000+cb000]
Fixes: a69170c65acd ("KVM: selftests: memslot_perf_test: Report optimal memory
slots")
Signed-off-by: Sean Christopherson
---
tools/testing/selftests/kvm/memslot_perf_test.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/kvm/memslot_perf_test.c
b/tools/testing/selftests/kvm/memslot_perf_test.c
index e698306bf49d..e6587e193490 100644
--- a/tools/testing/selftests/kvm/memslot_perf_test.c
+++ b/tools/testing/selftests/kvm/memslot_perf_test.c
@@ -265,6 +265,9 @@ static uint64_t get_max_slots(struct vm_data *data,
uint32_t host_page_size)
slots = data->nslots;
while (--slots > 1) {
pages_per_slot = mempages / slots;
+ if (!pages_per_slot)
+ continue;
+
rempages = mempages % pages_per_slot;
if (check_slot_pages(host_page_size, guest_page_size,
pages_per_slot, rempages))
--
2.39.0.rc1.256.g54fd8350bd-goog
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 02/14] KVM: selftests: Delete dead code in x86_64/vmx_tsc_adjust_test.c
Delete an unused struct definition in x86_64/vmx_tsc_adjust_test.c.
Signed-off-by: Sean Christopherson
---
tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
b/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
index 5943187e8594..ff8ecdf32ae0 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
@@ -49,11 +49,6 @@ enum {
NUM_VMX_PAGES,
};
-struct kvm_single_msr {
- struct kvm_msrs header;
- struct kvm_msr_entry entry;
-} __attribute__((packed));
-
/* The virtual machine object. */
static struct kvm_vm *vm;
--
2.39.0.rc1.256.g54fd8350bd-goog
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 00/14] KVM: selftests: Clang fixes, Makefile cleanup
Fix a variety of KVM selftests issues exposed by clang, and rework the
Makefile and .gitignore to reduce the maintenance burden of selftests.
For the Makefile, programmatically generate the list of targets by
looking for .c files, and opt-out via a dummy macro in the source
instead of forcing architectures to opt-in. The opt-out approach is
less error prone (harder to forget to add an arch), doesn't generate
unnecessary conflicts if multiple tests are added simultanesouly, and
makes it much easier to understand which tests aren't supported, e.g.
$ git grep TEST_UNSUPPORTED | grep aarch64
hardware_disable_test.c:TEST_UNSUPPORTED(aarch64);
max_guest_memory_test.c:TEST_UNSUPPORTED(aarch64);
system_counter_offset_test.c:TEST_UNSUPPORTED(aarch64);
This all started when trying to reproduce clang build errors reported by
Raghu and Aaron that were introduced by commit 6b6f71484bf4 ("KVM:
selftests: Implement memcmp(), memcpy(), and memset() for guest use").
Just getting selftests to compile with clang was a nightmare, as it took
me several hours to realize that "CC=clang make" and "make CC=clang"
aren't equivalent, and that the "include ../lib.mak" buried halfway through
the Makefile was overriding "CC=clang make".
After too many hours fighting to get clang working, my frustration with
the Makefile boiled over a bit...
Note, I have fixes for the RISC-V RSEQ bugs (outside of selftests/kvm) that
I'll post separately.
Tested on x86 and arm, build tested on s390x and RISC-V, all with both gcc
and clang.
Sean Christopherson (14):
KVM: selftests: Define literal to asm constraint in aarch64 as
unsigned long
KVM: selftests: Delete dead code in x86_64/vmx_tsc_adjust_test.c
KVM: selftests: Fix divide-by-zero bug in memslot_perf_test
KVM: selftests: Use pattern matching in .gitignore
KVM: selftests: Fix a typo in x86-64's kvm_get_cpu_address_width()
KVM: selftests: Rename UNAME_M to ARCH_DIR, fill explicitly for x86
KVM: selftests: Use proper function prototypes in probing code
KVM: selftests: Probe -no-pie with actual CFLAGS used to compile
KVM: selftests: Explicitly disable builtins for mem*() overrides
KVM: selftests: Include lib.mk before consuming $(CC)
KVM: selftests: Disable "gnu-variable-sized-type-not-at-end" warning
KVM: selftests: Use wildcards to find library source files
KVM: selftests: Use wildcards to find targets and test source files
KVM: selftests: Enable RSEQ test for RISC-V
tools/testing/selftests/kvm/.gitignore| 91 +--
tools/testing/selftests/kvm/Makefile | 229 +++---
.../selftests/kvm/aarch64/page_fault_test.c | 2 +-
.../selftests/kvm/access_tracking_perf_test.c | 3 +
.../selftests/kvm/dirty_log_perf_test.c | 3 +
.../selftests/kvm/hardware_disable_test.c | 4 +
.../testing/selftests/kvm/include/test_util.h | 11 +
.../selftests/kvm/lib/x86_64/processor.c | 2 +-
.../selftests/kvm/max_guest_memory_test.c | 4 +
.../kvm/memslot_modification_stress_test.c| 3 +
.../testing/selftests/kvm/memslot_perf_test.c | 6 +
tools/testing/selftests/kvm/steal_time.c | 3 +
.../kvm/system_counter_offset_test.c | 4 +
.../kvm/x86_64/vmx_tsc_adjust_test.c | 5 -
14 files changed, 80 insertions(+), 290 deletions(-)
base-commit: f1a1d3aff0cc2e68a9ebbd8810d7dcd8cfe2714b
--
2.39.0.rc1.256.g54fd8350bd-goog
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 01/14] KVM: selftests: Define literal to asm constraint in aarch64 as unsigned long
Define a literal '0' asm input constraint to aarch64/page_fault_test's
guest_cas() as an unsigned long to make clang happy.
tools/testing/selftests/kvm/aarch64/page_fault_test.c:120:16: error:
value size does not match register size specified by the constraint
and modifier [-Werror,-Wasm-operand-widths]
:: "r" (0), "r" (TEST_DATA), "r" (guest_test_memory));
^
tools/testing/selftests/kvm/aarch64/page_fault_test.c:119:15: note:
use constraint modifier "w"
"casal %0, %1, [%2]\n"
^~
%w0
Fixes: 35c581015712 ("KVM: selftests: aarch64: Add aarch64/page_fault_test")
Cc: Ricardo Koller
Signed-off-by: Sean Christopherson
---
tools/testing/selftests/kvm/aarch64/page_fault_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
index 95d22cfb7b41..beb944fa6fd4 100644
--- a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
+++ b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
@@ -117,7 +117,7 @@ static void guest_cas(void)
GUEST_ASSERT(guest_check_lse());
asm volatile(".arch_extension lse\n"
"casal %0, %1, [%2]\n"
-:: "r" (0), "r" (TEST_DATA), "r" (guest_test_memory));
+:: "r" (0ul), "r" (TEST_DATA), "r" (guest_test_memory));
val = READ_ONCE(*guest_test_memory);
GUEST_ASSERT_EQ(val, TEST_DATA);
}
--
2.39.0.rc1.256.g54fd8350bd-goog
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 00/37] KVM: Refactor the KVM/x86 TDP MMU into common code
On 12/13/22 00:26, Sean Christopherson wrote: I would strongly be in favor of discarding the shadow paging residue if x86 folks are willing to part ways with it 😄 Yes, absolutely. Something like to_shadow_page->to_mmu_data, sp->md, spt->hpt, spte->spte, sptep->hptep. "host" will be confusing if when the primary MMU is involved though, e.g. I always think of the primary MMU's page tables as the "host page tables". What if we keep the "S" for SPT(E) and repurpose it to mean Secondary PTE? Makes sense, so just to_shadow_page->to_secmmu_page? Paolo ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 00/37] KVM: Refactor the KVM/x86 TDP MMU into common code
On Mon, Dec 12, 2022, Paolo Bonzini wrote: > On 12/9/22 20:07, Oliver Upton wrote: > > > - Naming. This series does not change the names of any existing code. > > > So all the KVM/x86 Shadow MMU-style terminology like > > > "shadow_page"/"sp"/"spte" persists. Should we keep that style in > > > common code or move toward something less shadow-paging-specific? > > > e.g. "page_table"/"pt"/"pte". > > > > I would strongly be in favor of discarding the shadow paging residue if > > x86 folks are willing to part ways with it 😄 > > Yes, absolutely. Something like to_shadow_page->to_mmu_data, sp->md, > spt->hpt, spte->spte, sptep->hptep. "host" will be confusing if when the primary MMU is involved though, e.g. I always think of the primary MMU's page tables as the "host page tables". What if we keep the "S" for SPT(E) and repurpose it to mean Secondary PTE? ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 04/37] KVM: x86/mmu: Invert sp->tdp_mmu_page to sp->shadow_mmu_page
On 12/8/22 20:38, David Matlack wrote:
Invert the meaning of sp->tdp_mmu_page and rename it accordingly. This
allows the TDP MMU code to not care about this field, which will be used
in a subsequent commit to move the TDP MMU to common code.
No functional change intended.
Let's use a bit of the role instead.
Paolo
Signed-off-by: David Matlack
---
arch/x86/kvm/mmu/mmu.c | 1 +
arch/x86/kvm/mmu/mmu_internal.h | 2 +-
arch/x86/kvm/mmu/tdp_mmu.c | 3 ---
arch/x86/kvm/mmu/tdp_mmu.h | 5 -
4 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 355548603960..f7668a32721d 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -2180,6 +2180,7 @@ static struct kvm_mmu_page
*kvm_mmu_alloc_shadow_page(struct kvm *kvm,
sp->gfn = gfn;
sp->role = role;
+ sp->shadow_mmu_page = true;
hlist_add_head(&sp->hash_link, sp_list);
if (sp_has_gptes(sp))
account_shadowed(kvm, sp);
diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
index e32379c5b1ad..c1a379fba24d 100644
--- a/arch/x86/kvm/mmu/mmu_internal.h
+++ b/arch/x86/kvm/mmu/mmu_internal.h
@@ -52,7 +52,7 @@ struct kvm_mmu_page {
struct list_head link;
struct hlist_node hash_link;
- bool tdp_mmu_page;
+ bool shadow_mmu_page;
bool unsync;
u8 mmu_valid_gen;
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 7ccac1aa8df6..fc0b87ceb1ea 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -133,8 +133,6 @@ void kvm_tdp_mmu_put_root(struct kvm *kvm, struct
kvm_mmu_page *root,
if (!refcount_dec_and_test(&root->tdp_mmu_root_count))
return;
- WARN_ON(!is_tdp_mmu_page(root));
-
/*
* The root now has refcount=0. It is valid, but readers already
* cannot acquire a reference to it because kvm_tdp_mmu_get_root()
@@ -279,7 +277,6 @@ static void tdp_mmu_init_sp(struct kvm_mmu_page *sp,
tdp_ptep_t sptep,
sp->role = role;
sp->gfn = gfn;
sp->ptep = sptep;
- sp->tdp_mmu_page = true;
trace_kvm_mmu_get_page(sp, true);
}
diff --git a/arch/x86/kvm/mmu/tdp_mmu.h b/arch/x86/kvm/mmu/tdp_mmu.h
index 0a63b1afabd3..18d3719f14ea 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.h
+++ b/arch/x86/kvm/mmu/tdp_mmu.h
@@ -71,7 +71,10 @@ u64 *kvm_tdp_mmu_fast_pf_get_last_sptep(struct kvm_vcpu
*vcpu, u64 addr,
u64 *spte);
#ifdef CONFIG_X86_64
-static inline bool is_tdp_mmu_page(struct kvm_mmu_page *sp) { return
sp->tdp_mmu_page; }
+static inline bool is_tdp_mmu_page(struct kvm_mmu_page *sp)
+{
+ return !sp->shadow_mmu_page;
+}
#else
static inline bool is_tdp_mmu_page(struct kvm_mmu_page *sp) { return false; }
#endif
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 02/37] KVM: MMU: Move struct kvm_mmu_page_role into common code
On 12/8/22 20:38, David Matlack wrote:
+/*
+ * kvm_mmu_page_role tracks the properties of a shadow page (where shadow page
+ * also includes TDP pages) to determine whether or not a page can be used in
+ * the given MMU context.
+ */
+union kvm_mmu_page_role {
+ u32 word;
+ struct {
+ struct {
+ /* The address space ID mapped by the page. */
+ u16 as_id:8;
+
+ /* The level of the page in the page table hierarchy. */
+ u16 level:4;
+
+ /* Whether the page is invalid, i.e. pending
destruction. */
+ u16 invalid:1;
+ };
+
+ /* Architecture-specific properties. */
+ struct kvm_mmu_page_role_arch arch;
+ };
+};
+
Have you considered adding a tdp_mmu:1 field to the arch-independent
part? I think that as long as _that_ field is the same, there's no need
to have any overlap between TDP MMU and shadow MMU roles.
I'm not even sure if the x86 TDP MMU needs _any_ other role bit. It
needs of course the above three, and it also needs "direct" but it is
used exactly to mean "is this a TDP MMU page". So we could have
union {
struct {
u32 tdp_mmu:1;
u32 invalid:1;
u32 :6;
u32 level:8;
u32 arch:8;
u32 :8;
} tdp;
/* the first field must be "u32 tdp_mmu:1;" */
struct kvm_mmu_page_role_arch shadow;
};
Paolo
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 00/37] KVM: Refactor the KVM/x86 TDP MMU into common code
On 12/9/22 20:07, Oliver Upton wrote: - Naming. This series does not change the names of any existing code. So all the KVM/x86 Shadow MMU-style terminology like "shadow_page"/"sp"/"spte" persists. Should we keep that style in common code or move toward something less shadow-paging-specific? e.g. "page_table"/"pt"/"pte". I would strongly be in favor of discarding the shadow paging residue if x86 folks are willing to part ways with it 😄 Yes, absolutely. Something like to_shadow_page->to_mmu_data, sp->md, spt->hpt, spte->spte, sptep->hptep. Paolo ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 01/37] KVM: x86/mmu: Store the address space ID directly in kvm_mmu_page_role
On 12/12/22 18:39, Sean Christopherson wrote: The notion of address spaces is already existing architecture-neutral concept in KVM (e.g. see uses of KVM_ADDRESS_SPACE_NUM in virt/kvm/kvm_main.c), although SMM is the only use-case I'm aware of. Yes, SMM is currently the only use-case. It's possible that in the future Hyper-V VTLs will also have per-level protections. It wouldn't use as_id, but it would likely be recorded in the upper byte of the role. I'm not sure if Microsoft intends to port those to ARM as well. My preference would be to leave .smm in x86's page role What about defining a byte of arch_role and a macro to build it? Unless I'm missing something, e.g. a need to map GPAs differently for SMM vs. non-SMM, SMM could have been implemented with a simple flag in a memslot to mark the memslot as SMM-only. Unfortunately not, because there can be another region (for example video RAM at 0Ah) underneath SMRAM. In fact, KVM_MEM_X86_SMRAM was the first idea. It was uglier than multiple address spaces (https://lore.kernel.org/lkml/1431084034-8425-1-git-send-email-pbonz...@redhat.com). In retrospect there were probably ways to mix the best of the two designs, but it wasn't generic enough. And separate address spaces become truly nasty if the CPU can access multiple protected regions, e.g. if the CPU can access type X and type Y at the same time, then there would need to be memslots for "regular", X, Y, and X+Y. Without a usecase that's hard to say. It's just as possible that there would be a natural hierarchy of levels. Paolo ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 06/37] KVM: MMU: Move struct kvm_mmu_page to common code
On Mon, Dec 12, 2022 at 2:32 PM Paolo Bonzini wrote:
>
> On 12/8/22 20:38, David Matlack wrote:
> > This commit increases the size of struct kvm_mmu_page by 64 bytes on
> > x86_64 (184 bytes -> 248 bytes). The size of this struct can be reduced
> > in future commits by moving TDP MMU root fields into a separate struct
> > and by dynamically allocating fields only used by the Shadow MMU.
>
> I think it's already possible to use a union like
>
> union {
> struct kvm_mmu_page_arch arch;
> struct {
> struct work_struct work;
> void *data;
> };
> };
That would potentially corrupt
kvm_mmu_page_arch.nx_huge_page_disallowed and
possible_nx_huge_page_link, both of which still need to be used by the
TDP MMU on x86.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 33/37] KVM: Move kvm_arch_flush_remote_tlbs_memslot() to common code
On Mon, Dec 12, 2022 at 2:03 PM Ben Gardon wrote: > > On Thu, Dec 8, 2022 at 11:40 AM David Matlack wrote: > > > > Move kvm_arch_flush_remote_tlbs_memslot() to common code and drop > > "arch_" from the name. kvm_arch_flush_remote_tlbs_memslot() is just a > > range-based TLB invalidation where the range is defined by the memslot. > > Now that kvm_flush_remote_tlbs_range() can be called from common code we > > can just use that and drop a bunch of duplicate code from the arch > > directories. > > > > Note this adds a lockdep assertion for slot_lock being held when calling > > kvm_flush_remote_tlbs_memslot(), which was previously only asserted on > > x86. > > Besides the one lockdep assertion, is there any benefit to having this > wrapper function? Open-coding "kvm_flush_remote_tlbs_range(kvm, > memslot->base_gfn, memslot->npages);" is only a slightly longer line > and, IMO, just as readable. I'm happy to see this cleanup, but it > might be just as easy to drop the function. The wrapper makes lines shorter, adds a lockdep assertion, and is just as readable. What's the reason to drop it? ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 06/37] KVM: MMU: Move struct kvm_mmu_page to common code
On 12/8/22 20:38, David Matlack wrote:
This commit increases the size of struct kvm_mmu_page by 64 bytes on
x86_64 (184 bytes -> 248 bytes). The size of this struct can be reduced
in future commits by moving TDP MMU root fields into a separate struct
and by dynamically allocating fields only used by the Shadow MMU.
I think it's already possible to use a union like
union {
struct kvm_mmu_page_arch arch;
struct {
struct work_struct work;
void *data;
};
};
Paolo
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 10/37] KVM: MMU: Move struct kvm_page_fault to common code
On Mon, Dec 12, 2022 at 10:24:31AM -0800, Ben Gardon wrote:
> On Thu, Dec 8, 2022 at 11:39 AM David Matlack wrote:
> >
> > Move struct kvm_page_fault to common code. This will be used in a future
> > commit to move the TDP MMU to common code.
> >
> > No functional change intended.
> >
> > Signed-off-by: David Matlack
> > ---
[...]
> > diff --git a/include/kvm/mmu_types.h b/include/kvm/mmu_types.h
> > index a9da33d4baa8..9f0ca920bf68 100644
> > --- a/include/kvm/mmu_types.h
> > +++ b/include/kvm/mmu_types.h
> > @@ -66,4 +66,48 @@ struct kvm_mmu_page {
> > struct kvm_mmu_page_arch arch;
> > };
> >
> > +struct kvm_page_fault {
> > + /* The raw faulting address. */
> > + const gpa_t addr;
> > +
> > + /* Whether the fault was synthesized to prefetch a mapping. */
> > + const bool prefetch;
> > +
> > + /* Information about the cause of the fault. */
> > + const bool write;
> > + const bool exec;
> > +
> > + /* Shifted addr, or result of guest page table walk if shadow
> > paging. */
> > + gfn_t gfn;
>
> Is this redundant to have in common code? If we're not doing common
> shadow paging, then this is just addr shifted. Would this be better
> placed in the arch specific struct?
Yes it's redundant but it is actually used by the TDP MMU, unlike @addr.
So if anything I would rather move @addr to kvm_page_fault_arch.
>
> > +
> > + /* The memslot that contains @gfn. May be NULL. */
> > + struct kvm_memory_slot *slot;
> > +
> > + /* Maximum page size that can be created for this fault. */
> > + u8 max_level;
> > +
> > + /*
> > +* Page size that can be created based on the max_level and the page
> > +* size used by the host mapping.
> > +*/
> > + u8 req_level;
> > +
> > + /* Final page size that will be created. */
> > + u8 goal_level;
> > +
> > + /*
> > +* The value of kvm->mmu_invalidate_seq before fetching the host
> > +* mapping. Used to verify that the host mapping has not changed
> > +* after grabbing the MMU lock.
> > +*/
> > + unsigned long mmu_seq;
>
> Should this be ifdef'ed with KVM_ARCH_WANT_MMU_NOTIFIER?
I'll have to take a closer look, but probably yes.
>
> > +
> > + /* Information about the host mapping. */
> > + kvm_pfn_t pfn;
> > + hva_t hva;
> > + bool map_writable;
> > +
> > + struct kvm_page_fault_arch arch;
> > +};
> > +
> > #endif /* !__KVM_MMU_TYPES_H */
> > --
> > 2.39.0.rc1.256.g54fd8350bd-goog
> >
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 10/37] KVM: MMU: Move struct kvm_page_fault to common code
On 12/8/22 20:38, David Matlack wrote: + + /* Derived from mmu and global state. */ + const bool is_tdp; I think this could stay in the architecture-independent part. Paolo ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 33/37] KVM: Move kvm_arch_flush_remote_tlbs_memslot() to common code
On Thu, Dec 8, 2022 at 11:40 AM David Matlack wrote:
>
> Move kvm_arch_flush_remote_tlbs_memslot() to common code and drop
> "arch_" from the name. kvm_arch_flush_remote_tlbs_memslot() is just a
> range-based TLB invalidation where the range is defined by the memslot.
> Now that kvm_flush_remote_tlbs_range() can be called from common code we
> can just use that and drop a bunch of duplicate code from the arch
> directories.
>
> Note this adds a lockdep assertion for slot_lock being held when calling
> kvm_flush_remote_tlbs_memslot(), which was previously only asserted on
> x86.
Besides the one lockdep assertion, is there any benefit to having this
wrapper function? Open-coding "kvm_flush_remote_tlbs_range(kvm,
memslot->base_gfn, memslot->npages);" is only a slightly longer line
and, IMO, just as readable. I'm happy to see this cleanup, but it
might be just as easy to drop the function.
>
> Signed-off-by: David Matlack
> ---
> arch/arm64/kvm/arm.c | 6 --
> arch/mips/kvm/mips.c | 10 ++
> arch/riscv/kvm/mmu.c | 6 --
> arch/x86/kvm/mmu/mmu.c | 16 +---
> arch/x86/kvm/x86.c | 2 +-
> include/linux/kvm_host.h | 7 +++
> virt/kvm/kvm_main.c | 17 +++--
> 7 files changed, 22 insertions(+), 42 deletions(-)
>
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 0e0d4c4f79a2..4f1549c1d2d2 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1430,12 +1430,6 @@ void kvm_arch_sync_dirty_log(struct kvm *kvm, struct
> kvm_memory_slot *memslot)
>
> }
>
> -void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
> - const struct kvm_memory_slot *memslot)
> -{
> - kvm_flush_remote_tlbs(kvm);
> -}
> -
> static int kvm_vm_ioctl_set_device_addr(struct kvm *kvm,
> struct kvm_arm_device_addr *dev_addr)
> {
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index a25e0b73ee70..ecd8a051fd6b 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -209,7 +209,7 @@ void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
> /* Flush slot from GPA */
> kvm_mips_flush_gpa_pt(kvm, slot->base_gfn,
> slot->base_gfn + slot->npages - 1);
> - kvm_arch_flush_remote_tlbs_memslot(kvm, slot);
> + kvm_flush_remote_tlbs_memslot(kvm, slot);
> spin_unlock(&kvm->mmu_lock);
> }
>
> @@ -245,7 +245,7 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
> needs_flush = kvm_mips_mkclean_gpa_pt(kvm, new->base_gfn,
> new->base_gfn + new->npages - 1);
> if (needs_flush)
> - kvm_arch_flush_remote_tlbs_memslot(kvm, new);
> + kvm_flush_remote_tlbs_memslot(kvm, new);
> spin_unlock(&kvm->mmu_lock);
> }
> }
> @@ -997,12 +997,6 @@ int kvm_arch_flush_remote_tlb(struct kvm *kvm)
> return 1;
> }
>
> -void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
> - const struct kvm_memory_slot *memslot)
> -{
> - kvm_flush_remote_tlbs(kvm);
> -}
> -
> long kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long
> arg)
> {
> long r;
> diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
> index a8281a65cb3d..98bf3719a396 100644
> --- a/arch/riscv/kvm/mmu.c
> +++ b/arch/riscv/kvm/mmu.c
> @@ -406,12 +406,6 @@ void kvm_arch_sync_dirty_log(struct kvm *kvm, struct
> kvm_memory_slot *memslot)
> {
> }
>
> -void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
> - const struct kvm_memory_slot *memslot)
> -{
> - kvm_flush_remote_tlbs(kvm);
> -}
> -
> void kvm_arch_free_memslot(struct kvm *kvm, struct kvm_memory_slot *free)
> {
> }
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 19963ed83484..f2602ee1771f 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -6524,7 +6524,7 @@ static void kvm_rmap_zap_collapsible_sptes(struct kvm
> *kvm,
> */
> if (slot_handle_level(kvm, slot, kvm_mmu_zap_collapsible_spte,
> PG_LEVEL_4K, KVM_MAX_HUGEPAGE_LEVEL - 1, true))
> - kvm_arch_flush_remote_tlbs_memslot(kvm, slot);
> + kvm_flush_remote_tlbs_memslot(kvm, slot);
> }
>
> void kvm_mmu_zap_collapsible_sptes(struct kvm *kvm,
> @@ -6543,20 +6543,6 @@ void kvm_mmu_zap_collapsible_sptes(struct kvm *kvm,
> }
> }
>
> -void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
> - const struct kvm_memory_slot *memslot)
> -{
> - /*
> -* All current use cases for flushing the TLBs for a specific memslot
> -* related to dirty logging, and many do the TLB flush out of
> mmu_lock.
> -* The interaction between the various operations on memslot must be
> -* s
Re: [RFC PATCH 20/37] KVM: x86/mmu: Abstract away computing the max mapping level
On Mon, Dec 12, 2022 at 11:32 AM Ben Gardon wrote: > > On Thu, Dec 8, 2022 at 11:39 AM David Matlack wrote: > > > > Abstract away kvm_mmu_max_mapping_level(), which is an x86-specific > > function for computing the max level that a given GFN can be mapped in > > KVM's page tables. This will be used in a future commit to enable moving > > the TDP MMU to common code. > > > > Provide a default implementation for non-x86 architectures that just > > returns the max level. This will result in more zapping than necessary > > when disabling dirty logging (i.e. less than optimal performance) but no > > correctness issues. > > Apologies if you already implemented it in a later patch in this > series, but would it not at least be possible to port > host_pfn_mapping_level to common code and check that? > I'm assuming, though I could be wrong, that all archs map GFNs with at > most a host page table granularity mapping. > I suppose that doesn't strictly need to be included in this series, > but it would be worth addressing in the commit description. It's not implemented later in this series, but I agree it's something we should do. In fact, it's worth doing regardless of this series as a way to share more code across architectures (e.g. KVM/ARM has it's own version in arch/arm64/kvm/mmu.c:get_user_mapping_size()). ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH 1/3] arm: pmu: Fix overflow checks for PMUv3p5 long counters
On Fri, Dec 09, 2022 at 05:47:14PM +, Alexandru Elisei wrote:
> Hi,
>
> On Fri, Dec 02, 2022 at 04:55:25AM +, Ricardo Koller wrote:
> > PMUv3p5 uses 64-bit counters irrespective of whether the PMU is configured
> > for overflowing at 32 or 64-bits. The consequence is that tests that check
> > the counter values after overflowing should not assume that values will be
> > wrapped around 32-bits: they overflow into the other half of the 64-bit
> > counters on PMUv3p5.
> >
> > Fix tests by correctly checking overflowing-counters against the expected
> > 64-bit value.
> >
> > Signed-off-by: Ricardo Koller
> > ---
> > arm/pmu.c | 29 ++---
> > 1 file changed, 18 insertions(+), 11 deletions(-)
> >
> > diff --git a/arm/pmu.c b/arm/pmu.c
> > index cd47b14..eeac984 100644
> > --- a/arm/pmu.c
> > +++ b/arm/pmu.c
> > @@ -54,10 +54,10 @@
> > #define EXT_COMMON_EVENTS_LOW 0x4000
> > #define EXT_COMMON_EVENTS_HIGH 0x403F
> >
> > -#define ALL_SET0x
> > -#define ALL_CLEAR 0x0
> > -#define PRE_OVERFLOW 0xFFF0
> > -#define PRE_OVERFLOW2 0xFFDC
> > +#define ALL_SET0xULL
> > +#define ALL_CLEAR 0xULL
> > +#define PRE_OVERFLOW 0xFFF0ULL
> > +#define PRE_OVERFLOW2 0xFFDCULL
> >
> > #define PMU_PPI23
> >
> > @@ -538,6 +538,7 @@ static void test_mem_access(void)
> > static void test_sw_incr(void)
> > {
> > uint32_t events[] = {SW_INCR, SW_INCR};
> > + uint64_t cntr0;
> > int i;
> >
> > if (!satisfy_prerequisites(events, ARRAY_SIZE(events)))
> > @@ -572,9 +573,9 @@ static void test_sw_incr(void)
> > write_sysreg(0x3, pmswinc_el0);
> >
> > isb();
> > - report(read_regn_el0(pmevcntr, 0) == 84, "counter #1 after + 100
> > SW_INCR");
> > - report(read_regn_el0(pmevcntr, 1) == 100,
> > - "counter #0 after + 100 SW_INCR");
> > + cntr0 = (pmu.version < ID_DFR0_PMU_V3_8_5) ? 84 : PRE_OVERFLOW + 100;
>
> Hm... in the Arm ARM it says that counters are 64-bit if PMUv3p5 is
> implemented. But it doesn't say anywhere that versions newer than p5 are
> required to implement PMUv3p5.
>
> For example, for PMUv3p7, it says that the feature is mandatory in Arm8.7
> implementations. My interpretation of that is that it is not forbidden for
> an implementer to cherry-pick this version on older versions of the
> architecture where PMUv3p5 is not implemented.
>
> Maybe the check should be pmu.version == ID_DFR0_PMU_V3_8_5, to match the
> counter definitions in the architecture?
>
> Also, I found the meaning of those numbers to be quite cryptic. Perhaps
> something like this would be more resilient to changes to the value of
> PRE_OVERFLOW and easier to understand:
>
> + cntr0 = (pmu.version < ID_DFR0_PMU_V3_8_5) ?
> + (uint32_t)PRE_OVERFLOW + 100 :
> + (uint64_t)PRE_OVERFLOW + 100;
>
> I haven't tested the code, would that work?
Just checked. That works. It's much cleaner, thank you very much.
Will send a v2 a the end of the week, maybe after some more reviews.
>
> Thanks,
> Alex
>
> > + report(read_regn_el0(pmevcntr, 0) == cntr0, "counter #0 after + 100
> > SW_INCR");
> > + report(read_regn_el0(pmevcntr, 1) == 100, "counter #1 after + 100
> > SW_INCR");
> > report_info("counter values after 100 SW_INCR #0=%ld #1=%ld",
> > read_regn_el0(pmevcntr, 0), read_regn_el0(pmevcntr, 1));
> > report(read_sysreg(pmovsclr_el0) == 0x1,
> > @@ -584,6 +585,7 @@ static void test_sw_incr(void)
> > static void test_chained_counters(void)
> > {
> > uint32_t events[] = {CPU_CYCLES, CHAIN};
> > + uint64_t cntr1;
> >
> > if (!satisfy_prerequisites(events, ARRAY_SIZE(events)))
> > return;
> > @@ -618,13 +620,16 @@ static void test_chained_counters(void)
> >
> > precise_instrs_loop(22, pmu.pmcr_ro | PMU_PMCR_E);
> > report_info("overflow reg = 0x%lx", read_sysreg(pmovsclr_el0));
> > - report(!read_regn_el0(pmevcntr, 1), "CHAIN counter #1 wrapped");
> > + cntr1 = (pmu.version < ID_DFR0_PMU_V3_8_5) ? 0 : ALL_SET + 1;
> > + report(read_regn_el0(pmevcntr, 1) == cntr1, "CHAIN counter #1 wrapped");
> > +
> > report(read_sysreg(pmovsclr_el0) == 0x3, "overflow on even and odd
> > counters");
> > }
> >
> > static void test_chained_sw_incr(void)
> > {
> > uint32_t events[] = {SW_INCR, CHAIN};
> > + uint64_t cntr0, cntr1;
> > int i;
> >
> > if (!satisfy_prerequisites(events, ARRAY_SIZE(events)))
> > @@ -665,10 +670,12 @@ static void test_chained_sw_incr(void)
> > write_sysreg(0x1, pmswinc_el0);
> >
> > isb();
> > + cntr0 = (pmu.version < ID_DFR0_PMU_V3_8_5) ? 0 : ALL_SET + 1;
> > + cntr1 = (pmu.version < ID_DFR0_PMU_V3_8_5) ? 84 : PRE_OVERFLOW + 100;
> > report((read_sysreg(pmovsclr_el0) == 0x3) &&
> > -
Re: [RFC PATCH 20/37] KVM: x86/mmu: Abstract away computing the max mapping level
On Thu, Dec 8, 2022 at 11:39 AM David Matlack wrote:
>
> Abstract away kvm_mmu_max_mapping_level(), which is an x86-specific
> function for computing the max level that a given GFN can be mapped in
> KVM's page tables. This will be used in a future commit to enable moving
> the TDP MMU to common code.
>
> Provide a default implementation for non-x86 architectures that just
> returns the max level. This will result in more zapping than necessary
> when disabling dirty logging (i.e. less than optimal performance) but no
> correctness issues.
Apologies if you already implemented it in a later patch in this
series, but would it not at least be possible to port
host_pfn_mapping_level to common code and check that?
I'm assuming, though I could be wrong, that all archs map GFNs with at
most a host page table granularity mapping.
I suppose that doesn't strictly need to be included in this series,
but it would be worth addressing in the commit description.
>
> Signed-off-by: David Matlack
> ---
> arch/x86/kvm/mmu/tdp_mmu.c | 14 ++
> arch/x86/kvm/mmu/tdp_pgtable.c | 7 +++
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 7670fbd8e72d..24d1dbd0a1ec 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -1696,6 +1696,13 @@ void kvm_tdp_mmu_clear_dirty_pt_masked(struct kvm *kvm,
> clear_dirty_pt_masked(kvm, root, gfn, mask, wrprot);
> }
>
> +__weak int tdp_mmu_max_mapping_level(struct kvm *kvm,
> +const struct kvm_memory_slot *slot,
> +struct tdp_iter *iter)
> +{
> + return TDP_MAX_HUGEPAGE_LEVEL;
> +}
> +
> static void zap_collapsible_spte_range(struct kvm *kvm,
>struct kvm_mmu_page *root,
>const struct kvm_memory_slot *slot)
> @@ -1727,15 +1734,14 @@ static void zap_collapsible_spte_range(struct kvm
> *kvm,
> /*
> * If iter.gfn resides outside of the slot, i.e. the page for
> * the current level overlaps but is not contained by the
> slot,
> -* then the SPTE can't be made huge. More importantly, trying
> -* to query that info from slot->arch.lpage_info will cause an
> +* then the SPTE can't be made huge. On x86, trying to query
> +* that info from slot->arch.lpage_info will cause an
> * out-of-bounds access.
> */
> if (iter.gfn < start || iter.gfn >= end)
> continue;
>
> - max_mapping_level = kvm_mmu_max_mapping_level(kvm, slot,
> - iter.gfn,
> PG_LEVEL_NUM);
> + max_mapping_level = tdp_mmu_max_mapping_level(kvm, slot,
> &iter);
> if (max_mapping_level < iter.level)
> continue;
>
> diff --git a/arch/x86/kvm/mmu/tdp_pgtable.c b/arch/x86/kvm/mmu/tdp_pgtable.c
> index b07ed99b4ab1..840d063c45b8 100644
> --- a/arch/x86/kvm/mmu/tdp_pgtable.c
> +++ b/arch/x86/kvm/mmu/tdp_pgtable.c
> @@ -163,3 +163,10 @@ void tdp_mmu_arch_unlink_sp(struct kvm *kvm, struct
> kvm_mmu_page *sp,
> if (shared)
> spin_unlock(&kvm->arch.tdp_mmu_pages_lock);
> }
> +
> +int tdp_mmu_max_mapping_level(struct kvm *kvm,
> + const struct kvm_memory_slot *slot,
> + struct tdp_iter *iter)
> +{
> + return kvm_mmu_max_mapping_level(kvm, slot, iter->gfn, PG_LEVEL_NUM);
> +}
> --
> 2.39.0.rc1.256.g54fd8350bd-goog
>
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 17/37] KVM: Move struct kvm_gfn_range to kvm_types.h
On Thu, Dec 8, 2022 at 11:39 AM David Matlack wrote:
>
> Move struct kvm_gfn_range to kvm_types.h so that it's definition can be
> accessed in a future commit by arch/x86/include/asm/kvm/tdp_pgtable.h
> without needing to include the mega-header kvm_host.h.
>
> No functional change intended.
>
> Signed-off-by: David Matlack
Reviewed-by: Ben Gardon
> ---
> include/linux/kvm_host.h | 7 ---
> include/linux/kvm_types.h | 8
> 2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 22ecb7ce4d31..469ff4202a0d 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -256,13 +256,6 @@ int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu);
> #endif
>
> #ifdef KVM_ARCH_WANT_MMU_NOTIFIER
I don't have any problem with always having this defined, but might be
worth noting that it's now defined on all archs, regardless of
KVM_ARCH_WANT_MMU_NOTIFIER.
> -struct kvm_gfn_range {
> - struct kvm_memory_slot *slot;
> - gfn_t start;
> - gfn_t end;
> - pte_t pte;
> - bool may_block;
> -};
> bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range);
> bool kvm_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range);
> bool kvm_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range);
> diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
> index 59cf958d69df..001aad9ea987 100644
> --- a/include/linux/kvm_types.h
> +++ b/include/linux/kvm_types.h
> @@ -132,4 +132,12 @@ struct kvm_vcpu_stat_generic {
>
> #define KVM_STATS_NAME_SIZE48
>
> +struct kvm_gfn_range {
> + struct kvm_memory_slot *slot;
> + gfn_t start;
> + gfn_t end;
> + pte_t pte;
> + bool may_block;
> +};
> +
> #endif /* __KVM_TYPES_H__ */
> --
> 2.39.0.rc1.256.g54fd8350bd-goog
>
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 10/37] KVM: MMU: Move struct kvm_page_fault to common code
On Thu, Dec 8, 2022 at 11:39 AM David Matlack wrote:
>
> Move struct kvm_page_fault to common code. This will be used in a future
> commit to move the TDP MMU to common code.
>
> No functional change intended.
>
> Signed-off-by: David Matlack
> ---
> arch/x86/include/asm/kvm/mmu_types.h | 20 +++
> arch/x86/kvm/mmu/mmu.c | 19 +++
> arch/x86/kvm/mmu/mmu_internal.h | 79 ++--
> arch/x86/kvm/mmu/mmutrace.h | 2 +-
> arch/x86/kvm/mmu/paging_tmpl.h | 14 ++---
> arch/x86/kvm/mmu/tdp_mmu.c | 6 +--
> include/kvm/mmu_types.h | 44
> 7 files changed, 100 insertions(+), 84 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm/mmu_types.h
> b/arch/x86/include/asm/kvm/mmu_types.h
> index affcb520b482..59d1be85f4b7 100644
> --- a/arch/x86/include/asm/kvm/mmu_types.h
> +++ b/arch/x86/include/asm/kvm/mmu_types.h
> @@ -5,6 +5,7 @@
> #include
> #include
> #include
> +#include
>
> /*
> * This is a subset of the overall kvm_cpu_role to minimize the size of
> @@ -115,4 +116,23 @@ struct kvm_mmu_page_arch {
> atomic_t write_flooding_count;
> };
>
> +struct kvm_page_fault_arch {
> + const u32 error_code;
> +
> + /* x86-specific error code bits */
> + const bool present;
> + const bool rsvd;
> + const bool user;
> +
> + /* Derived from mmu and global state. */
> + const bool is_tdp;
> + const bool nx_huge_page_workaround_enabled;
> +
> + /*
> +* Whether a >4KB mapping can be created or is forbidden due to NX
> +* hugepages.
> +*/
> + bool huge_page_disallowed;
> +};
> +
> #endif /* !__ASM_KVM_MMU_TYPES_H */
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index e47f35878ab5..0593d4a60139 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -3092,7 +3092,8 @@ void kvm_mmu_hugepage_adjust(struct kvm_vcpu *vcpu,
> struct kvm_page_fault *fault
> struct kvm_memory_slot *slot = fault->slot;
> kvm_pfn_t mask;
>
> - fault->huge_page_disallowed = fault->exec &&
> fault->nx_huge_page_workaround_enabled;
> + fault->arch.huge_page_disallowed =
> + fault->exec && fault->arch.nx_huge_page_workaround_enabled;
>
> if (unlikely(fault->max_level == PG_LEVEL_4K))
> return;
> @@ -3109,7 +3110,7 @@ void kvm_mmu_hugepage_adjust(struct kvm_vcpu *vcpu,
> struct kvm_page_fault *fault
> */
> fault->req_level = kvm_mmu_max_mapping_level(vcpu->kvm, slot,
> fault->gfn,
> fault->max_level);
> - if (fault->req_level == PG_LEVEL_4K || fault->huge_page_disallowed)
> + if (fault->req_level == PG_LEVEL_4K ||
> fault->arch.huge_page_disallowed)
> return;
>
> /*
> @@ -3158,7 +3159,7 @@ static int direct_map(struct kvm_vcpu *vcpu, struct
> kvm_page_fault *fault)
> * We cannot overwrite existing page tables with an NX
> * large page, as the leaf could be executable.
> */
> - if (fault->nx_huge_page_workaround_enabled)
> + if (fault->arch.nx_huge_page_workaround_enabled)
> disallowed_hugepage_adjust(fault, *it.sptep,
> it.level);
>
> base_gfn = fault->gfn & ~(KVM_PAGES_PER_HPAGE(it.level) - 1);
> @@ -3170,7 +3171,7 @@ static int direct_map(struct kvm_vcpu *vcpu, struct
> kvm_page_fault *fault)
> continue;
>
> link_shadow_page(vcpu, it.sptep, sp);
> - if (fault->huge_page_disallowed)
> + if (fault->arch.huge_page_disallowed)
> account_nx_huge_page(vcpu->kvm, sp,
> fault->req_level >= it.level);
> }
> @@ -3221,7 +3222,7 @@ static int kvm_handle_noslot_fault(struct kvm_vcpu
> *vcpu,
>struct kvm_page_fault *fault,
>unsigned int access)
> {
> - gva_t gva = fault->is_tdp ? 0 : fault->addr;
> + gva_t gva = fault->arch.is_tdp ? 0 : fault->addr;
>
> vcpu_cache_mmio_info(vcpu, gva, fault->gfn,
> access & shadow_mmio_access_mask);
> @@ -3255,7 +3256,7 @@ static bool page_fault_can_be_fast(struct
> kvm_page_fault *fault)
> * generation number. Refreshing the MMIO generation needs to go down
> * the slow path. Note, EPT Misconfigs do NOT set the PRESENT flag!
> */
> - if (fault->rsvd)
> + if (fault->arch.rsvd)
> return false;
>
> /*
> @@ -3273,7 +3274,7 @@ static bool page_fault_can_be_fast(struct
> kvm_page_fault *fault)
> *SPTE is MMU-writable (determined later), the fault can be fixed
> *by setting the Writable bit, which can be done out of mmu_lock.
> */
> - if (
Re: [RFC PATCH 01/37] KVM: x86/mmu: Store the address space ID directly in kvm_mmu_page_role
On Mon, Dec 12, 2022 at 05:39:38PM +, Sean Christopherson wrote:
> On Fri, Dec 09, 2022, David Matlack wrote:
> > On Fri, Dec 9, 2022 at 9:25 AM Oliver Upton wrote:
> > >
> > > On Fri, Dec 09, 2022 at 10:37:47AM +0800, Yang, Weijiang wrote:
> > > > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > > > > index 4d188f056933..f375b719f565 100644
> > > > > --- a/arch/x86/kvm/mmu/mmu.c
> > > > > +++ b/arch/x86/kvm/mmu/mmu.c
> > > > > @@ -5056,7 +5056,7 @@ kvm_calc_cpu_role(struct kvm_vcpu *vcpu, const
> > > > > struct kvm_mmu_role_regs *regs)
> > > > > union kvm_cpu_role role = {0};
> > > > > role.base.access = ACC_ALL;
> > > > > - role.base.smm = is_smm(vcpu);
> > > > > + role.base.as_id = is_smm(vcpu);
> > > >
> > > > I'm not familiar with other architectures, is there similar conception
> > > > as
> > > > x86 smm mode?
> >
> > The notion of address spaces is already existing architecture-neutral
> > concept in KVM (e.g. see uses of KVM_ADDRESS_SPACE_NUM in
> > virt/kvm/kvm_main.c), although SMM is the only use-case I'm aware of.
>
> Yes, SMM is currently the only use-case.
>
> > Architectures that do not use multiple address spaces will just leave
> > as_id is as always 0.
>
> My preference would be to leave .smm in x86's page role. IMO, defining
> multiple
> address spaces to support SMM emulation was a mistake that should be
> contained to
> SMM, i.e. should never be used for any other feature. And with
> CONFIG_KVM_SMM,
> even x86 can opt out.
+1
I don't think something is architecture-neutral by virtue of it existing
in virt/kvm/*.
> Emulating something like TrustZone or EL3 would be quite similar.
/me shudders
I know it is for the sake of discussion, but for posterity: please no!
--
Best,
Oliver
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 06/37] KVM: MMU: Move struct kvm_mmu_page to common code
On Thu, Dec 8, 2022 at 11:39 AM David Matlack wrote:
>
> Move struct kvm_mmu_page to common code and all x86-specific fields into
> kvm_mmu_page_arch.
>
> This commit increases the size of struct kvm_mmu_page by 64 bytes on
> x86_64 (184 bytes -> 248 bytes). The size of this struct can be reduced
> in future commits by moving TDP MMU root fields into a separate struct
> and by dynamically allocating fields only used by the Shadow MMU.
>
> No functional change intended.
>
> Signed-off-by: David Matlack
I haven't reviewed every line of the mechanical refactor of adding
"arch." all over the place, but at a high level, this looks like a
good split on struct kvm_mmu_page.
> ---
> arch/x86/include/asm/kvm/mmu_types.h | 62 ++
> arch/x86/include/asm/kvm_host.h | 4 -
> arch/x86/kvm/mmu/mmu.c | 122 ++-
> arch/x86/kvm/mmu/mmu_internal.h | 83 --
> arch/x86/kvm/mmu/mmutrace.h | 4 +-
> arch/x86/kvm/mmu/paging_tmpl.h | 10 +--
> arch/x86/kvm/mmu/tdp_mmu.c | 8 +-
> arch/x86/kvm/mmu/tdp_mmu.h | 2 +-
> include/kvm/mmu_types.h | 32 ++-
> 9 files changed, 167 insertions(+), 160 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm/mmu_types.h
> b/arch/x86/include/asm/kvm/mmu_types.h
> index 35f893ebab5a..affcb520b482 100644
> --- a/arch/x86/include/asm/kvm/mmu_types.h
> +++ b/arch/x86/include/asm/kvm/mmu_types.h
> @@ -2,6 +2,8 @@
> #ifndef __ASM_KVM_MMU_TYPES_H
> #define __ASM_KVM_MMU_TYPES_H
>
> +#include
> +#include
> #include
>
> /*
> @@ -53,4 +55,64 @@ struct kvm_mmu_page_role_arch {
> u16 passthrough:1;
> };
>
> +struct kvm_rmap_head {
> + unsigned long val;
> +};
> +
> +struct kvm_mmu_page_arch {
> + struct hlist_node hash_link;
> +
> + bool shadow_mmu_page;
> + bool unsync;
> + u8 mmu_valid_gen;
> +
> +/*
> + * The shadow page can't be replaced by an equivalent huge page
> + * because it is being used to map an executable page in the guest
> + * and the NX huge page mitigation is enabled.
> + */
> + bool nx_huge_page_disallowed;
> +
> + /*
> +* Stores the result of the guest translation being shadowed by each
> +* SPTE. KVM shadows two types of guest translations: nGPA -> GPA
> +* (shadow EPT/NPT) and GVA -> GPA (traditional shadow paging). In
> both
> +* cases the result of the translation is a GPA and a set of access
> +* constraints.
> +*
> +* The GFN is stored in the upper bits (PAGE_SHIFT) and the shadowed
> +* access permissions are stored in the lower bits. Note, for
> +* convenience and uniformity across guests, the access permissions
> are
> +* stored in KVM format (e.g. ACC_EXEC_MASK) not the raw guest
> format.
> +*/
> + u64 *shadowed_translation;
> +
> + unsigned int unsync_children;
> +
> + /* Rmap pointers to all parent sptes. */
> + struct kvm_rmap_head parent_ptes;
> +
> + DECLARE_BITMAP(unsync_child_bitmap, 512);
> +
> + /*
> +* Tracks shadow pages that, if zapped, would allow KVM to create an
> NX
> +* huge page. A shadow page will have nx_huge_page_disallowed set but
> +* not be on the list if a huge page is disallowed for other reasons,
> +* e.g. because KVM is shadowing a PTE at the same gfn, the memslot
> +* isn't properly aligned, etc...
> +*/
> + struct list_head possible_nx_huge_page_link;
> +
> +#ifdef CONFIG_X86_32
> + /*
> +* Used out of the mmu-lock to avoid reading spte values while an
> +* update is in progress; see the comments in __get_spte_lockless().
> +*/
> + int clear_spte_count;
> +#endif
> +
> + /* Number of writes since the last time traversal visited this page.
> */
> + atomic_t write_flooding_count;
> +};
> +
> #endif /* !__ASM_KVM_MMU_TYPES_H */
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index ebcd7a0dabef..f5743a652e10 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -329,10 +329,6 @@ union kvm_cpu_role {
> };
> };
>
> -struct kvm_rmap_head {
> - unsigned long val;
> -};
> -
> struct kvm_pio_request {
> unsigned long linear_rip;
> unsigned long count;
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 11cef930d5ed..e47f35878ab5 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -350,7 +350,7 @@ static void count_spte_clear(u64 *sptep, u64 spte)
>
> /* Ensure the spte is completely set before we increase the count */
> smp_wmb();
> - sp->clear_spte_count++;
> + sp->arch.clear_spte_count++;
> }
>
> static void __set_spte(u64 *sptep, u64 spte)
> @@ -432,7 +432,7 @@ static u64 __get_spte_lockless(u64 *sptep)
>
Re: [RFC PATCH 02/37] KVM: MMU: Move struct kvm_mmu_page_role into common code
On Thu, Dec 8, 2022 at 11:39 AM David Matlack wrote:
>
> Move struct kvm_mmu_page_role into common code, and move all
> x86-specific fields into a separate sub-struct within the role,
> kvm_mmu_page_role_arch.
>
> Signed-off-by: David Matlack
> ---
> MAINTAINERS | 4 +-
> arch/x86/include/asm/kvm/mmu_types.h | 56 ++
> arch/x86/include/asm/kvm_host.h | 68 +---
> arch/x86/kvm/mmu/mmu.c | 156 +--
> arch/x86/kvm/mmu/mmu_internal.h | 4 +-
> arch/x86/kvm/mmu/mmutrace.h | 12 +--
> arch/x86/kvm/mmu/paging_tmpl.h | 20 ++--
> arch/x86/kvm/mmu/spte.c | 4 +-
> arch/x86/kvm/mmu/spte.h | 2 +-
> arch/x86/kvm/x86.c | 8 +-
> include/kvm/mmu_types.h | 37 +++
> 11 files changed, 202 insertions(+), 169 deletions(-)
> create mode 100644 arch/x86/include/asm/kvm/mmu_types.h
> create mode 100644 include/kvm/mmu_types.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 89672a59c0c3..7e586d7ba78c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11198,7 +11198,8 @@ W: http://www.linux-kvm.org
> T: git git://git.kernel.org/pub/scm/virt/kvm/kvm.git
> F: Documentation/virt/kvm/
> F: include/asm-generic/kvm*
> -F: include/kvm/iodev.h
> +F: include/kvm/
> +X: include/kvm/arm_*
> F: include/linux/kvm*
> F: include/trace/events/kvm.h
> F: include/uapi/asm-generic/kvm*
> @@ -11285,6 +11286,7 @@ L: k...@vger.kernel.org
> S: Supported
> T: git git://git.kernel.org/pub/scm/virt/kvm/kvm.git
> F: arch/x86/include/asm/kvm*
> +F: arch/x86/include/asm/kvm/
> F: arch/x86/include/asm/svm.h
> F: arch/x86/include/asm/vmx*.h
> F: arch/x86/include/uapi/asm/kvm*
> diff --git a/arch/x86/include/asm/kvm/mmu_types.h
> b/arch/x86/include/asm/kvm/mmu_types.h
> new file mode 100644
> index ..35f893ebab5a
> --- /dev/null
> +++ b/arch/x86/include/asm/kvm/mmu_types.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_KVM_MMU_TYPES_H
> +#define __ASM_KVM_MMU_TYPES_H
> +
> +#include
> +
> +/*
> + * This is a subset of the overall kvm_cpu_role to minimize the size of
> + * kvm_memory_slot.arch.gfn_track, i.e. allows allocating 2 bytes per gfn
> + * instead of 4 bytes per gfn.
> + *
> + * Upper-level shadow pages having gptes are tracked for write-protection via
> + * gfn_track. As above, gfn_track is a 16 bit counter, so KVM must not
> create
> + * more than 2^16-1 upper-level shadow pages at a single gfn, otherwise
> + * gfn_track will overflow and explosions will ensure.
> + *
> + * A unique shadow page (SP) for a gfn is created if and only if an existing
> SP
> + * cannot be reused. The ability to reuse a SP is tracked by its role, which
> + * incorporates various mode bits and properties of the SP. Roughly
> speaking,
> + * the number of unique SPs that can theoretically be created is 2^n, where n
> + * is the number of bits that are used to compute the role.
> + *
> + * Note, not all combinations of modes and flags below are possible:
> + *
> + * - invalid shadow pages are not accounted, so the bits are effectively 18
> + *
> + * - quadrant will only be used if has_4_byte_gpte=1 (non-PAE paging);
> + * execonly and ad_disabled are only used for nested EPT which has
> + * has_4_byte_gpte=0. Therefore, 2 bits are always unused.
> + *
> + * - the 4 bits of level are effectively limited to the values 2/3/4/5,
> + * as 4k SPs are not tracked (allowed to go unsync). In addition non-PAE
> + * paging has exactly one upper level, making level completely redundant
> + * when has_4_byte_gpte=1.
> + *
> + * - on top of this, smep_andnot_wp and smap_andnot_wp are only set if
> + * cr0_wp=0, therefore these three bits only give rise to 5
> possibilities.
> + *
> + * Therefore, the maximum number of possible upper-level shadow pages for a
> + * single gfn is a bit less than 2^13.
> + */
> +struct kvm_mmu_page_role_arch {
> + u16 has_4_byte_gpte:1;
> + u16 quadrant:2;
> + u16 direct:1;
> + u16 access:3;
> + u16 efer_nx:1;
> + u16 cr0_wp:1;
> + u16 smep_andnot_wp:1;
> + u16 smap_andnot_wp:1;
> + u16 ad_disabled:1;
> + u16 guest_mode:1;
> + u16 passthrough:1;
> +};
> +
> +#endif /* !__ASM_KVM_MMU_TYPES_H */
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 0a819d40131a..ebcd7a0dabef 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -37,6 +37,8 @@
> #include
> #include
>
> +#include
> +
> #define __KVM_HAVE_ARCH_VCPU_DEBUGFS
>
> #define KVM_MAX_VCPUS 1024
> @@ -286,72 +288,6 @@ enum x86_intercept_stage;
>
> struct kvm_kernel_irq_routing_entry;
>
> -/*
> - * kvm_mmu_page_role tracks the properties of a shadow page (where shadow
> page
> - * also includes TDP pages) t
Re: [RFC PATCH 01/37] KVM: x86/mmu: Store the address space ID directly in kvm_mmu_page_role
On Fri, Dec 09, 2022, David Matlack wrote:
> On Fri, Dec 9, 2022 at 9:25 AM Oliver Upton wrote:
> >
> > On Fri, Dec 09, 2022 at 10:37:47AM +0800, Yang, Weijiang wrote:
> > > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > > > index 4d188f056933..f375b719f565 100644
> > > > --- a/arch/x86/kvm/mmu/mmu.c
> > > > +++ b/arch/x86/kvm/mmu/mmu.c
> > > > @@ -5056,7 +5056,7 @@ kvm_calc_cpu_role(struct kvm_vcpu *vcpu, const
> > > > struct kvm_mmu_role_regs *regs)
> > > > union kvm_cpu_role role = {0};
> > > > role.base.access = ACC_ALL;
> > > > - role.base.smm = is_smm(vcpu);
> > > > + role.base.as_id = is_smm(vcpu);
> > >
> > > I'm not familiar with other architectures, is there similar conception as
> > > x86 smm mode?
>
> The notion of address spaces is already existing architecture-neutral
> concept in KVM (e.g. see uses of KVM_ADDRESS_SPACE_NUM in
> virt/kvm/kvm_main.c), although SMM is the only use-case I'm aware of.
Yes, SMM is currently the only use-case.
> Architectures that do not use multiple address spaces will just leave
> as_id is as always 0.
My preference would be to leave .smm in x86's page role. IMO, defining multiple
address spaces to support SMM emulation was a mistake that should be contained
to
SMM, i.e. should never be used for any other feature. And with CONFIG_KVM_SMM,
even x86 can opt out.
For all potential use cases I'm aware of, SMM included, separate address spaces
are overkill. The SMM use case is to define a region of guest memory that is
accessible if and only if the vCPU is operating in SMM. Emulating something
like
TrustZone or EL3 would be quite similar. Ditto for Intel's TXT Private Space
(though I can't imagine KVM ever emulating TXT :-) ).
Using separate address spaces means that userspace needs to define the
overlapping
GPA areas multiple times, which is inefficient for both memory and CPU usage.
E.g. for SMM, userspace needs to redefine all of "regular" memory for SMM in
addition to memory that is SMM-only. And more bizarelly, nothing prevents
userspace
from defining completely different memslot layouts for each address space, which
might may not add complexity in terms of code, but does make it more difficult
to
reason about KVM behavior at the boundaries between modes.
Unless I'm missing something, e.g. a need to map GPAs differently for SMM vs.
non-SMM, SMM could have been implemented with a simple flag in a memslot to mark
the memslot as SMM-only. Or likely even better, as an overlay to track
attributes,
e.g. similar to how private vs. shared memory will be handled for protected VMs.
That would be slightly less efficient for memslot searches for use cases where
all
memory is mutually exclusive, but simpler and more efficient overall.
And separate address spaces become truly nasty if the CPU can access multiple
protected regions, e.g. if the CPU can access type X and type Y at the same
time,
then there would need to be memslots for "regular", X, Y, and X+Y.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH 1/3] arm: pmu: Fix overflow checks for PMUv3p5 long counters
Hi, On Mon, Dec 12, 2022 at 09:05:02AM +, Marc Zyngier wrote: > Alex, > > On Sun, 11 Dec 2022 11:40:39 +, > Alexandru Elisei wrote: > > > > A simple "hey, you're wrong here, the PMU extensions do not follow the > > principles of the ID scheme for fields in ID registers" would have > > sufficed. > > This is what I did, and saved you the hassle of looking it up. The comment was about how you went about it, not about proving someone wrong. As expressive as it might be, I don't think that calling someone's suggestion "ludicrous" from the position of authority associated with being a maintainer is constructive; and can also be interpreted as a personal attack (you used **your** suggestion, not **this** suggestion). I didn't interpret it that way, just saying that it can be. > > > Guess you never made a silly mistake ever, right? > > It's not so much about making a silly mistake. I do that all the time. > But it is about the way you state these things, and the weight that > your reviews carry. You're a trusted reviewer, with a lot of > experience, and posting with an @arm.com address: what you say in a > public forum sticks. When you assert that the author is wrong, they > will take it at face value. This is how I stated things: "Hm... in the Arm ARM it says that counters are 64-bit if PMUv3p5 is implemented. But it doesn't say anywhere that versions newer than p5 are required to implement PMUv3p5." -> patently false, easily provable with the Arm ARM and by logic (as you did). My entire argument was based on this, so once this has been proven false, I would say that the rest of my argument falls apart. "For example, for PMUv3p7, it says that the feature is mandatory in Arm8.7 implementations. **My interpretation** of that is that it is not forbidden for an implementer to cherry-pick this version on older versions of the architecture where PMUv3p5 is not implemented." -> emphasis on the "my interpretation"; also easy to prove false because PMUv3p5+ is required to implement PMUv3p5, as per the architecture. "**Maybe** the check should be pmu.version == ID_DFR0_PMU_V3_8_5, to match the counter definitions in the architecture?" -> emphasis on the "maybe", and the question mark at the end. My intention wasn't to dictate something, my intention was to have a conversation about the patch, with the mindset that I might be wrong. What made you get the idea that I was asserting that the author is wrong? Where by "asserting the author is wrong" I understand framing my comment in such a way as to leave no room for further discussions. Or did you mean something else by that? Or, to put it another way, what about the way I stated things could have been done better (other than not being wrong, obviously)? > > > Otherwise, good job encouraging people to help review KVM/arm64 patches ;) > > What is the worse: no review? or a review that spreads confusion? > Think about it. I'm all for being nice, but I will call bullshit when That wasn't about calling people out on their mistakes. I was saying that the way you "call bullshit", as you put it, might be a put off for some people. Call me naive, but I like to think that not everyone that comments on a patch does it because they have to. > I see it asserted by people with a certain level of authority. > > And I've long made up my mind about the state of the KVM/arm64 review > process -- reviews rarely come from people who have volunteered to do > so, but instead from those who have either a vested interest in it, or > an ulterior motive. Hey ho... I genuinely don't know what to make of this. I can't even tell if it's directed at me or not. Thanks, Alex ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 7/7] KVM: selftests: Avoid infinite loop if ucall_alloc() fails
On 12/9/22 22:03, Sean Christopherson wrote:
From: Sean Christopherson
Date: Fri, 9 Dec 2022 12:55:44 -0800
Subject: [PATCH] KVM: selftests: Use magic value to signal ucall_alloc()
failure
Use a magic value to signal a ucall_alloc() failure instead of simply
doing GUEST_ASSERT(). GUEST_ASSERT() relies on ucall_alloc() and so a
failure puts the guest into an infinite loop.
Use -1 as the magic value, as a real ucall struct should never wrap.
Reported-by: Oliver Upton
Signed-off-by: Sean Christopherson
---
tools/testing/selftests/kvm/lib/ucall_common.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/ucall_common.c
b/tools/testing/selftests/kvm/lib/ucall_common.c
index 0cc0971ce60e..2f0e2ea941cc 100644
--- a/tools/testing/selftests/kvm/lib/ucall_common.c
+++ b/tools/testing/selftests/kvm/lib/ucall_common.c
@@ -4,6 +4,8 @@
#include "linux/bitmap.h"
#include "linux/atomic.h"
+#define GUEST_UCALL_FAILED -1
+
struct ucall_header {
DECLARE_BITMAP(in_use, KVM_MAX_VCPUS);
struct ucall ucalls[KVM_MAX_VCPUS];
@@ -41,7 +43,8 @@ static struct ucall *ucall_alloc(void)
struct ucall *uc;
int i;
- GUEST_ASSERT(ucall_pool);
+ if (!ucall_pool)
+ goto ucall_failed;
for (i = 0; i < KVM_MAX_VCPUS; ++i) {
if (!test_and_set_bit(i, ucall_pool->in_use)) {
@@ -51,7 +54,13 @@ static struct ucall *ucall_alloc(void)
}
}
- GUEST_ASSERT(0);
+ucall_failed:
+ /*
+* If the vCPU cannot grab a ucall structure, make a bare ucall with a
+* magic value to signal to get_ucall() that things went sideways.
+* GUEST_ASSERT() depends on ucall_alloc() and so cannot be used here.
+*/
+ ucall_arch_do_ucall(GUEST_UCALL_FAILED);
return NULL;
}
@@ -93,6 +102,9 @@ uint64_t get_ucall(struct kvm_vcpu *vcpu, struct ucall *uc)
addr = ucall_arch_get_ucall(vcpu);
if (addr) {
+ TEST_ASSERT(addr != (void *)GUEST_UCALL_FAILED,
+ "Guest failed to allocate ucall struct");
+
memcpy(uc, addr, sizeof(*uc));
vcpu_run_complete_io(vcpu);
} else {
base-commit: dc2efbe4813e0dc4368779bc36c5f0e636cb8eb2
--
Queued, thanks.
Paolo
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 0/7] KVM: selftests: Fixes for ucall pool + page_fault_test
On 12/9/22 02:52, Oliver Upton wrote: The combination of the pool-based ucall implementation + page_fault_test resulted in some 'fun' bugs. As has always been the case, KVM selftests is a house of cards. Small series to fix up the issues on kvm/queue. Patches 1-2 can probably be squashed into Paolo's merge resolution, if desired. Tested on Ampere Altra and a Skylake box, since there was a decent amount of munging in architecture-generic code. v1 -> v2: - Collect R-b from Sean (thanks!) - Use a common routine for split and contiguous VA spaces, with commentary on why arm64 is different since we all get to look at it now. (Sean) - Don't identity map the ucall MMIO hole - Fix an off-by-one issue in the accounting of virtual memory, discovered in fighting with #2 - Fix an infinite loop in ucall_alloc(), discovered fighting with the ucall_init() v. kvm_vm_elf_load() ordering issue Queued 3+5, thanks. Paolo ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 4/7] KVM: selftests: Correctly initialize the VA space for TTBR0_EL1
On 12/9/22 02:53, Oliver Upton wrote:
@@ -268,17 +305,17 @@ struct kvm_vm *vm_create(enum vm_guest_mode mode)
#ifdef __aarch64__
if (vm->pa_bits != 40)
vm->type = KVM_VM_TYPE_ARM_IPA_SIZE(vm->pa_bits);
+
+ /* selftests use TTBR0 only, meaning there is a single VA region. */
+ vm->has_split_va_space = false;
+#else
+ vm->has_split_va_space = true;
#endif
Even though there happens to be already a suitable #ifdef, I don't
really like them and don't think there should be a new bool unless
something else uses it.
However, the new comment is very useful, so I added it to kvm_util.c as
follows:
/testing/selftests/kvm/lib/kvm_util.c
b/tools/testing/selftests/kvm/lib/kvm_util.c
index 759a45540108..56d5ea949cbb 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -186,6 +186,15 @@ const struct vm_guest_mode_params vm_guest_mode_params[] =
{
_Static_assert(sizeof(vm_guest_mode_params)/sizeof(struct
vm_guest_mode_params) == NUM_VM_MODES,
"Missing new mode params?");
+/*
+ * Initializes vm->vpages_valid to match the canonical VA space of the
+ * architecture.
+ *
+ * The default implementation is valid for architectures which split the
+ * range addressed by a single page table into a low and high region
+ * based on the MSB of the VA. On architectures with this behavior
+ * the VA region spans [0, 2^(va_bits - 1)), [-(2^(va_bits - 1), -1].
+ */
__weak void vm_vaddr_populate_bitmap(struct kvm_vm *vm)
{
sparsebit_set_num(vm->vpages_valid,
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 0/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value
On Fri, 9 Dec 2022 16:44:45 +, James Clark wrote: > We noticed qemu failing to run because of an assert on our CI. I don't see > the issue anymore with > this fix. > > Applies to kvmarm/next (753d734f3f34) > > Thanks > > [...] Applied to fixes, thanks! [1/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value commit: aff234839f8b80ac101e6c2f14d0e44b236efa48 Cheers, M. -- Without deviation from the norm, progress is not possible. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH 1/3] arm: pmu: Fix overflow checks for PMUv3p5 long counters
Alex, On Sun, 11 Dec 2022 11:40:39 +, Alexandru Elisei wrote: > > A simple "hey, you're wrong here, the PMU extensions do not follow the > principles of the ID scheme for fields in ID registers" would have > sufficed. This is what I did, and saved you the hassle of looking it up. > Guess you never made a silly mistake ever, right? It's not so much about making a silly mistake. I do that all the time. But it is about the way you state these things, and the weight that your reviews carry. You're a trusted reviewer, with a lot of experience, and posting with an @arm.com address: what you say in a public forum sticks. When you assert that the author is wrong, they will take it at face value. > Otherwise, good job encouraging people to help review KVM/arm64 patches ;) What is the worse: no review? or a review that spreads confusion? Think about it. I'm all for being nice, but I will call bullshit when I see it asserted by people with a certain level of authority. And I've long made up my mind about the state of the KVM/arm64 review process -- reviews rarely come from people who have volunteered to do so, but instead from those who have either a vested interest in it, or an ulterior motive. Hey ho... M. -- Without deviation from the norm, progress is not possible. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
