Nested Virt is the final special case in legacy CPUID handling. Pass the
(poorly named) nested_hvm setting down into xc_cpuid_apply_policy() to break
the semantic dependency on HVM_PARAM_NESTEDHVM.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
If the bitmap is used to represent domU pages, the amount of memory is
limited to 8TB due to the 32bit value. Adjust the code to use 64bit
values as input. All callers already use some form of 64bit as input,
so no further adjustment is required.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
QEMU as recently switch its build system to use meson and the
./configure step with meson is more restrictive that the step used to
be, most installation path wants to be within prefix, otherwise we
have this error message:
ERROR: The value of the 'datadir' option is '/usr/share/qemu-xen' which must be a subdir of the prefix '/usr/lib/xen'.
In order to workaround the limitation, we will set prefix to the same
one as for the rest of Xen installation, and set all the other paths.
For reference, a thread in qemu-devel:
"configure with datadir outside of --prefix fails with meson"
https://lore.kernel.org/qemu-devel/20200918133012.GH2024@perard.uk.xensource.com/t/
And an issue in meson:
"artificial limitation of directories (forced to be in prefix)"
https://github.com/mesonbuild/meson/issues/2561
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Tested-by: Paul Durrant <paul@xen.org>
Acked-by: Wei Liu <wl@xen.org>
The caller of writev_exact should be notified about malloc errors
when dealing with partial writes.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
This patch wraps the EFI OutputString() method so that they can be
called with const arguments. The OutputString method does not modify
its argument, although the prototype is missing const, so it is necssary
to cast away the const when calling it.
Signed-off-by: Trammell Hudson <hudson@trmm.net>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
There's no other path causing a terminal unlink_pirq_port() to be called
(evtchn_bind_vcpu() relinks it right away) and hence _if_ pirq can
indeed be NULL when closing the port, list corruption would occur when
bypassing the unlink (unless the structure never gets linked again). As
we can't come here after evtchn_destroy() anymore, (late) domain
destruction also isn't a reason for a possible exception, and hence the
only alternative looks to be that the check was pointless in the first
place. While I haven't observed the case, from code inspection I'm far
from sure I can exclude this being possible, so it feels more safe to
re-arrange the code instead.
Fixes: c24536b636 ("replace d->nr_pirqs sized arrays with radix tree")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Before and after XSA-342 there has been an asymmetry in how not really
usable ports get treated in do_poll(): Ones beyond a certain boundary
(max_evtchns originally, valid_evtchns subsequently) did get refused
with -EINVAL, while lower ones were accepted despite there potentially
being no way to wake the vCPU again from its polling state. Arrange to
also honor evtchn_usable() output in the decision.
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Dario Faggioli <dfaggioli@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Callers have no business knowing the state of the Xen end of an event
channel.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Paul Durrant <Paul@xen.org>
is_pv_32bit_domain() is an expensive predicate, but isn't used for speculative
safety in this case. Swap to checking the Long Mode bit in the CPUID policy,
which is the architecturally correct behaviour.
is_canonical_address() is currently moderately simple, but it will become more
complicated when 5-level support is added. Rearrange write_msr() to collapse
the common checks.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Opencode the fs/gs helpers, as the optimiser is unable to rearrange the logic
down to a single X86_CR4_FSGSBASE check. This removes several jumps and
creates bigger basic blocks.
In load_segments(), optimise GS base handling substantially. The call to
svm_load_segs() already needs gsb/gss the correct way around, so hoist the
logic for the later path to use it as well. Swapping the inputs in GPRs is
far more efficient than using SWAPGS.
Previously, there was optionally one SWAPGS from the user/kernel mode check,
two SWAPGS's in write_gs_shadow() and two WRGSBASE's. Updates to GS (4 or 5
here) in quick succession stall all contemporary pipelines repeatedly. (Intel
Core/Xeon pipelines have segment register renaming[1], so can continue to
speculatively execute with one GS update in flight. Other pipelines cannot
have two updates in flight concurrently, and must stall dispatch of the second
until the first has retired.)
Rewrite the logic to have exactly two WRGSBASEs and one SWAPGS, which removes
two stalles all contemporary processors.
Although modest, the resulting delta is:
add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-106 (-106)
Function old new delta
paravirt_ctxt_switch_from 235 198 -37
context_switch 3582 3513 -69
in a common path.
[1] https://software.intel.com/security-software-guidance/insights/deep-dive-intel-analysis-speculative-behavior-swapgs-and-segment-registers
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Split into two functions. Passing a load of zeros in results in unnecessary
caller setup code.
Update the prefetching comment to note that the main point is the TLB fill.
Reorder the writes in svm_load_segs() to access the VMCB fields in ascending
order, which gets better next-line prefetch behaviour out of hardware. Update
the prefetch instruction to match.
The net delta is:
add/remove: 1/0 grow/shrink: 0/2 up/down: 38/-39 (-1)
Function old new delta
svm_load_segs_prefetch - 38 +38
__context_switch 967 951 -16
svm_load_segs 291 268 -23
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
To match the read side which is already split out. A future change will want
to use them.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
They are currently named after the FSGSBASE instructions, but are not thin
wrappers around said instructions, and therefore do not accurately reflect the
logic they perform, especially when it comes to functioning safely in non
FSGSBASE context.
Rename them to {read,write}_{fs,gs}_base() to avoid confusion.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
We can just as well specify the CRn structure fields directly in the
asm()s, just like done for all other ones.
Also drop the unecessary "volatile" from the asm()-s: There are no
relevant hidden side effects.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
This keeps at least gcc 10 from generating a separate function instance
in common/kexec.o alongside the inlining of the function in its sole
caller. I also think putting the address of the actual code storing the
registers is a better indication to consumers than that of an otherwise
unreferenced function.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
When storing all (data) segment registers in one go, prefer writing the
selector values directly to memory (as opposed to read_sreg()).
Also move the single register variant into the regs.h, dropping the
unecessary "volatile" from the asm() (there are no hidden side effects).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
SMMUv{1, 2} are both marked as security supported, so we would
technically have to issue an XSA for any IOMMU security bug.
However, at the moment, device passthrough is not security supported
on Arm and there is no plan to change that in the next few months.
Therefore, mark Arm SMMUv{1, 2} as supported but not security supported.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Some callers of vcpu_pause() will expect to access the latest vcpu
context when the function returns (see XENDOMCTL_{set,get}vcpucontext}.
However, the latest vCPU context can only be observed after
v->is_running has been observed to be false.
As there is no memory barrier instruction generated, a processor could
try to speculatively access the vCPU context before it was observed.
To prevent the corruption of the vCPU context, we need to insert a
memory barrier instruction after v->is_running is observed and before
the context is accessed. This barrier is added in sync_vcpu_execstate()
as it seems to be the place where we expect the synchronization to
happen.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Fix the resume path to load the shadow stack pointer from saved_ssp (not
saved_rsp), to match what suspend path does.
Fixes: 633ecc4a7c ("x86/S3: Save and restore Shadow Stack configuration")
Backport: 4.14
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Remove the code to compose a MSI message based on the information from
the MSI registers and the data in the interrupt remapping table.
Since the removal of read_msi_msg and its user there's no longer a
need for such code, as the last written (untranslated) MSI message is
cached internally by Xen.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Later additions look to have been put at the end, with MEM_ACCESS*
somewhere in the middle. Re-sort this part of the file, in the hope that
future additions will be made noticing the intentions here.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Inspired by some of Trammell's suggestions, this harvests some low
hanging fruit, without needing to be concerned about the definitions of
the EFI interfaces themselves.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Wei Liu <wl@xen.org>
Acked-by: Julien Grall <jgrall@amazon.com>
A 64bit IRET can restore NT - the faulting case is when NT is set in the live
flags. This change had an unintended consequence of causing the NT flag to
spontaneously disappear from guest context whenever a interrupt/exception
occurred.
In combination with a SYSENTER which sets both TF and NT, Xen's handling of
the #DB exceptions clears NT before it is even recorded suitably in the guest
kernel's view of what userspace was doing.
Reported-by: Andy Lutomirski <luto@kernel.org>
Fixes: 0e47f92b0 ("x86: force EFLAGS.IF on when exiting to PV guests")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
It is a matter of guest kernel policy what to do with offending userspace, and
terminating said userspace may not be the action chosen.
Linux explicitly tolerates this case.
Reported-by: Andy Lutomirski <luto@kernel.org>
Fixes: fdac951560 ("x86: clear EFLAGS.NT in SYSENTER entry path")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In addition, this removes the unqualified 0/1 passed to msi_data_reg()
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
At the moment, Xen will stop processing the Device Tree if a memory
bank is empty (size == 0).
Unfortunately, some of the Device Tree (such as on Colibri imx8qxp)
may contain such a bank. This means Xen will not be able to boot
properly.
Relax the check to just ignore the banks. FWIW this also seems to be the
behavior adopted by Linux.
Reported-by: Daniel Wagner <Daniel.Wagner2@itk-engineering.de>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Mark Renesas IPMMU-VMSA as "Supported, not security supported"
and remove dependencies on CONFIG_EXPERT.
We can't treat the IOMMU driver as "Supported" since the device
passthrough feature is not security supported on Arm today.
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Like for evtchn_destroy() looping over all possible event channels to
close them can take a significant amount of time. Unlike done there, we
can't alter domain properties (i.e. d->valid_evtchns) here. Borrow, in a
lightweight form, the paging domctl continuation concept, redirecting
the continuations to different sub-ops. Just like there this is to be
able to allow for predictable overall results of the involved sub-ops:
Racing requests should either complete or be refused.
Note that a domain can't interfere with an already started (by a remote
domain) reset, due to being paused. It can prevent a remote reset from
happening by leaving a reset unfinished, but that's only going to affect
itself.
This is part of XSA-344.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Especially closing of fully established interdomain channels can take
quite some time, due to the locking involved. Therefore we shouldn't
assume we can clean up still active ports all in one go. Besides adding
the necessary preemption check, also avoid pointlessly starting from
(or now really ending at) 0; 1 is the lowest numbered port which may
need closing.
Since we're now reducing ->valid_evtchns, free_xen_event_channel(),
and (at least to be on the safe side) notify_via_xen_event_channel()
need to cope with attempts to close / unbind from / send through already
closed (and no longer valid, as per port_is_valid()) ports.
This is part of XSA-344.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Neither d->evtchn_port_ops nor max_evtchns(d) may be used in an entirely
lock-less manner, as both may change by a racing evtchn_reset(). In the
common case, at least one of the domain's event lock or the per-channel
lock needs to be held. In the specific case of the inter-domain sending
by evtchn_send() and notify_via_xen_event_channel() holding the other
side's per-channel lock is sufficient, as the channel can't change state
without both per-channel locks held. Without such a channel changing
state, evtchn_reset() can't complete successfully.
Lock-free accesses continue to be permitted for the shim (calling some
otherwise internal event channel functions), as this happens while the
domain is in effectively single-threaded mode. Special care also needs
taking for the shim's marking of in-use ports as ECS_RESERVED (allowing
use of such ports in the shim case is okay because switching into and
hence also out of FIFO mode is impossihble there).
As a side effect, certain operations on Xen bound event channels which
were mistakenly permitted so far (e.g. unmask or poll) will be refused
now.
This is part of XSA-343.
Reported-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
... in order for send_guest_{global,vcpu}_virq() to be able to make use
of it.
This is part of XSA-343.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
While the function closes all ports, it does so without holding any
lock, and hence racing requests may be issued causing new ports to get
opened. This would have been problematic in particular if such a newly
opened port had a port number above the new implementation limit (i.e.
when switching from FIFO to 2-level) after the reset, as prior to
"evtchn: relax port_is_valid()" this could have led to e.g.
evtchn_close()'s "BUG_ON(!port_is_valid(d2, port2))" to trigger.
Introduce a counter of active ports and check that it's (still) no
larger then the number of Xen internally used ones after obtaining the
necessary lock in evtchn_reset().
As to the access model of the new {active,xen}_evtchns fields - while
all writes get done using write_atomic(), reads ought to use
read_atomic() only when outside of a suitably locked region.
Note that as of now evtchn_bind_virq() and evtchn_bind_ipi() don't have
a need to call check_free_port().
This is part of XSA-343.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Julien Grall <jgrall@amazon.com>
The recording of d->max_evtchns in evtchn_2l_init(), in particular with
the limited set of callers of the function, is insufficient. Neither for
PV nor for HVM guests the bitness is known at domain_create() time, yet
the upper bound in 2-level mode depends upon guest bitness. Recording
too high a limit "allows" x86 32-bit domains to open not properly usable
event channels, management of which (inside Xen) would then result in
corruption of the shared info and vCPU info structures.
Keep the upper limit dynamic for the 2-level case, introducing a helper
function to retrieve the effective limit. This helper is now supposed to
be private to the event channel code. The used in do_poll() and
domain_dump_evtchn_info() weren't consistent with port uses elsewhere
and hence get switched to port_is_valid().
Furthermore FIFO mode's setup_ports() gets adjusted to loop only up to
the prior ABI limit, rather than all the way up to the new one.
Finally a word on the change to do_poll(): Accessing ->max_evtchns
without holding a suitable lock was never safe, as it as well as
->evtchn_port_ops may change behind do_poll()'s back. Using
port_is_valid() instead widens some the window for potential abuse,
until we've dealt with the race altogether (see XSA-343).
This is XSA-342.
Reported-by: Julien Grall <jgrall@amazon.com>
Fixes: 48974e6ce5 ("evtchn: use a per-domain variable for the max number of event channels")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Julien Grall <jgrall@amazon.com>
While the allocation of a bucket is always performed with the per-domain
lock, the bucket may be accessed without the lock taken (for instance, see
evtchn_send()).
Instead such sites relies on port_is_valid() to return a non-zero value
when the port has a struct evtchn associated to it. The function will
mostly check whether the port is less than d->valid_evtchns as all the
buckets/event channels should be allocated up to that point.
Unfortunately a compiler is free to re-order the assignment in
evtchn_allocate_port() so it would be possible to have d->valid_evtchns
updated before the new bucket has finish to allocate.
Additionally on Arm, even if this was compiled "correctly", the
processor can still re-order the memory access.
Add a write memory barrier in the allocation side and a read memory
barrier when the port is valid to prevent any re-ordering issue.
This is XSA-340.
Reported-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
There is at least one path (SYSENTER with NT set, Xen converts to #GP) which
ends up injecting the #GP fault twice, first in compat_sysenter(), and then a
second time in compat_test_all_events(), due to the stale TBF_EXCEPTION left
in TRAPBOUNCE_flags.
The guest kernel sees the second fault first, which is a kernel level #GP
pointing at the head of the #GP handler, and is therefore a userspace
trigger-able DoS.
This particular bug has bitten us several times before, so rearrange
{compat_,}create_bounce_frame() to clobber TRAPBOUNCE on success, rather than
leaving this task to one area of code which isn't used uniformly.
Other scenarios which might result in a double injection (e.g. two calls
directly to compat_create_bounce_frame) will now crash the guest, which is far
more obvious than letting the kernel run with corrupt state.
This is XSA-339
Fixes: fdac951560 ("x86: clear EFLAGS.NT in SYSENTER entry path")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
To avoid ports potentially becoming invalid behind the back of certain
other functions (due to ->max_evtchn shrinking) because of
- a guest invoking evtchn_reset() and from a 2nd vCPU opening new
channels in parallel (see also XSA-343),
- alloc_unbound_xen_event_channel() produced channels living above the
2-level range (see also XSA-342),
drop the max_evtchns check from port_is_valid(). For a port for which
the function once returned "true", the returned value may not turn into
"false" later on. The function's result may only depend on bounds which
can only ever grow (which is the case for d->valid_evtchns).
This also eliminates a false sense of safety, utilized by some of the
users (see again XSA-343): Without a suitable lock held, d->max_evtchns
may change at any time, and hence deducing that certain other operations
are safe when port_is_valid() returned true is not legitimate. The
opportunities to abuse this may get widened by the change here
(depending on guest and host configuration), but will be taken care of
by the other XSA.
This is XSA-338.
Fixes: 48974e6ce5 ("evtchn: use a per-domain variable for the max number of event channels")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Julien Grall <jgrall@amazon.com>
When assigned to less trusted or un-trusted guests, devices may change
state behind our backs (they may e.g. get reset by means we may not know
about). Therefore we should avoid reading BARs from hardware once a
device is no longer owned by Dom0. Furthermore when we can't read a BAR,
or when we read zero, we shouldn't instead use the caller provided
address unless that caller can be trusted.
Re-arrange the logic in msix_capability_init() such that only Dom0 (and
only if the device isn't DomU-owned yet) or calls through
PHYSDEVOP_prepare_msix will actually result in the reading of the
respective BAR register(s). Additionally do so only as long as in-use
table entries are known (note that invocation of PHYSDEVOP_prepare_msix
counts as a "pseudo" entry). In all other uses the value already
recorded will get used instead.
Clear the recorded values in _pci_cleanup_msix() as well as on the one
affected error path. (Adjust this error path to also avoid blindly
disabling MSI-X when it was enabled on entry to the function.)
While moving around variable declarations (in many cases to reduce their
scopes), also adjust some of their types.
This is part of XSA-337.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
It's safer and faster to just use the cached last written
(untranslated) MSI message stored in msi_desc for the single user that
calls read_msi_msg.
This also prevents relying on the data read from the device MSI
registers in order to figure out the index into the IOMMU interrupt
remapping table, which is not safe.
This is part of XSA-337.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The current vPT code will migrate the emulated timers between vCPUs
(change the pt->vcpu field) while just holding the destination lock,
either from create_periodic_time or pt_adjust_global_vcpu_target if
the global target is adjusted. Changing the periodic_timer vCPU field
in this way creates a race where a third party could grab the lock in
the unlocked region of pt_adjust_global_vcpu_target (or before
create_periodic_time performs the vcpu change) and then release the
lock from a different vCPU, creating a locking imbalance.
Introduce a per-domain rwlock in order to protect periodic_time
migration between vCPU lists. Taking the lock in read mode prevents
any timer from being migrated to a different vCPU, while taking it in
write mode allows performing migration of timers across vCPUs. The
per-vcpu locks are still used to protect all the other fields from the
periodic_timer struct.
Note that such migration shouldn't happen frequently, and hence
there's no performance drop as a result of such locking.
This is XSA-336.
Reported-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Tested-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In the case that an HVM Stubdomain makes an XENMEM_acquire_resource hypercall,
the FIXME path will bypass rcu_unlock_domain() on the way out of the function.
Move the check to the start of the function. This does change the behaviour
of the get-size path for HVM Stubdomains, but that functionality is currently
broken and unused anyway, as well as being quite useless to entities which
can't actually map the resource anyway.
This is XSA-334.
Fixes: 83fa6552ce ("common: add a new mappable resource type: XENMEM_resource_grant_table")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This MSR doesn't exist on AMD hardware, and switching away from the safe
functions in the common MSR path was an erroneous change.
Partially revert the change.
This is XSA-333.
Fixes: 4fdc932b3c ("x86/Intel: drop another 32-bit leftover")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Wei Liu <wl@xen.org>
At the moment iommu_map() and iommu_unmap() take a page order rather than a
count, whereas iommu_iotlb_flush() takes a page count rather than an order.
This patch makes them consistent with each other, opting for a page count since
CPU page orders are not necessarily the same as those of an IOMMU.
NOTE: The 'page_count' parameter is also made an unsigned long in all the
aforementioned functions.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <julien@xen.org>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Wei Liu <wl@xen.org>
This patch adds a full I/O TLB flush to the error paths of iommu_map() and
iommu_unmap().
Without this change callers need constructs such as:
rc = iommu_map/unmap(...)
err = iommu_flush(...)
if ( !rc )
rc = err;
With this change, it can be simplified to:
rc = iommu_map/unmap(...)
if ( !rc )
rc = iommu_flush(...)
because, if the map or unmap fails the flush will be unnecessary. This saves
a stack variable and generally makes the call sites tidier.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The VT-d and AMD IOMMU implementations both use the general x86 IOMMU page
table allocator and ARM always shares page tables with CPU. Hence there is no
need to retain the free_page_table() method or the tasklet which invokes it.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This patch converts the VT-d code to use the new IOMMU page table allocator
function. This allows all the free-ing code to be removed (since it is now
handled by the general x86 code) which reduces TLB and cache thrashing as well
as shortening the code.
The scope of the mapping_lock in intel_iommu_quarantine_init() has also been
increased slightly; it should have always covered accesses to
'arch.vtd.pgd_maddr'.
NOTE: The common IOMMU needs a slight modification to avoid scheduling the
cleanup tasklet if the free_page_table() method is not present (since
the tasklet will unconditionally call it).
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Andrew Cooper
Olaf Hering
Anthony PERARD
Trammell Hudson
Jan Beulich
Julien Grall
Marek Marczykowski-Górecki
Roger Pau Monné
Oleksandr Tyshchenko
Paul Durrant