Commit Graph
42800 Commits
Author SHA1 Message Date
Jan Beulich f004fd7cbb x86emul: avoid using _PRE_EFLAGS() in a few cases
The macro expanding to quite a few insns, replace its use by simply
clearing the status flags when the to be executed insn doesn't depend
on their initial state, in cases where this is easily possible. (There
are more cases where the uses are hidden inside macros, and where some
of the users of the macros want guest flags put in place before running
the insn, i.e. the macros can't be updated as easily.)

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2021-06-29 12:33:37 +02:00
Jan Beulich f8582da041 x86/mm: pull a sanity check earlier in xenmem_add_to_physmap_one()
We should try to limit the failure reasons after we've started making
changes.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2021-06-29 11:03:29 +02:00
Jan Beulich 17e91570c5 x86/paging: deal with log-dirty stats overflow
While the precise values are unlikely of interest once they exceed 4
billion (allowing us to leave alone the domctl struct), we still
shouldn't wrap or truncate the actual values. It is in particular
problematic if the truncated values were zero (causing libxenguest to
skip an iteration altogether) or a very small value (leading to
premature exiting of the pre-copy phase).

Change the internal fields to unsigned long, and suitably saturate for
copying to guest context.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2021-06-29 11:02:35 +02:00
Jan Beulich f24ec0ed22 fully replace mfn_to_gmfn()
Convert the two remaining uses as well as Arm's stub to the properly
named and type-safe mfn_to_gfn(), dropping x86's definition (where we
already have mfn_to_gfn()).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <julien@xen.org>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2021-06-29 11:00:51 +02:00
Andrew Cooper c636a5fe59 Replace FSF street address with canonical URL (again)
As recommended in http://www.gnu.org/licenses/gpl-howto.en.html.

Exactly as per changeset 443701ef0c - Some errors have crept back in in
the past 6 years.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
2021-06-28 11:17:14 +01:00
Rahul Singh bb11edcec1 xen/arm: smmuv1: Set privileged attr to 'default'
Backport commit e19898077cfb642fe151ba22981e795c74d9e114
"iommu/arm-smmu: Set privileged attribute to 'default' instead of
'unprivileged'"

Original commit message:
    Currently the driver sets all the device transactions privileges
    to UNPRIVILEGED, but there are cases where the iommu masters wants
    to isolate privileged supervisor and unprivileged user.
    So don't override the privileged setting to unprivileged, instead
    set it to default as incoming and let it be controlled by the
    pagetable settings.

    Acked-by: Will Deacon <will.deacon@arm.com>
    Signed-off-by: Sricharan R <sricharan@codeaurora.org>
    Signed-off-by: Will Deacon <will.deacon@arm.com>

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
2021-06-25 14:54:17 -07:00
Rahul Singh 93fe1817e7 xen/arm: smmuv1: Fixed stream matching register allocation
SMR allocation should be based on the number of supported stream
matching register for each SMMU device.

Issue introduced by commit 5e08586afb
when backported the patches from Linux to XEN to fix the stream match
conflict issue when two devices have the same stream-id.

Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
2021-06-25 14:53:48 -07:00
Jan Beulich f591755823 IOMMU/PCI: don't let domain cleanup continue when device de-assignment failed
Failure here could in principle mean the device may still be issuing DMA
requests, which would continue to be translated by the page tables the
device entry currently points at. With this we cannot allow the
subsequent cleanup step of freeing the page tables to occur, to prevent
use-after-free issues. We would need to accept, for the time being, that
in such a case the remaining domain resources will all be leaked, and
the domain will continue to exist as a zombie.

However, with flushes no longer timing out (and with proper timeout
detection for device I/O TLB flushing yet to be implemented), there's no
way anymore for failures to occur, except due to bugs elsewhere. Hence
the change here is merely a "just in case" one.

In order to continue the loop in spite of an error, we can't use
pci_get_pdev_by_domain() anymore. I have no idea why it was used here in
the first place, instead of the cheaper list iteration.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
2021-06-25 14:06:55 +02:00
Andrew Cooper 01a2d001de libxencall: Bump SONAME following new functionality
Fixes: bef64f2c00 ("libxencall: introduce variant of xencall2() returning long")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Ian Jackson <iwj@xenproject.org>
2021-06-25 12:38:18 +01:00
Julien Grall 8a9b94982b tools/xenstored: Correctly read the requests header from the stream
Commit c0fe360f42 ("tools/xenstored: Extend restore code to handle
multiple input buffer") extend the read_buffered_state() to support
multiple input buffers. Unfortunately, the commit didn't go far
enough and still used sc->data (start of the buffers) for retrieving
the header. This would lead to read the wrong headers for second and
follow-up commands.

Use data in place for sc->data for the source of the memcpy()s.

Fixes: c0fe360f42 ("tools/xenstored: Extend restore code to handle multiple input buffer")
Reported-by: Raphael Ning <raphning@amazon.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
2021-06-25 10:40:06 +01:00
Julien Grall e87d8f60fa tools/xenstored: Remove redundant check in socket_can_process()
Commit 3adfb50315 ("tools/xenstored: Introduce a wrapper for
conn->funcs->can_{read, write}") consolidated the check
!conn->is_ignored in two new wrappers.

This means the check in socket_can_process() is now redundant. In
fact it should have been removed in orignal commit (as it was done
for the domain helpers).

Reported-by: Raphael Ning <raphning@amazon.com
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
2021-06-24 15:55:49 +01:00
Jan Beulich 190facdf87 libxc: make xc_domain_maximum_gpfn() endianness-agnostic
libxc generally uses uint32_t to represent domain IDs. This is fine as
long as addresses of such variables aren't taken, to then pass into
hypercalls: To the hypervisor, a domain ID is a 16-bit value. Introduce
a wrapper struct to deal with the issue. (On architectures with
arguments passed in registers, an intermediate variable would have been
created by the compiler already anyway, just one of the wrong type.)

The public interface change is both source and binary compatible for
the architectures we currently support.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Jackson <iwj@xenproject.org>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2021-06-24 16:40:57 +02:00
Jan Beulich 7ffbed8681 libxencall: drop bogus mentioning of xencall6()
There's no xencall6(), so the version script also shouldn't mention it.
If such a function would ever appear, it shouldn't land in version 1.0.

No change to the generated binary, nor abi-dumper's view of the object.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Jackson <iwj@xenproject.org>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2021-06-24 16:39:55 +02:00
Jan Beulich 6f02d1ea4a libxc: use multicall for memory-op on Linux (and Solaris)
Some sub-functions, XENMEM_maximum_gpfn and XENMEM_maximum_ram_page in
particular, can return values requiring more than 31 bits to represent.
Hence we cannot issue the hypercall directly when the return value of
ioctl() is used to propagate this value. This is the case for Linux
and Solaris (and hence needs changing), while the BSDs avoid using the
return value for dual purposes altogether, and MiniOS already wraps all
hypercalls in a multicall.

Suggested-by: Jürgen Groß <jgross@suse.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Jackson <iwj@xenproject.org>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2021-06-24 16:39:26 +02:00
Jan Beulich bef64f2c00 libxencall: introduce variant of xencall2() returning long
Some hypercalls, memory-op in particular, can return values requiring
more than 31 bits to represent. Hence the underlying layers need to make
sure they won't truncate such values.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Jackson <iwj@xenproject.org>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2021-06-24 16:39:02 +02:00
Jan Beulich 6409210a5f libxencall: osdep_hypercall() should return long
Some hypercalls, memory-op in particular, can return values requiring
more than 31 bits to represent. Hence the underlying layers need to make
sure they won't truncate such values. (Note that for Solaris the
function also gets renamed, to match the other OSes.)

Due to them merely propagating ioctl()'s return value, this change is
benign on Linux and Solaris. IOW there's an actual effect here only for
the BSDs and MiniOS, but even then further adjustments are needed at the
xencall<N>() level.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <iwj@xenproject.org>
2021-06-24 16:38:37 +02:00
Jan Beulich 198a2bc6f1 x86/HVM: wire up multicalls
To be able to use them from, in particular, the tool stack, they need to
be supported for all guest types. Note that xc_resource_op() already
does, so would not work without this on PVH Dom0.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Begrudingly acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <iwj@xenproject.org>
2021-06-24 16:35:39 +02:00
Jan Beulich f7db924309 VT-d: drop/move a few QI related constants
Replace uses of QINVAL_ENTRY_ORDER and QINVAL_INDEX_SHIFT, such that
the constants can be dropped. Move the remaining QINVAL_* ones to the
single source file using them.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
2021-06-24 16:30:51 +02:00
Jan Beulich a0eb19723d VT-d: centralize mapping of QI entries
Introduce a helper function to reduce redundancy. Take the opportunity
to express the logic without using the somewhat odd QINVAL_ENTRY_ORDER.
Also take the opportunity to uniformly unmap after updating queue tail
and dropping the lock (like was done so far only by
queue_invalidate_context_sync()).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
2021-06-24 16:30:32 +02:00
Jan Beulich e7059776f9 VT-d: don't lose errors when flushing TLBs on multiple IOMMUs
While no longer an immediate problem with flushes no longer timing out,
errors (if any) get properly reported by iommu_flush_iotlb_{dsi,psi}().
Overwriting such an error with, perhaps, a success indicator received
from another IOMMU will misguide callers. Record the first error, but
don't bail from the loop (such that further necessary invalidation gets
carried out on a best effort basis).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
2021-06-24 16:30:06 +02:00
Jan Beulich 80589800ae VT-d: clear_fault_bits() should clear all fault bits
If there is any way for one fault to be left set in the recording
registers, there's no reason there couldn't also be multiple ones. If
PPF set set (being the OR or all F fields), simply loop over the entire
range of fault recording registers, clearing F everywhere.

Since PPF is a r/o bit, also remove it from DMA_FSTS_FAULTS (arguably
the constant's name is ambiguous as well).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
2021-06-24 16:29:42 +02:00
Jan Beulich 3265588005 VT-d: adjust domid map updating when unmapping context
When an earlier error occurred, cleaning up the domid mapping data is
wrong, as references likely still exist. The only exception to this is
when the actual unmapping worked, but some flush failed (supposedly
impossible after XSA-373). The guest will get crashed in such a case
though, so add fallback cleanup to domain destruction to cover this
case. This in turn makes it desirable to silence the dprintk() in
domain_iommu_domid().

Note that no error will be returned anymore when the lookup fails - in
the common case lookup failure would already have caused
domain_context_unmap_one() to fail, yet even from a more general
perspective it doesn't look right to fail domain_context_unmap() in such
a case when this was the last device, but not when any earlier unmap was
otherwise successful.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
2021-06-24 16:29:13 +02:00
Jan Beulich f3401d65d9 VT-d: undo device mappings upon error
When
 - flushes (supposedly not possible anymore after XSA-373),
 - secondary mappings for legacy PCI devices behind bridges,
 - secondary mappings for chipset quirks, or
 - find_upstream_bridge() invocations
fail, the successfully established device mappings should not be left
around.

Further, when (parts of) unmapping fail, simply returning an error is
typically not enough. Crash the domain instead in such cases, arranging
for domain cleanup to continue in a best effort manner despite such
failures.

Finally make domain_context_unmap()'s error behavior consistent in the
legacy PCI device case: Don't bail from the function in one special
case, but always just exit the switch statement.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
2021-06-24 16:28:25 +02:00
Julien GralL 90bafdba8e tools/xenstored: Don't crash xenstored when Live-Update is cancelled
As Live-Update is asynchronous, it is possible to receive a request to
cancel it (either on the same connection or from a different one).

Currently, this will crash xenstored because do_lu_start() assumes
lu_status will be valid. This is not the case when Live-Update has been
cancelled. This will result to dereference a NULL pointer and
crash Xenstored.

Rework do_lu_start() to check if lu_status is NULL and return an
error in this case.

Fixes: af216a99fb ("tools/xenstore: add the basic framework for doing the live update")
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
2021-06-24 12:15:49 +01:00
Julien Grall c8be420025 tools/xenstored: Delay new transaction while Live-Update is pending
At the moment, Live-Update will, by default, not proceed if there are
in-flight transactions. It is possible force it by passing -F but this
will break any connection with in-flight transactions.

There are PV drivers out that may never terminate some transaction. On
host running such guest, we would need to use -F. Unfortunately, this
also risks to break well-behaving guests (and even dom0) because
Live-Update will happen as soon as the timeout is hit.

Ideally, we would want to preserve transactions but this requires
some work and a lot of testing to be able to use it in production.

As a stop gap, we want to limit the damage of -F. This patch will delay
any transactions that are started after Live-Update has been requested.

If the request cannot be delayed, the connection will be stalled to
avoid loosing requests.

If the connection has already a pending transaction before Live-Update,
then new transaction will not be delayed. This is to avoid the connection
to stall.

With this stop gap in place, domains with long running transactions will
still break when using -F, but other domains which starts a transaction
in the middle of Live-Update will continue to work.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
2021-06-24 12:14:56 +01:00
Julien Grall 54f73e6fc6 tools/xenstored: Dump delayed requests
Currently, only Live-Update request can be delayed. In a follow-up,
we will want to delay more requests (e.g. transaction start).
Therefore we want to preserve delayed requests across Live-Update.

Delayed requests are just complete "in" buffer. So the code is
refactored to allow sharing the code to dump "in" buffer.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
2021-06-24 12:12:54 +01:00
Daniel P. Smith 76a0aa9c4d maintainers: adding new reviewer for xsm
Would like to add myself as a reviewer for XSM.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2021-06-24 11:45:32 +01:00
Oleksandr Tyshchenko ae8d47ede9 iommu/arm: ipmmu-vmsa: Add compatible for Renesas R-Car M3-W+ SoC
The "renesas,r8a77961" string identifies M3-W+ (aka M3-W ES3.0)
instead of "renesas,r8a7796" since Linux commit:
"9c9f7891093b02eb64ca4e1c7ab776a4296c058f soc: renesas: Identify R-Car M3-W+".
Add new compatible to the Xen driver.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Acked-by: Julien Grall <jgrall@amazon.com>
2021-06-24 11:43:53 +01:00
Julien Grall c0fe360f42 tools/xenstored: Extend restore code to handle multiple input buffer
Currently, the restore code is considering the stream will contain at
most one in-flight request per connection. In a follow-up changes, we
will want to transfer multiple in-flight requests.

The function read_state_buffered() is now extended to restore multiple
in-flight request. Complete requests will be queued as delayed
requests, if there a partial request (only the last one can) then it
will used as the current in-flight request.

Note that we want to bypass the quota check for delayed requests as
the new Xenstore may have a lower limit.

Lastly, there is no need to change the specification as there was
no restriction on the number of in-flight requests preserved.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
2021-06-24 11:41:00 +01:00
Julien Grall 361f1deb42 tools/xenstored: delay_request: don't assume conn->in == in
delay_request() is currently assuming that the request delayed is
always conn->in. This is currently correct, but it is a call for
a latent bug as the function allows the caller to specify any request.

To prevent any future surprise, check if the request delayed is the
current one.

Fixes: c5ca1404b4 ("tools/xenstore: add support for delaying execution of a xenstore request")
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
2021-06-24 09:08:56 +01:00
Julien Grall 3adfb50315 tools/xenstored: Introduce a wrapper for conn->funcs->can_{read, write}
Currently, the callbacks can_read and can_write are called directly. This
doesn't allow us to add generic check and therefore requires duplication.

At the moment, one check that could benefit to be common is whether the
connection should ignored. The position is slightly different between
domain and socket because for the latter we want to check the state of
the file descriptor first.

In follow-up patches, there will be more potential generic checks.

This patch provides wrappers to read/write a connection and move
the check ->is_ignored after the callback for everyone.

This also requires to replace the direct call to domain_can_read()
and domain_can_write() with the new wrapper. At the same time,
both functions can now be static. Note that the implementations need
to be moved earlier in the file xenstored_domain.c to avoid
declaring the prototype.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
2021-06-24 09:08:42 +01:00
Julien Grall a8002238b4 tools/xenstored: xenstored_core.h should include fcntl.h
xenstored_core.h will consider live-udpate is not supported if
O_CLOEXEC doesn't exist. However, the header doesn't include the one
defining O_CLOEXEC (i.e. fcntl.h). This means that depending on
the header included, some source file will think Live-Update is not
supported.

I am not aware of any issue with the existing. Therefore this is just
a latent bug so far.

Prevent any potential issue by including fcntl.h in xenstored_core.h

Fixes: cd831ee438 ("tools/xenstore: handle CLOEXEC flag for local files and pipes")
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
2021-06-24 09:07:52 +01:00
Julien Grall 2aa9e00bd3 tools/xenstored: Limit the number of requests a connection can delay
Currently, only liveupdate request can be delayed. The request can only
be performed by a privileged connection (e.g. dom0). So it is fine to
have no limits.

In a follow-up patch we will want to delay request for unprivileged
connection as well. So it is best to apply a limit.

For now and for simplicity, only a single request can be delayed
for a given unprivileged connection.

Take the opportunity to tweak the prototype and provide a way to
bypass the quota check. This would be useful when the function
is called from the restore code.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
2021-06-24 09:07:30 +01:00
Julien Grall 65f19ed62a tools/xenstore: Don't assume conn->in points to the LU request
call_delayed() is currently assuming that conn->in is NULL when
handling delayed request. However, the connection is not paused.
Therefore new request can be processed and conn->in may be non-NULL
if we have only received a partial request.

Furthermore, as we overwrite conn->in, the current partial request
will not be transferred. This will result to corrupt the connection.

Rather than updating conn->in, stash the LU request in lu_status and
let each callback for delayed request to update conn->in when
necessary.

To keep a sane interface, the code to write the "OK" response the
LU request is moved in xenstored_core.c.

Fixes: c5ca1404b4 ("tools/xenstore: add support for delaying execution of a xenstore request")
Fixes: ed6eebf17d ("tools/xenstore: dump the xenstore state for live update")
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
2021-06-24 09:06:58 +01:00
Julien Grall 3ce271ef53 tools/xenstored: Introduce lu_get_connection() and use it
At the moment, dump_state_buffered_data() is taking two connections
in parameters (one is the connection to dump, the other is the
connection used to request LU). The naming doesn't help to
distinguish (c vs conn) them and this already lead to several mistake
while modifying the function.

To remove the confusion, introduce an help lu_get_connection() that
will return the connection used to request LU and use it
in place of the existing parameter.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
2021-06-24 09:02:59 +01:00
Julien Grall 82b1b19469 MAINTAINERS: Add myself as reviewers for tools/xenstore
I would like to help reviewing Xenstored patches. It is more convenient
to find them if I am CCed.

Signed-off-by: Julien Grall <julien@xen.org>
Acked-by: Juergen Gross <jgross@suse.com>
2021-06-24 09:02:56 +01:00
Andrew Cooper c7691f5e34 Revert "tools/firmware/ovmf: Use OvmfXen platform file is exist"
This reverts commit aad7b5c11d.

The change from OvmfX64 to OvmfXen causes a change in behaviour, whereby
OvmfXen maps its shared info page at the top of address space.  When trying to
migrate such a domain, XENMEM_maximum_gpfn returns a very large value.  This
has uncovered multiple issues:

 1) The userspace hypercall wrappers truncate all return values to int on
    Linux and Solaris, even on 64bit.  This needs fixing in libxenctrl.
 2) 32bit toolstacks can't migrate any domain with RAM above the 2^40 mark,
    because of virtual address constraints.  This needs fixing in OVMF.

Fixes for both of these aren't completely trivial.  Revert the change to
unblock staging in the meantime.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2021-06-22 18:00:08 +01:00
Nick Rosbrook c9b59f9032 golang/xenlight: do not negate ret when converting to Error
Commit 871e51d2d4 changed the sign on the xenlight error types (making
the values negative, same as the C-generated constants), but failed to
remove the code changing the sign before casting to Error().  This
results in error strings like "libxl error: <x>", rather than the
correct message. Fix all occurrances of this by running:

  gofmt -w -r 'Error(-ret) -> Error(ret)' xenlight.go

from tools/golang/xenlight.

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
2021-06-21 16:49:08 +01:00
Nick Rosbrook 1d95fd75df golang/xenlight: add SendTrigger wrapper
Add a warpper around libxl_send_trigger.

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
2021-06-21 16:49:08 +01:00
Nick Rosbrook 9b6d865e0a golang/xenlight: add DomainDestroy wrapper
Add a wrapper around libxl_domain_destroy.

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
2021-06-21 16:49:08 +01:00
Nick Rosbrook c089de0e2f golang/xenlight: rename Ctx receivers to ctx
As a matter of style, it is strange to see capitalized receiver names,
due to the significance of capitalized symbols in Go (although there is
in fact nothing special about a capitalized receiver name). Fix this in
xenlight.go by running:

  gofmt -w -r 'Ctx -> ctx' xenlight.go

from tools/golang/xenlight. There is no functional change.

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
2021-06-21 16:49:08 +01:00
Nick Rosbrook 1997940ad2 golang/xenlight: use struct pointers in keyed union fields
Currently, when marshalig Go types with keyed union fields, we assign the
value of the struct (e.g. DomainBuildInfoTypeUnionHvm) which implements the
interface of the keyed union field (e.g. DomainBuildInfoTypeUnion).
As-is, this means that if a populated DomainBuildInfo is marshaled to
e.g. JSON, unmarshaling back to DomainBuildInfo will fail.

When the encoding/json is unmarshaling data into a Go type, and
encounters a JSON object, it basically can either marshal the data into
an empty interface, a map, or a struct. It cannot, however, marshal data
into an interface with at least one method defined on it (e.g.
DomainBuildInfoTypeUnion). Before this check is done, however, the
decoder will check if the Go type is a pointer, and dereference it if
so. It will then use the type of this value as the "target" type.

This means that if the TypeUnion field is populated with a
DomainBuildInfoTypeUnion, the decoder will see a non-empty interface and
fail. If the TypeUnion field is populated with a
*DomainBuildInfoTypeUnionHvm, it dereferences the pointer and sees a
struct instead, allowing decoding to continue normally.

Since there does not appear to be a strict need for NOT using pointers
in these fields, update code generation to set keyed union fields to
pointers of their implementing structs.

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
2021-06-21 16:49:08 +01:00
Nick Rosbrook bc9f632e31 golang/xenlight: export keyed union interface types
For structs that have a keyed union, e.g. DomainBuildInfo, the TypeUnion
field must be exported so that package users can get/set the fields
within. This means that users are aware of the existence of the
interface type used in those fields (see [1]), so it is awkward that the
interface itself is not exported. However, the single method within the
interface must remain unexported so that users cannot mistakenly "implement"
those interfaces.

Since there seems to be no reason to do otherwise, export the keyed
union interface types.

[1] https://pkg.go.dev/xenbits.xenproject.org/git-http/xen.git/tools/golang/xenlight?tab=doc#DeviceUsbdev

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
2021-06-21 16:49:08 +01:00
Nick Rosbrook 1422d8db1b golang/xenlight: fix StringList toC conversion
The current implementation of StringList.toC does not correctly account
for how libxl_string_list is expected to be laid out in C, which is clear
when one looks at libxl_string_list_length in libxl.c. In particular,
StringList.toC does not account for the extra memory that should be
allocated for the "sentinel" entry. And, when using the "slice trick" to
create a slice that can address C memory, the unsafe.Pointer conversion
should be on a C.libxl_string_list, not *C.libxl_string_list.

Fix these problems by (1) allocating an extra slot in the slice used to
address the C memory, and explicity set the last entry to nil so the C
memory will be zeroed out, and (2) dereferencing csl in the
unsafe.Pointer conversion.

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
2021-06-21 16:49:08 +01:00
Nick Rosbrook b291ce703b golang/xenlight: update generated code
Re-generate code to reflect changes to libxl_types.idl from the
following commits:

0570d7f276 x86/msr: introduce an option for compatible MSR behavior selection
7e5cffcd1e viridian: allow vCPU hotplug for Windows VMs
9835246710 viridian: remove implicit limit of 64 VPs per partition

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
2021-06-21 16:49:08 +01:00
Roger Pau Monné 8af4b47f06 x86/ept: force WB cache attributes for grant and foreign maps
Force WB type for grants and foreign pages. Those are usually mapped
over unpopulated physical ranges in the p2m, and those ranges would
usually be UC in the MTRR state, which is unlikely to be the correct
cache attribute. It's also cumbersome (or even impossible) for the
guest to be setting the MTRR type for all those mappings as WB, as
MTRR ranges are finite.

Note that this is not an issue on AMD because WB cache attribute is
already set on grants and foreign mappings in the p2m and MTRR types
are ignored. Also on AMD Xen cannot force a cache attribute because of
the lack of ignore PAT equivalent, so the behavior here slightly
diverges between AMD and Intel (or EPT vs NPT/shadow).

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
2021-06-17 18:00:57 +02:00
Roger Pau Monné ed464d4e8a x86/mtrr: move epte_get_entry_emt to p2m-ept.c
This is an EPT specific function, so it shouldn't live in the generic
mtrr file. Such movement is also needed for future work that will
require passing a p2m_type_t parameter to epte_get_entry_emt, and
making that type visible to the mtrr users is cumbersome and
unneeded.

Moving epte_get_entry_emt out of mtrr.c requires making the helper to
get the MTRR type of an address from the mtrr state public. While
there rename the function to start with the mtrr prefix, like other
mtrr related functions.

While there fix some of the types of the function parameters.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
2021-06-17 17:58:11 +02:00
Andrew Cooper 4bcf6433ee tests: Introduce a TSX test
See the comment at the top of test-tsx.c for details.

This covers various complexities encountered while trying to address the
recent TSX deprecation on client parts.

A sample run on KabyLake with latest microcode and default tsx= looks like
this:

  root@host# ./test-tsx
  TSX tests
    Got 8 CPUs
  Testing MSR_TSX_FORCE_ABORT consistency
    CPU0 val 0x3
  Testing MSR_TSX_CTRL consistency
  Testing RTM behaviour
    Got Abort
  Testing PV default/max policies
    Max: RTM 1, HLE 1, TSX_FORCE_ABORT 0, RTM_ALWAYS_ABORT 0, TSX_CTRL 0
    Def: RTM 0, HLE 0, TSX_FORCE_ABORT 0, RTM_ALWAYS_ABORT 0, TSX_CTRL 0
  Testing HVM default/max policies
    Max: RTM 1, HLE 1, TSX_FORCE_ABORT 0, RTM_ALWAYS_ABORT 0, TSX_CTRL 0
    Def: RTM 0, HLE 0, TSX_FORCE_ABORT 0, RTM_ALWAYS_ABORT 0, TSX_CTRL 0
  Testing PV guest
    Created d7
    Cur: RTM 0, HLE 0, TSX_FORCE_ABORT 0, RTM_ALWAYS_ABORT 0, TSX_CTRL 0
    Cur: RTM 1, HLE 1, TSX_FORCE_ABORT 0, RTM_ALWAYS_ABORT 0, TSX_CTRL 0
  Testing HVM guest
    Created d8
    Cur: RTM 0, HLE 0, TSX_FORCE_ABORT 0, RTM_ALWAYS_ABORT 0, TSX_CTRL 0
    Cur: RTM 1, HLE 1, TSX_FORCE_ABORT 0, RTM_ALWAYS_ABORT 0, TSX_CTRL 0

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2021-06-15 20:50:32 +01:00
Andrew Cooper f7ad9ee7ca libs/guest: Move struct xc_cpu_policy into xg_private.h
... so tests can peek at the internals, without the structure being generally
available to users of the library.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2021-06-15 20:50:32 +01:00
Andrew Cooper b672695e74 x86/msr: Expose MSR_ARCH_CAPS in the raw and host policies
MSR_ARCH_CAPS is still not supported for guests yet (other than the hardware
domain), until the toolstack learns how to construct an MSR policy.

However, we want access to the host ARCH_CAPS_TSX_CTRL value in particular for
testing purposes.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2021-06-15 20:50:32 +01:00