Commit Graph
100 Commits
Author SHA1 Message Date
Roger Pau Monné 635dd1120a pci: fix locking around vPCI removal in pci_remove_device()
Currently vpci_deassign_device() is called without holding the per-domain
pci_lock in pci_remove_device(), which leads to:

Assertion 'rw_is_write_locked(&pdev->domain->pci_lock)' failed at ../drivers/vpci/vpci.c:47
[...]
Xen call trace:
   [<ffff82d040260eac>] R vpci_deassign_device+0x10d/0x1b9
   [<ffff82d04027932f>] S pci_remove_device+0x2b1/0x380
   [<ffff82d040260bd0>] F pci_physdev_op+0x197/0x19e
   [<ffff82d04032272d>] F do_physdev_op+0x342/0x12aa
   [<ffff82d0402f067a>] F pv_hypercall+0x58e/0x62b
   [<ffff82d0402012ba>] F lstar_enter+0x13a/0x140

Move the existing block that removes the device from the domain pdev_list ahead
and also issue the call to vpci_deassign_device() there.  It's fine to remove
the device from the domain list of assigned devices, as further functions only
care that the pdev domain field is correctly set to the owner of the device
about to be removed.

Moving the vpci_deassign_device() past the pci_cleanup_msi() call can be
dangerous, as doing the MSI cleanup ahead of having removed the vPCI handlers
could lead to stale data in vPCI MSI(-X) internal structures.

Fixes: 4f78438b45 ('vpci: use per-domain PCI lock to protect vpci structure')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
2024-02-29 17:00:40 +01:00
Roger Pau Monné c20850540a x86/altcall: always use a temporary parameter stashing variable
The usage in ALT_CALL_ARG() on clang of:

register union {
    typeof(arg) e;
    const unsigned long r;
} ...

When `arg` is the first argument to alternative_{,v}call() and
const_vlapic_vcpu() is used results in clang 3.5.0 complaining with:

arch/x86/hvm/vlapic.c:141:47: error: non-const static data member must be initialized out of line
         alternative_call(hvm_funcs.test_pir, const_vlapic_vcpu(vlapic), vec) )

Workaround this by pulling `arg1` into a local variable, like it's done for
further arguments (arg2, arg3...)

Originally arg1 wasn't pulled into a variable because for the a1_ register
local variable the possible clobbering as a result of operators on other
variables don't matter:

https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html#Local-Register-Variables

Note clang version 3.8.1 seems to already be fixed and don't require the
workaround, but since it's harmless do it uniformly everywhere.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Fixes: 2ce562b2a4 ('x86/altcall: use a union as register type for function parameters on clang')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2024-02-29 08:21:49 +01:00
Roger Pau Monné 54fd7b9974 xen/livepatch: group and document payload hooks
Group the payload hooks between the pre/post handlers, and the apply/revert
replacements.  Also attempt to comment the context in which the hooks are
executed.

No functional change.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
2024-02-28 16:57:25 +00:00
Roger Pau Monné e579677095 xen/livepatch: properly build the noapply and norevert tests
It seems the build variables for those tests where copy-pasted from
xen_action_hooks_marker-objs and not adjusted to use the correct source files.

Fixes: 6047104c3c ('livepatch: Add per-function applied/reverted state tracking marker')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
2024-02-28 16:57:25 +00:00
Roger Pau Monné cdae267ce1 xen/livepatch: fix norevert test attempt to open-code revert
The purpose of the norevert test is to install a dummy handler that replaces
the internal Xen revert code, and then perform the revert in the post-revert
hook.  For that purpose the usage of the previous common_livepatch_revert() is
not enough, as that just reverts specific functions, but not the whole state of
the payload.

Remove both common_livepatch_{apply,revert}() and instead expose
revert_payload{,_tail}() in order to perform the patch revert from the
post-revert hook.

Fixes: 6047104c3c ('livepatch: Add per-function applied/reverted state tracking marker')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
2024-02-28 16:57:25 +00:00
Roger Pau Monné d2daa40fb3 xen/livepatch: search for symbols in all loaded payloads
When checking if an address belongs to a patch, or when resolving a symbol,
take into account all loaded livepatch payloads, even if not applied.

This is required in order for the pre-apply and post-revert hooks to work
properly, or else Xen won't detect the instruction pointer belonging to those
hooks as being part of the currently active text.

Move the RCU handling to be used for payload_list instead of applied_list, as
now the calls from trap code will iterate over the payload_list.

Fixes: 8313c864fa ('livepatch: Implement pre-|post- apply|revert hooks')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
2024-02-28 16:57:25 +00:00
Roger Pau Monné a57b4074ab xen/livepatch: register livepatch regions when loaded
Currently livepatch regions are registered as virtual regions only after the
livepatch has been applied.

This can lead to issues when using the pre-apply or post-revert hooks, as at
that point the livepatch is not in the virtual regions list.  If a livepatch
pre-apply hook contains a WARN() it would trigger an hypervisor crash, as the
code to handle the bug frame won't be able to find the instruction pointer that
triggered the #UD in any of the registered virtual regions, and hence crash.

Fix this by adding the livepatch payloads as virtual regions as soon as loaded,
and only remove them once the payload is unloaded.  This requires some changes
to the virtual regions code, as the removal of the virtual regions is no longer
done in stop machine context, and hence an RCU barrier is added in order to
make sure there are no users of the virtual region after it's been removed from
the list.

Fixes: 8313c864fa ('livepatch: Implement pre-|post- apply|revert hooks')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
2024-02-28 16:57:25 +00:00
Roger Pau Monné 576528a2a7 x86/spec: do not print thunk option selection if not built-in
Since the thunk built-in enable is printed as part of the "Compiled-in
support:" line, avoid printing anything in "Xen settings:" if the thunk is
disabled at build time.

Note the BTI-Thunk option printing is also adjusted to print a colon in the
same way the other options on the line do.

Requested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2024-02-27 14:58:40 +01:00
Roger Pau Monné 8441fa806a x86/spec: fix INDIRECT_THUNK option to only be set when build-enabled
Attempt to provide a more helpful error message when the user attempts to set
spec-ctrl=bti-thunk option but the support is build-time disabled.

While there also adjust the command line documentation to mention
CONFIG_INDIRECT_THUNK instead of INDIRECT_THUNK.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2024-02-27 14:58:20 +01:00
Roger Pau Monné 6e9507f7d5 x86/spec: print the built-in SPECULATIVE_HARDEN_* options
Just like it's done for INDIRECT_THUNK and SHADOW_PAGING.

Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2024-02-27 14:57:52 +01:00
Roger Pau Monné ca7c872c70 xen: introduce Kconfig function alignment option
And use it to replace CODE_ALIGN in assembly.  This allows to generalize the
way the code alignment gets set across all architectures.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
2024-02-27 10:17:59 +01:00
Roger Pau Monné 60e00f77a5 x86/spec: fix BRANCH_HARDEN option to only be set when build-enabled
The current logic to handle the BRANCH_HARDEN option will report it as enabled
even when build-time disabled. Fix this by only allowing the option to be set
when support for it is built into Xen.

Fixes: 2d6f36daa0 ('x86/nospec: Introduce CONFIG_SPECULATIVE_HARDEN_BRANCH')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2024-02-26 16:06:42 +01:00
Roger Pau Monné 2ce562b2a4 x86/altcall: use a union as register type for function parameters on clang
The current code for alternative calls uses the caller parameter types as the
types for the register variables that serve as function parameters:

uint8_t foo;
[...]
alternative_call(myfunc, foo);

Would expand roughly into:

register unint8_t a1_ asm("rdi") = foo;
register unsigned long a2_ asm("rsi");
[...]
asm volatile ("call *%c[addr](%%rip)"...);

However with -O2 clang will generate incorrect code, given the following
example:

unsigned int func(uint8_t t)
{
    return t;
}

static void bar(uint8_t b)
{
    int ret_;
    register uint8_t di asm("rdi") = b;
    register unsigned long si asm("rsi");
    register unsigned long dx asm("rdx");
    register unsigned long cx asm("rcx");
    register unsigned long r8 asm("r8");
    register unsigned long r9 asm("r9");
    register unsigned long r10 asm("r10");
    register unsigned long r11 asm("r11");

    asm volatile ( "call %c[addr]"
                   : "+r" (di), "=r" (si), "=r" (dx),
                     "=r" (cx), "=r" (r8), "=r" (r9),
                     "=r" (r10), "=r" (r11), "=a" (ret_)
                   : [addr] "i" (&(func)), "g" (func)
                   : "memory" );
}

void foo(unsigned int a)
{
    bar(a);
}

Clang generates the following assembly code:

func:                                   # @func
        movl    %edi, %eax
        retq
foo:                                    # @foo
        callq   func
        retq

Note the truncation of the unsigned int parameter 'a' of foo() to uint8_t when
passed into bar() is lost.  clang doesn't zero extend the parameters in the
callee when required, as the psABI mandates.

The above can be worked around by using a union when defining the register
variables, so that `di` becomes:

register union {
    uint8_t e;
    unsigned long r;
} di asm("rdi") = { .e = b };

Which results in following code generated for `foo()`:

foo:                                    # @foo
        movzbl  %dil, %edi
        callq   func
        retq

So the truncation is not longer lost.  Apply such workaround only when built
with clang.

Reported-by: Matthew Grooms <mgrooms@shrew.net>
Link: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=277200
Link: https://github.com/llvm/llvm-project/issues/12579
Link: https://github.com/llvm/llvm-project/issues/82598
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2024-02-26 10:18:01 +01:00
Roger Pau Monné ef101f5251 xen/cmdline: fix printf format specifier in no_config_param()
'*' sets the width field, which is the minimum number of characters to output,
but what we want in no_config_param() is the precision instead, which is '.*'
as it imposes a maximum limit on the output.

Fixes: 68d757df8d ('x86/pv: Options to disable and/or compile out 32bit PV support')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2024-02-26 10:17:40 +01:00
Roger Pau Monné f0622dd4fd xen/livepatch: fix norevert test hook setup typo
The test code has a typo in using LIVEPATCH_APPLY_HOOK() instead of
LIVEPATCH_REVERT_HOOK().

Fixes: 6047104c3c ('livepatch: Add per-function applied/reverted state tracking marker')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
2024-02-26 10:13:46 +01:00
Roger Pau Monné 4917b9a423 mm: add the __must_check attribute to {gfn,mfn,dfn}_add()
It's not obvious from just the function name whether the incremented value will
be stored in the parameter, or returned to the caller.  That has leads to bugs
in the past as callers may assume the incremented value is stored in the
parameter.

Add the __must_check attribute to the function to easily spot callers that
don't consume the returned value, which signals an error in the caller logic.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2024-02-19 12:44:50 +01:00
Roger Pau Monné d9bfc2810e iommu/x86: fix IVMD/RMRR range checker loop increment
mfn_add() doesn't store the incremented value in the parameter, and instead
returns it to the caller.  As a result, the loop in iommu_unity_region_ok()
didn't make progress.  Fix it by storing the incremented value.

Fixes: e45801dea1 ('iommu/x86: introduce a generic IVMD/RMRR range validity helper')
Coverity-ID: 1592056
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2024-02-14 14:18:06 +01:00
Roger Pau Monné a185f88e98 iommu/vt-d: switch to common RMRR checker
Use the newly introduced generic unity map checker.

Also drop the message recommending the usage of iommu_inclusive_mapping: the
ranges would end up being mapped anyway even if some of the checks above
failed, regardless of whether iommu_inclusive_mapping is set.  Plus such option
is not supported for PVH, and it's deprecated.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2024-02-13 09:37:20 +01:00
Roger Pau Monné e45801dea1 iommu/x86: introduce a generic IVMD/RMRR range validity helper
IVMD and RMRR ranges are functionally equivalent, and as so could use the same
validity checker.

Move the IVMD to x86 common IOMMU code and adjust the function to take a pair
of [start, end] mfn parameters.

So far only the AMD-Vi side is adjusted to use the newly introduced helper, the
VT-d side will be adjusted in a further change.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2024-02-13 09:36:52 +01:00
Roger Pau Monné 83afa31358 amd-vi: fix IVMD memory type checks
The current code that parses the IVMD blocks is relaxed with regard to the
restriction that such unity regions should always fall into memory ranges
marked as reserved in the memory map.

However the type checks for the IVMD addresses are inverted, and as a result
IVMD ranges falling into RAM areas are accepted.  Note that having such ranges
in the first place is a firmware bug, as IVMD should always fall into reserved
ranges.

Fixes: ed6c77ebf0 ('AMD/IOMMU: check / convert IVMD ranges for being / to be reserved')
Reported-by: Ox <oxjo@proton.me>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Tested-by: oxjo <oxjo@proton.me>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2024-02-06 11:56:13 +01:00
Roger Pau Monné 583f1d0950 x86/spec-ctrl: Expose BHI_CTRL to guests
The CPUID feature bit signals the presence of the BHI_DIS_S control in
SPEC_CTRL MSR, first available in Intel AlderLake and Sapphire Rapids CPUs

Xen already knows how to context switch MSR_SPEC_CTRL properly between guest
and hypervisor context.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2024-02-01 19:52:44 +00:00
Roger Pau Monné 478e4787fa x86/spec-ctrl: Expose RRSBA_CTRL to guests
The CPUID feature bit signals the presence of the RRSBA_DIS_{U,S} controls in
SPEC_CTRL MSR, first available in Intel AlderLake and Sapphire Rapids CPUs.

Xen already knows how to context switch MSR_SPEC_CTRL properly between guest
and hypervisor context.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2024-02-01 19:52:44 +00:00
Roger Pau Monné 4dd6760706 x86/spec-ctrl: Expose IPRED_CTRL to guests
The CPUID feature bit signals the presence of the IPRED_DIS_{U,S} controls in
SPEC_CTRL MSR, first available in Intel AlderLake and Sapphire Rapids CPUs.

Xen already knows how to context switch MSR_SPEC_CTRL properly between guest
and hypervisor context.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2024-02-01 19:52:44 +00:00
Roger Pau Monné cb4ecb3cc1 pci: fail device assignment if phantom functions cannot be assigned
The current behavior is that no error is reported if (some) phantom functions
fail to be assigned during device add or assignment, so the operation succeeds
even if some phantom functions are not correctly setup.

This can lead to devices possibly being successfully assigned to a domU while
some of the device phantom functions are still assigned to dom0.  Even when the
device is assigned domIO before being assigned to a domU phantom functions
might fail to be assigned to domIO, and also fail to be assigned to the domU,
leaving them assigned to dom0.

Since the device can generate requests using the IDs of those phantom
functions, given the scenario above a device in such state would be in control
of a domU, but still capable of generating transactions that use a context ID
targeting dom0 owned memory.

Modify device assign in order to attempt to deassign the device if phantom
functions failed to be assigned.

Note that device addition is not modified in the same way, as in that case the
device is assigned to a trusted domain, and hence partial assign can lead to
device malfunction but not a security issue.

This is XSA-449 / CVE-2023-46839

Fixes: 4e9950dc1b ('IOMMU: add phantom function support')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2024-01-30 14:28:01 +01:00
Roger Pau Monne 4a7e71aa08 x86/iommu: cleanup unused functions
Remove xen_in_range() and vpci_is_mmcfg_address() now that hey are unused.

Adjust comments to point to the new functions that replace the existing ones.

No functional change.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
2024-01-29 17:25:15 +00:00
Roger Pau Monne 0e1bd15a1d x86/iommu: switch hwdom IOMMU to use a rangeset
The current loop that iterates from 0 to the maximum RAM address in order to
setup the IOMMU mappings is highly inefficient, and it will get worse as the
amount of RAM increases.  It's also not accounting for any reserved regions
past the last RAM address.

Instead of iterating over memory addresses, iterate over the memory map regions
and use a rangeset in order to keep track of which ranges need to be identity
mapped in the hardware domain physical address space.

On an AMD EPYC 7452 with 512GiB of RAM, the time to execute
arch_iommu_hwdom_init() in nanoseconds is:

x old
+ new
    N           Min           Max        Median           Avg        Stddev
x   5 2.2364154e+10  2.338244e+10 2.2474685e+10 2.2622409e+10 4.2949869e+08
+   5       1025012       1033036       1026188     1028276.2     3623.1194
Difference at 95.0% confidence
        -2.26214e+10 +/- 4.42931e+08
        -99.9955% +/- 9.05152e-05%
        (Student's t, pooled s = 3.03701e+08)

Execution time of arch_iommu_hwdom_init() goes down from ~22s to ~0.001s.

Note there's a change for HVM domains (ie: PVH dom0) that get switched to
create the p2m mappings using map_mmio_regions() instead of
p2m_add_identity_entry(), so that ranges can be mapped with a single function
call if possible.  Note that the interface of map_mmio_regions() doesn't
allow creating read-only mappings, but so far there are no such mappings
created for PVH dom0 in arch_iommu_hwdom_init().

No change intended in the resulting mappings that a hardware domain gets.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2024-01-29 17:25:15 +00:00
Roger Pau Monne 79fcc0e9d7 x86/iommu: remove regions not to be mapped
Introduce the code to remove regions not to be mapped from the rangeset
that will be used to setup the IOMMU page tables for the hardware domain.

This change also introduces two new functions: remove_xen_ranges() and
vpci_subtract_mmcfg() that copy the logic in xen_in_range() and
vpci_is_mmcfg_address() respectively and remove the ranges that would otherwise
be intercepted by the original functions.

Note that the rangeset is still not populated.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
2024-01-29 17:23:35 +00:00
Roger Pau Monné c378ff8983 x86/entry: fix jump into restore_all_guest without %rbx correctly set
e047b8d0fa went too far when limiting obtaining the vCPU pointer.  While the
code in ist_dispatch_done does indeed only need the vCPU pointer when PV32 is
enabled, the !PV32 path will end up jumping into restore_all_guest which does
require rbx == vCPU pointer.

Fix by moving the fetching of the vCPU pointer to be done outside of the PV32
code block.

Fixes: e047b8d0fa ('x86/entry: replace two GET_CURRENT() uses')
Reported-by: Edwin Torok <edwin.torok@cloud.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2024-01-26 19:07:30 +00:00
Roger Pau Monne 024e7131be tools: don't expose XENFEAT_hvm_pirqs by default
The HVM pirq feature allows routing interrupts from both physical and emulated
devices over event channels, this was done a performance improvement.  However
its usage is fully undocumented, and the only reference implementation is in
Linux.  It defeats the purpose of local APIC hardware virtualization, because
when using it interrupts avoid the usage of the local APIC altogether.

It has also been reported to not work properly with certain devices, at least
when using some AMD GPUs Linux attempts to route interrupts over event
channels, but Xen doesn't correctly detect such routing, which leads to the
hypervisor complaining with:

(XEN) d15v0: Unsupported MSI delivery mode 7 for Dom15

When MSIs are attempted to be routed over event channels the entry delivery
mode is set to ExtINT, but Xen doesn't detect such routing and attempts to
inject the interrupt following the native MSI path, and the ExtINT delivery
mode is not supported.

Disable HVM PIRQs by default and provide a per-domain option in xl.cfg to
enable such feature.  Also for backwards compatibility keep the feature enabled
for any resumed domains that don't have an explicit selection.

Note that the only user of the feature (Linux) is also able to handle native
interrupts fine, as the feature was already not used if Xen reported local APIC
hardware virtualization active.

Link: https://github.com/QubesOS/qubes-issues/issues/7971
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2024-01-25 17:46:57 +00:00
Roger Pau Monne 72cad62abb x86/hvm: make X86_EMU_USE_PIRQ optional
Allow selecting X86_EMU_USE_PIRQ for HVM guests, so it's no longer mandated to
be always on.

There's no restriction in Xen that forces such feature to be always on for HVM
guests, as for example PVH guests don't support it, as such allow toolstack to
select whether to enabled it on a per-domain basis.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2024-01-25 17:46:57 +00:00
Roger Pau Monné 610775d0dd x86/p2m-pt: fix off by one in entry check assert
The MMIO RO rangeset overlap check is bogus: the rangeset is inclusive so the
passed end mfn should be the last mfn to be mapped (not last + 1).

Fixes: 6fa1755644 ('amd/npt/shadow: replace assert that prevents creating 2M/1G MMIO entries')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@cloud.com>
2024-01-25 16:09:04 +01:00
Roger Pau Monné 6bdb965178 x86/intel: ensure Global Performance Counter Control is setup correctly
When Architectural Performance Monitoring is available, the PERF_GLOBAL_CTRL
MSR contains per-counter enable bits that is ANDed with the enable bit in the
counter EVNTSEL MSR in order for a PMC counter to be enabled.

So far the watchdog code seems to have relied on the PERF_GLOBAL_CTRL enable
bits being set by default, but at least on some Intel Sapphire and Emerald
Rapids this is no longer the case, and Xen reports:

Testing NMI watchdog on all CPUs: 0 40 stuck

The first CPU on each package is started with PERF_GLOBAL_CTRL zeroed, so PMC0
doesn't start counting when the enable bit in EVNTSEL0 is set, due to the
relevant enable bit in PERF_GLOBAL_CTRL not being set.

Check and adjust PERF_GLOBAL_CTRL during CPU initialization so that all the
general-purpose PMCs are enabled.  Doing so brings the state of the package-BSP
PERF_GLOBAL_CTRL in line with the rest of the CPUs on the system.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2024-01-17 10:40:52 +01:00
Roger Pau Monné c2ce346647 CirrusCI: drop FreeBSD 12
Went EOL by the end of December 2023, and the pkg repos have been shut down.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2024-01-15 12:20:11 +01:00
Roger Pau Monné c27c8922f2 x86/iommu: introduce a rangeset to perform hwdom IOMMU setup
This change just introduces the boilerplate code in order to use a rangeset
when setting up the hardware domain IOMMU mappings.  The rangeset is never
populated in this patch, so it's a non-functional change as far as the mappings
the domain gets established.

Note there will be a change for HVM domains (ie: PVH dom0) when the code
introduced here gets used: the p2m mappings will be established using
map_mmio_regions() instead of p2m_add_identity_entry(), so that ranges can be
mapped with a single function call if possible.  Note that the interface of
map_mmio_regions() doesn't allow creating read-only mappings, but so far there
are no such mappings created for PVH dom0 in arch_iommu_hwdom_init().

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2024-01-09 14:07:49 +01:00
Roger Pau Monne 23db507a01 x86/amd: Extend CPU erratum #1474 fix to more affected models
Erratum #1474 has now been extended to cover models from family 17h ranges
00-2Fh, so the errata now covers all the models released under Family
17h (Zen, Zen+ and Zen2).

Additionally extend the workaround to Family 18h (Hygon), since it's based on
the Zen architecture and very likely affected.

Rename all the zen2 related symbols to fam17, since the errata doesn't
exclusively affect Zen2 anymore.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2023-12-21 12:19:40 +00:00
Roger Pau Monné 94039d97e2 amd-vi: set IOMMU page table levels based on guest reported paddr width
However take into account the minimum number of levels required by unity maps
when setting the page table levels.

The previous setting of the page table levels for PV guests based on the
highest RAM address was bogus, as there can be other non-RAM regions past the
highest RAM address that need to be mapped, for example device MMIO.

For HVM we also take amd_iommu_min_paging_mode into account, however if unity
maps require more than 4 levels attempting to add those will currently fail at
the p2m level, as 4 levels is the maximum supported.

Fixes: 0700c962ac ('Add AMD IOMMU support into hypervisor')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-12-21 11:44:55 +01:00
Roger Pau Monné ad3a4b7c76 x86/p2m: move and rename paging_max_paddr_bits()
The function also supports non-paging domains, and hence it being placed in
p2m.h and named with the paging_ prefix is misleading.

Move to x86 domain.c and rename to domain_max_paddr_bits().  Moving to a
different header is non trivial, as the function depends on helpers declared in
p2m.h.  There's no performance reason for the function being inline.

Note the function is safe to use against PV or system domains, as it does check
whether the domain is using external paging, and if not the returned physical
address width is the host (native) value.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-12-21 11:44:04 +01:00
Roger Pau Monné 3af0a4f922 iommu/vt-d: do not assume page table levels for quarantine domain
Like XSA-445, do not assume IOMMU page table levels on VT-d are always set
based on DEFAULT_DOMAIN_ADDRESS_WIDTH and instead fetch the value set by
intel_iommu_domain_init() from the domain iommu structure.  This prevents
changes to intel_iommu_domain_init() possibly getting the levels out of sync
with what intel_iommu_quarantine_init() expects.

No functional change, since on Intel domains are hardcoded to use
DEFAULT_DOMAIN_ADDRESS_WIDTH.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-12-19 13:48:09 +01:00
Roger Pau Monne 948e033031 automation/alpine: add elfutils-dev
In preparation for adding some livepatch-build-tools test update the Alpine
container to also install elfutils-dev.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2023-12-05 18:02:51 +00:00
Roger Pau Monne 902377b690 xen/livepatch: fix livepatch tests
The current set of in-tree livepatch tests in xen/test/livepatch started
failing after the constify of the payload funcs array, and the movement of the
status data into a separate array.

Fix the tests so they respect the constness of the funcs array and also make
use of the new location of the per-func state data.

Fixes: 82182ad7b4 ('livepatch: do not use .livepatch.funcs section to store internal state')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
2023-11-29 10:46:42 +00:00
Roger Pau Monne b3d3267286 cirrus-ci: update FreeBSD versions
FreeBSD 14.0 has already been released, so switch to the release version image,
and introduce a FreeBSD 15.0 version to track current FreeBSD unstable
(development) branch.

Sample output at:

https://github.com/royger/xen/runs/19105278189

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
2023-11-29 10:46:42 +00:00
Roger Pau Monné 82182ad7b4 livepatch: do not use .livepatch.funcs section to store internal state
Currently the livepatch logic inside of Xen will use fields of struct
livepatch_func in order to cache internal state of patched functions.  Note
this is a field that is part of the payload, and is loaded as an ELF section
(.livepatch.funcs), taking into account the SHF_* flags in the section
header.

The flags for the .livepatch.funcs section, as set by livepatch-build-tools,
are SHF_ALLOC, which leads to its contents (the array of livepatch_func
structures) being placed in read-only memory:

Section Headers:
  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align
[...]
  [ 4] .livepatch.funcs  PROGBITS         0000000000000000  00000080
       0000000000000068  0000000000000000   A       0     0     8

This previously went unnoticed, as all writes to the fields of livepatch_func
happen in the critical region that had WP disabled in CR0.  After 8676092a0f
however WP is no longer toggled in CR0 for patch application, and only the
hypervisor .text mappings are made write-accessible.  That leads to the
following page fault when attempting to apply a livepatch:

----[ Xen-4.19-unstable  x86_64  debug=y  Tainted:   C    ]----
CPU:    4
RIP:    e008:[<ffff82d040221e81>] common/livepatch.c#apply_payload+0x45/0x1e1
[...]
Xen call trace:
   [<ffff82d040221e81>] R common/livepatch.c#apply_payload+0x45/0x1e1
   [<ffff82d0402235b2>] F check_for_livepatch_work+0x385/0xaa5
   [<ffff82d04032508f>] F arch/x86/domain.c#idle_loop+0x92/0xee

Pagetable walk from ffff82d040625079:
 L4[0x105] = 000000008c6c9063 ffffffffffffffff
 L3[0x141] = 000000008c6c6063 ffffffffffffffff
 L2[0x003] = 000000086a1e7063 ffffffffffffffff
 L1[0x025] = 800000086ca5d121 ffffffffffffffff

****************************************
Panic on CPU 4:
FATAL PAGE FAULT
[error_code=0003]
Faulting linear address: ffff82d040625079
****************************************

Fix this by moving the internal Xen function patching state out of
livepatch_func into an area not allocated as part of the ELF payload.  While
there also constify the array of livepatch_func structures in order to prevent
further surprises.

Note there's still one field (old_addr) that gets set during livepatch load.  I
consider this fine since the field is read-only after load, and at the point
the field gets set the underlying mapping hasn't been made read-only yet.

Fixes: 8676092a0f ('x86/livepatch: Fix livepatch application when CET is active')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
2023-11-27 15:16:01 +01:00
Roger Pau Monne c22fe7213c automation: switch to multi-platform images when possible
Instead of using specific architecture image, switch to using multi-arch ones
and specify the desired architecture using the --platform option.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
2023-11-22 13:37:17 +00:00
Roger Pau Monne 3b2ba63994 automation: set architecture in docker files
Pass the desired architecture of the image in the FROM instruction if the
image is possibly multi-platform.

This allows using the x86 Dockerfiles on OS X on arm64 hardware.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
2023-11-22 13:37:17 +00:00
Roger Pau Monne b78b4de1c5 automation: update tests to use Debian Bookworm
Switch tests using Stretch to Bookworm, as Stretch is EOL.

Note the packages are not removed from the Stretch dockerfile, because the
tests in stable branches will run using the old containers.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
2023-11-22 13:37:17 +00:00
Roger Pau Monne b5739330d7 automation: fix jessie/stretch images to use archive.debian.org apt repos
Otherwise it's impossible to build the images.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
2023-11-22 13:37:17 +00:00
Roger Pau Monne 7440355a29 automation: add Xen timestamps to all tests
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
2023-11-22 13:37:17 +00:00
Roger Pau Monne 24532be907 automation: remove com1= parameter on QEMU smoke tests
The serial is already setup by the firmware, be consistent with the rest of the
QEMU tests and don't specify a com1 setup.

Note it's also bogus, as the extra ',' will make the intended DPS argument to
be parsed as the io-base.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
2023-11-22 13:37:17 +00:00
Roger Pau Monne b576497e3b automation: remove CR characters from serial output
The gitlab CI webpage seems to have issues displaying the \CR\CR\LF "\r\r\n"
sequence on the web interface used as line returns by the Linux kernel serial
output.  This leads to the QEMU tests output looking like:

(XEN) Guest Loglevel: Nothing (Rate-limited: Errors and warnings)
(XEN) *** Serial input to DOM0 (type 'CTRL-a' three times to switch input)
(XEN) Freed 664kB init memory
mapping kernel into physical memory
about to get started...
qemu-system-x86_64: terminating on signal 15 from pid 52 (timeout)

This not helpful, so strip the CR characters from the output that goes to
stdout, leaving the output in the serial output file untouched.

All usages of `tee` are adjusted to strip CR from the output.

Fixes: 3030a73bf8 ('automation: add a QEMU based x86_64 Dom0/DomU test')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
2023-11-22 13:37:17 +00:00
Roger Pau Monne fe1e4668b3 iommu/amd-vi: use correct level for quarantine domain page tables
The current setup of the quarantine page tables assumes that the quarantine
domain (dom_io) has been initialized with an address width of
DEFAULT_DOMAIN_ADDRESS_WIDTH (48).

However dom_io being a PV domain gets the AMD-Vi IOMMU page tables levels based
on the maximum (hot pluggable) RAM address, and hence on systems with no RAM
above the 512GB mark only 3 page-table levels are configured in the IOMMU.

On systems without RAM above the 512GB boundary amd_iommu_quarantine_init()
will setup page tables for the scratch page with 4 levels, while the IOMMU will
be configured to use 3 levels only.  The page destined to be used as level 1,
and to contain a directory of PTEs ends up being the address in a PTE itself,
and thus level 1 page becomes the leaf page.  Without the level mismatch it's
level 0 page that should be the leaf page instead.

The level 1 page won't be used as such, and hence it's not possible to use it
to gain access to other memory on the system.  However that page is not cleared
in amd_iommu_quarantine_init() as part of re-initialization of the device
quarantine page tables, and hence data on the level 1 page can be leaked
between device usages.

Fix this by making sure the paging levels setup by amd_iommu_quarantine_init()
match the number configured on the IOMMUs.

Note that IVMD regions are not affected by this issue, as those areas are
mapped taking the configured paging levels into account.

This is XSA-445 / CVE-2023-46835

Fixes: ea38867831 ('x86 / iommu: set up a scratch page in the quarantine domain')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-11-14 12:30:00 +00:00
Roger Pau Monné 06719e449f x86/intel: fix indentation
Adjust line to use a hard tab and align using spaces afterwards, instead of
using all spaces.

Fixes: fc3090a47b ('x86/boot: Clear XD_DISABLE from the early boot path')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2023-11-13 15:01:26 +01:00
Roger Pau Monne e3c409d59a x86/x2apic: introduce a mixed physical/cluster mode
The current implementation of x2APIC requires to either use Cluster Logical or
Physical mode for all interrupts.  However the selection of Physical vs Logical
is not done at APIC setup, an APIC can be addressed both in Physical or Logical
destination modes concurrently.

Introduce a new x2APIC mode called Mixed, which uses Logical Cluster mode for
IPIs, and Physical mode for external interrupts, thus attempting to use the
best method for each interrupt type.

Using Physical mode for external interrupts allows more vectors to be used, and
interrupt balancing to be more accurate.

Using Logical Cluster mode for IPIs allows fewer accesses to the ICR register
when sending those, as multiple CPUs can be targeted with a single ICR register
write.

A simple test calling flush_tlb_all() 10000 times on a tight loop on AMD EPYC
9754 with 512 CPUs gives the following figures in nano seconds:

x mixed
+ phys
* cluster
    N           Min           Max        Median           Avg        Stddev
x  25 3.5131328e+08 3.5716441e+08 3.5410987e+08 3.5432659e+08     1566737.4
+  12  1.231082e+09  1.238824e+09 1.2370528e+09 1.2357981e+09     2853892.9
Difference at 95.0% confidence
	8.81472e+08 +/- 1.46849e+06
	248.774% +/- 0.96566%
	(Student's t, pooled s = 2.05985e+06)
*  11 3.5099276e+08 3.5561459e+08 3.5461234e+08 3.5415668e+08     1415071.9
No difference proven at 95.0% confidence

So Mixed has no difference when compared to Cluster mode, and Physical mode is
248% slower when compared to either Mixed or Cluster modes with a 95%
confidence.

Note that Xen uses Cluster mode by default, and hence is already using the
fastest way for IPI delivery at the cost of reducing the amount of vectors
available system-wide.

Make the newly introduced mode the default one.

Note the printing of the APIC addressing mode done in connect_bsp_APIC() has
been removed, as with the newly introduced mixed mode this would require more
fine grained printing, or else would be incorrect.  The addressing mode can
already be derived from the APIC driver in use, which is printed by different
helpers.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Henry Wang <Henry.Wang@arm.com>
2023-11-07 09:59:48 +00:00
Roger Pau Monné 87f37449d5 x86/i8259: do not assume interrupts always target CPU0
Sporadically we have seen the following during AP bringup on AMD platforms
only:

microcode: CPU59 updated from revision 0x830107a to 0x830107a, date = 2023-05-17
microcode: CPU60 updated from revision 0x830104d to 0x830107a, date = 2023-05-17
CPU60: No irq handler for vector 27 (IRQ -2147483648)
microcode: CPU61 updated from revision 0x830107a to 0x830107a, date = 2023-05-17

This is similar to the issue raised on Linux commit 36e9e1eab777e, where they
observed i8259 (active) vectors getting delivered to CPUs different than 0.

On AMD or Hygon platforms adjust the target CPU mask of i8259 interrupt
descriptors to contain all possible CPUs, so that APs will reserve the vector
at startup if any legacy IRQ is still delivered through the i8259.  Note that
if the IO-APIC takes over those interrupt descriptors the CPU mask will be
reset.

Spurious i8259 interrupt vectors however (IRQ7 and IRQ15) can be injected even
when all i8259 pins are masked, and hence would need to be handled on all CPUs.

Continue to reserve PIC vectors on CPU0 only, but do check for such spurious
interrupts on all CPUs if the vendor is AMD or Hygon.  Note that once the
vectors get used by devices detecting PIC spurious interrupts will no longer be
possible, however the device driver should be able to cope with spurious
interrupts.  Such PIC spurious interrupts occurring when the vector is in use
by a local APIC routed source will lead to an extra EOI, which might
unintentionally clear a different vector from ISR.  Note this is already the
current behavior, so assume it's infrequent enough to not cause real issues.

Finally, adjust the printed message to display the CPU where the spurious
interrupt has been received, so it looks like:

microcode: CPU1 updated from revision 0x830107a to 0x830107a, date = 2023-05-17
cpu1: spurious 8259A interrupt: IRQ7
microcode: CPU2 updated from revision 0x830104d to 0x830107a, date = 2023-05-17

Amends: 3fba06ba9f ('x86/IRQ: re-use legacy vector ranges on APs')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-11-02 10:50:59 +01:00
Roger Pau Monné 26a449ce32 x86/x2apic: remove usage of ACPI_FADT_APIC_CLUSTER
The ACPI FADT APIC_CLUSTER flag mandates that when the interrupt delivery is
Logical mode APIC must be configured for Cluster destination model.  However in
apic_x2apic_probe() such flag is incorrectly used to gate whether Physical mode
can be used.

Since Xen when in x2APIC mode only uses Logical mode together with Cluster
model completely remove checking for ACPI_FADT_APIC_CLUSTER, as Xen always
fulfills the requirement signaled by the flag.

Fixes: eb40ae41b6 ('x86/Kconfig: add option for default x2APIC destination mode')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-11-02 10:50:26 +01:00
Roger Pau Monne 474fc7d3c6 iommu/vt-d: fix SAGAW capability parsing
SAGAW is a bitmap field, with bits 1, 2 and 3 signaling support for 3, 4 and 5
level page tables respectively.  According to the Intel VT-d specification, an
IOMMU can report multiple SAGAW bits being set.

Commit 859d11b279 claims to replace the open-coded find_first_set_bit(), but
it's actually replacing an open coded implementation to find the last set bit.
The change forces the used AGAW to the lowest supported by the IOMMU instead of
the highest one between 1 and 2.

Restore the previous SAGAW parsing by using fls() instead of
find_first_set_bit(), in order to get the highest (supported) AGAW to be used.

However there's a caveat related to the value the AW context entry field must
be set to when using passthrough mode:

"When the Translation-type (TT) field indicates pass-through processing (10b),
this field must be programmed to indicate the largest AGAW value supported by
hardware." [0]

Newer Intel IOMMU implementations support 5 level page tables for the IOMMU,
and signal such support in SAGAW bit 3.

Enabling 5 level paging support (AGAW 3) is too risky at this point in the Xen
4.18 release, so instead put a bodge to unconditionally disable passthough
mode if SAGAW has any bits greater than 2 set.  Ignore bit 0; it's reserved in
current specifications, but had a meaning in the past and is unlikely to be
reused in the future.

Note the message about unhandled SAGAW bits being set is printed
unconditionally, regardless of whether passthrough mode is enabled.  This is
done in order to easily notice IOMMU implementations with not yet supported
SAGAW values.

[0] Intel VT Directed Spec Rev 4.1

Fixes: 859d11b279 ('VT-d: prune SAGAW recognition')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2023-10-19 21:52:52 +01:00
Roger Pau Monne 94a5127ebe iommu: fix quarantine mode command line documentation
With the addition of per-device quarantine page tables the sink page is now
exclusive for each device, and thus writable.  Update the documentation to
reflect the current implementation.

Fixes: 14dd241aad ('IOMMU/x86: use per-device page tables for quarantining')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2023-10-19 21:52:52 +01:00
Roger Pau Monné 4bb882fe6e x86/pvh: fix identity mapping of low 1MB
The mapping of memory regions below the 1MB mark was all done by the PVH dom0
builder code, causing the region to be avoided by the arch specific IOMMU
hardware domain initialization code.  That lead to the IOMMU being enabled
without reserved regions in the low 1MB identity mapped in the p2m for PVH
hardware domains.  Firmware which happens to be missing RMRR/IVMD ranges
describing E820 reserved regions in the low 1MB would transiently trigger IOMMU
faults until the p2m is populated by the PVH dom0 builder:

AMD-Vi: IO_PAGE_FAULT: 0000:00:13.1 d0 addr 00000000000eb380 flags 0x20 RW
AMD-Vi: IO_PAGE_FAULT: 0000:00:13.1 d0 addr 00000000000eb340 flags 0
AMD-Vi: IO_PAGE_FAULT: 0000:00:13.2 d0 addr 00000000000ea1c0 flags 0
AMD-Vi: IO_PAGE_FAULT: 0000:00:14.5 d0 addr 00000000000eb480 flags 0x20 RW
AMD-Vi: IO_PAGE_FAULT: 0000:00:12.0 d0 addr 00000000000eb080 flags 0x20 RW
AMD-Vi: IO_PAGE_FAULT: 0000:00:14.5 d0 addr 00000000000eb400 flags 0
AMD-Vi: IO_PAGE_FAULT: 0000:00:12.0 d0 addr 00000000000eb040 flags 0

Those errors have been observed on the osstest pinot{0,1} boxes (AMD Fam15h
Opteron(tm) Processor 3350 HE).

Rely on the IOMMU arch init code to create any identity mappings for reserved
regions in the low 1MB range (like it already does for reserved regions
elsewhere), and leave the mapping of any holes to be performed by the dom0
builder code.

Fixes: 6b4f6a31ac ('x86/PVH: de-duplicate mappings for first Mb of Dom0 memory')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2023-10-19 09:52:43 +02:00
Roger Pau Monne 9c114178ff libxl: limit bootloader execution in restricted mode
Introduce a timeout for bootloader execution when running in restricted mode.

Allow overwriting the default time out with an environment provided value.

This is part of XSA-443 / CVE-2023-34325

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2023-10-11 06:36:50 +01:00
Roger Pau Monne 1f762642d2 libxl: add support for running bootloader in restricted mode
Much like the device model depriv mode, add the same kind of support for the
bootloader.  Such feature allows passing a UID as a parameter for the
bootloader to run as, together with the bootloader itself taking the necessary
actions to isolate.

Note that the user to run the bootloader as must have the right permissions to
access the guest disk image (in read mode only), and that the bootloader will
be run in non-interactive mode when restricted.

If enabled bootloader restrict mode will attempt to re-use the user(s) from the
QEMU depriv implementation if no user is provided on the configuration file or
the environment.  See docs/features/qemu-deprivilege.pandoc for more
information about how to setup those users.

Bootloader restrict mode is not enabled by default as it requires certain
setup to be done first (setup of the user(s) to use in restrict mode).

This is part of XSA-443 / CVE-2023-34325

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2023-10-11 06:36:50 +01:00
Roger Pau Monne 5fc98b9708 iommu/amd-vi: flush IOMMU TLB when flushing the DTE
The caching invalidation guidelines from the AMD-Vi specification (48882—Rev
3.07-PUB—Oct 2022) seem to be misleading on some hardware, as devices will
malfunction (see stale DMA mappings) if some fields of the DTE are updated but
the IOMMU TLB is not flushed. This has been observed in practice on AMD
systems.  Due to the lack of guidance from the currently published
specification this patch aims to increase the flushing done in order to prevent
device malfunction.

In order to fix, issue an INVALIDATE_IOMMU_PAGES command from
amd_iommu_flush_device(), flushing all the address space.  Note this requires
callers to be adjusted in order to pass the DomID on the DTE previous to the
modification.

Some call sites don't provide a valid DomID to amd_iommu_flush_device() in
order to avoid the flush.  That's because the device had address translations
disabled and hence the previous DomID on the DTE is not valid.  Note the
current logic relies on the entity disabling address translations to also flush
the TLB of the in use DomID.

Device I/O TLB flushing when ATS are enabled is not covered by the current
change, as ATS usage is not security supported.

This is XSA-442 / CVE-2023-34326

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-10-11 06:36:49 +01:00
Roger Pau Monné ffa1437686 tools/xenpvboot: remove as unable to convert to Python 3
The script heavily relies on the urlgrabber python module, which doesn't seem
to be packaged by all distros; it's missing from newer Debian versions at
least.

Also the usage of the commands module has been deprecated since Python 2.6, and
removed in Python 3, so the code would need to be re-written to not rely on
urlgrabber and the commands modules.

Arguably the purpose of the xenpvnetboot bootloader can also be achieved with
an isolated script that fetches the kernel and ramdisk before attempting to
launch the domain, without having to run in libxl context.  There are no xl.cfg
options consumed by the bootloader apart from the base location and the
subpaths of the kernel and ramdisk to fetch.

Any interested parties in keeping such functionality are free to submit an
updated script that works with Python 3.

Resolves: xen-project/xen#172
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2023-10-06 18:17:42 +01:00
Roger Pau Monné 48a3fd1432 domain: expose newly introduced hypercalls as XENFEAT
XENFEAT_runstate_phys_area is exposed to all architectures, while
XENFEAT_vcpu_time_phys_area is currently only implemented for x86, and hence
the feature flag is also only exposed on x86.

Additionally add dummy guards with TODOs in the respective hypercall
implementations, to signal the intention to control the availability of those
hypercalls on a guest-by-guest basis from the toolstack.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2023-10-06 18:16:31 +01:00
Roger Pau Monné 6ba8384592 domain: fix misaligned unmap address in {,un}map_guest_area()
unmap_domain_page_global() expects the provided address to be page aligned, or
else some of the called functions will trigger assertions, like
modify_xen_mappings() on x86 or destroy_xen_mappings() on Arm.

The following assert has been reported by osstest arm 32bit tests:

  (XEN) Assertion 'IS_ALIGNED(s, PAGE_SIZE)' failed at arch/arm/mm.c:1243
  (XEN) ----[ Xen-4.18-rc  arm32  debug=y  Not tainted ]----
  (XEN) CPU:    0
  (XEN) PC:     00271a38 destroy_xen_mappings+0x50/0x5c
  [...]
  (XEN) Xen call trace:
  (XEN)    [<00271a38>] destroy_xen_mappings+0x50/0x5c (PC)
  (XEN)    [<00235aa8>] vunmap+0x30/0x1a0 (LR)
  (XEN)    [<0026ad88>] unmap_domain_page_global+0x10/0x20
  (XEN)    [<00208e38>] unmap_guest_area+0x90/0xec
  (XEN)    [<00208f98>] domain_kill+0x104/0x180
  (XEN)    [<00239e3c>] do_domctl+0x8ac/0x14fc
  (XEN)    [<0027ae34>] do_trap_guest_sync+0x570/0x66c
  (XEN)    [<002019f0>] arch/arm/arm32/entry.o#return_from_trap+0/0x4

Fixes: eadc288cbb ('domain: map/unmap GADDR based shared guest areas')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2023-10-06 18:16:31 +01:00
Roger Pau Monne 9a499a84a2 mem_sharing/fork: do not attempt to populate vcpu_info page
Instead let map_vcpu_info() and it's call to get_page_from_gfn()
populate the page in the child as needed.  Also remove the bogus
copy_domain_page(): should be placed before the call to map_vcpu_info(),
as the later can update the contents of the vcpu_info page.

Note that this eliminates a bug in copy_vcpu_settings(): The function did
allocate a new page regardless of the GFN already having a mapping, thus in
particular breaking the case of two vCPU-s having their info areas on the same
page.

Fixes: 41548c5472 ('mem_sharing: VM forking')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2023-10-05 14:08:14 +01:00
Roger Pau Monne d09e44e5d8 console/serial: bump buffer from 16K to 32K
Testing on a Kaby Lake box with 8 CPUs leads to the serial buffer
being filled halfway during dom0 boot, and thus a non-trivial chunk of
Linux boot messages are dropped.

Increasing the buffer to 32K does fix the issue and Linux boot
messages are no longer dropped.  There's no justification either on
why 16K was chosen, and hence bumping to 32K in order to cope with
current systems generating output faster does seem appropriate to have
a better user experience with the provided defaults.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
2023-09-22 09:04:29 +01:00
Roger Pau Monné e4ca4e261d x86/amd: do not expose HWCR.TscFreqSel to guests
OpenBSD 7.3 will unconditionally access HWCR if the TSC is reported as
Invariant, and it will then attempt to also unconditionally access PSTATE0 if
HWCR.TscFreqSel is set (currently the case on Xen).

The motivation for exposing HWCR.TscFreqSel was to avoid warning messages from
Linux.  It has been agreed that Linux should be changed instead to not
complaint about missing HWCR.TscFreqSel when running virtualized.

The relation between HWCR.TscFreqSel and PSTATE0 is not clearly written down in
the PPR, but it's natural for OSes to attempt to fetch the P0 frequency if the
TSC is stated to increment at the P0 frequency.

Exposing PSTATEn (PSTATE0 at least) with all zeroes is not a suitable solution
because the PstateEn bit is read-write, and OSes could legitimately attempt to
set PstateEn=1 which Xen couldn't handle.

Furthermore, the TscFreqSel bit is model specific and was never safe to expose
like this in the first place.  At a minimum it should have had a toolstack
adjustment to know not to migrate such a VM.

Therefore, simply remove the bit.  Note the HWCR itself is an architectural
register, and does need to be accessible by the guest.  Since HWCR contains
both architectural and non-architectural bits, going forward care must be taken
to assert the exposed value is correct on newer CPU families.

Reported-by: Solène Rapenne <solene@openbsd.org>
Link: https://github.com/QubesOS/qubes-issues/issues/8502
Fixes: 14b95b3b85 ('x86/AMD: expose HWCR.TscFreqSel to guests')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2023-09-18 15:07:49 +02:00
Roger Pau Monné 709f6c8ce6 x86/irq: fix reporting of spurious i8259 interrupts
The return value of bogus_8259A_irq() is wrong: the function will
return `true` when the IRQ is real and `false` when it's a spurious
IRQ.  This causes the "No irq handler for vector ..." message in
do_IRQ() to be printed for spurious i8259 interrupts which is not
intended (and not helpful).

Fix by inverting the return value of bogus_8259A_irq().

Fixes: 132906348a ('x86/i8259: Handle bogus spurious interrupts more quietly')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-08-30 10:03:53 +02:00
Roger Pau Monné 3e033172b0 x86/iommu: pass full IO-APIC RTE for remapping table update
So that the remapping entry can be updated atomically when possible.

Doing such update atomically will avoid Xen having to mask the IO-APIC
pin prior to performing any interrupt movements (ie: changing the
destination and vector fields), as the interrupt remapping entry is
always consistent.

This also simplifies some of the logic on both VT-d and AMD-Vi
implementations, as having the full RTE available instead of half of
it avoids to possibly read and update the missing other half from
hardware.

While there remove the explicit zeroing of new_ire fields in
ioapic_rte_to_remap_entry() and initialize the variable at definition
so all fields are zeroed.  Note fields could be also initialized with
final values at definition, but I found that likely too much to be
done at this time.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-08-01 11:48:39 +02:00
Roger Pau Monné f7065b24f4 amd: disable C6 after 1000 days on Zen2
As specified on Errata 1474:

"A core will fail to exit CC6 after about 1044 days after the last
system reset. The time of failure may vary depending on the spread
spectrum and REFCLK frequency."

Detect when running on AMD Zen2 and setup a timer to prevent entering
C6 after 1000 days of uptime.  Take into account the TSC value at boot
in order to account for any time elapsed before Xen has been booted.
Worst case we end up disabling C6 before strictly necessary, but that
would still be safe, and it's better than not taking the TSC value
into account and hanging.

Disable C6 by updating the MSR listed in the revision guide, this
avoids applying workarounds in the CPU idle drivers, as the processor
won't be allowed to enter C6 by the hardware itself.

Print a message once C6 is disabled in order to let the user know.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-07-31 15:05:48 +02:00
Roger Pau Monné a478b38c01 iommu/vtd: rename io_apic_read_remap_rte() local variable
Preparatory change to unify the IO-APIC pin variable name between
io_apic_read_remap_rte() and amd_iommu_ioapic_update_ire(), so that
the local variable can be made a function parameter with the same name
across vendors.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
2023-07-28 09:40:42 +02:00
Roger Pau Monné ef7995ed1b x86/ioapic: RTE modifications must use ioapic_write_entry
Do not allow to write to RTE registers using io_apic_write and instead
require changes to RTE to be performed using ioapic_write_entry.

This is in preparation for passing the full contents of the RTE to the
IOMMU interrupt remapping handlers, so remapping entries for IO-APIC
RTEs can be updated atomically when possible.

While immediately this commit might expand the number of MMIO accesses
in order to update an IO-APIC RTE, further changes will benefit from
getting the full RTE value passed to the IOMMU handlers, as the logic
is greatly simplified when the IOMMU handlers can get the complete RTE
value in one go.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-07-28 09:40:20 +02:00
Roger Pau Monné cdc48cb5a7 x86/ioapic: add a raw field to RTE struct
Further changes will require access to the full RTE as a single value
in order to pass it to IOMMU interrupt remapping handlers.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2023-07-28 09:39:44 +02:00
Roger Pau Monné baa6ea7003 vpci: add permission checks to map_range()
Just like it's done for the XEN_DOMCTL_memory_mapping hypercall, add
the permissions checks to vPCI map_range(), which is used to map the
BARs into the domain p2m.

Adding those checks requires that for x86 PVH hardware domain builder
the permissions are set before initializing the IOMMU, or else
attempts to initialize vPCI done as part of IOMMU device setup will
fail due to missing permissions to create the BAR mappings.

While moving the call to dom0_setup_permissions() convert the panic()
used for error handling to a printk, the caller will already panic if
required.

Fixes: 9c244fdef7 ('vpci: add header handlers')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Rahul Singh <rahul.singh@arm.com>
Tested-by: Rahul Singh <rahul.singh@arm.com>
2023-07-27 09:49:25 +02:00
Roger Pau Monne 6d21cedbaa libxl: add support for parsing MSR features
Introduce support for handling MSR features in
libxl_cpuid_parse_config().  The MSR policies are added to the
libxl_cpuid_policy like the CPUID one, which gets passed to
xc_cpuid_apply_policy().

This allows existing users of libxl to provide MSR related features as
key=value pairs to libxl_cpuid_parse_config() without requiring the
usage of a different API.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2023-07-26 10:56:27 +01:00
Roger Pau Monne d638fe233c libxl: use the cpuid feature names from cpufeatureset.h
The current implementation in libxl_cpuid_parse_config() requires
keeping a list of cpuid feature bits that should be mostly in sync
with the contents of cpufeatureset.h.

Avoid such duplication by using the automatically generated list of
cpuid features in INIT_FEATURE_NAMES in order to map feature names to
featureset bits, and then translate from featureset bits into cpuid
leaf, subleaf, register tuple.

Note that the full contents of the previous cpuid translation table
can't be removed.  That's because some feature names allowed by libxl
are not described in the featuresets, or because naming has diverged
and the previous nomenclature is preserved for compatibility reasons.

Should result in no functional change observed by callers, albeit some
new cpuid features will be available as a result of the change.

While there constify cpuid_flags name field.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2023-07-26 10:56:27 +01:00
Roger Pau Monne e2b1da9b8f libxl: split logic to parse user provided CPUID features
Move the CPUID value parsers out of libxl_cpuid_parse_config() into a
newly created cpuid_add() local helper.  This is in preparation for
also adding MSR feature parsing support.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2023-07-26 10:56:27 +01:00
Roger Pau Monne 5b80cecb74 libxl: introduce MSR data in libxl_cpuid_policy
Add a new array field to libxl_cpuid_policy in order to store the MSR
policies.

Adding the MSR data in the libxl_cpuid_policy_list type is done so
that existing users can seamlessly pass MSR features as part of the
CPUID data, without requiring the introduction of a separate
domain_build_info field, and a new set of handlers functions.

Note that support for parsing the old JSON format is kept, as that's
required in order to restore domains or received migrations from
previous tool versions.  Differentiation between the old and the new
formats is done based on whether the contents of the 'cpuid' field is
an array or a map JSON object.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2023-07-26 10:56:27 +01:00
Roger Pau Monne 4825d19603 libxl: change the type of libxl_cpuid_policy_list
Currently libxl_cpuid_policy_list is an opaque type to the users of
libxl, and internally it's an array of xc_xend_cpuid objects.

Change the type to instead be a structure that contains one array for
CPUID policies, in preparation for it also holding another array for
MSR policies.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2023-07-26 10:56:27 +01:00
Roger Pau Monne ed742cf1b6 libs/guest: introduce support for setting guest MSRs
Like it's done with CPUID, introduce support for passing MSR values to
xc_cpuid_apply_policy().  The chosen format for expressing MSR policy
data matches the current one used for CPUID.  Note that existing
callers of xc_cpuid_apply_policy() can pass NULL as the value for the
newly introduced 'msr' parameter in order to preserve the same
functionality, and in fact that's done in libxl on this patch.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2023-07-26 10:56:27 +01:00
Roger Pau Monné 813da5f0e7 x86/ioapic: sanitize IO-APIC pins before enabling lapic LVTERR/ESR
The current logic to init the local APIC and the IO-APIC does init the
local APIC LVTERR/ESR before doing any sanitization on the IO-APIC pin
configuration.  It's already noted on enable_IO_APIC() that Xen
shouldn't trust the IO-APIC being empty at bootup.

At XenServer we have a system where the IO-APIC 0 is handed to Xen
with pin 0 unmasked, set to Fixed delivery mode, edge triggered and
with a vector of 0 (all fields of the RTE are zeroed).  Once the local
APIC LVTERR/ESR is enabled periodic injections from such pin cause the
local APIC to in turn inject periodic error vectors:

APIC error on CPU0: 00(40), Received illegal vector
APIC error on CPU0: 40(40), Received illegal vector
APIC error on CPU0: 40(40), Received illegal vector
APIC error on CPU0: 40(40), Received illegal vector
APIC error on CPU0: 40(40), Received illegal vector
APIC error on CPU0: 40(40), Received illegal vector

That prevents Xen from booting.

Move the masking of the IO-APIC pins ahead of the setup of the local
APIC.  This has the side effect of also moving the detection of the
pin where the i8259 is connected, as such detection must be done
before masking any pins.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-07-17 10:31:10 +02:00
Roger Pau Monné b831326ee2 cmdline: only set ask mode if vga= is present
Commit 9473d9a241 set the ASK mode without checking if there was a
`vga` option provided in the command line.  This breaks existing
behavior, so exit early without changes if `vga` is not present in the
command line.

Fixes: 9473d9a241 ('cmdline: parse multiple instances of the vga option')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-07-10 18:35:09 +02:00
Roger Pau Monné fd62986f6b iommu/amd-vi: avoid pointless flushes in invalidate_all_domain_pages()
Fix invalidate_all_domain_pages() to only attempt to flush the domains
that have IOMMU enabled, otherwise the flush is pointless.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-07-10 12:19:25 +02:00
Roger Pau Monné 9473d9a241 cmdline: parse multiple instances of the vga option
Parse all instances of the vga= option on the command line, in order
to always enforce the last selection on the command line.

Note that it's not safe to parse just the last occurrence of the vga=
option, as then a command line with `vga=current vga=keep` would
result in current being ignored.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-07-10 12:19:03 +02:00
Roger Pau Monné 60474e8e28 multiboot2: do not set StdOut mode unconditionally
Only initialize StdOut if the current StdOut mode is unusable.  This
avoids forcefully switching StdOut to the maximum supported
resolution, and thus very likely changing the GOP mode without having
first parsed the command line options.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-07-10 12:18:39 +02:00
Roger Pau Monné c93aa5c5d0 multiboot2: parse vga= option when setting GOP mode
Introduce support for passing the command line to the efi_multiboot2()
helper, and parse the vga= option if present.

Add support for the 'gfx' and 'current' vga options, ignore the 'keep'
option, and print a warning message about other options not being
currently implemented.

Note that the multboot2 command line string must always be
zero-terminated according to the multiboot2 specification, and hence
there's no need to pass the string size to efi_multiboot2().

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-07-10 12:17:45 +02:00
Roger Pau Monné 82b28deb25 iommu/vtd: fix address translation for leaf entries
Fix two issues related to leaf address lookups in VT-d:

* When translating an address that falls inside of a superpage in the
  IOMMU page tables the fetching of the PTE value wasn't masking of the
  contiguous related data, which caused the returned data to be
  corrupt as it would contain bits that the caller would interpret as
  part of the address.

* When the requested leaf address wasn't mapped by a superpage the
  returned value wouldn't have any of the low 12 bits set, thus missing
  the permission bits expected by the caller.

Take the opportunity to also adjust the function comment to note that
when returning the full PTE the bits above PADDR_BITS are removed.

Fixes: c71e55501a ('VT-d: have callers specify the target level for page table walks')
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>
2023-06-19 15:46:03 +02:00
Roger Pau Monné 5ecbb77974 iommu/amd-vi: fix checking for Invalidate All support in amd_iommu_resume()
The iommu local variable does not point to to a valid amd_iommu element
after the call to for_each_amd_iommu().  Instead check whether any IOMMU
on the system doesn't support Invalidate All in order to perform the
per-domain and per-device flushes.

Fixes: 9c46139de8 ('amd iommu: Support INVALIDATE_IOMMU_ALL command.')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-06-13 14:41:32 +02:00
Roger Pau Monné f4be887879 livepatch: apply_alternatives() is only used for livepatch
Guard it with CONFIG_LIVEPATCH.  Note alternatives are applied at boot
using _apply_alternatives().

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2023-06-13 11:13:39 +02:00
Roger Pau Monne 6915a12064 x86/hvm: print valid CR4 bits in case of error
Some of the current users of hvm_cr4_guest_valid_bits() don't print
the valid mask in case of error, and thus the resulting error messages
are not as helpful as they could be.

Amend callers to always print the value of hvm_cr4_guest_valid_bits()
together with the rejected bits in the checked value. Also take the
opportunity and adjust all the users to use the same print formatter.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2023-06-08 11:48:53 +01:00
Roger Pau Monné ee045f3a4a vpci/header: cope with devices not having vpci allocated
When traversing the list of pci devices assigned to a domain cope with
some of them not having the vpci struct allocated. It should be
possible for the hardware domain to have read-only devices assigned
that are not handled by vPCI, such support will be added by further
patches.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-05-26 09:18:37 +02:00
Roger Pau Monné 053ffa783e x86/iommu: adjust type in arch_iommu_hwdom_init()
The 'i' iterator index stores a PDX, not a PFN, and hence the initial
assignation of start (which stores a PFN) needs a conversion from PFN
to PDX.

This is harmless currently, as the PDX compression skips the bottom
MAX_ORDER bits which cover the low 1MB, but still do the conversion
from PDX to PFN for type correctness.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2023-05-25 14:57:14 +02:00
Roger Pau Monné c7908869ac pci: fix pci_get_pdev() to always account for the segment
When a domain parameter is provided to pci_get_pdev() the search
function would match against the bdf, without taking the segment into
account.

Fix this and also account for the passed segment.

Fixes: 8cf6e07389 ('PCI: simplify (and thus correct) pci_get_pdev{,_by_domain}()')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-05-22 16:11:55 +02:00
Roger Pau Monne eda98ea870 x86/amd: fix legacy setting of SSBD on AMD Family 17h
The current logic to set SSBD on AMD Family 17h and Hygon Family 18h
processors requires that the setting of SSBD is coordinated at a core
level, as the setting is shared between threads.  Logic was introduced
to keep track of how many threads require SSBD active in order to
coordinate it, such logic relies on using a per-core counter of
threads that have SSBD active.

Given the current logic, it's possible for a guest to under or
overflow the thread counter, because each write to VIRT_SPEC_CTRL.SSBD
by the guest gets propagated to the helper that does the per-core
active accounting.  Overflowing the counter is not so much of an
issue, as this would just make SSBD sticky.

Underflowing however is more problematic: on non-debug Xen builds a
guest can perform empty writes to VIRT_SPEC_CTRL that would cause the
counter to underflow and thus the value gets saturated to the max
value of unsigned int.  At which points attempts from any thread to
set VIRT_SPEC_CTRL.SSBD won't get propagated to the hardware anymore,
because the logic will see that the counter is greater than 1 and
assume that SSBD is already active, effectively loosing the setting
of SSBD and the protection it provides.

Fix this by introducing a per-CPU variable that keeps track of whether
the current thread has legacy SSBD active or not, and thus only
attempt to propagate the value to the hardware once the thread
selected value changes.

This is XSA-431 / CVE-2022-42336

Fixes: b2030e6730 ('amd/virt_ssbd: set SSBD at vCPU context switch')
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-05-16 17:17:24 +02:00
Roger Pau Monné 4c507d8a6b iommu/amd-vi: fix assert comparing boolean to enum
Or else when iommu_intremap is set to iommu_intremap_full the assert
triggers.

Fixes: 1ba66a870e ('AMD/IOMMU: without XT, x2APIC needs to be forced into physical mode')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-05-12 09:35:36 +02:00
Roger Pau Monné eaa324bfeb x86/trampoline: load the GDT located in the trampoline page
When booting the BSP the portion of the code executed from the
trampoline page will be using the GDT located in the hypervisor
.text.head section rather than the GDT located in the relocated
trampoline page.

If skip_realmode is not set the GDT located in the trampoline page
will be loaded after having executed the BIOS call, otherwise the GDT
from .text.head will be used for all the protected mode trampoline
code execution.

Note that both gdt_boot_descr and gdt_48 contain the same entries, but
the former is located inside the hypervisor .text section, while the
later lives in the relocated trampoline page.

This is not harmful as-is, as both GDTs contain the same entries, but
for consistency with the APs switch the BSP trampoline code to also
use the GDT on the relocated trampoline page.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2023-05-03 13:37:19 +02:00
Roger Pau Monné 0946068e7f x86/head: check base address alignment
Ensure that the base address is 2M aligned, or else the page table
entries created would be corrupt as reserved bits on the PDE end up
set.

We have encountered a broken firmware where grub2 would end up loading
Xen at a non 2M aligned region when using the multiboot2 protocol, and
that caused a very difficult to debug triple fault.

If the alignment is not as required by the page tables print an error
message and stop the boot.  Also add a build time check that the
calculation of symbol offsets don't break alignment of passed
addresses.

The check could be performed earlier, but so far the alignment is
required by the page tables, and hence feels more natural that the
check lives near to the piece of code that requires it.

Note that when booted as an EFI application from the PE entry point
the alignment check is already performed by
efi_arch_load_addr_check(), and hence there's no need to add another
check at the point where page tables get built in
efi_arch_memory_setup().

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-05-03 13:36:25 +02:00
Roger Pau Monné 19c6cbd909 xen/vcpu: ignore VCPU_SSHOTTMR_future
The usage of VCPU_SSHOTTMR_future in Linux prior to 4.7 is bogus.
When the hypervisor returns -ETIME (timeout in the past) Linux keeps
retrying to setup the timer with a higher timeout instead of
self-injecting a timer interrupt.

On boxes without any hardware assistance for logdirty we have seen HVM
Linux guests < 4.7 with 32vCPUs give up trying to setup the timer when
logdirty is enabled:

CE: Reprogramming failure. Giving up
CE: xen increased min_delta_ns to 1000000 nsec
CE: Reprogramming failure. Giving up
CE: Reprogramming failure. Giving up
CE: xen increased min_delta_ns to 506250 nsec
CE: xen increased min_delta_ns to 759375 nsec
CE: xen increased min_delta_ns to 1000000 nsec
CE: Reprogramming failure. Giving up
CE: Reprogramming failure. Giving up
CE: Reprogramming failure. Giving up
Freezing user space processes ...
INFO: rcu_sched detected stalls on CPUs/tasks: { 14} (detected by 10, t=60002 jiffies, g=4006, c=4005, q=14130)
Task dump for CPU 14:
swapper/14      R  running task        0     0      1 0x00000000
Call Trace:
 [<ffffffff90160f5d>] ? rcu_eqs_enter_common.isra.30+0x3d/0xf0
 [<ffffffff907b9bde>] ? default_idle+0x1e/0xd0
 [<ffffffff90039570>] ? arch_cpu_idle+0x20/0xc0
 [<ffffffff9010820a>] ? cpu_startup_entry+0x14a/0x1e0
 [<ffffffff9005d3a7>] ? start_secondary+0x1f7/0x270
 [<ffffffff900000d5>] ? start_cpu+0x5/0x14
INFO: rcu_sched detected stalls on CPUs/tasks: { 26} (detected by 24, t=60002 jiffies, g=6922, c=6921, q=7013)
Task dump for CPU 26:
swapper/26      R  running task        0     0      1 0x00000000
Call Trace:
 [<ffffffff90160f5d>] ? rcu_eqs_enter_common.isra.30+0x3d/0xf0
 [<ffffffff907b9bde>] ? default_idle+0x1e/0xd0
 [<ffffffff90039570>] ? arch_cpu_idle+0x20/0xc0
 [<ffffffff9010820a>] ? cpu_startup_entry+0x14a/0x1e0
 [<ffffffff9005d3a7>] ? start_secondary+0x1f7/0x270
 [<ffffffff900000d5>] ? start_cpu+0x5/0x14
INFO: rcu_sched detected stalls on CPUs/tasks: { 26} (detected by 24, t=60002 jiffies, g=8499, c=8498, q=7664)
Task dump for CPU 26:
swapper/26      R  running task        0     0      1 0x00000000
Call Trace:
 [<ffffffff90160f5d>] ? rcu_eqs_enter_common.isra.30+0x3d/0xf0
 [<ffffffff907b9bde>] ? default_idle+0x1e/0xd0
 [<ffffffff90039570>] ? arch_cpu_idle+0x20/0xc0
 [<ffffffff9010820a>] ? cpu_startup_entry+0x14a/0x1e0
 [<ffffffff9005d3a7>] ? start_secondary+0x1f7/0x270
 [<ffffffff900000d5>] ? start_cpu+0x5/0x14

Thus leading to CPU stalls and a broken system as a result.

Workaround this bogus usage by ignoring the VCPU_SSHOTTMR_future in
the hypervisor.  Old Linux versions are the only ones known to have
(wrongly) attempted to use the flag, and ignoring it is compatible
with the behavior expected by any guests setting that flag.

Note the usage of the flag has been removed from Linux by commit:

c06b6d70feb3 xen/x86: don't lose event interrupts

Which landed in Linux 4.7.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Henry Wang <Henry.Wang@arm.com> # CHANGELOG
Acked-by: Jan Beulich <jbeulich@suse.com>
2023-05-03 13:36:05 +02:00
Roger Pau Monne f6c3cb2162 x86/shadow: restore dropped check in sh_unshadow_for_p2m_change()
As a result of 241702e064 the
mfn_valid() check in sh_unshadow_for_p2m_change() was lost.  That
allows sh_remove_shadows() to be called with gfns that have no backing
page, causing an ASSERT to trigger in debug builds or dereferencing an
arbitrary pointer partially under guest control in non-debug builds:

RIP:    e008:[<ffff82d0402dcf2c>] sh_remove_shadows+0x19f/0x722
RFLAGS: 0000000000010246   CONTEXT: hypervisor (d0v2)
[...]
Xen call trace:
   [<ffff82d0402dcf2c>] R sh_remove_shadows+0x19f/0x722
   [<ffff82d0402e28f4>] F arch/x86/mm/shadow/hvm.c#sh_unshadow_for_p2m_change+0xab/0x2b7
   [<ffff82d040311931>] F arch/x86/mm/p2m-pt.c#write_p2m_entry+0x19b/0x4d3
   [<ffff82d0403131b2>] F arch/x86/mm/p2m-pt.c#p2m_pt_set_entry+0x67b/0xa8e
   [<ffff82d040302c92>] F p2m_set_entry+0xcc/0x149
   [<ffff82d040305a50>] F unmap_mmio_regions+0x17b/0x2c9
   [<ffff82d040241e5e>] F do_domctl+0x11f3/0x195e
   [<ffff82d0402c7e10>] F hvm_hypercall+0x5b1/0xa2d
   [<ffff82d0402adc72>] F vmx_vmexit_handler+0x130f/0x1cd5
   [<ffff82d040203602>] F vmx_asm_vmexit_handler+0xf2/0x210

****************************************
Panic on CPU 1:
Assertion 'mfn_valid(gmfn)' failed at arch/x86/mm/shadow/common.c:2203
****************************************

Fix this by restoring the mfn_valid() check in
sh_unshadow_for_p2m_change(), unifying it with the rest of the checks
that are done at the start of the function.

This is XSA-430 / CVE-2022-42335

Fixes: 241702e064 ('x86/shadow: slightly consolidate sh_unshadow_for_p2m_change() (part II)')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-04-25 15:44:54 +02:00
Roger Pau Monné 569632a583 CHANGELOG: add gnttab_max_{maptrack_,}frames option changes
Note in the changelog that the purpose of
gnttab_max_{maptrack_,}frames command line options has been changed.

Fixes: b2ea81d2b9 ('xen/grants: repurpose command line max options')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Henry Wang <Henry.Wang@arm.com>
2023-04-19 11:03:30 +02:00
Roger Pau Monné 7a502b4fbc vpci/msix: restore PBA access length and alignment restrictions
Accesses to the PBA array have the same length and alignment
limitations as accesses to the MSI-X table:

"For all accesses to MSI-X Table and MSI-X PBA fields, software must
use aligned full DWORD or aligned full QWORD transactions; otherwise,
the result is undefined."

Introduce such length and alignment checks into the handling of PBA
accesses for vPCI.  This was a mistake of mine for not reading the
specification correctly.

Note that accesses must now be aligned, and hence there's no longer a
need to check that the end of the access falls into the PBA region as
both the access and the region addresses must be aligned.

Fixes: b177892d2d ('vpci/msix: handle accesses adjacent to the MSI-X table')
Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-03-29 14:56:33 +02:00