The PV MSR functionality is now implemented as a separate set of domctls.
This is a revert of parts of c/s65e3554908
"x86/PV: support data breakpoint extension registers"
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Migrating PV domains using MSRs is not supported. This uses the new
XEN_DOMCTL_get_vcpu_msrs and will fail the migration with an explicit error.
This is an improvement upon the current failure of
"No extended context for VCPUxx (ENOBUFS)"
Support for migrating PV domains which are using MSRs will be included in the
migration v2 work.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Despite my 'Reviewed-by' tag on c/s 65e3554908 "x86/PV: support data
breakpoint extension registers", I have re-evaluated my position as far as the
hypercall interface is concerned.
Previously, for the sake of not modifying the migration code in libxc,
XEN_DOMCTL_get_ext_vcpucontext would jump though hoops to return -ENOBUFS if
and only if MSRs were in use and no buffer was present.
This is fragile, and awkward from a toolstack point-of-view when actually
sending MSR content in the migration stream. It also complicates fixing a
further race condition, between querying the number of MSRs for a vcpu, and
the vcpu touching a new one.
As this code is still only in unstable, take this opportunity to redesign the
interface. This patch introduces the brand new XEN_DOMCTL_{get,set}_vcpu_msrs
subops.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Generate JSON map handle according to KeyedUnion discriminator.
The original JSON output for a keyed union is like:
{
...
"u" : { FIELDS }
...
}
The discriminator is not generated, so that the parser won't be able to
figure out the fields in the incoming stream.
So we need to change this to something more sensible. For example, for
keyed union libxl_domain_type, which has a discriminator called "type",
we generate following for HVM guest:
{
...
"type.hvm" : { HVM FIELDS }
...
}
Parser then can know the type of this union and how to interpret the
incoming stream.
Note that we change the existing API here. However the original output is
quite broken anyway, we cannot make sensible use of it and I doubt that
there's existing user of existing API. So we are acutally fixing a
problem.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
This changeset introduces following functions:
* libxl_defbool_parse_json
* libxl__bool_parse_json
* libxl_uuid_parse_json
* libxl_mac_parse_json
* libxl_bitmap_parse_json
* libxl_cpuid_policy_list_parse_json
* libxl_string_list_parse_json
* libxl_key_value_list_parse_json
* libxl_hwcap_parse_json
* libxl__int_parse_json
* libxl__uint{8,16,32,64}_parse_json
* libxl__string_parse_json
They will be used in later patch to convert the libxl__json_object
tree of a builtin type to libxl_FOO struct.
Also remove declaration of libxl_domid_gen_json as libxl_domid uses
yajl_gen_integer to generate JSON object.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Anthony Perard <anthony.perard@citrix.com>
Given a JSON string, we need to convert it to libxl_FOO struct.
The approach is:
JSON string -> libxl__json_object -> libxl_FOO struct
With this approach we can make use of libxl's infrastructure to do the
first half (JSON string -> libxl__json_object).
Second half is done by auto-generated code by libxl's IDL
infrastructure. IDL patch(es) will come later.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
This json_fn is in fact used to generate string representation of a json
data structure. We will introduce another json function to parse json
data structure in later changeset, so rename json_fn to json_gen_fn to
clarify.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
yajl_gen_integer cannot cope with uint64_t, because it takes a signed
long long. If we pass to it an uint64_t number which is between INT_MAX
and UINT_MAX, it generates a negative number. Later when we feed this
generated number into parser, the result gets signed extended, which is
wrong.
A new function called libxl__uint64_gen_json is introduced to handle
uint64_t. It utilises yajl_gen_number to generate numbers.
Also removed a duplicated definition of MemKB while I was there.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Print out a warning and suggest user use "-V" option when invoking "xl
create". Also remove that option in manpage. This will introduce a
minor functional regression but it's very easy to work around.
The rationale behind this change is that, this option is actually not
part of domain configuration. It just affects whether a vncviewer
should be automatically spawn, but has nothing to do with how a domain
should be constructed. And this option is also bogus, considering if you
migrate a domain to a remote host and the receiver spawns a vncviewer on
the receiving side then it either dies silently or occupies resource.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
DECLARE_HYPERCALL_BUFFER_SHADOW() is like DECLARE_HYPERCALL_BUFFER()
except it is backed by an already allocated hypercall buffer.
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
This helps the compiler spot printf formatting errors.
Fix up resulting errors in xenctrl_osdep_ENOSYS.c. Substitute %p for the
slightly less bad %lx when trying to format an opaque structure.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
NMI and MCE interrupt handlers have no right to modify their exception frame
or underlying vcpu registers. Apply liberal quantities of 'const' to 'struct
cpu_user_regs *' throughout the codebase.
The Double Fault handler, while an IST handler, reloads some extra
architectural state back into its regs parameter. As this is for printing
purposes and on a terminal error path, the const requirements for #DF are
relaxed.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
To aid analysis of eventual errors, print EFI status codes with error
messages where available.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Andrew validly points out that the use of the macro on the restore path
can't rely on the CPUID bits for the guest already being in place (as
their setting by the tool stack in turn requires the other restore
operations already having taken place). And even worse, using
hvm_cpuid() is invalid here because that function assumes to be used in
the context of the vCPU in question.
Reverting to the behavior prior to the change from checking
cpu_has_sm?p to hvm_vcpu_has_sm?p() would break the other (non-restore)
use of the macro. So let's revert to the prior behavior only for the
restore path, by adding a respective second parameter to the macro.
Obviously the two cpu_has_* uses in the macro should really also be
converted to hvm_cpuid() based checks at least for the non-restore
path.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: David Vrabel <david.vrabel@citrix.com>
Otherwise functions in these sections can end up between .text and .rodata
which is after _etext and therefore gets made non-executable.
This matches x86 (although it was done there for different reasons).
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
Cc: Jan Beulich <JBeulich@suse.com>
cpufreq parameter can take more options than currently
documented. Include these with some comments regarding
their intention.
Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
After Andrew's commit 07884c9, all writes to password-protected
MSR's are performed using wrmsr_amd_safe.
Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
c/s bac6334b5 "move domain to cpupool0 before destroying it" introduced a
problem when destroying a HVM domain with PCI passthrough enabled. The
moving of the domain to cpupool0 includes moving the pirqs to the cpupool0
cpus, but the event channel infrastructure already is unusable for the
domain. So just avoid moving pirqs for dying domains.
Signed-off-by: Juergen Gross <jgross@suse.com>
Do not clobber errors from certain codepaths. Clobbering of -EINVAL from
failing "evc->size <= PV_XSAVE_SIZE(_xcr0_accum)" was a pre-existing bug.
However, clobbering -EINVAL/-EFAULT from the get codepath was a bug
unintentionally introduced by 090ca8c1 "x86/domctl: two functional fixes to
XEN_DOMCTL_[gs]etvcpuextstate".
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Virtual environments such as Xen HVM containers and VirtualBox do not
necessarily provide support for feature masking MSRs.
As their presence is detected by model numbers alone, and their use predicated
on command line parameters, use the safe() variants of {wr,rd}msr() to avoid
dying with an early #GP fault.
In fact, use the password variants in all cases because:
a) they are safe to use even if not strictly required
b) have a more useful function prototype for this purposes
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Interacting with the vcpu itself should be protected by vcpu_pause().
Buggy/naive toolstacks might encounter adverse interaction with a vcpu context
switch, or increase of xcr0_accum. There are no much problems with current
in-tree code.
Explicitly permit a NULL guest handle as being a request for size. It is the
prevailing Xen style, and without it, valgrind's ioctl handler is unable to
determine whether evc->buffer actually got written to.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
xstate_ctxt_size(xfeature_mask) is runtime constant after boot, and for bounds
checking when handling xsave state. Avoid reloading xcr0 twice to obtain a
number which has already been calculated.
Also annotate xfeature_mask as __read_mostly as it is only ever written once.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
When firmware-first mode is being indicated by firmware, we shouldn't
be modifying AER registers - these are considered to be owned by
firmware in that case. Violating this is being reported to result in
SMI storms. While circumventing the workaround means re-exposing
affected hosts to the XSA-59 issues, this in any event seems better
than not booting at all. Respective messages are being issued to the
log, so the situation can be diagnosed.
The basic building blocks were taken from Linux 3.15-rc. Note that
this includes a block of code enclosed in #ifdef CONFIG_X86_MCE - we
don't define that symbol, and that code also wouldn't build without
suitable machine check side code added; that should happen eventually,
but isn't subject of this change.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reported-by: Malcolm Crossley <malcolm.crossley@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Malcolm Crossley <malcolm.crossley@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Yang Zhang <yang.z.zhang@intel.com>
... and propagate this from hvm_inject_msi(). In the course of this I
spotted further room for cleanup:
- vmsi_inj_irq()'s struct domain * parameter was unused
- vmsi_deliver() pointlessly passed on dest_ExtINT to vmsi_inj_irq()
(which that one validly refused to handle)
- vmsi_inj_irq()'s sole caller guarantees a proper delivery mode (i.e.
rather than printing an obscure message we can just BUG())
- some formatting and log message quirks
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
There are a number of ways this operation can go wrong, all of which
got ignored so far.
In the context of this I wonder whether map_domain_emuirq_pirq()
returning 0 in the "already mapped" case is really intended to be that
way (this is why the subsequent NULL check here can't be an ASSERT()).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Coverity-ID: 1220092 "Unsigned compare against 0"
Coverity-ID: 1220093 "Out-of-bounds read"
Both of these are cased by the the while() loop in the fail path, which
results in an infinite loop and memory corruption from rangeset_destroy().
Move hvm_ioreq_server_free_rangesets() up and use it for cleanup on the
failure path.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
If the memory map is not shared between HAP and IOMMU we fail to set
correct IOMMU mappings for memory types other than p2m_ram_rw.
This patchs adds IOMMU support for the following memory types:
p2m_grant_map_rw, p2m_map_foreign, p2m_ram_ro, p2m_grant_map_ro and
p2m_ram_logdirty.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Tim Deegan <tim@xen.org>
Tested-by: David Zhuang <david.zhuang@oracle.com>
Prevent the usage of global logdirty if the domain is using the IOMMU,
and also prevent passthrough of devices if logdirty is enabled.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Tim Deegan <tim@xen.org>
This at once:
- prevents the page from being reassigned under our feet
- ensures that the domain owns the page, which stops a domain from giving a
grant mapping, MMIO region, other non-RAM as a hypercall input/output.
We need to hold the p2m lock while doing the lookup until we have the
reference.
This also requires that during domain 0 building current is set to an actual
dom0 vcpu, so take care of this at the same time as the p2m is temporarily
loaded.
Lastly when dumping the guest stack we need to make sure that the guest hasn't
pointed its sp off into the weeds and/or misaligned it, which could lead to
hypervisor traps. Solve this by using the new function and checking alignment
first.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>
In particular we need to make sure the guest has write permissions to buffers
which it passes as output buffers for hypercalls, otherwise the guest can
overwrite memory which it shouldn't be able to write (like r/o grant table
mappings).
This is XSA-98.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>
handle_mmio() is currently unsafe for pvh guests. A call to it would
result in call to vioapic_range that will crash xen since the vioapic
ptr in struct hvm_domain is not initialized for pvh guests.
However, one path exists for such a call. If a pvh guest, dom0 or domU,
unintentionally touches non-existing memory, an EPT violation would occur.
This would result in unconditional call to hvm_hap_nested_page_fault. In
that function, because get_gfn_type_access returns p2m_mmio_dm for non
existing mfns by default, handle_mmio() will get called. This would result
in xen crash instead of the guest crash. This patch addresses that.
Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
If a buggy BIOS programs an ACPI table with to small an entry length
then acpi_table_entries gets stuck in an infinite loop.
To aid debugging, report the error and exit the loop.
Based on Linux kernel commit 369d913b242cae2205471b11b6e33ac368ed33ec
Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
Use < instead of <= (which I wrongly suggested), return -ENODATA
instead of -EINVAL, and make description match code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
... making the code more generic and limiting address space consumption
(however small it might be) to just those machines that need this
mapping (this is an erratum workaround after all).
At the same time properly map the full needed range from the base
address instead of just the third page and fix some formatting.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
- pirq_info() returns NULL for a non-allocated pIRQ, and hence we
mustn't unconditionally de-reference it, and we need to invoke it
another time after having called map_domain_emuirq_pirq()
- don't use printk(), namely without XENLOG_GUEST, for error reporting
This is XSA-96.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
With all AMD mcheck initialization unified now after
commit 518576c, these two function definitions can be removed.
Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
validate_xsave() is called codepaths which load new vcpu xsave state from
XEN_DOMCTL_{setvcpuextstate,sethvmcontext}, usually as part of migration. In
both cases, this is the xfeature_mask of the saving Xen rather than the
restoring Xen.
Given that the xsave state itself is checked for consistency and validity on
the current cpu, checking whether it was valid for the cpu before migration is
not interesting (or indeed relevant, as the error can't be distinguished from
the other validity checking).
This change removes the need to pass the saving Xen's xfeature_mask,
simplifying the toolstack code and migration stream format in this area.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Andrew Cooper
Ian Campbell
Wei Liu
David Vrabel
Jan Beulich
Aravind Gopalakrishnan
Juergen Gross
Roger Pau Monné
Konrad Rzeszutek Wilk
Christian Wolter
Mukesh Rathor
Malcolm Crossley