While commit 75066cd4ea ("x86emul: fix {,i}mul and {,i}div") indeed did
as its title says, it broke the 3-operand form by uniformly using AL/AX/
EAX/RAX as second source operand. Fix this and add tests covering both
cases.
Reported-by: Andrei Lutas <vlutas@bitdefender.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Now that we require use of the {evex} pseudo-prefix, we can also use
the q-suffixed encoding of VPCMPESTRI, which is available as of 2.29
just like {evex} is.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
For both VT-x and SVM, the RDTSCP intercept will trigger if the pipeline
supports the instruction, but the guest may not have RDTSCP in its featureset.
Bring the vmexit handlers in line with the main emulator behaviour by
optionally handing back #UD.
Next on the AMD side, if RDTSCP actually ends up being intercepted on a debug
build or first-gen SVM hardware which lacks NRIP, we first update regs->rcx,
then call __get_instruction_length() asking for RDTSC. As the two
instructions are different (and indeed, different lengths!),
__get_instruction_length_from_list() fails and hands back a #GP fault.
This can demonstrated by putting a guest into tsc_mode="always emulate" and
executing an RDTSCP instruction:
(d1) --- Xen Test Framework ---
(d1) Environment: HVM 64bit (Long mode 4 levels)
(d1) Test rdtscp
(d1) TSC mode 1
(XEN) emulate.c:147:d1v0 __get_instruction_length: Mismatch between expected and actual instruction:
(XEN) emulate.c:152:d1v0 insn_index 8, opcode 0xf0031 modrm 0
(XEN) emulate.c:154:d1v0 rip 0x10475f, nextrip 0x104762, len 3
(XEN) SVM insn len emulation failed (1): d1v0 64bit @ 0008:0010475f -> 0f 01 f9 0f 31 5b 31 ff 31 c0 e9 c2 db ff ff 00
(d1) ******************************
(d1) PANIC: Unhandled exception at 0008:000000000010475f
(d1) Vec 13 #GP[0000]
(d1) ******************************
First, teach __get_instruction_length() to cope with RDTSCP, and improve
svm_vmexit_do_rdtsc() to ask for the correct instruction. Move the regs->rcx
adjustment into this function to ensure it gets done after we are done
potentially raising faults.
Reported-by: Paul Durrant <paul.durrant@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Brian Woods <brian.woods@amd.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
The helper pte_xen_addr computes the MFN based on the virtual
address and generates the PTE. This can be r
At the same time, make va a vaddr_t to make clear it holds virtual address.
Signed-off-by: Julien Grall <julien.grall@arm.com>
With being able to specify a dom0_mem value depending on host memory
size on x86 make it easy for distros to specify a default dom0 size by
adding a CONFIG_DOM0_MEM item which presets the dom0_mem boot parameter
value.
It will be used only if no dom0_mem parameter was specified in the
boot parameters.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Under desc->lock taken:
An IRQ with _IRQ_GUEST flag set always has an action.
An IRQ with _IRQ_DISABLED flag cleared always has an action.
Those flags checks cover all accesses to desc->action in do_IRQ,
so we can skip desc->action check in non-debug build.
Keep in place for debug build to help diagnostics potential
misconfiguration.
Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
Acked-by: Julien Grall <julien.grall@arm.com>
This action is excessive because for an invalid LR there is no need
to write another invalid value to a register. So we can skip it here,
saving a peripheral register write.
Keep clearing the LR for the DEBUG build. This would make dumped
invalid LRs be zero. That is more obvious than picking state bits
from a non-zero value.
Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
The page merging logic makes use of bits 1-8 and bit 63 of a PTE, which
used to be specified as 'ignored'. However, bits 5 and 6 are now specified
as 'accessed' and 'dirty' bits and their use only remains safe as long as
the DTE 'Host Access Dirty' bits remain unused by Xen, or by hardware
before the domain starts running. (XSA-275 disabled the operation of the
code after domain creation completes).
With the page merging logic present in its current form there are no spare
ignored bits in the PTE at all, but PV-IOMMU support will require at least
one spare bit to track which PTEs are added by hypercall.
This patch removes the code, freeing up the remaining PTE ignored bits
for other use, including PV-IOMMU support, as well as significantly
simplifying and shortening the source by ~170 lines. There may be some
marginal performance cost (but none has been observed in manual testing
with a passed-through NVIDIA GPU) since higher order mappings will now be
ruled out until a mapping order parameter is passed to iommu_ops. That will
be dealt with by a subsequent patch though.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Brian Woods <brian.woods@amd.com>
Xen mapping is first create using a 2MB page and then shatterred in 4KB
page for fine-graine permission. However, it is not safe to break-down
superpage page without going to an intermediate step invalidating
the entry.
As we are changing Xen mappings, we cannot go through the intermediate
step. The only solution is to create Xen mapping using 4KB entries
directly. As the Xen should always access the mappings according with
the runtime permission, it is then possible to set-up the permissions
while create the mapping.
We are still playing with the fire as there are still some
break-before-make issue in setup_pagetables (i.e switch between 2 sets of
page-tables). But it should slightly be better than the current state.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reported-by: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
Reported-by: Jan-Peter Larsson <Jan-Peter.Larsson@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Tested-by: Matthew Daley <mattd@bugfuzz.com>
A follow-up patch will add support for preemption in p2m_cache_flush_range.
Because of the complexity for the 2 loops, it would be necessary to add
preemption in both of them.
This can be avoided by merging the 2 loops together and still keeping
the code fairly simple to read and extend.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The function leave_hypervisor_tail is called before each return to the
guest vCPU. It has two main purposes:
1) Process physical CPU work (e.g rescheduling) if required
2) Prepare the physical CPU to run the guest vCPU
2) will always be done once we finished to process physical CPU work. At
the moment, it is done part of the last iterations of 1) making adding
some extra indentation in the code.
This could be streamlined by moving out 2) of the loop. At the same
time, 1) is moved in a separate function making more obvious what is
happening.
All those changes will help a follow-up patch where we would want to
introduce some vCPU work before returning to the guest vCPU.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
With the recent changes, a P2M entry may be populated but may not be
valid. In some situation, it would be useful to know whether the entry
has been marked available to guest in order to perform a specific
action. So extend p2m_get_entry to return the value of bit[0] (valid bit).
Signed-off-by: Julien Grall <julien.grall@arm.com>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Currently, we only allow to flush cache on regions mapped as p2m_ram_{rw,ro}.
There are no real problem in cache flushing any RAM regions such as grants
and foreign mapping. Therefore, relax the check to allow flushing the
cache on any RAM region.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The function will be easier to re-use in a follow-up patch if you have
only the begin and end.
At the same time, rename the function to reflect the change in the
prototype.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Currently a Stage-2 translation fault could happen:
1) MMIO emulation
2) Another pCPU was modifying the P2M using Break-Before-Make
3) Guest Physical address is not mapped
A follow-up patch will re-purpose the valid bit in an entry to generate
translation fault. This would be used to do an action on each entry to
track pages used for a given period.
When receiving the translation fault, we would need to walk the pages
table to find the faulting entry and then toggle valid bit. We can't use
p2m_lookup() for this purpose as it only tells us the mapping exists.
So this patch adds a new function to walk the page-tables and updates
the entry. This function will also handle 2) as it also requires walking
the page-table.
The function is able to cope with both table and block entry having the
validate bit unset. This gives flexibility to the function clearing the
valid bits. To keep the algorithm simple, the fault will be propating
one-level down. This will be repeated until a block entry has been
reached.
At the moment, there are no action done when reaching a block/page entry
but setting the valid bit to 1.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
A follow-up patch will re-purpose the valid bit of LPAE entries to
generate fault even on entry containing valid information.
This means that when translating a guest VA to guest PA (e.g IPA) will
fail if the Stage-2 entries used have the valid bit unset. Because of
that, we need to fallback to walk the page-table in software to check
whether the fault was expected.
This patch adds the software page-table walk on all the translation
fault. It would be possible in the future to avoid pointless walk when
the fault in PAR_EL1 is not a translation fault.
This function has only worked for guest RAM pages (no foreing mappings or
MMIO mappings) because we require the page to belong to the domain for
getting a reference. This means we can deny all non guest RAM pages.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The LPAE format allows to store information in an entry even with the
valid bit unset. In a follow-up patch, we will take advantage of this
feature to re-purpose the valid bit for generating a translation fault
even if an entry contains valid information.
So we need a different way to know whether an entry contains valid
information. It is possible to use the information hold in the p2m_type
to know for that purpose. Indeed all entries containing valid
information will have a valid p2m type (i.e p2m_type != p2m_invalid).
This patch introduces a new helper p2m_is_valid, which implements that
idea, and replace most of lpae_is_valid call with the new helper. The ones
remaining are for TLBs handling and entries accounting.
With the renaming there are 2 others changes required:
- Generate table entry with a valid p2m type
- Detect new mapping for proper stats accounting
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
A lot of the headers are not necessary, so remove them. At the same
time, re-order them alphabetically.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
A couple of places in the code will need to clear/set flags in HCR_EL2
for a given vCPU and then replicate into the hardware. Introduce
helpers and replace open-coded version.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Use __map_domain_page macro to deal with page_info directly.
No functional change.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Remove trailing whitespaces. Turn bool_t into bool. Annotate a section
for CONFIG_SEPARATE_XENHEAP.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The out label is followed by a semicolon only. Use return directly.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Today the memory size of dom0 can be specified only in terms of bytes
(either an absolute value or "host-mem - value"). When dom0 shouldn't
be auto-ballooned this requires nearly always a manual adaption of the
Xen boot parameters to reflect the actual host memory size.
Add more possibilities to specify memory sizes. Today we have:
dom0_mem= List of ( min:<size> | max:<size> | <size> )
with <size> being a positive or negative size value (e.g. 1G).
Modify that to:
dom0_mem= List of ( min:<sz> | max:<sz> | <sz> )
<sz>: <size> | [<size>+]<frac>%
<frac>: integer value < 100
With the following semantics:
<frac>% specifies a fraction of host memory size in percent.
<sz> is a percentage of host memory plus an offset.
So <sz> being 1G+25% on a 256G host would result in 65G.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Modify parse_size_and_unit() to support a value followed by a '%'
character. In this case ps is required to be non-NULL to ensure the
caller can detect that case. The returned value will be the integer
value s was pointing to and *ps will point to the '%' character.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
While actually benign (operands are either register or memory ones
anyway), I think it is better to use != instead of == for such checks.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
We skipped build stage for those branches. We want to skip test state
for those branches too.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Doug Goldstein <cardoe@cardoe.com>
By default on capable hardware, SECONDARY_EXEC_ENABLE_VMCS_SHADOWING is
activated unilaterally. The VMCS Link pointer is initialised to ~0, but the
VMREAD/VMWRITE bitmap pointers are not.
This causes the 16bit IVT and Bios Data Area get interpreted as the read/write
permission bitmap for guests which blindly execute VMREAD/VMWRITE
instructions.
This is not a security issue because the VMCS Link pointer being ~0 causes
VMREAD/VMWRITE to complete with VMFailInvalid (rather than modifying a
potential shadow VMCS), and the contents of MFN 0 has already been determined
not to contain any interesting data because of L1TF's ability to read that 4k
frame.
Leave VMCS Shadowing disabled by default, and toggle it in
nvmx_{set,clear}_vmcs_pointer(). This isn't the most efficient course of
action, but it is the most simple way of leaving nested-virt working as it did
before.
While editing construct_vmcs(), collect all default secondary_exec_control
modifications together. The disabling of PML is latently buggy because it
happens after secondary_exec_control are written into the VMCS, although there
is an unconditional update later which writes the correct value into hardware.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
A large amount of the information here is obsolete since Xen 4.7
To being with, however, this patch marks a change in style for section
headings, due to how HTML anchors are generated. Having more than one
parameter per heading makes an awkward anchor, especially when brace globbing
is used. Furthermore, the misc suffixes such as (AMD only) get included, as
do the escaping for the underscores.
Markdown doesn't require escaped underscores in headings (I'm not entirely
sure how we ended up with that style), so remove them and fully expand the
glob syntax. Also adjust com1,com2 while at it, which is the only other
multi-parameter heading. Move the misc suffixes into an "Applicability:" note
alongside the information about defaults.
This results in the headings being unadorned, and identical to how they are
expressed on the command line and in code.
For cpuid_mask_cpu option, collapse the long line of almost identical strings
using [] globbing. The result is much shorter and clearer to read. Add a
warning that this option no longer masks all features on Fam15h and above, due
to not making use of the leaf 7 masks.
For the remainder of the cpuid_mask_* options, collapse them all together into
a single description.
Finally, leave an explicit note explaining that people should not be using
these options for migration safety.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Adjust the default line to note that the default is now selectable in Kconfig.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
* vwfi needs a closing `. rmrr needs one as well, and the opening ' switched
to `
* The com1/com2 example lines are already verbatim blocks and shouldn't
escape their underscores. This ends up in the rendered output.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
* Add "uint64_t raw" to seg_desc_t to remove the opencoded uint64_t casting
in this function. Change the parameter to be of type seg_desc_t.
* Rename the 'pa' parameter to 'gaddr', because it lives in GFN space rather
than physical address space.
* Use gfn_t and mfn_t rather than unsigned longs.
* Check the alignment and proposed new descriptor before taking a page
reference.
* Use the more flexible ACCESS_ONCE() accessor in preference to
write_atomic()
No expected change in behaviour.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The struct suffix is redundant in the name, and a future change will want to
turn it into a union, rather than a structure. As this represents a segment
descriptor, give it an appropriate typedef.
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>
Now, as well as a `code block', with
| Something: some status
we tolerate a definition list which in pandoc terms looks like this
|Term
|: Definition
This ought not usually be be used for features but it will be useful
for linking to the release notes, because markup is not allowed in
code blocks but is in definitions.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Juergen Gross <jgross@suse.com>
In fact this was not markdown content, but just a string. We are
however going to make it be markdown content. So adjust the comments,
and the consumer.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Juergen Gross <jgross@suse.com>
If our run of pandoc to convert pieces of markup in our hand, into
html, fails, print the json that was rejected.
No change in non-error cases.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Juergen Gross <jgross@suse.com>
The code in viridian_synic_wrmsr() duplicates logic in vlapic_reg_write()
to update the ICR, ICR2 and TASKPRI registers. Instead of doing this,
make vlapic_reg_write() non-static and call it.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Rename "offset" to "reg" for consistency with the rest of the vlapic API.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
When viridian APIC assist is active, the code in vlapic_has_pending_irq()
may end up re-calling vlapic_find_highest_isr() after emulating an EOI
whereas simply moving the call after the EOI emulation removes the need
for this duplication.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
To be able to use it for the hot-plugged CPUs as well.
The reason why we link proc_info_list in ".rodata" section is that
it context should never be modified.
This patch also renames ".init.proc.info" section to ".proc.info"
as "init" prefix is not actual anymore.
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
The Xenstore domid isn't set for HVM domains. This will result in
failure when booting a HVM domain on a system with Xenstore not running
in dom0.
Same applies for console domid, so set both.
This is broken since commit a2d9a6fa1f ("tools/libxenctrl: use new
xenforeignmemory API to seed grant table").
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Which, on x86, requires fiddling with the INTx bit in PCI config space,
since for internally used MSI we can't delegate this to Dom0.
ns16550_init_postirq() also needs (benign) re-ordering of its
operations.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
In order for a Xen internal PCI device driver to enable MSI on the
device, we need another hook which the driver can use to create the IRQ
(doing this in the init_preirq hook is too early, since IRQ code hasn't
got initialized at that time yet, and doing it in init_postirq is too
late because at least on x86 smp_intr_init() needs to know the IRQ
number).
On x86 this additionally requires a slight ordering change to IRQ
initialization, to facilitate calling the new hook between basic
initialization and the call path leading to smp_intr_init().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Even if unlikely, donate_page() should not ignore the possible need to
obtain a domain reference. To make people look more closely when they
add new uses of domain_adjust_tot_pages(), force its return value to be
checked. This in turn requires a benign change to assign_pages().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>