Recent AMD processors may report up to 128 logical processors in CPUID
leaf 1. Doubling this value produces 0 (which OSes sincerely dislike),
as the respective field is only 8 bits wide. Suppress doubling the value
(and its leaf 0x80000008 counterpart) in such a case.
Note that while there's a similar overflow in intel_xc_cpuid_policy(),
that one is being left alone for now.
Note further that while it was considered to suppress the multiplication
by 2 altogether if the host topology already provides at least one bit
of thread ID within APIC IDs, it was decided to avoid more change here
than really needed at this point.
Also zap leaf 4 (and at the same time leaf 2) EDX output for AMD, as it
should have been from the beginning.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
libxc/x86: correct overflow avoidance check in AMD CPUID handling
Commit df29d03f1d ("libxc/x86: avoid certain overflows in CPUID APIC ID
adjustments" introduced a one bit too narrow mask when checking whether
multiplying by 1 (in particular in leaf 1) would result in overflow.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: df29d03f1d
master date: 2019-09-24 10:50:33 +0200
master commit: c9c7ac508b
master date: 2019-09-25 15:50:58 +0200
Don't allow the hardware domain write access the PCI config space of
devices marked as read-only.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 79f9ba7838
master date: 2019-09-17 16:13:39 +0200
The loop in FOR_EACH_IOREQ_SERVER is backwards hence the cleanup on
failure needs to be done forwards.
Fixes: 97a5a3e301 ('x86/hvm/ioreq: maintain an array of ioreq servers rather than a list')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
master commit: 215f2576b0
master date: 2019-09-10 16:32:47 +0200
7a0 is an integer field, not a mask - taking the logical and of the hardware
and policy values results in nonsense. Instead, take the policy value
directly.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@cirtrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: b50d78d0ea
master date: 2019-09-10 13:33:21 +0100
Commit 2634b997af ("x86/shadow: don't enable shadow mode with too small
a shadow allocation") was incomplete: The adjustment done there to
shadow_enable() is also needed in shadow_one_bit_enable(). The (new)
problem report was (apparently) a failed PV guest migration followed by
another migration attempt for that same guest. Disabling log-dirty mode
after the first one had left a couple of shadow pages allocated (perhaps
something that also wants fixing), and hence the second enabling of
log-dirty mode wouldn't have allocated anything further.
Reported-by: James Wang <jnwang@suse.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Tim Deegan <tim@xen.org>
master commit: 8b25551baa
master date: 2019-09-05 09:56:42 +0200
setup_clear_cpu_cap() is __init and hence may not be called post-boot.
Note that opt_pku nevertheless is not getting __initdata added - see
e.g. commit 43fa95ae6a ("mm: make opt_bootscrub non-init").
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 41c7700a00
master date: 2019-08-29 15:10:07 +0200
The level passed to ept_invalidate_emt corresponds to the EPT entry
passed as the mfn parameter, which is a pointer to an EPT page table,
hence the entries in that page table will have one level less than the
parent.
Fix the call to atomic_write_ept_entry to pass the correct level, ie:
one level less than the parent.
Fixes: 50fe6e7370 ('pvh dom0: add and remove foreign pages')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>.
master commit: b806c91275
master date: 2019-08-28 16:57:36 +0200
Since guest resource management work it's now possible to have a page
assigned to a domain without a valid M2P entry. Some paths in the code
rely on the fact a GFN returned from mfn_to_gfn() for such a page
is not valid as well, i.e. see arch_iommu_populate_page_table().
For systems without 512GB contiguous RAM M2P entries were already
correctly initialised on boot with INVALID_M2P_ENTRY (~0UL) but
on systems where M2P could be covered by a single 1GB page directory
0x77 poison was used instead. That eventually resulted in a crash
during IOMMU construction on systems without shared PTs enabled.
While here fix up compat M2P entries as well.
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 6c093931a7
master date: 2019-08-27 13:40:42 +0100
The current usage of need_iommu_pt_sync in p2m for non-translated
guests is wrong because it doesn't correctly handle a relaxed PV
hardware domain, that has need_sync set to false, but still need
entries to be added from calls to {set/clear}_identity_p2m_entry.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
Tested-by: Roman Shaposhnik <roman@zededa.com>
master commit: 77a994f3f8
master date: 2019-08-20 14:24:05 +0100
Code in intel.c:early_init_intel() modifies IA32_MISC_ENABLE MSR. Those
modifications must be restored after resuming from S3 (see e.g. Linux wakeup
code), otherwise bad things may happen (e.g. wakeup code may cause #GP when
trying to set IA32_EFER.NXE [1]).
This bug was noticed on a ThinkPad x230 with NX disabled in the BIOS:
Xen could correctly boot, but crashed when resuming from suspend.
Applying this patch fixed the problem.
[1] Intel SDM vol 3: "If the execute-disable capability is not
available, a write to set IA32_EFER.NXE produces a #GP exception."
Signed-off-by: Michał Kowalczyk <mkow@invisiblethingslab.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: c3cfa5b308
master date: 2019-08-19 15:07:34 +0100
The XPTI work restricted the visibility of most of memory, but missed a few
aspects when it came to the TSS.
Given that the TSS is just an object in percpu data, the 4k mapping for it
created in setup_cpu_root_pgt() maps adjacent percpu data, making it all
leakable via Meltdown, even when XPTI is in use.
Furthermore, no care is taken to check that the TSS doesn't cross a page
boundary. As it turns out, struct tss_struct is aligned on its size which
does prevent it straddling a page boundary.
Rework the TSS types while making this change. Rename tss_struct to tss64, to
mirror the existing tss32 structure we have in HVM's Tast Switch logic. Drop
tss64's alignment and __cacheline_filler[] field.
Introduce tss_page which contains a single tss64 and keeps the rest of the
page clear, so no adjacent data can be leaked. Move the definition from
setup.c to traps.c, which is a more appropriate place for it to live.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
master commit: 7888440625
master date: 2019-08-12 14:10:09 +0100
Combining of buddies happens only such that the resulting larger buddy
is still order-aligned. To cross a zone boundary while merging, the
implication is that both the buddy [0, 2^n-1] and the buddy
[2^n, 2^(n+1)-1] are free.
Ideally we want to fix the allocator, but for now we can just prevent
adding the MFN 0 in the allocator to avoid merging across zone
boundaries.
On x86, the MFN 0 is already kept away from the buddy allocator. So the
bug can only happen on Arm platform where the first memory bank is
starting at 0.
As this is a specific to the allocator, the MFN 0 is removed in the common code
to cater all the architectures (current and future).
[Stefano: improve commit message]
Reported-by: Jeff Kubascik <jeff.kubascik@dornerworks.com>
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
master commit: 762b9a2d99
master date: 2019-08-09 11:12:55 -0700
Future changes are going to need to page align some percpu data.
Shuffle the exact link order of items within the BSS to give
.bss.percpu.page_aligned appropriate alignment, even on CPU0, which uses
.bss.percpu itself.
Insert explicit alignment such that there won't be a gap between
__per_cpu_start and the first actual per-CPU object. The POINTER_ALIGN
for __bss_end is to cover the lack of SMP_CACHE_BYTES alignment, as the
loops which zero the BSS use pointer-sized stores on all architectures.
Rework __DEFINE_PER_CPU() so the caller passes in all attributes, and
adjust DEFINE_PER_CPU{,_READ_MOSTLY}() to match. This has the added bonus
that it is now possible to grep for .bss.percpu and find all the users.
Finally, introduce DEFINE_PER_CPU_PAGE_ALIGNED() which specifies the
section attribute and verifies the type's alignment.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Make DEFINE_PER_CPU_PAGE_ALIGNED() verify the alignment rather than
specifying it. It is the underlying type which should be suitably aligned.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 6c9639a72f
master date: 2019-08-09 16:36:58 +0200
There is no point causing the CPU to performed a locked update of the
descriptors on first use.
Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: af292b41e9
master date: 2019-08-07 13:34:56 +0100
Current code calls apic_x2apic_probe which does some initialization
and setup before having enabled x2APIC mode (if it's not already
enabled by the firmware).
This can lead to issues if the APIC ID doesn't match the x2APIC ID, as
apic_x2apic_probe calls init_apic_ldr_x2apic_cluster which depending
on the APIC mode might set cpu_2_logical_apicid using the APIC ID
instead of the x2APIC ID (because x2APIC might not be enabled yet).
Fix this by enabling x2APIC before calling apic_x2apic_probe.
As a remark, this was discovered while I was trying to figure out why
one of my test boxes didn't report any iommu faults. The root cause
was that the iommu MSI address field was set using the stale value in
cpu_2_logical_apicid, and thus the iommu fault interrupt would get
lost. Even if the MSI address field gets sets to a correct value
afterwards as soon as a single iommu fault is pending no further
interrupts would get injected, so losing a single iommu fault
interrupt is fatal.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 260940578d
master date: 2019-08-07 12:09:51 +0200
Currently cpu_sig struct is not updated during boot if no microcode blob
is specified by "ucode=[<interger>| scan]".
It will result in cpu_sig.rev being 0 which affects APIC's
check_deadline_errata() and retpoline_safe() functions.
Fix this by getting ucode revision early during boot and SMP bring up.
While at it, protect early_microcode_update_cpu() for cases when
microcode_ops is NULL.
Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 2bb2c55cf8
master date: 2019-08-01 18:45:32 +0100
Booting with spec-ctrl=0 results in Xen printing "None MD_CLEAR".
(XEN) Support for HVM VMs: None MD_CLEAR
(XEN) Support for PV VMs: None MD_CLEAR
Add a check about X86_FEATURE_MD_CLEAR to avoid to print "None".
Signed-off-by: James Wang <jnwang@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 2adc580bd5
master date: 2019-07-31 14:53:13 +0100
c/s 201f852eaf added start_info.h and kconfig.h to reloc.c, but only updated
start_info.h in RELOC_DEPS.
This causes reloc.c to not be regenerated when Kconfig changes. It is most
noticeable when enabling CONFIG_PVH and finding the resulting binary crash
early with:
(d9) (XEN)
(d9) (XEN) ****************************************
(d9) (XEN) Panic on CPU 0:
(d9) (XEN) Magic value is wrong: c2c2c2c2
(d9) (XEN) ****************************************
(d9) (XEN)
(d9) (XEN) Reboot in five seconds...
(XEN) d9v0 Triple fault - invoking HVM shutdown action 1
Reported-by: Paul Durrant <paul.durrant@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: 78c0000c87
master date: 2019-07-31 11:19:45 +0100
EPT differs from NPT and shadow when translating page orders to levels
in the physmap page tables. EPT page tables level for order 0 pages is
0, while NPT and shadow instead use 1, ie: EPT page tables levels
starts at 0 while NPT and shadow starts at 1.
Fix the p2m_entry_modify call in atomic_write_ept_entry to always add
one to the level, in order to match NPT and shadow usage.
While there also add a check to ensure p2m_entry_modify is never
called with level == 0. That should allow to catch future errors
related to the level parameter.
Fixes: c7a4c088ad ('x86/mm: split p2m ioreq server pages special handling into helper')
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>
master commit: ec2ab491b5
master date: 2019-07-16 09:05:28 +0200
On some machines (for example Thinkpad P52), UEFI GOP reports
framebuffer located above 4GB (0x4000000000 on that machine). This
address does not fit in {xen,dom0}_vga_console_info.u.vesa_lfb.lfb_base
field, which is 32bit. The overflow here cause all kind of memory
corruption when anything tries to write something on the screen,
starting with zeroing the whole framebuffer in vesa_init().
Fix this similar to how it's done in Linux: add ext_lfb_base field at
the end of the structure, to hold upper 32bits of the address. Since the
field is added at the end of the structure, it will work with older
Linux versions too (other than using possibly truncated address - no
worse than without this change). Thanks to ABI containing size of the
structure (start_info.console.dom0.info_size), Linux can detect when
this field is present and use it appropriately then.
Since this change public interface and use __XEN_INTERFACE_VERSION__,
bump __XEN_LATEST_INTERFACE_VERSION__.
Note: if/when backporting this change to Xen <= 4.12, #if in xen.h needs
to be extended with " || defined(__XEN__)".
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 9cf11fdcd9
master date: 2019-05-17 14:48:23 +0200
After a reboot of a guest only the first pci device configuration will
be retrieved from Xenstore resulting in loss of any further assigned
passed through pci devices.
The main reason is that all passed through pci devices reside under a
common root device "0" in Xenstore. So when the device list is rebuilt
from Xenstore after a reboot the sub-devices below that root device
need to be selected instead of using the root device number as a
selector.
Fix that by adding a new member to struct libxl_device_type which when
set is used to get the number of devices. Add such a member for pci to
get the correct number of pci devices instead of implying it from the
number of pci root devices (which will always be 1).
While at it fix the type of libxl__device_pci_from_xs_be() to match
the one of the .from_xenstore member of struct libxl_device_type. This
fixes a latent bug checking the return value of a function returning
void.
Signed-off-by: Juergen Gross <jgross@suse.com>
Tested-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
(cherry picked from commit c19434d928)
Backport-requested-by: Pasi Kärkkäinen <pasik@iki.fi>
Backport-requested-by: Roman Shaposhnik <roman@zededa.com>
Backport-requested-by: Rich Persaud <persaur@gmail.com>
DMA-ing to the stack is considered bad practice. In this case, if a
timeout occurs because of a sluggish device which is processing the
request, the completion notification will corrupt the stack of a
subsequent deeper call tree.
Place the poll_slot in a percpu area and DMA to that instead.
Fix the declaration of saddr in struct qinval_entry, to avoid a shift by
two. The requirement here is that the DMA address is dword aligned,
which is covered by poll_slot's type.
This change does not address other issues. Correlating completions
after a timeout with their request is a more complicated change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
master commit: 8970834eb9
master date: 2019-07-24 14:40:10 +0100
Following 6ff560f7f ("x86/SMP: don't try to stop already stopped CPUs")
an incorrect condition was placed into kexec transition path
leaving crashing CPU always online breaking kdump kernel entering.
Correct it by unifying the condition with smp_send_stop().
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
master commit: f56813f347
master date: 2019-07-22 18:02:24 +0100
When there are sufficiently many devices listed in the ACPI tables (no
matter if they actually exist), output may take way longer than the
watchdog would like.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Brian Woods <brian.woods@amd.com>
master commit: df2030c34c
master date: 2019-07-22 12:03:46 +0200
The interrupt remapping in-use bitmaps were leaked in all cases. The
ring buffers and the mapping of the MMIO space were leaked for any IOMMU
that hadn't been enabled yet.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Brian Woods <brian.woods@amd.com>
master commit: 6ae22e7aac
master date: 2019-07-22 11:59:01 +0200
Some logging messages made more sense as argo debug
logs rather than standard Xen logs. Use argo_dprintk
to only print this info if argo DEBUG is enabled.
Signed-off-by: Nicholas Tsirakis <tsirakisn@ainfosec.com>
Reviewed-by: Christopher Clark <christopher.w.clark@gmail.com>
master commit: 7abd7c21b9
master date: 2019-06-19 21:14:05 +0100
When a message is requeue'd in Xen's internal queue, the queue
entry contains the length of the message so that Xen knows to
send a VIRQ to the respective domain when enough space frees up
in the ring. Due to a small bug, however, Xen doesn't populate
the length of the msg if a given write fails, so this length is
always reported as zero. This causes Xen to spuriously wake up
a domain even when the ring doesn't have enough space.
This patch makes sure that the msg len is properly reported by
populating it in the event of a write failure.
Signed-off-by: Nicholas Tsirakis <tsirakisn@ainfosec.com>
Reviewed-by: Christopher Clark <christopher.w.clark@gmail.com>
master commit: 8966a3e9ab
master date: 2019-06-12 21:06:18 +0100
In its current state, if the destination ring is full, sendv()
will requeue the message and return the rc of pending_requeue(),
which will return 0 on success. This prevents the caller from
distinguishing the difference between a successful write and a
message that needs to be resent at a later time.
Instead, capture the -EAGAIN value returned from ringbuf_insert()
and *only* overwrite it if the rc of pending_requeue() is non-zero.
This allows the caller to make intelligent decisions on -EAGAIN and
still be alerted if the pending message fails to requeue.
Signed-off-by: Nicholas Tsirakis <tsirakisn@ainfosec.com>
Reviewed-by: Christopher Clark <christopher.w.clark@gmail.com>
master commit: 480800c769
master date: 2019-06-11 20:27:28 +0100
Calling virt_to_mfn() in the context switch path is a lot
of wasted cycles for a result which is constant after boot.
Begin by documenting how Xen handles the GDTs across context switch.
The loop in write_full_gdt_ptes() is unnecessary, because
NR_RESERVED_GDT_PAGES is 1. Dropping it makes the code substantially
more clear, and with it dropped, write_full_gdt_ptes() becomes more
obviously a poor name, so rename it to update_xen_slot_in_full_gdt().
Furthermore, load_full_gdt() is completely independent of the current
CPU, and load_default_gdt() only needs the current CPU's regular
GDT. (This is a change in behaviour, as previously it may have used the
compat GDT, but either will do.)
Add two extra per-cpu variables which cache the L1e for the regular and compat
GDT, calculated in cpu_smpboot_alloc()/trap_init() as appropriate, so
update_xen_slot_in_full_gdt() doesn't need to waste time performing the same
calculation on every context switch.
One performance scenario of Jüergen's (time to build the hypervisor on
an 8 CPU system, with two single-vCPU MiniOS VMs constantly interrupting
dom0 with events) shows the following, average over 5 measurements:
elapsed user system
Unpatched 66.51 232.93 109.21
Patched 57.00 225.47 105.47
which is a substantial improvement.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Tested-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 564d261687
master date: 2019-07-10 09:40:25 -0500
In preparation for core scheduling, carve out the GDT related
functionality (writing GDT related PTEs, loading default of full GDT)
into sub-functions.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 12dce7ea5a
master date: 2019-07-04 16:02:52 +0200
VMExit doesn't switch all state. The FS/GS/TS/LDTR/GSBASE segment
information, and SYSCALL/SYSENTER MSRs may still be cached in hardware, rather
than up-to-date in the VMCB.
Export svm_sync_vmcb() via svmdebug.h so svm_vmcb_dump() can use it, and bring
the VMCB into sync in current context.
As a minor optimisation, switch svm_sync_vmcb() to use svm_vm{load,save}_pa(),
as svm->vmcb_pa is always correct, and this avoids a redundant __pa()
translation behind the scenes.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Brian Woods <brian.woods@amd.com>
master commit: 7d161f6537
master date: 2019-06-19 19:54:22 +0100
This code was never updated when the 32bit build of Xen was dropped.
* Expand the now-redundant ptr_reg macro.
* The number of iterations in the loop can be halfed by using 64bit writes,
without consuming any extra execution resource in the pipeline. Adjust all
numbers/offsets appropriately.
* Replace dec with sub to avoid a eflags stall, and position it to be
macro-fused with the related jmp.
* With no need to preserve eflags across the body of the loop, replace lea
with add which has 1/3'rd the latency on basically all 64bit hardware.
A quick userspace perf test on my Haswell dev box indicates that the old
version takes ~1385 cycles on average (ignoring outliers), and the new version
takes ~1060 cyles, or about 77% of the time.
Reported-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 260acc521d
master date: 2019-06-18 15:47:52 +0100
The p2m_altp2m_lazy_copy is responsible for lazily populating an
altp2m view when the guest traps out due to no EPT entry being present
in the active view. Currently, in addition to taking a number of
unused argements, the whole calling convention has a number of
redundant p2m lookups: the function reads the hostp2m, even though the
caller has just read the same hostp2m entry; and then the caller
re-reads the altp2m entry that the function has just read (and possibly set).
Rework this function to make it a bit more rational. Specifically:
- Pass the current hostp2m entry values we have just read for it to
use to populate the altp2m entry if it finds the entry empty.
- If the altp2m entry is not empty, pass out the values we've read so
the caller doesn't need to re-walk the tables
- Either way, return with the gfn 'locked', to make clean-up handling
more consistent.
Rename the function to better reflect this functionality.
While we're here, change bool_t to bool, and return true/false rather
than 1/0.
It's a bit grating to do both the p2m_lock() and the get_gfn(),
knowing that they boil down to the same thing at the moment; but we
have to maintain the fiction until such time as we decide to get rid
of it entirely.
Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Tested-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
x86/altp2m: Fix style errors introduced with c/s 9abcac7ff
Drop introduced trailing whitespace, excessively long lines, mal-indention,
superfluous use of PRI macros for int-or-smaller types, and incorrect PRI
macros for gfns and mfns.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
master commit: 9abcac7ff1
master date: 2019-05-28 14:10:36 +0100
master commit: df4e4cafd2
master date: 2019-05-29 14:39:28 +0100
Ever since its introducion in c/s 436fb462 "x86/microcode: enable boot
time (pre-Dom0) loading", the allocation has gone un-freed, and has its final
use as part of constructing dom0.
Xen already consideres it an error to have more than a single unaccounted-for
module (again, logic from the same change), and will only pass the first one
to dom0 as the initrd.
Instead of having an 8 byte pointer to a bitmap which won't exceed 4 bits wide
in any production scenario (dom0 kernel, initrd, XSM blob and microcode blob),
allocate module_map[] on the stack and add a sanity bound for mbi->mods_count.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
master commit: 9b757bdc17
master date: 2019-05-13 10:35:37 +0100
c/s 9383de210 "x86/altp2m: support for setting restrictions for an array of
pages" introduced this logic, but do_hvm_op() was already capable of handling
-ERESTART correctly.
More problematic however is a continuation from compat_altp2m_op(). The arg
written back into register state points into the hypercall XLAT area, not at
the original parameter passed by the guest. It may be truncated by the
vmentry, but definitely won't be correct on the next invocation.
Delete the hypercall_create_continuation() call, and return -ERESTART, which
will cause the compat case to start working correctly.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 8228577ad1
master date: 2019-04-09 19:34:41 +0100
There are a number of bugs. There are no read/write hooks on the HVM side, so
guest accesses fall into the "read/write-discard" defaults, which bypass the
correct faulting behaviour and the Intel special case.
For the PV side, writes are discarded (again, bypassing proper faulting),
except for a pinned dom0, which is permitted to actually write the values
other than 0. This is pointless with read hook implementing the Intel special
case.
However, implementing the Intel special case is itself pointless. First of
all, OS software can't guarentee to read back 0 in the first place, because a)
this behaviour isn't guarenteed in the SDM, and b) there are SMM handlers
which use the CPUID instruction. Secondly, when a guest executes CPUID, this
doesn't typically result in Xen executing a CPUID instruction in practice.
With the dom0 special case removed, there are now no writes to this MSR other
than Xen's microcode loading facilities, which means that the value held in
the MSR will be properly up-to-date. Forward it directly, without jumping
through any hoops.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 013896cb8b
master date: 2019-04-05 11:09:08 +0100
p2m_set_mem_access() (and other places) treat view 0 as the
hostp2m, but p2m_get_mem_access() does not. Correct that
inconsistency.
Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
master commit: 44f3c3cdd3
master date: 2019-04-03 15:14:30 +0100
Bring libxl's in line with the public header, and update xen-cpuid's to
the latest information available in Intel's documentation (SDM ver 068
and ISA extensions ver 035), with (as before) the exception on MAWAU.
Some pre-existing strings get changed to match SDM naming. This should
be benign in xen-cpuid, and I hope it's also acceptable in libxl, where
people actually using the slightly wrong names would have to update
their guest config files.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
[Backport restricted to just xen-cpuid.c]
master commit: 787619a064
master date: 2019-03-14 16:38:39 +0100
The logic in xstate_init() is a rementent of the pre-featuremask days.
Collect the xstate features in generic_identify(), like all other feature
leaves, after which identify_cpu() will apply the known_feature[] mask derived
from the automatically generated CPUID information.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
master commit: 65c165d659
master date: 2019-03-12 13:57:13 +0000
The for loop that deals with MSI masking is coded as follows:
for ( pos = 0; pos < entry->msi.nvec; ++pos, ++entry )
Thus the loop termination condition is dereferencing a struct pointer that
is being incremented by the loop.
A block of MSI entries stores the number of vectors in entry[0].msi.nvec,
with all subsequent entries using a value of 0. Therefore, for a block of
two or more MSIs will terminate the loop early, as entry[1].msi.nvec is 0.
However, for a single MSI, ++entry moves the pointer out of bounds, and a
bogus read is used for the termination condition. In the case that the
loop body gets entered, there are subsequent OoB writes which clobber
adjacent memory in the heap.
This patch simply initializes a stack variable to the value of
entry->msi.nvec before starting the loop and then uses that in the
termination condition instead.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 56ad626532
master date: 2019-07-02 12:00:42 +0100
This fixes booting of old non-PV-OPS kernels which historically
looked for OSXSAVE instead of XSAVE bit in CPUID to check whether
XSAVE feature is enabled. If such a guest appears to be started on
an XSAVE enabled CPU and the feature is explicitly cleared in
policy, leaked OSXSAVE bit from Xen will lead to guest crash early in
boot.
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 902888922e
master date: 2019-06-28 13:17:53 +0100
Just like for other family values of 0x17 (see "x86/AMD: correct certain
Fam17 checks"), commit 3157bb4e13 ("Add MSR support for various feature
AMD processor families") made the original check for Fam11 here include
families all the way up to Fam17. The involved MSR (0xC0010055),
however, is fully reserved starting from Fam16, and the two bits of
interest are reserved for Fam12 and onwards (albeit I admit I wasn't
able to find any Fam13 doc). Restore the upper bound to be Fam11.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 5c2926f576
master date: 2019-06-18 16:34:51 +0200
Commit 3157bb4e13 ("Add MSR support for various feature AMD processor
families") converted certain checks for Fam11 to include families all
the way up to Fam17. The commit having no description, it is hard to
tell whether this was a mechanical dec->hex conversion mistake, or
indeed intended. In any event the NB_CFG handling needs to be restricted
to Fam16 and below: Fam17 doesn't really have such an MSR anymore. As
per observation it's read-zero / write-discard now, so make PV uniformly
(with the exception of pinned Dom0 vCPU-s) behave so, just like HVM
already does.
Mirror the NB_CFG behavior to MSR_FAM10H_MMIO_CONF_BASE as well, except
that here the vendor/model check is kept in place (for now at least).
A non-MMCFG extended config space access mechanism still appears to
exist, but code to deal with it will need to be written down the road,
when it can actually be tested.
Reported-by: Pu Wen <puwen@hygon.cn>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: e0fbf3bf98
master date: 2019-06-18 16:33:53 +0200
UBSAN reports:
(XEN) ================================================================================
(XEN) UBSAN: Undefined behaviour in x86_64/mm.c:1108:31
(XEN) left shift of 255 by 24 places cannot be represented in type 'int'
(XEN) ----[ Xen-4.13-unstable x86_64 debug=y Tainted: H ]----
(XEN) CPU: 60
(XEN) RIP: e008:[<ffff82d0802a54ce>] ubsan.c#ubsan_epilogue+0xa/0xc2
<snip>
(XEN) Xen call trace:
(XEN) [<ffff82d0802a54ce>] ubsan.c#ubsan_epilogue+0xa/0xc2
(XEN) [<ffff82d0802a6009>] __ubsan_handle_shift_out_of_bounds+0x15d/0x16c
(XEN) [<ffff82d08033abd7>] check_descriptor+0x191/0x3dd
(XEN) [<ffff82d0804ef920>] do_update_descriptor+0x7f/0x2b6
(XEN) [<ffff82d0804efb75>] compat_update_descriptor+0x1e/0x20
(XEN) [<ffff82d0804fa1cc>] pv_hypercall+0x87f/0xa6f
(XEN) [<ffff82d080501acb>] do_entry_int82+0x53/0x58
(XEN) [<ffff82d08050702b>] entry_int82+0xbb/0xc0
(XEN)
(XEN) ================================================================================
As this is a constant, express it in longhand for correctness, and consistency
with the surrounding code.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
master commit: bd5be40ce2
master date: 2019-06-06 20:04:33 +0100
Jan Beulich
Roger Pau Monné
Andrew Cooper
Igor Druzhinin
Michał Kowalczyk
Julien Grall
Juergen Gross
Sergey Dyasli
James Wang
Marek Marczykowski-Górecki
Nicholas Tsirakis
Tamas K Lengyel
Razvan Cojocaru
Paul Durrant