The first attempt resulted in several "Free of address-of
expression (BAD_FREE)" issues, because of code which relies on the fact that
any pointer in the same page is ok to pass to unmap_domain_page()
Model this property to remove the issues.
Coverity IDs: 1135356 113536{0,1} 1401300 141809{0,1} 1438864
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
There are issues as reported by osstest when Xen is running nested on
itself and the L1 Xen is using x2APIC. While those are being
investigated, disable announcing the x2APIC feature in CPUID when nested
HVM mode is enabled.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
Gdbsx support in the hypervisor is rarely used and it is opening a
way for dom0 to modify the running hypervisor by very easy means.
Remove the possibility to read/write hypervisor memory, it was never
used by gdbsx.
Add a Kconfig option to control support of gdbsx. Default is on.
While at it correct a wrong comment in related code and remove dead
code.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
debugger_trap_entry() is not needed without CONFIG_CRASH_DEBUG, so only
include it if CONFIG_CRASH_DEBUG is defined.
While at it remove CONFIG_HAS_GDBSX as it can easily be replaced by
CONFIG_CRASH_DEBUG.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The hvm and pae parameters are a remnant of legacy migration. They have 0
passed in from libxl_stream_read.c's process_record(), and are discarded in
xc_domain_restore().
While dropping these, update the doxygen comment to be accurate, and simplify
the other hvm vs pv handling in xc_domain_restore().
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
The jmp after setting %cr0 is redundant with the following ljmp.
The CPUID to protect the jump to higher mappings was inserted due to an
abundance of caution/paranoia before Spectre was public. It doesn't usefully
protect against an attack, which is able to leak memory with one single
instruction's worth of onward speculation.
Only CPU Hotplug (if used at all) will use this path while guests are
executing. An attacker would have to be running and primed on an adjacent
thread while a hotplug event occurred, to gain one single data sample, and
have some other way of inferring that a hotplug event has occurred, which it
won't know directly.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
We will provide a header file for Hyper-V hypercalls.
No functional change.
Signed-off-by: Wei Liu <liuwe@microsoft.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
...if a specified domid is already in use.
XEN_DOMCTL_createdomain allows a domid to be specified by its caller and
will correctly fail if that domid is already in use. However the errno
returned in this case will be EINVAL, making it indistinguishable from
several other failures. Also a value of EINVAL does not seem appropriate
as the specified domid is valid [1] but just not (transiently) available.
[1] any invalid value passed in is ignored and causes Xen to choose an
unused and valid value.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
...for patches not (yet) upstream.
This patch is simply adding a comment to reserve save record number space
to avoid the risk of clashes between existent downstream changes made by
Amazon and future upstream changes which may be incompatible.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Reviewed-by: Wei Liu <wl@xen.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
Using memcpy() may result in multiple individual byte accesses
(dependening how memcpy() is implemented and how the resulting insns,
e.g. REP MOVSB, get carried out in hardware), which isn't what we
want/need for carrying out guest insns as correctly as possible. Fall
back to memcpy() only for accesses not 2, 4, or 8 bytes in size.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
cr-daily-branch ought to be called via cr-for-branches so that we take
the lock. Otherwise strange things can occur if cron runs
cr-daily-branch in the same directory - in particular, it will be
likely to update the osstest revision, breaking everything.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Most users pass a vcpu pointer, and only stopmachine_action() takes an integer
parameter. Switch to using void * to substantially reduce the number of
explicit casts.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien@xen.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
The use of return_reg() assumes ARM's 32bit ABI. Therefore, a failure such as
-EINVAL will appear as a large positive number near 4 billion to a 64bit ARM
guest which happens to use continue_hypercall_on_cpu().
Introduce a new arch_hypercall_tasklet_result() hook which is implemented by
both architectures, and drop the return_reg() macros. This logic will be
extended in a later change to make continuations out of the tasklet work.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <julien@xen.org>
Like %cr2 for pagefaults, %dr6 contains ancillary information for debug
exceptions, and needs similar handling.
For xendevicemodel_inject_event(), no ABI change is needed (although an API
one would be ideal). Switch from 'cr2' to 'extra' in variable names which
don't constitute an API change, and update the documentation to match.
For the monitor interface, vm_event_debug needs extending with a pending_dbg
field. This shall behave like the VT-x PENDING_DBG control. Extend
hvm_monitor_debug() and for now, always pass in 0 - this will be fixed
eventually, when other hypervisor bugfixes are complete.
While modifying hvm_monitor_debug(), take the opportunity to correct trap type
and instruction length from unsigned long to unsigned int, as they are both
tiny values.
Finally, adjust xen-access.c to the new expectations. Introspection tools
intercepting debug exceptions should mirror the new pending_dbg field into
xendevicemodel_inject_event() for %dr6 to be processed correctly for the
guest.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Petre Pircalabu <ppircalabu@bitdefender.com>
Despite as suggested in c/s 685e922d6f, not all HVM_PARAMs are handled
in the same way. HVM_PARAM_PAE_ENABLED is a toolstack-only value, and
the xc_cpuid_apply_policy() used to be the only consumer.
Reinstate the old behaviour (mad as it is) to avoid regressions.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
... to separate code from data. In particular, trampoline_realmode_entry's
write to trampoline_cpu_started clobbers the I-cache line containing
trampoline_protmode_entry, which won't be great for AP startup performance.
Reformat the comments for trampoline_gdt to reduce their volume.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
get_page_light()'s use of cmpxchg() is a full barrier already anyway.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
All that really matters is whether writability of a page changes; in
particular e.g. page table -> page table (but different levels)
transitions do not require unmapping the page from the IOMMU again.
Note that the XSA-288 fix did arrange for PGT_none pages not needing
special consideration here.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
There's been effectively no use of the field for HVM.
Also shrink the field to unsigned int, even if this doesn't immediately
yield any space benefit for the structure itself. The resulting 32-bit
padding slot can eventually be used for some other field. The change in
size makes accesses slightly more efficient though, as no REX.W prefix
is going to be needed anymore on the respective insns.
Mirror the HVM side change here (dropping of setting the field to
VGCF_online) also to Arm, on the assumption that it was cloned like
this originally. VGCF_online really should simply and consistently be
the guest view of the inverse of VPF_down, and hence needs representing
only in the get/set vCPU context interfaces.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
This is an architectural definition, so move it to x86-defns.h and add
an X86_ prefix. This in particular allows removing the inclusion of
irq_vectors.h by virtually every source file, due to irq.h and
hvm/vmx/vmcs.h having needed to include it: Changes to IRQ vector usage
shouldn't really trigger full rebuilds.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The legacy vectors have been actively used on CPU 0 only. CPUs not
sharing vector space with CPU 0 can easily re-use them, slightly
increasing the relatively scarce resource of total vectors available in
the system. As a result the legacy vector range simply becomes a
sub-range of the dynamic one, with an extra check performed in
_assign_irq_vector() (we can't rely on the
"per_cpu(vector_irq, new_cpu)[vector] >= 0" check in the subsequent
loop, as we need to also exclude vectors of disabled legacy IRQs).
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
There's no reason to have the PIC vectors (which are typically entirely
unused on 64-bit systems anyway) right below the high priority ones. Put
them in the lowest possible range, and shift the dynamic vector range up
accordingly. This is to reduce the priority of PIC vectors in the LAPIC
vs all other ones.
Note that irq_move_cleanup_interrupt(), despite using
FIRST_DYNAMIC_VECTOR, does not get touched, as PIC interrupts aren't
movable.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
In 5655ce8b1e ("x86/IRQ: make internally used IRQs also honor the
pending EOI stack") it was mentioned that both the check_eoi_deferral
per-CPU variable and the cpu_has_pending_apic_eoi() were added just to
have as little impact on existing behavior as possible, to reduce the
risk of a last minute regression in 4.13.
Upon closer inspection, dropping the variable is an option only if all
callers of ->end() would assume the responsibility of also calling
flush_ready_eoi(). Therefore only drop the cpu_has_pending_apic_eoi()
guard now.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
This is for it to be next to do_IRQ(). Beyond the actual code movement
this
- drops the leading underscores,
- passes in desc and vector, rather than irq,
- flips the order of two ASSERT()s,
- changes i and sp to unsigned int,
- restricts the scope of d and sp,
- corrects style.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
This is to avoid forward declarations of static functions. Beyond the
actual code movement this does
- u8 -> uint8_t,
- convert to Xen style,
- drop unnecessary parentheses and alike,
- strip trailing white space.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The emulated RTC is synchronized with the PV wallclock; any write to the
RTC will update struct domain's 'time_offset_seconds' field and call
update_domain_wallclock().
However, the value of 'time_offset_seconds' is not preserved in any save
record and indeed, when the RTC save record is loaded, the CMOS values
will be updated based on an offset value which may or may not have been
set by the toolstack [1]. This may result in making bogus values available
to the guest and messing up any calculations done in the call to
alarm_timer_update() at the end of rtc_load().
This patch extends the RTC save record to contain an offset value, which
will be zero filled on load of an older record. The 'time_offset_secoonds'
field in struct domain is also modified into a 'time_offset' struct,
containing a 'seconds' field and a boolean 'set' field.
The code in rtc_load() then uses the new value in the save record to
update the value of struct domain's 'time_offset.seconds' unless
'time_offset.set' is true, which will only be the case if the toolstack has
already performed a XEN_DOMCTL_settimeoffset.
[1] There is currently no way for a toolstack to read the value of
'time_offset_seconds' from struct domain. In the past, any hope of
preservation of the value across a guest life-cycle operation was based
on relying on qemu-dm to write a value into xenstore whenever the RTC
was updated, in response to an IOREQ with type IOREQ_TYPE_TIMEOFFSET
being sent by Xen; see:
https://xenbits.xen.org/gitweb/?p=qemu-xen-traditional.git;a=blob;f=i386-dm/helper2.c#l457
but this behaviour was never forward-ported into upstream QEMU, which
completely ignores that IOREQ type.
In either case, nothing in xl or libxl ever samples the value of
RTC offset from xenstore so any offset adjustment to a non-zero value
performed by the guest (which in the case of Windows is highly likely
as it normally writes RTC in local time, whereas Xen maintains time in
UTC) is completely lost with the de-facto toolstack, and always has
been. Instead, PV drivers are relied upon to paper over this gaping
hole.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <julien@xen.org>
According to the Intel SDM, "virtualize x2APIC mode" and "virtualize
APIC accesses" can't be enabled at the same time, or else a
vm{launch/entry} failure will happen. This was seen when running Xen
nested and with x2APIC enabled:
(XEN) d3v0 VMLAUNCH error: 0x7
[...]
(XEN) *** Control State ***
(XEN) PinBased=0000003f CPUBased=b6a075fe SecondaryExec=000014fb
[...]
Fix this by making sure nvmx_update_secondary_exec_control clears the
incompatible bits from the host vmcs before merging it with the nested
vmcs.
This fixes a regression reported by osstest in the
test-amd64-amd64-qemuu-nested-intel job.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
x86 PVH is completely obsolete - it was intended for legacy PVH before that
idea was abandoned. There was an RFC series for ARM in 2015, but there is
plenty of outstanding work which hasn't been done yet.
No functional change. New types can be (re)introduced with the code which
actually implements them.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien@xen.org>
Acked-by: Wei Liu <wl@xen.org>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
This record is specific to x86, and should have had a prefix to being with.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
The numbers aren't referenced directly, and explicit numbering makes an
unnecesserily large diff when inserting something new in the middle.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
vcpu_guest_context_any_t is currently allocated on the stack, and copied from
a mutable buffer which is freed immediately after its use here. Mutate the
buffer in place instead of duplicating it.
The code is as it is due to how it was developed. Originally,
process_vcpu_basic() operated on a const pointer from the X86_VCPU_BASIC
record, but during upstreaming, the addition of Remus support required
buffering of X86_VCPU_BASIC records each checkpoint.
By the time process_vcpu_basic() runs, we are commited to completing state
restoration and unpausing the guest.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
A previous commit that removed Context.CheckOpen revealed
an ineffectual assignent to err in Context.Cpupoolinfo, as
there is no error return type.
Since it appears that the intent is to return an error here,
add an error return value to the function signature.
Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Remove the exported global context variable, 'Ctx.' Generally, it is
better to not export global variables for use through a Go package.
However, there are some exceptions that can be found in the standard
library.
Add a NewContext function instead, and remove the Open, IsOpen, and
CheckOpen functions as a result.
Also, comment-out an ineffectual assignment to 'err' inside the function
Context.CpupoolInfo so that compilation does not fail.
Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Since there are many AVX{,2} insns having legacy SIMD counterparts, have
macros covering both in one go. This (imo) improves readability and helps
prepare for optionally disabling SIMD support in the emulator.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
It's used only by CASE_SIMD_ALL_FP(), which can equally well be
implemented in terms of CASE_SIMD_{PACKED,SCALAR}_FP().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Since there are many AVX{,2} insns having legacy MMX and SIMD
counterparts, have a macro covering all three in one go. This (imo)
improves readability (simply by the shrunk number of lines) and helps
prepare for optionally disabling MMX and SIMD support in the emulator.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Clang complains:
In file included from synic.c:15:
/builds/xen-project/xen/xen/include/asm/guest/hyperv-tlfs.h:900:18: error: field 'hv_vp_set' with variable sized type 'struct hv_vpset' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
struct hv_vpset hv_vp_set;
^
1 error generated.
/builds/xen-project/xen/xen/Rules.mk:198: recipe for target 'synic.o' failed
make[6]: *** [synic.o] Error 1
Comment out the last variable size array from hv_tlb_flush_ex to fix
clang builds.
Fixes: bbba482664 ("x86: import hyperv-tlfs.h from Linux")
Signed-off-by: Wei Liu <liuwe@microsoft.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Use hv_synic_sint in hyperv-tlfs.h instead.
This requires adding the missing "polling" member to hv_synic_sint.
No functional change.
Signed-off-by: Wei Liu <liuwe@microsoft.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Andrew Cooper