| Age | Commit message (Collapse) | Author |
|---|
|
https://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux.git
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd
Pull iommufd updates from Jason Gunthorpe:
"Several bug fixes, probably found by AI based on the emails:
- Several edge cases in the self test
- Reject malformed invalidation commands in case the VMM let them
through
- Rework how the invalidation interface works to the drivers to be
more robust and fix several Sashiko issues
- Correct several error unwind paths
- UAF during eventq file release
- Reject more malformed usage of the dirty tracking by the VMM"
* tag 'for-linus-iommufd' of git://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd:
iommupt: Do not allow PTEs to be split when dirty logging
iommufd/iova_bitmap: Ensure that iova_bitmap_set() handles the full IOVA
iommufd: Reject a zero-length dirty bitmap request
iommufd/iova_bitmap: Fail GET_DIRTY_BITMAP when a bitmap page cannot be pinned
iommufd: Fix event queue release use-after-free
iommufd: Keep dmabuf PFNs MMIO when filling another domain
iommufd: Keep reserved IOVA ranges when a replace within one IOAS fails
iommufd: Fix iommufd_hw_capabilities reference in kernel-doc
iommu/vt-d: Convert nested cache invalidation to the core array loop
iommu/arm-smmu-v3-iommufd: Convert cache invalidation to the core array loop
iommufd/selftest: Convert cache invalidation mocks to the core array loop
iommufd: Iterate the cache invalidation array in the core
iommu/arm-smmu-v3-iommufd: Reject unsupported bits in invalidation commands
iommufd/selftest: Skip when the mock device is not there
iommufd/selftest: Return dmabuf fd from IOMMU_TEST_OP_DMABUF_GET again
|
|
arm_smmu_insert_master() skips inserting a stream whose StreamID duplicates
one the same master already owns (bridged PCI devices can present duplicate
IDs), leaving that master->streams[i].node zeroed and unlinked from the
smmu->streams rb-tree.
Both the insert error-rollback loop and arm_smmu_remove_master() then call
rb_erase() on every master->streams[i].node unconditionally. rb_erase() on
a zeroed node sees a NULL parent, treats the node as the tree root and sets
root->rb_node = NULL, silently emptying the whole SID tree and breaking SID
lookups (and DMA) for every other master on the SMMU.
Mark each node with RB_CLEAR_NODE() after sort_nonatomic() reorders the
array, since sorting relocates the entries and would leave the earlier
self-referential RB_CLEAR_NODE() pointer stale. An un-inserted node then
stays RB_EMPTY_NODE() and is skipped in both erase loops; inserted nodes
are linked by rb_find_add() and erased as before.
Fixes: b00d24997a11 ("iommu/arm-smmu-v3: Fix iommu_device_probe bug due to duplicated stream ids")
Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Mostafa Saleh <smostafa@google.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
A queue position, the wrap bit combined with the index, is Q_WRP | Q_IDX.
It is a (max_n_shift + 1)-bit value that wraps at twice the queue capacity.
queue_inc_cons(), queue_sync_cons_ovf(), and queue_inc_prod_n() currently
compute such positions by open-coding the two macros at each call site.
Add a Q_POS() macro and switch the open-coded sites to it.
A subsequent change will build a Q_DIFF() on it, to count the entries that
a queue pointer moved past.
No functional change intended.
Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
The IOPF teardown is done in arm_smmu_remove_master_domain() when releasing
the master_domain on detach, under the global arm_smmu_asid_lock mutex.
A later change will add an IOPF workqueue flush to that teardown, which can
block on a user-faulting page-fault handler. Holding the arm_smmu_asid_lock
across it would stall every unrelated attachment in the system.
Split the teardown out of arm_smmu_remove_master_domain(), to a new helper
arm_smmu_attach_release() that runs after arm_smmu_asid_lock is released.
No functional change: the old master_domain belongs to no other device, so
freeing it outside the lock stays safe, still under iommu_group->mutex.
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
arm_smmu_device_shutdown() only calls arm_smmu_device_disable(), and never
the impl_ops->device_disable() hook. On tegra241-cmdqv, that hook quiesces
every VINTF through tegra241_vintf_hw_deinit(), which issues a CMD_SYNC on
the shared CMDQ while it is still enabled.
arm_smmu_disable_action() already runs this hook before disabling the SMMU,
to cover an unbind or a probe failure, and no devres release ever follows a
reboot or a kexec to run it again. Have arm_smmu_device_shutdown() call it
directly, instead of reimplementing part of what it already does.
Fixes: 9ff145a25c5c ("iommu/tegra241-cmdqv: Fix CMD_SYNC use-after-free on teardown")
Cc: stable@vger.kernel.org
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Assisted-by: LLM
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
Each of these have their own unique situation, populate the tlbi right at
the top and pass it into arm_smmu_domain_inv_range(). They will diverge
further when the iommupt invalidation scheme is introduced.
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Tested-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
Store the required cmd data in the tlbi and just copy it out when
processing each item in the invs list. The cmd form only depends on if the
instance supports range invalidation or not, otherwise it is always the
same.
This avoids redundant calculations for each invs entry.
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Tested-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
Summarize if any of the inv entries will use range invalidation and if any
need ARM_SMMU_OPT_FULL_CONT_RANGE_INV. The next patch will use this to
avoid range invalidation pre-calculations unless the range commands will
be used.
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
The server IOMMU drivers focus on invalidation latency by default,
over-invalidating if necessary, to round the invalidation range up to a
single command. I think this represents a trade off for DMA non-FQ and SVA
where stalling the operation is overall worse than re-loading the IOTLB.
For instance AMD and VT-d both round the range up to the largest aligned
power of two and invalidate that. This causes over-invalidation but that
is preferred on real HW over trying to issue a number of smaller range
invalidations.
SMMUv3 on the other hand will try to do up to 512 single invalidations,
otherwise falls back to full invalidation, or it will try to issue an
string of range invalidations for every 5 bits of IOVA range to perfectly
cover it.
While the range-invalidation optimization is pretty good we can still do
better and reduce it to only 2 invalidations at maximum using the idea
Robin came up with to overlap two range invalidations in the middle. This
preserves the exact coverage of the current range invalidation and still
caps the number of range invalidations at 2.
This works because the range invalidation can start at any IOVA, so we can
place a range invalidation forwards from the start and backwards from the
end, overlapping in the middle if the scale isn't precise enough.
In the cases where this produces 2 range invalidations it continues to
produce range invalidations that don't overlap, but the exact split point
is different than the original algorithm due to the new calculation
method. In cases where the original algorithm would produce 3 or more
range invalidations this will produce 2 range invalidations with an
overlap.
The SVA under invalidation errata work around is maintained by "rounding
up" to generate a single range invalidation that over-covers the entire
range.
Since the normal path is now the only one with a loop, split them into two
functions and fold a simplified version of arm_smmu_inv_size_too_big()
directly into the normal flow in a way that directly limits the number of
single invalidation commands generated, again focusing on controlling
latency.
The end result is any gather is converted into either:
- One invalidate all
- One or two range invalidation operations
- At most 512 single invalidation ops
The current code switches at 511, but this turns every table removal
into a full invalidation. Change to 512 to try to minimize the amount
of full invalidation when using a 4k translation granule.
arm_smmu_domain_inv() is modified to always pass in the tgsz because the
new logic relies on it being valid.
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Tested-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
pgsize is a constant property of the domain, it is the base translation
granule of the page table (4k, 16k, 64k) in log2.
Store it to the struct arm_smmu_domain based on how the page table was
created.
Pass it around in the tlbi.
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Tested-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
These parameters go to a lot of different functions and the next patches
will add more. Put them into a struct to keep things tidy.
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Tested-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
The erratum (MMU-700: #3777127, S3: #3673557) deals with under
invalidation of a CONT PTE grouping in the SMMU. The recommended work
around is to use a range invalidation that spans the entire CONT. The only
user of CONT in the kernel right now is through SVA sharing a CPU page
table that contains a CONT created by the mm.
Previously it was thought that this errata was dealt with because the
driver always uses range invalidation. However, there is a subtle detail
in the errata that the invalidation range must fully enclose the entire
CONT for it to work.
It seems that two sequential range invalidations, with a split point
falling inside a CONT grouping, will not prevent the errata.
The SMMU's range invalidation generation algorithm does not produce a
single invalidation for a single SVA invalidation request, nor does the mm
carefully align the SVA invalidation ranges to accommodate the splitting
of the invalidation into several ranges.
Thus, when processing a SVA invalidation, the range invalidation splitting
routine can generate a range invalidation that is split in the middle of
the CONT and risk under invalidation from this errata. This condition
could be triggered by a malicious userspace manipulating the TLB gathers
via mmap/mprotect/munmap.
Update the errata list to the include the S3 variation, detect the IOMMUs
that have it, and then have SVA invalidations use a simplified version of
the over invalidation algorithm from the tlbi rework series. This ensures
that a single range invalidation is issued for a single MMU notifier
callback and now the range invalidation is guarenteed to cover any posible
CONT.
Future work to add CONT to iommu_domain page tables should either use this
one-invalidate/one-range invalidation algorithm or disable CONT support in
the iommu_domain.
Cc: stable@vger.kernel.org
Cc: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
Fixes: 3f1ce8e85ee0 ("iommu/arm-smmu-v3: Share process page tables")
Reviewed-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Pranjal Shrivastava <peaan@google.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Tested-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
struct arm_smmu_invs marks its flexible array member inv[] with the
__counted_by(max_invs).
arm_smmu_v3_invs_test() uses invs1 to invs5 which has num_invs = 3
but omit max_invs.
This causes the following UBSAN error:
[ 2.333240] UBSAN: array-index-out-of-bounds in drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:1116:21
[ 2.333604] # arm_smmu_v3_invs_test: lib/ubsan.c:228: array-index-out-of-bounds in drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
[ 2.334129] index 1 is out of range for type 'struct arm_smmu_inv[] __counted_by(max_invs)' (aka 'struct arm_smmu_inv[]')
[ 2.335375] CPU: 6 UID: 0 PID: 155 Comm: kunit_try_catch Tainted: G N 7.2.0-g8dcb331fbb72 #2 PREEMPT
[ 2.335759] Tainted: [N]=TEST
[ 2.335791] Hardware name: linux,dummy-virt (DT)
[ 2.336254] Call trace:
[ 2.336932] show_stack+0x18/0x24 (C)
[ 2.338443] __dump_stack+0x28/0x38
[ 2.338494] dump_stack_lvl+0x54/0x6c
[ 2.338518] dump_stack+0x18/0x24
[ 2.338541] ubsan_epilogue+0x10/0x44
[ 2.338565] __ubsan_handle_out_of_bounds+0xb8/0xbc
[ 2.338834] arm_smmu_invs_merge+0x688/0x8ac
[ 2.338862] arm_smmu_v3_invs_test+0xd4/0x598
[ 2.338890] kunit_try_run_case+0x64/0x160
[ 2.338914] kunit_generic_run_threadfn_adapter+0x28/0x4c
[ 2.338955] kthread+0x10c/0x12c
[ 2.338984] ret_from_fork+0x10/0x20
[ 2.339433] ---[ end trace ]---
Explicitly set max_invs = 3 on the test arrays to match the number of
elements.
Fixes: 15a2a5645ad7 ("iommu/arm-smmu-v3: Introduce a per-domain arm_smmu_invs array")
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
arm_smmu_v3_invs_test_verify() validates the array bounds using
KUNIT_EXPECT_EQ(), which triggers a failure and continues execution.
If invs->num_invs was smaller than expected, the next loop over
num_invs would blindly read past the end of the invs->inv[] array.
Switch to KUNIT_ASSERT_EQ() to fail and stop the test on bound errors.
Fixes: 15a2a5645ad7 ("iommu/arm-smmu-v3: Introduce a per-domain arm_smmu_invs array")
Reported-by: Sashiko <>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
arm_smmu_invs_merge() and arm_smmu_invs_alloc() can return NULL or
errors which are checked by the driver but not the test.
Add KUNIT_ASSERT_NOT_ERR_OR_NULL() after calling them to fail the
test instead of accessing NULL or ERR pointers.
Fixes: 15a2a5645ad7 ("iommu/arm-smmu-v3: Introduce a per-domain arm_smmu_invs array")
Reported-by: Sashiko <>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
arm_smmu_v3_test_debug_print_used_bits() hardcodes arm_smmu_get_ste_used()
instead of using the provided writer->ops->get_used() callback.
This caused Context Descriptors to be incorrectly parsed as STEs.
Fixes: 56e1a4cc2588 ("iommu/arm-smmu-v3: Add unit tests for arm_smmu_write_entry")
Reported-by: Sashiko <>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
Commit 6fabce53f6b9 ("iommu/arm-smmu-v3: Add a missing dma_wmb() for hitless STE update")
adds a dma_wmb() to arm_smmu_write_entry() to make sure stream tables
and context descriptors are observed first.
However, STE L1 table descriptors are configured directly
via WRITE_ONCE(), where before that they were zeroed with memset()
inside dma_direct_alloc() then written to abort in via memset() also
in arm_smmu_init_initial_stes() without a barrier in both cases which
means that the SMMUv3 can observe the allocated table before the
written descriptors causing it to fetch random data.
Similarly in arm_smmu_write_cd_l1_desc() where the L1 CD is written
after dma_alloc_coherent() with no barriers.
Add dma_wmb() in both cases.
Fixes: 48ec83bcbcf5 ("iommu/arm-smmu: Add initial driver support for ARM SMMUv3 devices")
Reported-by: Sashiko <>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
All three queues are sized from the maxima the hardware advertises in IDR1
and allocated at probe, up to 4 MB each on a 4K-page kernel. The capture
kernel already disables two of them: arm_smmu_device_reset() drops
CR0_EVTQEN and CR0_PRIQEN. It still allocates both at full size.
A kdump capture kernel runs from a small crashkernel reservation, and every
SMMUv3 instance pays that cost again, up to 12 MB apiece. It goes to queues
that either serve the handful of devices used to save the dump or are
switched off outright, and it is memory the dump itself needs.
Size all three queues at one page worth of entries when is_kdump_kernel().
The queues carry commands and fault records rather than DMA data, so dump
throughput is unaffected. A shallower command queue only bounds how many
commands may be in flight before a sync, which does not matter for the few
devices that save the dump. The cmdq_max_n_shift parameter therefore does
not apply in a capture kernel.
Suggested-by: Kyle McMartin <jkkm@meta.com>
Assisted-by: LLM
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Tested-by: Yuanhe Shu <xiangzao@linux.alibaba.com>
Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
The command queue depth comes straight from the maximum the hardware
advertises in IDR1, which reaches megabytes of coherent DMA per queue.
A system with several SMMUv3 instances pays that per instance, and the
Tegra241 CMDQV pays it again for every VCMDQ it preallocates.
Queue depth only bounds how many commands may be in flight before a sync.
A machine driving a handful of devices, or one with a tight memory budget,
has no use for the maximum, and no way to say so.
Add cmdq_max_n_shift, a cap on the depth given as the log2 of the entry
count, the form the hardware itself takes in the LOG2SIZE field of
CMDQ_BASE. It defaults to the largest depth the driver allocates, so an
unset parameter changes nothing. Decide the depth in
arm_smmu_cmdq_max_n_shift(), which applies the parameter to the IDR1
value, so the queue is allocated at the requested size. The Tegra241 CMDQV
sizes its VCMDQs from IDR1 itself, so route that through the same helper.
Floor the request at one page worth of entries. Without the floor, a small
request trips the CMDQ_BATCH_ENTRIES check in arm_smmu_device_hw_probe()
and the SMMU fails to probe. The floor also costs nothing: coherent DMA is
page granular, so a shallower queue occupies the same memory as one that
fills the page.
Assisted-by: LLM
Reviewed-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
The Google Tensor G5 SoC (Laguna) has a silicon errata with the SMMU
integration where the AxPROT[2] signal is always 1, which makes all
transactions appear as instructions.
To avoid F_PERMISSION faults (due to the VMSAv8-64 requirement that
unprivilieged writable buffers are treated as Privileged Execute-Never
(PXN)), as a software workaround we make use of the INSTCFG override
feature to force all incoming traffic to data.
To enable this workaround, the "arm,instdata-override" DT property can be
provided in the device tree entry for the SMMU, similar to how existing
errata workarounds are handled by this driver.
Note this is added for "stage-1" and "stage-2" but we intentionally leave
out "bypass" as nothing is downstream of the SMMU that makes use of the
Instruction/Data attribute.
[peter.griffin: ported to upstream, updated commit message, remove
feature flag]
Signed-off-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
Replace acpi_get_first_physical_node() that is slated for removal
with acpi_bus_get_primary_device() that takes a reference to the
device it is about to return.
This addresses a potential use-after-free that may occur if the
device returned by acpi_get_first_physical_node() is removed right
after dropping its ACPI companion's physical_node_lock in that
function.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://patch.msgid.link/3636226.QJadu78ljV@rafael.j.wysocki
|
|
arm_vsmmu_cache_invalidate() allocated a buffer for the entire user request
array, walked the array converting each of the commands, and issued those
converted commands to the cmdq in CMDQ_BATCH_ENTRIES sized chunks, carrying
the sub-array bookkeeping all on its own.
The iommufd core now iterates the invalidation array and re-invokes the op
with the not-yet-handled sub-array, so the driver only has to proceed with
a single chunk per call.
Instead of a per-array allocation, use a fixed on-stack batch to copy from
the userspace array. If the copy fails due to nonzero padding (VMM violates
the ABI), fail the entire batch.
Convert the whole batch before issuing any of it: a malformed command is a
userspace bug, so the first illegal command fails the batch as a unit,
issuing nothing and leaving array->entry_num at zero, the same way the copy
above bails on nonzero padding. A batch that converts cleanly is issued in
full, so the op returns either a handled count with no error or zero with
an error.
A zero-length array now returns success once the data type gets validated,
matching the documented probe behavior, rather than the -EINVAL that the
full-array copy helper would previously return.
This also fixes two long-standing bugs:
1) On a conversion failure the old code reported commands that it had
converted but not yet issued, so user space advanced its consumer
index past invalidations that never reached the cmdq.
2) A zero-length array was rejected with -EINVAL, although the uAPI
documents it as a valid request that only probes the data type.
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Link: https://patch.msgid.link/5223275dbc8ef00af233f3fee01efa09e45f26b5.1788127877.git.nicolinc@nvidia.com
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
|
|
The arm_vsmmu_cache_invalidate() op hands a guest's invalidation commands
to the trusted main command queue after enforcing only the VMID or the SID,
and passes the rest of the command through to the queue unchanged.
That lets a guest set bits the host never meant to forward: a reserved or
undefined bit makes a command malformed; per the Arm SMMUv3 specification,
in its section 4.1.3 "Command errors", a CERROR_ILL is raised, among other
cases, when:
A valid command opcode is used and a Reserved or undefined field is
optionally detected as non-zero, which results in the command being
treated as malformed.
Restrict each opcode to the fields that the driver supports and reject the
command with -EIO if it sets any other bit, before the command reaches the
queue. This stops the host from forwarding any bit whose meaning it does
not control. Document this contract in the uAPI header, so user space must
take the responsibility to forward valid commands only.
Some fields and whole opcodes are legal only on an SMMU that implements
the matching feature, so accept them conditionally:
- NUM, SCALE, TG and TTL need FEAT_RANGE_INV.
- SCALE bit 25, for values above 31, and TTL == 0b01 with a 16KB TG
need SMMU_IDR5.DS, gated on ARM_SMMU_FEAT_DS so that a VMM exposing
DS from the reported IDR5 register keeps working.
- ASID is limited to asid_bits, since its upper 8 bits are RES0 on an
SMMU that only supports 8-bit ASIDs.
- ATC_INV needs FEAT_ATS. Per the specification's section 4.5 "ATS and
PRI", CMD_ATC_INV is ILLEGAL when:
SMMU_IDR0.ATS == 0 and this command is issued on a Non-secure or
Secure Command queue.
- SSV, SSID and Global need a non-zero ssid_bits. Without it, setting
them is not illegal but CONSTRAINED UNPREDICTABLE, which a guest
should not be able to provoke. Global also takes effect only when
SSV == 1, broadening the invalidation from the one SSID to all the
PASIDs of the single device that the SID field addresses.
Some values inside the accepted fields are Reserved too:
- NUM == 0, SCALE == 0 and TTL == 0 together are a Reserved combination
and cause a CERROR_ILL.
- NUM, SCALE and TTL turn RES0 when TG == 0.
- An ATC_INV Size above 52, the invalidate-all span, is permitted to
raise a CERROR_ILL.
Reject these Reserved values the same way. In contrast, an out-of-range
address or ID value is defined as CONSTRAINED UNPREDICTABLE that would
be scoped to the guest itself, so it does not deserve a check.
Fixes: d68beb276ba2 ("iommu/arm-smmu-v3: Support IOMMU_HWPT_INVALIDATE using a VIOMMU object")
Cc: stable@vger.kernel.org
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Assisted-by: Claude:claude-fable-5
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Link: https://patch.msgid.link/40f4aabf98e434d6ac9f42a8d278b92200f202ca.1788127877.git.nicolinc@nvidia.com
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
|
|
This is another run of the Coccinelle script for converting kmalloc()
family of allocations to kmalloc_obj() via the existing rules in
scripts/coccinelle/api/kmalloc_objs.cocci
This catches both the set of kmalloc() uses added since the first
kmalloc_obj() conversions in v7.0 and adds a large group missed in the
first pass due to Coccinelle not interacting well with the cleanup.h
scoped_...() family of macros[1]. I worked around this with spatch's
"--macro-file" argument to a file with all the scoped_...() macros mapped
to Coccinelle's YACFE_ITERATOR[2] as that was the closest viable control
flow indicator I could find.
Build tested allmodconfig on x86, arm64, arm, loongarch, mips, powerpc,
riscv, and s390 with no new warnings.
Link: https://lore.kernel.org/lkml/202609021314.8A9C0B8@keescook/ [1]
Link: https://github.com/coccinelle/coccinelle/blob/master/standard.h [2]
Signed-off-by: Kees Cook <kees+treewide@kernel.org>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux
Pull iommu updates from Joerg Roedel:
"ARM SMMUv2:
- Device-tree binding updates for Qualcomm Eliza, Maili, Shikra and
IPQ9650 SoCs
- Add support for Qualcomm SM8450
- Numerous fixes for lifetime and ordering issues found by Sashiko in
the Qualcomm driver
ARM SMMUv3:
- Fix interrupt type in device-tree binding example for NVIDIA CMDQV
- Numerous fixes for issues identified by Sashiko in the NVIDIA CMDQV
driver
- Work around TLB erratum T264-SMMU-3 on Tegra264 by repeating the
invalidation sequence
- Add support for HAFT (hardware access flag in table entries) when
using SVA
- Probe for 52-bit addressing with a page size smaller than 64k
('DS') but don't do anything with it for now
- Minor driver improvements (remove sort_nonatomic(), use
readl_relaxed_poll_timeout_atomic(), fix IOPF teardown ordering)
Intel VT-d:
- Consolidation of complex enablement logic into a clean,
priority-based state machine
- Support for the DMA_REMAP_OPT_OUT flag from the VT-d v5.2
specification
- An update to cache_tag_flush_devtlb_psi() to use full-range
constants instead of modifying shared variables for
CACHE_TAG_NESTING_DEVTLB
- A fix for the UCTP context-table slot when copying root entries
- Fixes for several pre-existing issues reported by Sashiko
- General code cleanup and refinement
AMD IOMMU:
- Add SNP page-mode-0 support, enabling passthrough, v2 DMA page
tables and host SVA on supporting systems
- Fix invalid PPR handling, COMPLETE_PPR responses and guest-mode
reporting
- Improve Southbridge IOAPIC validation and remove the dependency on
hard-coded device IDs
- Fix PCI-device lifetime, debugfs and diagnostic issues
IOMMU core and IOMMUFD:
- Restore serialization of the shared MSI-page list
- Fix SVA-handle publication and several IOMMUFD reference and error
path leaks
- Return the expected zero result for invalid generic page-table
translations
- Allocate per-CPU IOVA magazines lazily to reduce memory use on
large systems
PCI ATS:
- Make VF support checks account for the associated PF and validate
that VF and PF Smallest Translation Unit settings agree
Platform drivers:
- Fix Qualcomm runtime-PM, probe unwind, fault reporting and page
table initialization races
- Rework Rockchip state handling and fix clock, probe and stale-fault
handling
- Fix smaller issues in the MSM and MediaTek drivers
Device-tree bindings:
- Add new Qualcomm SMMU compatibles, convert the OMAP IOMMU binding
to YAML, and fix the Tegra264 CMDQV interrupt example
Various smaller cleanups, documentation fixes and a Rust IOMMU
safety/readability improvement"
* tag 'iommu-updates-v7.3' of git://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux: (93 commits)
iommu/amd: Add SNP page mode 0 support
iommu/amd: Fix GN bit setting in COMPLETE_PPR_REQUEST command
iommu/amd: Rate limit INVALID_PPR_REQUEST error logging
iommu/amd: Fix missing CMD_COMPLETE_PPR response for invalid PPR requests
iommu/amd: Introduce PPR_TAG_LAST_PAGE() macro
iommu/amd: Fix incorrect device ID in invalid PASID error message
iommu/vt-d: Flush context cache with correct SID when tearing down aliases
iommu/vt-d: Tear down scalable-mode context on probe failure
iommu/vt-d: Fix iopf_refcount leak on RID domain replacement
iommu/vt-d: Clear Present bit before tearing down copied context entry
iommu/vt-d: Fix copied_tables bitmap leak on error in copy_translation_tables
iommu/vt-d: Cache max domain ID to avoid redundant calculation
iommu/vt-d: Support the new DMA_REMAP_OPT_OUT flag bit
iommu/vt-d: Remove dmar_disabled
iommu/vt-d: Remove the 'force_on' variable
iommu/vt-d: Call dmar_can_force_on() for tboot opt-in
iommu/vt-d: Use dmar_can_force_on() for platform opt-in
iommu/vt-d: Consolidate dmar policy management and force_on logic
iommu/vt-d: Remove dead code when CONFIG_INTEL_IOMMU is not set
iommu/vt-d: Force requesting ACS when tboot is enabled
...
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux
Pull arm64 updates from Will Deacon:
"There's a reasonable amount of stuff here, including a bunch of
updates to the perf PMU drivers and some MPAM updates to expose the
memory bandwidth counters via resctrl.
On the architecture side, some highlights include support for BBML3
and steps towards support for an architectural NMI solution, all
wrapped up in a web of fixes for latent issues identified by Sashiko.
ACPI:
- Combine reads of AMU counters into a single FFH feedback counter op
Confidential computing:
- Fix smp_processor_id() in preemptible context when retrieving an
attestation token inside a realm
- Convert pKVM over to a "CC platform"
- Clean-up our SWIOTLB configuration in preparation for reworking the
handling of encrypted/decryped DMA buffers in the dma-mapping tree
CPU errata handling:
- Work around broken device memory ordering on NVIDIA Olympus cores
- Fix broken 'nospectre_bhb' command-line option
- Select the idle loop backend instruction on the command-line
CPU features:
- Replace our BBML2-noabort feature with the new architectural BBML3
feature
- Disable in-kernel BTI for recent versions of Clang due to issues
with livepatch that are still being investigated
- Clean-up documentation describing which ID register fields are
exposed to userspace
Interrupts:
- Preliminary work towards supporting FEAT_NMI, which cleans up our
IRQ entry code and fixes some latent issues with pseudo-NMI
- Support for an SDEI backend to trigger an NMI backtrace
Memory management:
- Treat all devices as coherent when CLIDR_EL1.LoC == 0
- Fix no-map handling of sub-page-sized regions
- Second attempt at unmapping the linear aliases of the kernel data
and bss sections
- Fix EFI runtime calls when software-PAN is enabled
Miscellaneous:
- Add Mark Rutland as a reviewer!
- Tidy-up our futex cmpxchg logic when using the new LSUI
instructions
- Drop the requirement on DYNAMIC_FTRACE_WITH_CALL_OPS when
selecting HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
- Fix a false-positive KCSCAN splat in the delay loop
- Use a portable typedef for 128-bit scalar types in our UAPI headers
- Non-critical fixes for Sashiko reports all over
MPAM:
- Hook MPAM memory bandwidth counters into resctrl's counter
assignment interface
- Fix a quirk in the MPAM bandwidth counting on Nvidia T241 so that
it also applies to 63 bit counters
Perf:
- Workarounds for hardware issues in the CMN-S3 PMU (Graviton 5) and
CPU PMU (NVIDIA Olympus again!)
- Add support for the DDR PMU on Marvell CN20K SoCs
- Add support for Picoheart implementations of the DCW PCIe PMU
- Add support for Channel/Rank/Bank filtering in the CXL PMU driver
- Add support for 64-bit counters in the CSPMU device
- Add support for revision 2 of the CMN S3 PMU
Ptrace:
- Fix a decade-old bug in our handling of seccomp and tracing on
syscall entry
- Fix regset handling for inactive SVE and SSVE registers
Selftests
- Add some tests for the decade-old bug that we just tried to fix in
our syscall entry path
- Fix SVE test crash on SME-only CPUs"
* tag 'arm64-upstream' of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux: (95 commits)
arm64/efi: Avoid voluntary preemption with efi_mm installed
arm64: bti: Disable in-kernel BTI with recent versions of Clang
arm64: entry: Avoid unnecessary local_irq_disable() on kernel exit
irqchip/gic-v3: make the unmasking of pseudo-NMIs explicit when handling IRQs
arm64: Disable KCSAN instrumentation in delay.o
arm_mpam: Disable driver unbind to avoid UAF
arm_mpam: Fix a NULL pointer dereference on unbinding after an error interrupt
perf: arm_pmuv3: Zero initialize hw_id branch stack field
arm64: mm: Unmap kernel data/bss entirely from the linear map
iommu/arm-smmu-v3-sva: Use system_supports_bbml3() to detect CPU feature
perf/arm-cmn: Support CMN S3 r2
perf/arm-cmn: Plumb in new filter types
perf/arm-cmn: Refactor event filter data
perf/arm-cmn: Refactor event filter programming
perf/arm-cmn: Rename filter variables for clarity
arm64: mm: fix accidental linear mapping of no-map reserved memory
tools: Ensure tools copy of linux/filter.h exports the UAPI
kselftest/arm64: Fix abi test compilation errors
arch: arm64: add early_param idle=<wfi|yield|nop>
arm64: entry: mask DAIF before returning from C EL1 handlers
...
|
|
'qualcomm/msm', 'rockchip', 'ti/omap', 'riscv', 'intel/vt-d', 'amd/amd-vi', 'core' and 'typos' into next
|
|
Commit 94104e3cfa80 ("arm64: cpufeature: Rename BBML2_NOABORT as BBML3")
renamed the cpu_supports_bbml2_noabort() helper to cpu_supports_bbml3(),
as the Linux-defined "noabort" semantics have now been incorporated into
the architecture under the BBML3 feature.
Update the caller in the SMMUv3 SVA driver to use the new function.
There is a slightly oddity in that the SMMUv3 architecture already
defined BBML2 in such a way that aborts were prohibited, so we use the
BBML3 feature on the CPU to enable BBML2 in the SMMU.
Fixes: 94104e3cfa80 ("arm64: cpufeature: Rename BBML2_NOABORT as BBML3")
Signed-off-by: Will Deacon <will@kernel.org>
|
|
The number of master->num_streams per master device is typically very
small in practice. Sorting this array takes a very small amount of
time, so there is no practical risk of triggering a soft lockup that
would necessitate calling cond_resched() during the sort.
Replace sort_nonatomic() with the standard sort(). Since this is the
only remaining in-tree caller of sort_nonatomic(), this change paves
the way to eventually remove the unused sort_nonatomic() API from the
core library.
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
An SMMU implementing SMMU_IDR5.DS extends the range invalidation commands:
the SCALE field grows a 6th bit, raising its maximum value from 31 to 39,
and TTL == 0b01 becomes a valid level hint for a 16KB translation granule.
Add a new ARM_SMMU_FEAT_DS feature detecting the DS bit, and widen the
CMDQ_TLBI_0_SCALE field to its architectural 6 bits. Mask the scale value
explicitly in arm_smmu_cmdq_batch_add_range(), so the range invalidation
path emits the same commands as before, keeping the pre-existing 5-bit
truncation of a scale above 31.
Also list DS as a valid IDR5 field in the iommu_hw_info_arm_smmuv3 kdoc:
iommufd has always reported the raw IDR5 register, so a VMM may conclude
from that bit alone that it can expose DS to its guest.
Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Assisted-by: Claude:claude-fable-5
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
The arm_smmu_write_reg_sync() helper is currently implemented using
readl_relaxed_poll_timeout() (that relies on usleep_range() internally)
which becomes a critical issue when used in the gerror irq handler.
If the SMMU hits a gerror and enters Service Failure Mode
(GERROR_SFM_ERR), the gerror handler calls arm_smmu_device_disable() in
hard-irq context. This becomes a problem as arm_smmu_device_disable()
inevitably calls arm_smmu_write_reg_sync() which might attempt to sleep
inside a hard-irq context.
Fix this by converting the arm_smmu_write_reg_sync to use the
readl_relaxed_poll_timeout_atomic() polling helper.
(Discovered while running Sashiko locally on another patch series).
Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 48ec83bcbcf5 ("iommu/arm-smmu: Add initial driver support for ARM SMMUv3 devices")
Signed-off-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
Since table access flags cannot be software-managed, if process
pagetables are using HAFT then SVA must require the SMMU to support and
enable it too, otherwise page aging is liable to get out of whack.
For unbinding, we can't disable HAFT atomically with HA as might be
desired, but luckily we can get away with just not disabling HA either.
Cc: stable@vger.kernel.org
Fixes: 62df5870ebf7 ("arm64: Enable ARCH_HAS_NONLEAF_PMD_YOUNG")
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
tegra241_cmdqv_isr() logs the error-map registers on every error interrupt.
A malfunctioning device, or a guest deliberately faulting its own VCMDQs,
can raise these interrupts rapidly, and the unconditional dev_warn() then
floods the kernel log.
Rate-limit the message with dev_warn_ratelimited(), and pass the error-map
registers straight to it so their four MMIO reads run only when the limiter
prints, instead of building the string on every interrupt.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
tegra241_cmdqv_init_structures() allocates VINTF0 with kzalloc_obj(), inits
it, and preallocates its logical VCMDQs. Two of its error paths leak.
When tegra241_cmdqv_init_vintf() fails it returns before VINTF0 reaches the
cmdqv->vintfs[] array, so the devres unwind on probe failure cannot reach
it; free it directly there.
A later VCMDQ preallocation failure instead leaves VINTF0 published, and so
this time the unwind does reach tegra241_cmdqv_remove_vintf(), which then
frees it from vintf->hyp_own. But tegra241_vintf_hw_init() sets that flag
only afterward, from a HW read-back, so the still-uninited VINTF0 reads as
guest-owned and leaks, with mutex_destroy() and ida_destroy() run on fields
it never set up.
Decide ownership from vintf->idx instead, the index assigned when its id is
allocated: idx 0 is the kernel-owned VINTF0, while idx >= 1 marks a guest
VINTF. So the in-kernel free decision in tegra241_cmdqv_remove_vintf() and
tegra241_vintf_free_lvcmdq() now keys on idx too, and hyp_own stays a pure
HW-readback state.
Fixes: 918eb5c856f6 ("iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
tegra241_vintf_init_vsid() maps a guest vSID to a single physical Stream ID
taken from master->streams[0], and only warns when the device does not have
exactly one stream. A device with several streams gets only its first one
mapped, so a guest vSID invalidation cannot reach the others' ATC and IOTLB
entries; a device with none makes master->streams a ZERO_SIZE_PTR, read out
of bounds.
Reject the mapping with -EOPNOTSUPP if master->num_streams is not one.
Fixes: 4dc0d12474f9 ("iommu/tegra241-cmdqv: Add user-space use support")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
tegra241_vintf_init_vsid() programs the guest-provided vSID into SID_MATCH,
whose VIRT_SID field spans bits [20:1] with bit 0 as the match-enable flag.
The HW therefore matches only a 20-bit Stream ID.
The bound check rejects only virt_sid > UINT_MAX, which admits a value far
wider than the field. The write "virt_sid << 1 | 0x1" then drops every bit
above 20: a virt_sid of 0x80000000 lands as SID_MATCH = 0x1, a valid match
on vSID 0, so the entry aliases the wrong Stream ID. Because vdev->virt_id
is guest-controlled, a VMM can trigger it.
Validate virt_sid against the field width with FIELD_MAX(), and program the
register with FIELD_PREP() so the value and the field stay consistent.
Fixes: 4dc0d12474f9 ("iommu/tegra241-cmdqv: Add user-space use support")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
tegra241_cmdqv_remove() tears each VINTF down first, then calls free_irq().
Tearing a VINTF down frees vintf0 and clears cmdqv->vintfs[0]. An error in
that window makes tegra241_cmdqv_isr() read the stale slot and hand it to
tegra241_vintf0_handle_error(), which dereferences a NULL or freed pointer.
Free the IRQ before tearing the VINTFs down. free_irq() waits for in-flight
handlers to finish and blocks new ones, so no ISR can observe a VINTF as it
is torn down.
Note: a user-owned VINTF (viommu) could outlive this teardown, which unmaps
cmdqv->base and frees cmdqv->vintfs, so a later viommu close then touches
freed memory. This is neither introduced nor fixed here: a physical IOMMU
is not a pluggable device, so iommufd by design holds no reference on the
one behind a viommu, and this teardown is not expected while that viommu is
still alive.
Fixes: 918eb5c856f6 ("iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
__tegra241_cmdqv_probe() uses devm_krealloc() to grow @smmu into the larger
tegra241_cmdqv, which frees the original @smmu once it relocates. A failure
after that returned NULL, and the caller then dereferenced the freed @smmu
on its fallback path.
Return an int and take @smmu by reference instead, then update *smmu to the
reallocated pointer after devm_krealloc() succeeds, so the caller and its
fallback path both use the live @smmu rather than the freed original.
Fixes: 918eb5c856f6 ("iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
__tegra241_cmdqv_probe() requests the error IRQ before it has allocated the
cmdqv->vintfs array and set cmdqv->num_vintfs. A CMDQV left enabled with a
latched error across a kexec fires the IRQ as soon as it is requested, and
tegra241_cmdqv_isr() then walks the uninitialized cmdqv->vintfs array.
Request the IRQ only after cmdqv->vintfs is allocated and zeroed, so that
a latched interrupt firing early runs the ISR against a valid array of NULL
slots that it safely skips.
Fixes: 918eb5c856f6 ("iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
tegra241_vintf0_handle_error() reads both 64-bit LVCMDQ error-map registers
but used the register-local __ffs64() bit directly as the vintf->lvcmdqs[]
index. For the second register that selects the wrong queue instead of 64 *
i + bit, clearing the wrong queue's error status.
The index is unbounded too: a bit at or beyond num_lvcmdqs_per_vintf would
walk the read off vintf->lvcmdqs[].
tegra241_cmdqv_isr() has the same flaw one level up: a VINTF_ERR_MAP bit
at or beyond num_vintfs would walk the read off cmdqv->vintfs[].
Use 64 * i + bit for the index and clear the snapshot with the local bit.
In both handlers, WARN_ON_ONCE() and skip an out-of-bounds index. Only a
malfunctioning device sets such a bit, so the _ONCE form keeps a wedged map
from flooding the log.
Note that 64 * i + bit is not reachable with the current configuration as a
VINTF is pre-assigned with 2 lvcmdqs, this is not treated as bug fix but an
defensive hardening.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
A user VINTF is torn down by tegra241_cmdqv_deinit_vintf(), which runs from
the destroy callback and from the init-failure unwind in the alloc handler.
It clears the cmdqv->vintfs[] slot and lets the iommufd core free it, but
nothing serializes that against the error interrupt: tegra241_cmdqv_isr()
reads cmdqv->vintfs[idx] and dereferences the vintf. A concurrent error can
make the ISR read a slot mid-clear (a NULL deref) or use a vintf which is
about to be freed (a use-after-free).
deinit_vintf() also returns idx to the IDA before clearing the slot, so a
concurrent create that reuses idx can publish its new vintf into the slot,
only for this teardown to erase it again with the stale NULL store.
On the other end, tegra241_cmdqv_init_vintf() publishes a new vintf with a
plain store to the cmdqv->vintfs[] slot, and the ISR dereferences fields of
a published vintf such as vintf->base. A plain store gives no ordering on a
weakly-ordered CPU, and a stale VINTF_ERR_MAP bit on a reused idx can make
the ISR pick a vintf the moment it is published, before its fields are set
or tegra241_vintf_hw_init() runs.
The cmdqv->vintfs[0] slot stays NULL until tegra241_cmdqv_init_structures()
first creates VINTF0, so the slot 0 read needs the same NULL check.
Publish every slot with an smp_store_release(), and read each slot in the
ISR with an smp_load_acquire() under a NULL check, so the ISR always sees
a fully built vintf or NULL. Also make deinit_vintf() clear the slot, and
synchronize_irq() prior to returning idx to the IDA, so no vintf is freed
under a running handler and no reused idx is clobbered.
Fixes: 4dc0d12474f9 ("iommu/tegra241-cmdqv: Add user-space use support")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
tegra241_vintf_init_lvcmdq() stores the freshly allocated vcmdq pointer to
the vintf->lvcmdqs[] array, before tegra241_vcmdq_alloc_smmu_cmdq() builds
the vcmdq->cmdq. The error ISR dereferences that cmdq, so a latched LVCMDQ
error (e.g. one inherited across a kexec) firing in this window would make
tegra241_vintf0_handle_error() pass the still-zeroed arm_smmu_cmdq down to
__arm_smmu_cmdq_skip_err(), dereferencing NULL queue register pointers.
Drop the store from tegra241_vintf_init_lvcmdq() and publish the vcmdq at
the end of the allocation instead, with an smp_store_release() that pairs
with an smp_load_acquire() in the ISR, which can see a fully built LVCMDQ
or NULL.
The user-owned LVCMDQ allocation moves accordingly, publishing the vcmdq
once tegra241_vcmdq_hw_init_user() succeeds, using a plain store since a
user VINTF's lvcmdqs[] has no lockless reader -- the error ISR only walks
the VINTF0 array.
Fixes: 918eb5c856f6 ("iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
Nvidia Tegra264 SMMU is affected by an erratum where a TLB entry can
survive an invalidation that races with concurrent traffic targeting
the same entry. The hardware-recommended software workaround is to
issue every CFGI/TLBI command (each followed by CMD_SYNC) twice, and
that infrastructure is already in place behind
arm_smmu_erratum_repeat_tlbi_cfgi_key.
Neither IDR nor IIDR flags this Tegra264-specific bug, so hardware
detection is not possible. Tegra264 is device-tree-only (no ACPI/IORT
support) and already has a dedicated "nvidia,tegra264-smmu" compatible,
so DT-probe is the only viable detection path.
Enable the workaround on instances matching the existing
"nvidia,tegra264-smmu" compatible by calling static_branch_enable() on
arm_smmu_erratum_repeat_tlbi_cfgi_key. Document the erratum in
Documentation/arch/arm64/silicon-errata.rst.
Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
A guest with access to VCMDQ generates its own invalidation commands
and must apply any invalidation errata before submitting them. If the
host also repeats those commands, each affected invalidation is issued
four times instead of twice.
Add IOMMU_HW_INFO_ARM_SMMUV3_ERRATA_REPEAT_TLBI_CFGI to report the
CFGI/TLBI-repeat erratum to user space. This allows the VMM to expose
the erratum to the guest or apply the workaround itself.
Use the raw __arm_smmu_cmdq_issue_cmdlist() helper for user-provided
invalidations so the host does not apply the workaround a second time.
Add arm_smmu_erratum_repeat_tlbi_cfgi() to query the static key when
populating the SMMUv3 hardware information.
Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
Tegra264 SMMU instances need every CFGI/TLBI command sequence issued
twice, with the second issue executing only after the first issue's
CMD_SYNC has completed:
TLBI/CFGI ... CMD_SYNC TLBI/CFGI ... CMD_SYNC
ATC_INV is not affected and must never be doubled.
Add arm_smmu_erratum_repeat_tlbi_cfgi_key and a file-local
arm_smmu_erratum_cmd_needs_repeating() helper that gates on the static
key first and then range-checks the opcode (CFGI_STE .. ATC_INV).
Rename the existing arm_smmu_cmdq_issue_cmdlist() to
__arm_smmu_cmdq_issue_cmdlist() and add a thin wrapper of the original
name that re-issues the same cmdlist a second time when the predicate
fires. Register the new condition with
arm_smmu_cmdq_batch_force_sync() too.
No callers enable the static key yet, so there is no functional change.
A subsequent change will enable the key on affected instances.
Suggested-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
arm_smmu_cmdq_batch_add_cmd_p() carries two distinct reasons for
flushing the current batch with a CMD_SYNC before appending the
new command:
- The batch's pre-assigned cmdq does not support the new command.
- The Arm erratum 2812531 workaround (ARM_SMMU_OPT_CMDQ_FORCE_SYNC)
forces a SYNC at one entry before the batch is full.
Lift those checks into a new arm_smmu_cmdq_batch_force_sync() helper
so that adding another force-sync condition becomes a one-line
addition. No functional change.
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
arm_smmu_impl_remove() is registered as a devres action in
arm_smmu_impl_probe(), before arm_smmu_init_queues() allocates
smmu->cmdq.q.base. On a devres unwind, whether a failed probe or an
unbind, the queue is freed first and arm_smmu_impl_remove() then runs
tegra241_cmdqv_remove_vintf(), whose VINTF deinit issues a CMD_SYNC on
the freed memory.
Observed during testing with a QEMU hack that makes the VCMDQ fail to
enable, so the impl reset fails and probe aborts into the devres unwind:
platform NVDA200C:00: tegra241_cmdqv: VINTF0: VCMDQ0/LVCMDQ0: failed to enable, STATUS=0x00000000
platform NVDA200C:00: tegra241_cmdqv: VINTF0: VCMDQ0/LVCMDQ0: GERRORN=0x0, GERROR=0x4, CONS=0x0
platform NVDA200C:00: tegra241_cmdqv: VINTF0: VCMDQ0/LVCMDQ0: uncleared error detected, resetting
arm-smmu-v3 arm-smmu-v3.0.auto: failed to reset impl
arm-smmu-v3 arm-smmu-v3.0.auto: probe with driver arm-smmu-v3 failed with error -110
Unable to handle kernel paging request at virtual address ffff8000891e0098
...
Internal error: Oops: 0000000096000047 [#1] SMP
...
Call trace:
arm_smmu_cmdq_issue_cmdlist+0x320/0x6fc (P)
tegra241_vcmdq_hw_deinit+0x98/0x168
tegra241_vintf_hw_deinit+0x5c/0x1b0
tegra241_cmdqv_remove_vintf+0x34/0xec
tegra241_cmdqv_remove+0x40/0x9c
arm_smmu_impl_remove+0x20/0x30
devm_action_release+0x14/0x20
devres_release_all+0xa8/0x110
device_unbind_cleanup+0x18/0x84
really_probe+0x1f0/0x29c
Drop the VINTF deinit from tegra241_cmdqv_remove_vintf() so the unwind no
longer touches the freed queue. Quiesce the VINTFs earlier instead. Add a
device_disable() impl op and run it from arm_smmu_disable_action() while
the CMDQ is still up. That handles a live unbind. A failed reset is already
handled because tegra241_vintf_hw_init() deinits the VINTF on its own error
path. tegra241_cmdqv_remove_vintf() is also used by the iommufd viommu
destroy path, so quiesce there too.
Fixes: 4dc0d12474f9 ("iommu/tegra241-cmdqv: Add user-space use support")
Cc: stable@vger.kernel.org
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
arm_smmu_device_remove() manually frees the IOPF queue, destroys the
vmid_map and disables the device, while the IRQs and queues are devm
managed. devm unwinds only after remove() returns, so the cleanup runs
in the wrong order. The IOPF queue is freed before the event-queue IRQ
whose handler uses it.
Manage all of it with devm so the unwind order is correct. Free the IOPF
queue and vmid_map via devm actions, and disable the device from one
registered after arm_smmu_device_reset().
This is also a prerequisite for fixing a Tegra241 CMDQV CMD_SYNC
use-after-free in the subsequent patch.
Cc: stable@vger.kernel.org
Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
|
arm_smmu_get_ste_update_safe() declares the eats_s1chk and eats_trans
locals as __le64, but initializes them from FIELD_PREP(), which returns a
host-endian value, and passes them through cpu_to_le64() at the use sites.
Sparse reports the following warnings:
>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:1122:38: sparse: sparse: cast from restricted __le64
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:1124:33: sparse: sparse: cast from restricted __le64
Declare both locals as u64 so the type matches FIELD_PREP() and the
existing cpu_to_le64() at the use sites performs the host-to-little-endian
conversion. No functional change.
Fixes: 7cad80048595 ("iommu/arm-smmu-v3: Mark EATS_TRANS safe when computing the update sequence")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/all/202606151017.QU0evpH9-lkp@intel.com/
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Will Deacon <will@kernel.org>
|
