Currently all the users are within x86/mm.c. But that will change once
we split PV specific mm code to another file. Lift that to page.h
along side _PAGE_* in preparation for later patches.
No functional change. Add some spaces around "|" while moving.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
And at once make it an inline function. Add declarations of
replace_grant_{p2m,pv}_mapping to respective header files.
The code movement will be done later.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
And at once make create_grant_host_mapping an inline function. This
requires making create_grant_{p2m,pv}_mapping non-static. Provide
{p2m,pv}/grant_table.h. Include the headers where necessary.
The two functions create_grant_{p2m,pv}_mapping will be moved later in
a dedicated patch with all their helpers.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
To avoid breaking the build elsewhere, the l{1..4}e_{from,get}_page() macros
are switched to using __mfn_to_page() and __page_to_mfn().
Most changes are wrapping or removing _mfn()/mfn_x() from existing callsites.
However, {alloc,free}_l1_table() are switched to using __map_domain_page(), as
their pfn parameters are otherwise unused. get_page() has one pfn->mfn
correction in a printk(), and __get_page_type()'s IOMMU handling has its gfn
calculation broken out for clarity.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Make sure local symbols are correctly associated with their source
files: I've just run across a cpufreq.c#create_bounce_frame stack trace
entry. Since we have multiple entry.S, don't use __FILE__ there to
fully disambiguate things.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
This shrinks the size from 48 to 40 bytes bytes on 64-bit builds.
Switch to gfn_t at once.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
While benign to 32-bit arches, this shrinks the size from 56 to 48
bytes on 64-bit ones (while still leaving a 16-bit hole).
Take the opportunity and consistently use bool/true/false for all
is_sub_page uses.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
They're violating name space rules, and we don't really need them. When
followed by "gnttab_", also drop that.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
In particular use grant_ref_t and grant_handle_t where appropriate.
Also switch other nearby type uses to their canonical variants where
appropriate and introduce INVALID_MAPTRACK_HANDLE.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
When no memory is available in the hypervisor, rather than immediately
failing the request, try to steal a handle from another vCPU.
Reported-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Processing of transitive grants must not use the fast path, or else
reference counting breaks due to the skipped recursive call to
__acquire_grant_for_copy() (its __release_grant_for_copy()
counterpart occurs independent of original pin count). Furthermore
after re-acquiring temporarily dropped locks we need to verify no grant
properties changed if the original pin count was non-zero; checking
just the pin counts is sufficient only for well-behaved guests. As a
result, __release_grant_for_copy() needs to mirror that new behavior.
Furthermore a __release_grant_for_copy() invocation was missing on the
retry path of __acquire_grant_for_copy(), and gnttab_set_version() also
needs to bail out upon encountering a transitive grant.
This is part of XSA-226.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
There is no guarantee that the compiler would actually translate them
to branches instead of calls, so only ones with a known recursion limit
are okay:
- __release_grant_for_copy() can call itself only once, as
__acquire_grant_for_copy() won't permit use of multi-level transitive
grants,
- __acquire_grant_for_copy() is fine to call itself with the last
argument false, as that prevents further recursion,
- __acquire_grant_for_copy() must not call itself to recover from an
observed change to the active entry's pin count
This is part of XSA-226.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
__virt_to_maddr() is used very frequently, but has a large footprint due to
its assertions and comparasons.
Rearange its logic to drop one assertion entirely, encoding its check in a
second assertion (with no additional branch, and the comparason performed with
a 32bit immediate rather than requiring a movabs).
Bloat-o-meter net report is:
add/remove: 0/0 grow/shrink: 1/72 up/down: 3/-2169 (-2166)
along with a reduction of 32 assertion frames (895 down to 861)
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
They are only referenced by physical address (either the HSA MSR, or via
VMSAVE/VMLOAD which take a physical operand). Allocating xenheap pages and
storing their virtual address is wasteful.
Allocate them with domheap pages instead, taking the opportunity to suitably
NUMA-position them. This avoids Xen needing to perform a virt to phys
translation on every context switch.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
* Drop trailing whitespace.
* Move amd_nonfatal_mcheck_init() into .init.text and drop a trailing return.
* Drop unnecessary wmb()'s. Because of Xen's implementation, they are only
compiler barriers anyway, and each wrmsr() is already fully serialising.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The only way alloc_boot_pages will return 0 is during the error case.
Although, Xen will panic in the error path. So the check in the caller
is pointless.
Looking at the loop, my understanding is it will try to allocate in
smaller chunk if a bigger chunk fail. Given that alloc_boot_pages can
never check, the loop seems unecessary.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
alloc_boot_pages will panic if it is not possible to allocate. So the
check in the caller is pointless.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
alloc_boot_pages will panic if it is not possible to allocate. So the
check in the caller is pointless.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In psr.c, we defined some macros but the coding style is not good.
Use '(1u << X)' to replace '(1<<X)'.
Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Holding any lock while accessing the maptrack entry fields is
pointless, as these entries are protected by their associated active
entry lock (which is being acquired later, before re-validating the
fields read without holding the lock).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The function is the same on both x86 and arm. Lift it to sched.h to
save a function call, make it take a pointer to vcpu to avoid
resolving current every time it gets called.
Take the chance to change one of its callers to only use one current
in code.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
When the record length for sending the p2m frames in a migration
stream is too large, the issued error message is not very helpful:
xc: Record (0x00000003, x86 PV P2M frames) length 0x8 exceeds max
(0x800000): Internal error
When printing the error use the size which was tested instead that of
the record header length.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Remove the opencoded rcu_lock_domain_by_any_id(). Drop the PIN_FAIL()s and
return GNTST_* values directly.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Drop pointless debugging messages, reduce variable scope, and correct the type
of an induction variable.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
* Drop trailing whitespace
* Style corrections
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Simplify the error labels to just "unlock" and "out". This fixes an erroneous
path where a failure of rcu_lock_domain_by_any_id() still results in
rcu_unlock_domain() being called.
This is only not an XSA by luck. rcu_unlock_domain() is a nop other than
decrementing the preempt count, and nothing reads the preempt count outside of
a debug build.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
timer_deadline is only ever updated via this_cpu() in timer_softirq_action(),
so is not going to change behind the back of the currently running cpu.
Update hpet_broadcast_{enter,exit}() to cache the value in a local variable to
avoid the repeated RELOC_HIDE() penalty.
handle_hpet_broadcast() reads the timer_deadlines of remote cpus, but there is
no need to force the read for cpus which are not present in the mask. One
requirement is that we only sample the value once (which happens as a side
effect of RELOC_HIDE()), but is made more explicit with ACCESS_ONCE().
Bloat-o-meter shows a modest improvement:
add/remove: 0/0 grow/shrink: 0/3 up/down: 0/-144 (-144)
function old new delta
hpet_broadcast_exit 335 313 -22
hpet_broadcast_enter 327 278 -49
handle_hpet_broadcast 572 499 -73
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Regardless of copy operations only setting GNTPIN_hst*, GNTPIN_dev*
also need to be taken into account when deciding whether to clear
_GTF_{read,writ}ing. At least for consistency with code elsewhere the
read part better doesn't use any mask at all.
This is XSA-230.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The way the lock is currently being used in get_maptrack_handle(), it
protects only the maptrack limit: The function acts on current's list
only, so races on list accesses are impossible even without the lock.
Otoh list access races are possible between __get_maptrack_handle() and
put_maptrack_handle(), due to the invocation of the former for other
than current from steal_maptrack_handle(). Introduce a per-vCPU lock
for list accesses to become race free again. This lock will be
uncontended except when it becomes necessary to take the steal path,
i.e. in the common case there should be no meaningful performance
impact.
When in get_maptrack_handle adds a stolen entry to a fresh, empty,
freelist, we think that there is probably no concurrency. However,
this is not a fast path and adding the locking there makes the code
clearly correct.
Also, while we are here: the stolen maptrack_entry's tail pointer was
not properly set. Set it.
This is CVE-2017-12136 / XSA-228.
Reported-by: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Pagetable entries must be aligned to function correctly. Disallow attempts
from the guest to have a grant PTE created at a misaligned address, which
would result in corruption of the L1 table with largely-guest-controlled
values.
This is CVE-2017-12137 / XSA-227.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
While re-ordering the include alphabetically in arch/arm/domain.c, I got
a complitation error because grant_table.h is using gfn_t before been
defined:
In file included from domain.c:14:0:
xen/xen/include/xen/grant_table.h:153:29: error: unknown type name gfn_t
gfn_t *gfn, uint16_t *status);
^
Fix it by including xen/mm.h in it.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Always map the PCI MMCFG region as strongly uncacheable. Nothing good will
happen if stray MTRR settings end up converting UC- to WC.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The hypercall argument translation area lives in the per-domain mappings in
PML4 slot 260. Nothing currently resides in the lower canonical half above
the 4GB boundary in a 32bit PV guest.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
They don't belong there. Removing them causes build errors in several
places. Add the forward declarations in those places.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
This infrastructure is used exclusively by the x86 do_mmu_update() hypercall.
Mapping and unmapping domain pages is probably not the slow part of that
function, but even with an opencoded caching implementation, Bloat-o-meter
reports:
function old new delta
do_mmu_update 6815 6573 -242
The !CONFIG_DOMAIN_PAGE stub code has a mismatch between mapping and
unmapping, which is a latent bug.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The check is useless because pointer arithmetic ensures "info" is
always non-zero.
Replace it with an ASSERT for socket_info. The only caller of
free_socket_resources already ensures socket_info is not NULL before
calling it.
Coverity-ID: 1416344
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Wei Liu
Andrew Cooper
Jan Beulich
Julien Grall
Yi Sun
Juergen Gross
Boris Ostrovsky