literal strings are not meant to be modified. So we should use const
char * rather than char * when we want to store a pointer to them.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
literal strings are not meant to be modified. So we should use const
char * rather than char * when we want to shore a pointer to them.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Acked-by: Tim Deegan <tim@xen.org>
The function sh_audit_flags() is returning pointer to literal strings.
They should not be modified, so the return is now const and this is
propagated to the callers.
Take the opportunity to fix the coding style in the declaration of
sh_audit_flags.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Tim Deegan <tim@xen.org>
Both name and opt_name are pointing to literal string. So mark both of
the fields as const.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
The string 'name' will never get modified by the function, so mark it
as const.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Currently perform_gunzip() can only be called once because the
the internal state (e.g allocate) is not fully re-initialized.
This works fine if you are only booting dom0. But this will break when
booting multiple using the dom0less that uses compressed kernel images.
This can be resolved by re-initializing bytes_out, malloc_ptr,
malloc_count every time perform_gunzip() is called.
Note the latter is only re-initialized for hardening purpose as there is
no guarantee that every malloc() are followed by free() (It should in
theory!).
Take the opportunity to check the return of alloc_heap_pages() to return
an error rather than dereferencing a NULL pointer later on failure.
Reported-by: Charles Chiou <cchiou@ambarella.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
As of the conversion to safe_strcpy() years ago there has been no need
anymore to use snprintf() to prevent storing a not-nul-terminated string.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Use ENXIO instead of EINVAL to cover the two cases of the address not
satisfying the requirements. This will make an issue here better stand
out at the call site.
Also add a missing compat-mode related size check: If the sizes
differed, other code in the function would need changing. Accompany this
by a change to the initial sizeof() expression, tying it to the type of
the variable we're actually after (matching e.g. the alignof() added by
XSA-327).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Backport 588888a7399db352d2b1a41c9d5b3bf0fd482390
"iommu/arm-smmu: Intelligent SMR allocation" from the Linux kernel
This patch fix the stream match conflict issue when two devices have the
same stream-id.
Only difference while applying this patch with regard to Linux patch are
as follows:
1. Spinlock is used in place of mutex when attaching a device to the
SMMU via arm_smmu_master_alloc_smes(..) function call.Replacing the
mutex with spinlock is fine here as we are configuring the hardware
via registers and it is very fast.
2. move iommu_group_alloc(..) function call in arm_smmu_add_device(..)
function from the start of the function to the end.
Original commit message:
iommu/arm-smmu: Intelligent SMR allocation
Stream Match Registers are one of the more awkward parts of the SMMUv2
architecture; there are typically never enough to assign one to each
stream ID in the system, and configuring them such that a single ID
matches multiple entries is catastrophically bad - at best, every
transaction raises a global fault; at worst, they go *somewhere*.
To address the former issue, we can mask ID bits such that a single
register may be used to match multiple IDs belonging to the same device
or group, but doing so also heightens the risk of the latter problem
(which can be nasty to debug).
Tackle both problems at once by replacing the simple bitmap allocator
with something much cleverer. Now that we have convenient in-memory
representations of the stream mapping table, it becomes straightforward
to properly validate new SMR entries against the current state, opening
the door to arbitrary masking and SMR sharing.
Another feature which falls out of this is that with IDs shared by
separate devices being automatically accounted for, simply associating a
group pointer with the S2CR offers appropriate group allocation almost
for free, so hook that up in the process.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Julien GralL <jgrall@amazon.com>
Backport commit d3097e39302083d58922a3d1032d7d59a63d263d
"iommu/arm-smmu: Add a stream map entry iterator" from the Linux kernel.
This patch is the preparatory work to fix the stream match conflict
when two devices have the same stream-id.
Original commit message:
iommu/arm-smmu: Add a stream map entry iterator
We iterate over the SMEs associated with a master config quite a lot in
various places, and are about to do so even more. Let's wrap the idiom
in a handy iterator macro before the repetition gets out of hand.
Tested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Backport commit 8e8b203eabd8b9e96d02d6339e4abce3e5a7ea4b
"iommu/arm-smmu: Keep track of S2CR state" from the Linux kernel.
This patch is the preparatory work to fix the stream match conflict
when two devices have the same stream-id.
Original commit message:
iommu/arm-smmu: Keep track of S2CR state
Making S2CRs first-class citizens within the driver with a high-level
representation of their state offers a neat solution to a few problems:
Firstly, the information about which context a device's stream IDs are
associated with is already present by necessity in the S2CR. With that
state easily accessible we can refer directly to it and obviate the need
to track an IOMMU domain in each device's archdata (its earlier purpose
of enforcing correct attachment of multi-device groups now being handled
by the IOMMU core itself).
Secondly, the core API now deprecates explicit domain detach and expects
domain attach to move devices smoothly from one domain to another; for
SMMUv2, this notion maps directly to simply rewriting the S2CRs assigned
to the device. By giving the driver a suitable abstraction of those
S2CRs to work with, we can massively reduce the overhead of the current
heavy-handed "detach, free resources, reallocate resources, attach"
approach.
Thirdly, making the software state hardware-shaped and attached to the
SMMU instance once again makes suspend/resume of this register group
that much simpler to implement in future.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Backport commit 1f3d5ca43019bff1105838712d55be087d93c0da
"iommu/arm-smmu: Consolidate stream map entry state" from the Linux
kernel.
This patch is the preparatory work to fix the stream match conflict
when two devices have the same stream-id.
Original commit message:
iommu/arm-smmu: Consolidate stream map entry state
In order to consider SMR masking, we really want to be able to validate
ID/mask pairs against existing SMR contents to prevent stream match
conflicts, which at best would cause transactions to fault unexpectedly,
and at worst lead to silent unpredictable behaviour. With our SMMU
instance data holding only an allocator bitmap, and the SMR values
themselves scattered across master configs hanging off devices which we
may have no way of finding, there's essentially no way short of digging
everything back out of the hardware. Similarly, the thought of power
management ops to support suspend/resume faces the exact same problem.
By massaging the software state into a closer shape to the underlying
hardware, everything comes together quite nicely; the allocator and the
high-level view of the data become a single centralised state which we
can easily keep track of, and to which any updates can be validated in
full before being synchronised to the hardware itself.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Backport commit 21174240e4f4439bb8ed6c116cdbdc03eba2126e
"iommu/arm-smmu: Handle stream IDs more dynamically" from the Linux
ernel.
This patch is the preparatory work to fix the stream match conflict
when two devices have the same stream-id.
Original commit message:
iommu/arm-smmu: Handle stream IDs more dynamically
Rather than assuming fixed worst-case values for stream IDs and SMR
masks, keep track of whatever implemented bits the hardware actually
reports. This also obviates the slightly questionable validation of SMR
fields in isolation - rather than aborting the whole SMMU probe for a
hardware configuration which is still architecturally valid, we can
simply refuse masters later if they try to claim an unrepresentable ID
or mask (which almost certainly implies a DT error anyway).
Acked-by: Will Deacon <will.deacon@arm.com>
Tested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Currently in order to link existing DTB into Xen image
we need to either specify option CONFIG_DTB_FILE on the
command line or manually add it into .config.
Add Kconfig entry: CONFIG_DTB_FILE
to be able to provide the path to DTB we want to embed
into Xen image. If no path provided - the dtb will not
be embedded.
Remove the line: AFLAGS-y += -DCONFIG_DTB_FILE=\"$(CONFIG_DTB_FILE)\"
as it is not needed since Kconfig will define it in a header
with all the other config options.
Move definition of _sdtb into dtb.S to prevent defining it
if there is no reference to it or if someone protects
_sdtb with #ifdef rather than with .ifnes. If the latter,
we will get a compiler error.
Signed-off-by: Michal Orzel <michal.orzel@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Introduce two feature flags to tell the domain whether it is
direct-mapped or not. It allows the guest kernel to make informed
decisions on things such as swiotlb-xen enablement.
The introduction of both flags (XENFEAT_direct_mapped and
XENFEAT_not_direct_mapped) allows the guest kernel to avoid any
guesswork if one of the two is present, or fallback to the current
checks if neither of them is present.
XENFEAT_direct_mapped is always set for not auto-translated guests.
For auto-translated guests, only Dom0 on ARM is direct-mapped. Also,
see is_domain_direct_mapped() which refers to auto-translated guests:
xen/include/asm-arm/domain.h:is_domain_direct_mapped
xen/include/asm-x86/domain.h:is_domain_direct_mapped
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
CC: jbeulich@suse.com
CC: andrew.cooper3@citrix.com
CC: julien@xen.org
All cpu identification registers that we store in the cpuinfo structure
are 64bit on arm64 and 32bit on arm32 so storing the values in 32bit on
arm64 is removing the higher bits which might contain information in the
future.
This patch is changing the types in cpuinfo to register_t (which is
32bit on arm32 and 64bit on arm64) and adding the necessary paddings
inside the unions.
For consistency uint64_t entries are also changed to register_t on 64bit
systems.
It is also fixing all prints using directly the bits values from cpuinfo
to use PRIregister and adapt the printed value to print all bits
available on the architecture.
Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
In rare cases, the path to the daemon socket cannot be created as it is
longer than PATH_MAX. Instead of failing with a NULL pointer dereference,
terminate the application with an error message.
This bug was discovered and resolved using Coverity Static Analysis
Security Testing (SAST) by Synopsys, Inc.
Signed-off-by: Norbert Manthey <nmanthey@amazon.de>
Reviewed-by: Thomas Friebel <friebelt@amazon.de>
Reviewed-by: Julien Grall <jgrall@amazon.co.uk>
Reviewed-by: Juergen Gross <jgross@suse.com>
In case a command fails, also free the memory. As this is for the CLI
client, currently the leaked memory is freed right after receiving the
error, as the application terminates next.
Similarly, if the allocation fails, do not use the NULL pointer
afterwards, but instead error out.
This bug was discovered and resolved using Coverity Static Analysis
Security Testing (SAST) by Synopsys, Inc.
Signed-off-by: Norbert Manthey <nmanthey@amazon.de>
Reviewed-by: Thomas Friebel <friebelt@amazon.de>
Reviewed-by: Julien Grall <jgrall@amazon.co.uk>
Reviewed-by: Juergen Gross <jgross@suse.com>
At the moment, flush_page_to_ram() is both cleaning and invalidate to
PoC the page.
The goal of flush_page_to_ram() is to prevent corruption when the guest
has disabled the cache (the cache line may be dirty) and the guest to
read previous content.
Per this definition, the invalidating the line is not necessary. So
invalidating the cache is unnecessary. In fact, it may be counter-
productive as the line may be (speculatively) accessed a bit after.
So this will incurr an expensive access to the memory.
More generally, we should avoid interferring too much with cache.
Therefore, flush_page_to_ram() is updated to only clean to PoC the page.
The performance impact of this change will depend on your
workload/processor.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
As of commit a6066af5b1 ("xen/init: Annotate all command line
parameter infrastructure as const") .init.setup has been part of .init.
As of commit 544ad7f5ca ("xen/init: Move initcall infrastructure into
.init.data") .initcall* have been part of .init. Hence neither can be
encountered as a stand-alone section in the final binaries anymore.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Like Intel, AMD also records the date in their blobs. The field was
merely misnamed as "data_code" so far; this was perhaps meant to be
"date_code". Split it into individual fields, just like we did for Intel
some time ago, and extend the message logged after a successful update.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
When an IO-APIC pin is switched from level to edge trigger mode the
IRR bit is cleared, so it can be used as a way to EOI an interrupt at
the IO-APIC level.
Such EOI however does not get forwarded to the dpci code like it's
done for the local APIC initiated EOI. This change adds the code in
order to notify dpci of such EOI, so that dpci and the interrupt
controller are in sync.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Top word writes just update the destination of the interrupt, but
since there's no change on the masking or the triggering mode no
guest interrupt injection can result of such write. Add an assert to
that effect.
Requested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
15 apparently once used to be the last valid type to request a callback
for, and the dimension of the respective array. The arrays meanwhile are
larger than this (in a benign way, i.e. no caller ever sets a mask bit
higher than 15), dimensioned by SH_type_unused. Have the ASSERT()s
follow suit and add build time checks at the call sites.
Sadly at least some Clang versions aren't as flexible with
_Static_assert() as gcc is - they demand a truly integer constant
expression, while gcc also permits constant variables.
Also adjust a comment naming the wrong of the two functions.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Tim Deegan <tim@xen.org>
Leaving the hooks untouched is at best a latent risk: There may well be
cases where some flush is needed, which then needs carrying out the
"register" way.
Switch from u<N> to uint<N>_t while needing to touch the function
headers anyway.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
For one FECTL must be written last - the interrupt shouldn't be unmasked
without first having written the data and address needed to actually
deliver it. In the common case (when dma_msi_set_affinity() doesn't end
up bailing early) this happens from init_vtd_hw(), but for this to
actually have the intended effect we shouldn't subsequently overwrite
what was written there - this is only benign when old and new settings
match. Instead we should restore the registers ahead of calling
init_vtd_hw(), just for the unlikely case of dma_msi_set_affinity()
bailing early.
In the moved code drop some stray casts as well.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
We shouldn't blindly unmask the interrupt when resuming. vtd_resume()
will restore the intended state.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
In this case the compiler is recognizing that no valid array indexes
remain (in x2apic_cluster()'s access to per_cpu(cpu_2_logical_apicid,
...)), but oddly enough isn't really consistent about the checking it
does (see the code comment).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
epte_get_entry_emt will only be called for HVM domains, so the
is_hvm_domain check is unnecessary. It's a remnant of PVHv1.
Shouldn't result in any functional change.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Using RING_GET_RESPONSE() on a shared ring is easy to use incorrectly
(i.e., by not considering that the other end may alter the data in the
shared ring while it is being inspected). Safe usage of a response
generally requires taking a local copy.
Provide a RING_COPY_RESPONSE() macro to use instead of
RING_GET_RESPONSE() and an open-coded memcpy(). This takes care of
ensuring that the copy is done correctly regardless of any possible
compiler optimizations.
Use a volatile source to prevent the compiler from reordering or
omitting the copy.
This generalizes similar RING_COPY_REQUEST() macro added in 3f20b8def0.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
The individual decompression CUs need to only surface their top level
functions to other code. Arrange for everything else to be static, to
make sure no undue uses of that code exist or will appear without
explicitly noticing. (In some cases this also results in code size
reduction, but since this is all init-only code this probably doesn't
matter very much.)
In the LZO case also take the opportunity and convert u8 where lines
get touched anyway.
The downside is that the top level functions will now be non-static
in stubdom builds of libxenguest, but I think that's acceptable. This
does require declaring them first, though, as the compiler warns about
the lack of declarations.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
15 apparently once used to be the last valid type to request a callback
for, and the dimension of the respective array. The arrays meanwhile are
larger than this (in a benign way, i.e. no caller ever sets a mask bit
higher than 15), dimensioned by SH_type_unused. Have the ASSERT()s
follow suit and add build time checks at the call sites.
Also adjust a comment naming the wrong of the two functions.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Tim Deegan <tim@xen.org>
The last use of the compat foreign API was dropped in commit
38eeb3864d "tools/xenstored: Drop mapping of the ring via foreign
map".
Therefore, we don't need to define XC_WANT_COMPAT_MAP_FOREIGN_API.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Commit e1de4c196a ("x86/timer: Fix boot on Intel systems using ITSSPRC
static PIT clock gating") was reported to cause boot failures on certain
AMD Ryzen systems. Until we can figure out what the actual issue there
is, skip this new part of HPET setup by default. Introduce a "hpet"
command line option to allow enabling this on hardware where it's really
needed for Xen to boot successfully (i.e. where the PIT doesn't drive
the timer interrupt).
Since it makes little sense to introduce just "hpet=legacy-replacement",
also allow for a boolean argument as well as "broadcast" to replace the
separate "hpetbroadcast" option.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ian Jackson <iwj@xenproject.org>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
Julien Grall
George Dunlap
Jan Beulich
Rahul Singh
Michal Orzel
Stefano Stabellini
Bertrand Marquis
Norbert Manthey
Roger Pau Monné
Marek Marczykowski-Górecki
Ian Jackson