- Jun 26, 2024
-
-
George Dunlap authored
Signed-off-by:
George Dunlap <george.dunlap@cloud.com>
-
George Dunlap authored
Note that this trace is SVM-specific. Most HVM handler traces are shared between VMX and SVM because the underlying instruction set is largely the equivalent; but in this case, the instructions are different enough that there's no sensible way to share HVM handler traces between them. Keeping the target VMCB address should allow future analysis of which L2 vcpu within an L1 is running. Signed-off-by:
George Dunlap <george.dunlap@cloud.com>
-
George Dunlap authored
Basically this means adding VMEXIT strings for XSETBV exit, and adding the handlers and strings for them. Signed-off-by:
George Dunlap <george.dunlap@cloud.com>
-
George Dunlap authored
There are already "HVM handler" trace records for writing to XCRs in the context of an HVM guest. This trace is currently taken in hvmemul_write_xcr. However, both VMX and SVM vmexits call hvm_handle_xsetbv as a result of an XSETBV vmexit, and hvm_handle_xsetbv calls x86emul_write_xcr directly, bypassing the trace, resulting in no "HVM handler" trace record for that VMEXIT. For maximal DRY-ness, we would want hvm_handle_xsetbv to call hvmemul_write_xcr; but since the intent seems to be for hvmemul_* to be only accesible via hvm_emulate(), just duplicate the trace. Signed-off-by:
George Dunlap <george.dunlap@cloud.com> --- CC: Andrew Cooper <andrew.cooper@cloud.com> CC: Jan Beulich <jbeulich@suse.com> CC: Roger Pau Monne <roger.pau@cloud.com>
-
George Dunlap authored
There's a general issue with both PIO and MMIO reads (as detailed in the comment); do a work-around for now. Signed-off-by:
George Dunlap <george.dunlap@cloud.com>
-
George Dunlap authored
hvmemul_read_cr ends up being called fairly freqently during emulation; these are generally not necessary for understanding a trace, and make processing more complicated (because they show up as extra trace records within a vmexit/entry "arc" that must be classified). Leave the trace in write_cr, as any hypothetical writes to CRs *will* be necessary to understand traces. Signed-off-by:
George Dunlap <george.dunlap@cloud.com>
-
George Dunlap authored
Unfortunately I've forgotten exactly why I made this change. I suspect that there were other traces (like MMIO traces) which were being put before the NPF trace; but to understand the trace record you'd want to have the NPF information first. Signed-off-by:
George Dunlap <georg.dunlap@cloud.com>
-
George Dunlap authored
Commit 37f074a3 ("x86/msr: introduce guest_rdmsr()") introduced a function to combine the MSR_READ handling between PV and HVM. Unfortunately, by returning directly, it skipped the trace generation, leading to gaps in the trace record, as well as xenalyze errors like this: hvm_generic_postprocess: d2v0 Strange, exit 7c(VMEXIT_MSR) missing a handler Replace the `return` with `goto out`. Fixes: 37f074a3 ("x86/msr: introduce guest_rdmsr()") Signed-off-by:
George Dunlap <george.dunlap@cloud.com>
-
George Dunlap authored
Adding an HVM_TRAP trace record is redundant for EXCEPTION_AC: it adds trace volume without adding any information, and xenalyze already knows not to expect it. Remove it. Signed-off-by:
George Dunlap <george.dunlap@cloud.com>
-
George Dunlap authored
VMX combines all exceptions into a single VMEXIT exit reason, and so needs a separate HVM_HANDLER trace record (HVM_TRAP) to say which exception happened; but for a number of exceptions, no further information is put into the trace log. SVM, by contrast, has a separate VMEXIT exit reason for each exception vector, so HVM_TRAP would be redundant. NB that VMEXIT_EXCEPTION_DB doesn't have an HVM_HANDLER for SVM yet; but for VMX, there's a specific HVM_HANDLER trace record which includes more information; so SVM really should record information as well. Signed-off-by:
George Dunlap <george.dunlap@cloud.com>
-
George Dunlap authored
Generally speaking, a VMEXIT/VMENTRY cycle should have at least three trace records: the VMEXIT trace (which contains the processor-specific exit code), a more generic Xen-based Xen event (an HVM_HANDLER trace record), and a VMEXIT trace; and any given VMEXIT exit reason should only have a single HVM_HANDLER trace type. Having duplicate or missing HVM_HANDLER traces is generally indicative of a problem that's crept in in the hypervisor tracing scheme. This is property is checked in hvm_generic_postprocess(), and violations are flagged with a warning. In order to do this, when an HVM trace record that doesn't have a specific post-processor happens, information about the HVM trace record is stored in hvm_data->inflight.generic. Unfortunately, while the check was being done in all "modes", the relevant information was only being copied into inflight.generic in summary mode. This resulted in spurious warnings about missing HVM_HANDLER traces when running in dump mode. Since running in dump mode is often critical to understanding how the warnings came about, just collect the information always as well. That said, the data from the trace doesn't appear to be used by anyone; so to save some time, don't bother copying it. Signed-off-by:
George Dunlap <george.dunlap@cloud.com>
-
George Dunlap authored
On SVM, VMEXIT that occur in when an L1 is in non-root mode ("nested exits") are tagged with the TRC_HVM_NESTEDFLAG flag. Expand xenalyze to do basic handling of these records: - Refactor hvm_process to filter out both TRC_HVM_NESTEDFLAG and TRC_64_FLAG when deciding how to process - Create separate "trace volume" sub-categories for them (HVM_VOL_*), tweaking layout so things continue to align - Visually distinquish nested from non-nested vmexits and vmentries in dump mode At the moment, nested VMEXITs which are passed through to the L1 hypervisor won't have any HVM handlers; note in hvm_data when a vmexit was nested, and don't warn when no handlers occur. While here: - Expand one of the error messages with a bit more information - Replace repeated `ri->event & TRC_64_FLAG` instances with `flag64` - Remove a stray whitespace at the end of a line - Add a missing newline to a warning statement Signed-off-by:
George Dunlap <george.dunlap@cloud.com>
-
George Dunlap authored
The non-nested HVM code knows how to provide PCID functionality (non-zero values in the lower 12 bits of CR3 when running in 64-bit mode), but the nested code doesn't. If the L2 decides to use the PCID functionality, the L0 will fail the next L1 VMENTRY. Long term we definitely want to enable this feature, but for now, just hide it from guests when nested HVM is enabled. Signed-off-by:
George Dunlap <george.dunlap@cloud.com>
-
George Dunlap authored
NOTE: This patch is be partially superseded by Andrew Cooper's "x86: AMD CPUID handling improvements" series. Currently, the CPUID leaf for SVM features (extd 0xa.edx) is manually twiddled: - hvm_max_policy takes host_policy and clamps it to supported features (with some features unilaterally enabled because they're always emulated - hvm_default_policy is copied from there - When recalculate_policy() is called for a guest, if SVM is clear, then the entire leaf is zeroed out. Move to a mode where the extended features are off by default, and enabled when nested_virt is enabled. In cpufeatureset.h, define a new featureset word for the AMD SVM features, and declare all of the bits defined in x86/include/asm/hvm/svm/svm.h. Mark the ones we currently pass through to the "max policy" as HAP-only and optional. In cpu-policy.h, define FEATURESET_ead, and convert the un-named space in struct_cpu_policy into the appropriate union. FIXME: Do this in a prerequisite patch, and change all references to p->extd.raw[0xa]. Update x86_cpu_X_to_Y and Y_to_X to copy this into and out of the appropriate leaf. Populate this during boot in generic_identify(). Add the new featureset definition into libxl_cpuid.c. Update the code in calculate_hvm_max_policy() to do nothing with the "normal" CPUID bits, and use the feature bit to unconditionally enable VMCBCLEAN. FIXME Move this to a follow-up patch. In recalculate_cpuid_policy(), enable max_fs when nested_hvm() is true. Signed-off-by:
George Dunlap <george.dunlap@cloud.com> --- CC: Andrew Cooper <andrew.cooper3@citrix.com> CC: Jan Beulich <jbeulich@suse.com> CC: Roger Pau Monne <roger.pau@cloud.com>
-
Jan Beulich authored
Pull in the gcc14 build fix there. Signed-off-by:
Jan Beulich <jbeulich@suse.com> Reviewed-by:
Juergen Gross <jgross@suse.com> Release-Acked-by:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
- Jun 25, 2024
-
-
Alessandro Zucchelli authored
Rule 21.2 reports identifiers reserved for the C and POSIX standard libraries: or, and, not and xor are reserved identifiers because they constitute alternate spellings for the corresponding operators (they are defined as macros by iso646.h); however Xen doesn't use standard library headers, so there is no risk of overlap. This addresses violations arising from x86_emulate/x86_emulate.c, where label statements named as or, and and xor appear. No functional change. Signed-off-by:
Alessandro Zucchelli <alessandro.zucchelli@bugseng.com> Acked-by:
Stefano Stabellini <sstabellini@kernel.org> Release-Acked-by:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Federico Serafini authored
Rule 13.6 states that "The operand of the `sizeof' operator shall not contain any expression which has potential side effects". Define service B.UNEVALEFF as an extension of Rule 13.6 to check for unevalued side effects also for typeof and alignof operators. Update ECLAIR configuration to deviate uses of BUILD_BUG_ON and alternative_v?call[0-9] for both Rule 13.6 and B.UNEVALEFF. Add service B.UNEVALEFF to the accepted.ecl guidelines to check "violations" in the weekly analysis. Signed-off-by:
Federico Serafini <federico.serafini@bugseng.com> Signed-off-by:
Nicola Vetrini <nicola.vetrini@bugseng.com> Reviewed-by:
Stefano Stabellini <sstabellini@kernel.org> Release-Acked-By:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Jan Beulich authored
The odd DEFINE_XEN_GUEST_HANDLE(), inconsistent with all other similar constructs, should have caught my attention. Turns out it was needed for the build to succeed merely because the corresponding #ifndef had a typo. That typo in turn broke compat mode guests, by having query-size requests of theirs wire into the domain_crash() at the bottom of the switch(). Fixes: 8c3bb4d8 ("xen/gnttab: Perform compat/native gnttab_query_size check") Signed-off-by:
Jan Beulich <jbeulich@suse.com> Reviewed-by:
Roger Pau Monné <roger.pau@citrix.com> Release-Acked-by:
Oleksii Kurochko <Oleksii.kurochko@gmail.com>
-
Jan Beulich authored
When re-working them to avoid UB on guest address calculations, I failed to add explicit type checks in exchange for the implicit ones that until then had happened in assignments that were there anyway. Fixes: 43d5c5d5 ("xen: avoid UB in guest handle arithmetic") Signed-off-by:
Jan Beulich <jbeulich@suse.com> Acked-by:
Andrew Cooper <andrew.cooper3@citrix.com> Release-Acked-by:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Signed-off-by:
Anthony PERARD <anthony.perard@vates.tech> Acked-by:
Stefano Stabellini <sstabellini@kernel.org> Release-Acked-By:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
- Jun 24, 2024
-
-
Andrew Cooper authored
Commit 4c5d78a1 ("x86/pagewalk: Re-implement the pagetable walker") intentionally renamed guest_walk_tables()'s 'pfec' parameter to 'walk' because it's not a PageFault Error Code, despite the name of some of the constants passed in. Sadly the constants-cleanup I've been meaning to do since then still hasn't come to pass. Update the declaration to match, to placate MISRA. Fixes: 4c5d78a1 ("x86/pagewalk: Re-implement the pagetable walker") Signed-off-by:
Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by:
Stefano Stabellini <sstabellini@kernel.org> Release-Acked-by:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Alessandro Zucchelli authored
This addresses violations of MISRA C:2012 Rule 7.3 which states as following: the lowercase character `l' shall not be used in a literal suffix. The file common/unlzo.c defines the non-compliant constant LZO_BLOCK_SIZE with having a lowercase 'l'. It is now defined as '256*1024L'. No functional change. Signed-off-by:
Alessandro Zucchelli <alessandro.zucchelli@bugseng.com> Reviewed-by:
Stefano Stabellini <sstabellini@kernel.org> Release-Acked-by:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Federico Serafini authored
Add more accepted guidelines to the monitored set to check them at each commit. Signed-off-by:
Federico Serafini <federico.serafini@bugseng.com> Acked-by:
Stefano Stabellini <sstabellini@kernel.org> Release-Acked-by:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Federico Serafini authored
MISRA C Rule 5.5 states that "Identifiers shall be distinct from macro names". Update ECLAIR configuration to deviate: - macros expanding to their own name; - clashes between macros and non-callable entities; - clashes related to the selection of specific implementations of string handling functions. Signed-off-by:
Federico Serafini <federico.serafini@bugseng.com> Reviewed-by:
Stefano Stabellini <sstabellini@kernel.org> Release-Acked-by:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Federico Serafini authored
Update ECLAIR configuration to deviate some cases where not using the return value of a function is not dangerous. Signed-off-by:
Federico Serafini <federico.serafini@bugseng.com> Acked-by:
Stefano Stabellini <sstabellini@kernel.org> Release-Acked-by:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Michal Orzel authored
As a follow up to commit cb1ddafd ("xen/arm/static-shmem: Static-shmem should be direct-mapped for direct-mapped domains") add a check to request that both host and guest physical address must be supplied for direct mapped domains. Otherwise return an error to prevent unwanted behavior. Signed-off-by:
Michal Orzel <michal.orzel@amd.com> Fixes: 988f1c7e ("xen/arm: static-shmem: fix "gbase/pbase used uninitialized" build failure") Reviewed-by:
Julien Grall <jgrall@amazon.com> Release-Acked-By:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Andrew Cooper authored
Hide the legacy __ro_after_init definition in xen/cache.h for RISC-V, to avoid its use creeping in. Only mm.c needs adjusting as a consequence No functional change. Signed-off-by:
Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by:
Oleksii Kurochko <oleksii.kurochko@gmail.com> Release-Acked-by:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Andrew Cooper authored
This subop appears to have been missed from the compat checks. Fixes: 5ce8fafa ("Dynamic grant-table sizing") Signed-off-by:
Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by:
Jan Beulich <jbeulich@suse.com> Release-Acked-By:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Andrew Cooper authored
... with a C local to avoid ambiguities over _ and - as separators. Also adjust arch-x86/xen.h which is out-of-order relative to the other arch-x86/ files. Signed-off-by:
Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by:
Stefano Stabellini <sstabellini@kernel.org> Release-Acked-By:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Andrew Cooper authored
* Fix tabs/spaces mismatch for certain rows * Insert lines between header files to improve legibility Signed-off-by:
Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by:
Stefano Stabellini <sstabellini@kernel.org> Release-Acked-By:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
The emulation_count field is set only conditionally right now. Convert all field setting to an initializer, thus guaranteeing that field to be set to 0 (default initialized) when GUEST_PAGING_LEVELS != 3. Rework trace_shadow_emulate() to be consistent with the other trace helpers. Coverity-ID: 1598430 Fixes: 9a86ac1a ("xentrace 5/7: Additional tracing for the shadow code") Signed-off-by:
Jan Beulich <jbeulich@suse.com> Signed-off-by:
Andrew Cooper <andrew.cooper3@citrix.com> Acked-by:
Roger Pau Monné <roger.pau@citrix.com> Acked-by:
Jan Beulich <jbeulich@suse.com> Release-acked-by:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Andrew Cooper authored
sh_trace_gfn_va() is very similar to sh_trace_gl1e_va(), and a rather shorter name than trace_shadow_emulate_other(). It's only referenced in CONFIG_HVM=y builds, so give it a __maybe_unused to placate randconfig builds. No functional change. Signed-off-by:
Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by:
Jan Beulich <jbeulich@suse.com> Release-Acked-By:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Andrew Cooper authored
trace_shadow_fixup() and trace_not_shadow_fault() both write out identical trace records. Reimplement them in terms of a common sh_trace_gl1e_va(). No functional change. Signed-off-by:
Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by:
Jan Beulich <jbeulich@suse.com> Release-Acked-By:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Andrew Cooper authored
The ((GUEST_PAGING_LEVELS - 2) << 8) expression in the event field is common to all shadow trace events, so introduce sh_trace() as a very thin wrapper around trace(). Then, rename trace_shadow_gen() to sh_trace_va() to better describe what it is doing, and to be more consistent with later cleanup. No functional change. Signed-off-by:
Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by:
Jan Beulich <jbeulich@suse.com> Release-Acked-By:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
Andrew Cooper authored
`xl devd` has been observed leaking /var/log/xldevd.log into children. Note this is specifically safe; dup2() leaves O_CLOEXEC disabled on newfd, so after setting up stdout/stderr, it's only the logfile fd which will close on exec(). Link: https://github.com/QubesOS/qubes-issues/issues/8292 Reported-by:
Demi Marie Obenour <demi@invisiblethingslab.com> Signed-off-by:
Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by:
Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Reviewed-by:
Demi Marie Obenour <demi@invisiblethingslab.com> Acked-by:
Anthony PERARD <anthony.perard@vates.tech> Release-Acked-by:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
- Jun 21, 2024
-
-
When the left shift amount is up to 31, the shifted quantity wants to be of unsigned int (or wider) type. While there also adjust types: get doesn't alter the array and returns a boolean, while both don't really accept negative "nr". Drop a stray blank each as well. Signed-off-by:
Jan Beulich <jbeulich@suse.com> Reviewed-by:
Andrew Cooper <andrew.cooper3@citrix.com> Release-Acked-By:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
There exists bitshifts in the IOAPIC code where signed integers are shifted to the left by up to 31 bits, which is undefined behaviour. This patch fixes this by changing the integers from signed to unsigned. Signed-off-by:
Matthew Barnes <matthew.barnes@cloud.com> Reviewed-by:
Jan Beulich <jbeulich@suse.com> Reviewed-by:
Andrew Cooper <andrew.cooper3@citrix.com> Release-Acked-By:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
- Jun 20, 2024
-
-
Jan Beulich authored
As was made noticeable by the last of the commits referenced below, using a fixed-size type for such purposes is not only against ./CODING_STYLE, but can lead to actual issues. Switch to using size_t instead, thus also allowing calculations to be lighter-weight in 32-bit builds. No functional change for 64-bit builds. Link: https://gitlab.com/xen-project/xen/-/jobs/7136417308 Fixes: b145b4a3 ("livepatch: Handle arbitrary size names with the list operation") Fixes: 5083e0ff ("livepatch: Add metadata runtime retrieval mechanism") Fixes: 43d5c5d5 ("xen: avoid UB in guest handle arithmetic") Reported-by:
Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by:
Jan Beulich <jbeulich@suse.com> Reviewed-by:
Ross Lagerwall <ross.lagerwall@citrix.com> Release-Acked-by:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
fixup_irqs() is used to evacuate interrupts from to be offlined CPUs. Given the CPU is to become offline, the normal migration logic used by Xen where the vector in the previous target(s) is left configured until the interrupt is received on the new destination is not suitable. Instead attempt to do as much as possible in order to prevent loosing interrupts. If fixup_irqs() is called from the CPU to be offlined (as is currently the case for CPU hot unplug) attempt to forward pending vectors when interrupts that target the current CPU are migrated to a different destination. Additionally, for interrupts that have already been moved from the current CPU prior to the call to fixup_irqs() but that haven't been delivered to the new destination (iow: interrupts with move_in_progress set and the current CPU set in ->arch.old_cpu_mask) also check whether the previous vector is pending and forward it to the new destination. This allows us to remove the window with interrupts enabled at the bottom of fixup_irqs(). Such window wasn't safe anyway: references to the CPU to become offline are removed from interrupts masks, but the per-CPU vector_irq[] array is not updated to reflect those changes (as the CPU is going offline anyway). Signed-off-by:
Roger Pau Monné <roger.pau@citrix.com> Reviewed-by:
Jan Beulich <jbeulich@suse.com> Release-Acked-by:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-
After the following commit: 3bc14e4f ("tools/libs/light: Add vlan field to libxl_device_nic") xl list -l aborts with a double free error if a domain has at least one vif defined: $ sudo xl list -l free(): double free detected in tcache 2 Aborted Orginally, the vlan field was called vid and was defined as an integer. It was appropriate to call libxl__xs_read_checked() with gc passed as the string data was copied to a different variable. However, the final version uses a string data type and the call should have been changed to use NOGC instead of gc to allow that data to live past the gc controlled lifetime, in line with the other string fields. This patch makes the change to pass NOGC instead of gc and moves the new code to be next to the other string fields (fixing a couple of errant tabs along the way), as recommended by Jason. Fixes: 3bc14e4f ("tools/libs/light: Add vlan field to libxl_device_nic") Signed-off-by:
Leigh Brown <leigh@solinno.co.uk> Reviewed-by:
Jason Andryuk <jason.andryuk@amd.com> Acked-by:
Anthony PERARD <anthony.perard@vates.tech> Release-acked-by:
Oleksii Kurochko <oleksii.kurochko@gmail.com>
-