When CONFIG_PDX_COMPRESSION=n, pdx_init_mask(), pdx_region_mask() and
pfn_pdx_hole_setup() are just stubs doing nothing. It does not make
sense to keep the two loops iterating over all the memory banks.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Hari Limaye <hari.limaye@arm.com>
Tested-by: Hari Limaye <hari.limaye@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Harden the code by panicing if direct map is too small for current memory
layout taking into account possible PDX compression. Otherwise the assert
is observed:
Assertion '(mfn_to_pdx(maddr_to_mfn(ma)) - directmap_base_pdx) < (DIRECTMAP_SIZE >> PAGE_SHIFT)' failed at ./arch/arm/include/asm/mmu/mm.h:72
At the moment, we don't set max_pdx denoting maximum usable PDX which
should be based on max_page. Consolidate setting of max_page and max_pdx
in init_pdx() for both arm32 and arm64. max_pdx will be used in the
future to set up frametable mappings respecting the PDX grouping.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Hari Limaye <hari.limaye@arm.com>
Tested-by: Hari Limaye <hari.limaye@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Add a new entry under "Virtual Hardware, Hypervisor" for guest PSCI
support on ARM. This documents support for all mandatory functions of
PSCI 1.1, and separately lists the supported optional functions.
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
At the moment, we unconditionally allocate space for grant table region
membank and add it in the membanks array to find_unallocated_memory() to
find unused memory. In case of CONFIG_GRANT_TABLE=n, the size of the
region is empty and assertion in rangeset_remove_range() fails when
booting hwdom or 1:1 domU without IOMMU. Example:
(XEN) Assertion 's <= e' failed at common/rangeset.c:189
...
(XEN) Xen call trace:
(XEN) [<00000a0000218b5c>] rangeset_remove_range+0xbc/0x2d4 (PC)
(XEN) [<00000a00002b8370>] find_unallocated_memory+0x140/0x208 (LR)
(XEN) [<00000a00002cc28c>] make_hypervisor_node+0x310/0x7e0
...
Same issue would occur when booting hwdom with LLC coloring enabled.
Fix it by performing conditional allocation and configuration.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Function cpupool_do_sysctl is designed for doing cpupool related sysctl
operations, and shall be wrapped.
The following static functions are only called by cpupool_do_sysctl(), then
shall be wrapped too:
- cpupool_get_next_by_id
- cpupool_destroy
- cpupool_unassign_cpu_helper
- cpupool_unassign_cpu
Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Juergen Gross <jgross@suse.com>
Update manual pages to include detailed information about the altp2m_count
configuration parameter.
Signed-off-by: Petr Beneš <w1benny@gmail.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
Introduce a new altp2m_count parameter to control the maximum number of altp2m
views a domain can use. By default, if altp2m_count is unspecified and altp2m
is enabled, the value is set to 10, reflecting the legacy behavior.
This change is preparatory; it establishes the groundwork for the feature but
does not activate it.
Signed-off-by: Petr Beneš <w1benny@gmail.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Acked-by: Nick Rosbrook <enr0n@ubuntu.com>
Encapsulate the altp2m options within a struct. This change is preparatory
and sets the groundwork for introducing additional parameter in subsequent
commit.
Signed-off-by: Petr Beneš <w1benny@gmail.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
Acked-by: Julien Grall <jgrall@amazon.com> # arm
Reviewed-by: Jan Beulich <jbeulich@suse.com> # hypervisor
The check of this_softirq_pending must be performed with irqs disabled, but
this property was broken by an attempt to optimise entry/exit latency.
Commit c227233ad64c in Linux (which we copied into Xen) was fixed up by
edc8fc01f608 in Linux, which we have so far missed.
Going to sleep without waking on interrupts is nonsensical outside of
play_dead(), so overload this to select between two possible MWAITs, the
second using the STI shadow to cover MWAIT for exactly the same reason as we
do in safe_halt().
Fixes: b17e0ec72e ("x86/mwait-idle: enable interrupts before C1 on Xeons")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In order elide IPIs, we must be able to identify whether a target CPU is in
MWAIT at the point it is woken up. i.e. the store to wake it up must also
identify the state.
Create a new in_mwait variable beside __softirq_pending, so we can use a
CMPXCHG to set the softirq while also observing the status safely. Implement
an x86 version of arch_pend_softirq() which does this.
In mwait_idle_with_hints(), advertise in_mwait, with an explanation of
precisely what it means. X86_BUG_MONITOR can be accounted for simply by not
advertising in_mwait.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
x86 is the only architecture wanting an optimisation here, but the
test_and_set_bit() is a store into the monitored line (i.e. will wake up the
target) and, prior to the removal of the broken IPI-elision algorithm, was
racy, causing unnecessary IPIs to be sent.
To do this in a race-free way, the store to the monited line needs to also
sample the status of the target in one atomic action. Implement a new arch
helper with different semantics; to make the softirq pending and decide about
IPIs together. For now, implement the default helper. It will be overridden
by x86 in a subsequent change.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
With the recent simplifications, it becomes obvious that smp_mb() isn't the
right barrier. Strictly speaking, MONITOR is ordered as a load, but smp_rmb()
isn't correct either, as this only pertains to local ordering. All we need is
a compiler barrier().
Merge the barier() into the monitor() itself, along with an explantion.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
cpuidle_wakeup_mwait() is a TOCTOU race. The cpumask_and() sampling
cpuidle_mwait_flags can take a arbitrary period of time, and there's no
guarantee that the target CPUs are still in MWAIT when writing into
mwait_wakeup(cpu).
The consequence of the race is that we'll fail to IPI certain targets. Also,
there's no guarantee that mwait_idle_with_hints() will raise a TIMER_SOFTIRQ
on it's way out.
The fundamental bug is that the "in_mwait" variable needs to be in the
monitored line, and not in a separate cpuidle_mwait_flags variable, in order
to do this in a race-free way.
Arranging to fix this while keeping the old implementation is prohibitive, so
strip the current one out in order to implement the new one cleanly. In the
interim, this causes IPIs to be used unconditionally which is safe albeit
suboptimal.
Fixes: 3d521e933e ("cpuidle: mwait on softirq_pending & remove wakeup ipis")
Fixes: 1adb34ea84 ("CPUIDLE: re-implement mwait wakeup process")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
When ITS is enabled and PCI devices that are behind an SMMU generate an
MSI interrupt, SMMU fault will be observed as there is currently no
mapping in p2m table for the ITS translation register (GITS_TRANSLATER).
A mapping is required in the iommu page tables so that the device can
generate the MSI interrupt writing to the GITS_TRANSLATER register.
The GITS_TRANSLATER register is a 32-bit register, and there is nothing
else in a page containing it, so map that page.
Add new host_addr parameter to vgic_v3_its_init_virtual to prepare the
foundation for DomU MSI support where guest doorbell address can differ
for the host's. Note that the 1:1 check in arm_iommu_map_page remains
for now, as virtual ITSes are currently only created for hwdom where the
doorbell mapping is always 1:1.
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Current code skip the mapping for PCI bridge MMIO region to dom0 when
pci_passthrough_enabled flag is set. Mapping should be skip when
has_vpci(d) is enabled for the domain, as we need to skip the mapping
only when VPCI handler are registered for ECAM.
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Handle phantom functions in iommu_add_dt_pci_sideband_ids(). Each phantom
function will have a unique requestor ID (RID)/BDF. On ARM, we need to
map/translate the RID/BDF to an AXI stream ID for each phantom function
according to the pci-iommu device tree mapping [1]. The RID/BDF -> AXI stream ID
mapping in DT could allow phantom devices (i.e. devices with phantom functions)
to use different AXI stream IDs based on the (phantom) function.
[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/pci-iommu.txt
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The main purpose of this patch is to add a way to register PCI device
(which is behind the IOMMU) using the generic PCI-IOMMU DT bindings [1]
before assigning that device to a domain.
This behaves similarly to the existing iommu_add_dt_device API, except it
handles PCI devices, and it is to be invoked from the add_device hook in the
SMMU driver.
The function dt_map_id to translate an ID through a downstream mapping
(which is also suitable for mapping Requester ID) was borrowed from Linux
(v5.10-rc6) and updated according to the Xen code base.
[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/pci-iommu.txt
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
MISRA C Directive 4.10 states that "Precautions shall be taken in order
to prevent the contents of a header file being included more than
once".
Add a SAF tag and update the comment on top of cpufeatures.h.
Add a header inclusion guard to compile.h.
Generate header guards for hypercall-defs.h
Update ECLAIR configuration to:
- extend existing deviation to other comments explicitly saying a file
is intended for multiple inclusion;
- extend existing deviation to other autogenerated files;
- tag the guidelines as clean.
Update deviations.rst accordingly.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
MISRA C Rule 21.6 states that "The Standard Library input/output
functions shall not be used".
Xen does not use the functions provided by the Standard Library,
uses -nostdlib to ensure this.
Xen implements a set of functions that share the same names as their
Standard Library equivalent. The implementation of these functions is
available in source form: if some undefined or unspecified behavior
does arise in the implementation, it falls under the jurisdiction of
other MISRA guidelines.
Update ECLAIR configuration to deviate violations of the rule,
tag it as clean and add it to the monitored set.
Updated docs/misra/deviations.rst accordingly.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
When support was originally introduced, ebx, ecx and edx were reserved and
should have been zeroed in recalculate_misc() to avoid leaking into guests.
Since then, fields have been added into ebx. Guests can't load microcode, so
shouldn't see ucode_size, and while in principle we do want to support larger
RAP sizes in guests, virtualising this for guests depends on AMD procuding any
official documentation for ERAPS, which is long overdue and with no ETA.
This patch will cause a difference in guests on Zen5 CPUs, but as the main
ERAPS feature is hidden, guests should be ignoring the rap_size field too.
Fixes: e9b4fe2636 ("x86/cpuid: support LFENCE always serialising CPUID bit")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
As a result of the addition of a new field in libxl domain build info
structure the golang types need to be regnerated, this was missing as part
of 22650d6054.
Regenerate the headers now.
Reported-by: Juergen Gross <jgross@suse.com>
Fixes: 22650d6054 ('x86/hvmloader: select xen platform pci MMIO BAR UC or WB MTRR cache attribute')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Nick Rosbrook <enr0n@ubuntu.com>
For exception recovery in the stubs, the registered address for fixup is the
return address of the CALL entering the stub.
In invoke_stub(), the '.Lret%=:' label is the wrong side of the 'post'
parameter. The 'post' parameter is non-empty in cases where the arithmetic
flags of the operation need recovering.
Split the line to separate 'pre' and 'post', making it more obvious that the
return address label was in the wrong position.
However, in the case that an exception did occur, we want to skip 'post' as
it's logically part of the operation which had already failed. Therefore, add
a new skip label and use that for the exception recovery path.
This is XSA-470 / CVE-2025-27465
Fixes: 79903e50db ("x86emul: catch exceptions occurring in stubs")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Current script shows message:
Don't forget to add the subject and message to ...
Then perform:
git send-email -to xen-devel@lists.xenproject.org ...
which has wrong option '-to'.
It may be confused for user.
Set double dashes for longer options to avoid that.
Fixes: e1f912cbf7 ("scripts/add_maintainers.pl: New script")
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
At least GNU ld 2.35 takes this option to (also) mean what newer
versions have controllable by --enable-reloc-section. From there being
no relocations in check.efi (as we don't pass the option there) we infer
that we need to involve mkreloc, we'd end up with two sets of
relocations, which clearly isn't going to work. Furthermore the
relocations ld emits in this case also aren't usable: For bsp_idt[] we
end up with PE_BASE_RELOC_LOW ones, which efi_arch_relocate_image()
(deliberately) doesn't know how to deal with. (Related to that is also
why we check the number of relocations produced: The linker simply
didn't get this right there, yet.)
We also can't add the option to what we use when linking check.efi: That
ld version then would produce relocations, but 4 of them (instead of the
expected two). That would make us pass --disable-reloc-section, which
however only ld 2.36 and newer understand.
For such older binutils versions we therefore need to accept the slight
inconsistency in DLL characteristics that the earlier commit meant to
eliminate.
Fixes: f2148773b8 ("x86/EFI: sanitize DLL characteristics in binary")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
See the respective bullet point in the Caveats section of
https://gcc.gnu.org/gcc-15/changes.html.
While I'm unaware of us currently relying on the pre-gcc15 behavior,
let's still play safe and retain what unknowingly we may have been
relying upon.
According to my observations, on x86 generated code changes
- somewhere deep in modify_bars(), presumably from the struct map_data
initializer in apply_map() (a new MOVQ),
- in vpci_process_pending(), apparently again from the struct map_data
initializer (and again a new MOVQ),
- near the top of find_cpio_data(), presumably from the struct cpio_data
initializer (a MOVW changing to a MOVQ).
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
For features which are unconditionally set in the max policies, making the
default policy to match the host can be done with a conditional clear.
This is simpler than the unconditional clear, conditional set currently
performed.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
After commit 4df2e99d73 ("console/serial: set the default transmit
buffer size in Kconfig"), the default value is set by Kconfig option
CONFIG_SERIAL_TX_BUFSIZE. Moreover it was bumped to 32KB by commit
d09e44e5d8 ("console/serial: bump buffer from 16K to 32K").
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The result of the command is undefined according to the specification if
the "string2" argument in tr is shorter than "string1". GNU tr behaves
correctly by extending "string2" to repeat the last character.
Fixes: eb61a4fb14 ("xen: fix header guard generation for asm-generic headers")
Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
With introduction of the new byteswap infrastructure the build of
libxenguest for stubdoms was broken. Fix that again.
Fixes: 60dcff871e ("xen/decompressors: Remove use of *_to_cpup() helpers")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
Instead of declaring __ro_after_init_{start,end} in each architecture's
asm/setup.h, move these declarations to the common header xen/sections.h.
This centralizes the declarations and reduces duplication across
architectures.
No functional change intended.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Users rely on SYSCTL_coverage_op hypercall to interact with the coverage data,
that is, according operations shall be wrapped around with CONFIG_SYSCTL.
Right now, it is compiled under CONFIG_COVERAGE, so we shall make
CONFIG_COVERAGE depend on CONFIG_SYSCTL.
Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The following functions are only used to deal with XEN_SYSCTL_physinfo,
then they shall be wrapped:
- arch_do_physinfo()
- get_outstanding_claims()
Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
The following function is only to serve spinlock profiling via
XEN_SYSCTL_lockprof_op, so it shall be wrapped:
- spinlock_profile_control()
Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
perfc_control() and perfc_copy_info() are responsible for providing control
of perf counters via XEN_SYSCTL_perfc_op in DOM0, so they both shall
be wrapped.
Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Users could only access trace buffers via hypercall XEN_SYSCTL_tbuf_op,
so we shall make CONFIG_TRACEBUFFER depend on CONFIG_SYSCTL
Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
We introduce a new Kconfig CONFIG_SYSCTL, which shall only be disabled
on some dom0less systems or PV shim on x86, to reduce Xen footprint.
Making SYSCTL without prompt is transient and it will be adjusted in the final
patch. And the consequence of introducing "CONFIG_SYSCTL=y" in .config file
generated from pvshim_defconfig is transient too, which will also be adjusted
in the final patch.
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Having them in the general .init.data section is somewhat wasteful, due
to involved padding. Move them into .init.data.page_aligned, and place
that right after .init.bss.stack_aligned.
Overall .init.data* shrinks by slightly over 2 pages in the build I'm
looking at.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
A boot loader can load files from outside ESP.
In these cases device could be not provided or path could
be something not supported.
In these cases allows to boot anyway, all information
could be provided using UKI or using other boot loader
features.
Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
This patch wraps the suspend/resume console callbacks and related code within
CONFIG_SYSTEM_SUSPEND blocks. This ensures that these functions and their
calls are only included in the build when CONFIG_SYSTEM_SUSPEND is enabled.
This addresses Misra Rule 2.1 violations.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
When not using CONFIG_BIGMEM there are some restrictions in the address
width for allocations of the domain structure, as it's PDX truncated to 32
bits it's stashed into page_info structure for domain allocated pages.
The current logic to calculate this limit is based on the internals of the
PDX compression used, which is not strictly required. Instead simplify the
logic to rely on the existing PDX to PFN conversion helpers used elsewhere.
This has the added benefit of allowing alternative PDX compression
algorithms to be implemented without requiring to change the calculation of
the domain structure allocation boundary.
As a side effect introduce pdx_to_paddr() conversion macro and use it.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
I was presented with this:
(XEN) NX (Execute Disable) protection active
(XEN) d0 has maximum 416 PIRQs
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Error creating d0: -95
(XEN) ****************************************
which is less than helpful. It turns out to be the -EOPNOTSUPP from
shadow_domain_init().
The real bug here is create_dom0() unconditionally assuming the presence of
SHADOW_PAGING. Rework it to panic() rather than choosing a dom0_cfg which is
guaranteed to fail. This results in:
(XEN) NX (Execute Disable) protection active
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Neither HAP nor Shadow available for PVH domain
(XEN) ****************************************
which is rather more helpful.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Most of the patch (handling of CPUIDLE_FLAG_IBRS) is fine, but the
adjustements to mwait_idle() are not; spec_ctrl_enter_idle() does more than
just alter MSR_SPEC_CTRL.IBRS.
The only reason this doesn't need an XSA is because the unconditional
spec_ctrl_{enter,exit}_idle() in mwait_idle_with_hints() were left unaltered,
and thus the MWAIT remained properly protected.
There (would have been) two problems. In the ibrs_disable (== deep C) case:
* On entry, VERW and RSB-stuffing are architecturally skipped.
* On exit, there's a branch crossing the WRMSR which reinstates the
speculative safety for indirect branches.
All this change did was double up the expensive operations in the deep C case,
and fail to optimise the intended case.
I have an idea of how to plumb this more nicely, but it requires larger
changes to legacy IBRS handling to not make spec_ctrl_enter_idle() vulnerable
in other ways. In the short term, simply take out the perf hit.
Fixes: 08acdf9a26 ("x86/mwait-idle: disable IBRS during long idle")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Commit 48d32458bc ("x86, idle: add barriers to CLFLUSH workaround") was
inherited from Linux and added MFENCEs around the AAI65 errata fix.
The SDM now states:
Executions of the CLFLUSH instruction are ordered with respect to each
other and with respect to writes, locked read-modify-write instructions,
and fence instructions[1].
with footnote 1 reading:
Earlier versions of this manual specified that executions of the CLFLUSH
instruction were ordered only by the MFENCE instruction. All processors
implementing the CLFLUSH instruction also order it relative to the other
operations enumerated above.
I.e. the MFENCEs came about because of an incorrect statement in the SDM.
The Spec Update (no longer available on Intel's website) simply says "issue a
CLFLUSH", with no mention of MFENCEs.
As this erratum is specific to Intel, it's fine to remove the the MFENCEs; AMD
CPUs of a similar vintage do sport otherwise-unordered CLFLUSHs.
Move the feature bit into the BUG range (rather than FEATURE), and move the
workaround into monitor() itself.
The erratum check itself must use setup_force_cpu_cap(). It needs activating
if any CPU needs it, not if all of them need it.
Fixes: 48d32458bc ("x86, idle: add barriers to CLFLUSH workaround")
Fixes: 96d1b237ae ("x86/Intel: work around Xeon 7400 series erratum AAI65")
Link: https://web.archive.org/web/20090219054841/http://download.intel.com/design/xeon/specupdt/32033601.pdf
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>