[PATCH 04/19] iommu/arm-smmu-v3: Make STE programming independent of the callers
Jason Gunthorpe
jgg at nvidia.com
Wed Dec 27 07:46:48 PST 2023
On Tue, Dec 19, 2023 at 09:42:27PM +0800, Michael Shavit wrote:
> +static void arm_smmu_write_entry(struct arm_smmu_entry_writer *writer,
> + __le64 *cur, const __le64 *target,
> + __le64 *staging_entry)
> +{
> + bool cleanup_sync_required = false;
> + u8 entry_qwords_used_diff = 0;
> + int i = 0;
> +
> + entry_qwords_used_diff =
> + writer->ops.get_used_qword_diff_indexes(cur, target);
> + if (WARN_ON_ONCE(entry_qwords_used_diff == 0))
> + return;
A no change update is actually API legal, eg we can set the same
domain twice in a row. It should just do nothing.
If the goal is to improve readability I'd split this into smaller
functions and have the main function look like this:
compute_used(..)
if (hweight8(entry_qwords_used_diff) > 1) {
set_v_0(..);
set(qword_start=1,qword_end=N);
set(qword_start=0,qword_end=1); // V=1
} else if (hweight8(entry_qwords_used_diff) == 1) {
set_unused(..);
critical = ffs(..);
set(qword_start=critical,qword_end=critical+1);
set(qword_start=0,qword_end=N);
} else { // hweight8 == 0
set(qword_start=0,qword_end=N);
}
Then the three different programming algorithms are entirely clear in
code. Make the generic set() function skip the sync if nothing
changed.
> + if (hweight8(entry_qwords_used_diff) > 1) {
> + /*
> + * If transitioning to the target entry with a single qword
> + * write isn't possible, then we must first transition to an
> + * intermediate entry. The intermediate entry may either be an
> + * entry that melds bits of the target entry into the current
> + * entry without disrupting the hardware, or a breaking entry if
> + * a hitless transition to the target is impossible.
> + */
> +
> + /*
> + * Compute a staging entry that has all the bits currently
> + * unused by HW set to their target values, such that comitting
> + * it to the entry table woudn't disrupt the hardware.
> + */
> + memcpy(staging_entry, cur, writer->entry_length);
> + writer->ops.set_unused_bits(staging_entry, target);
> +
> + entry_qwords_used_diff =
> + writer->ops.get_used_qword_diff_indexes(staging_entry,
> + target);
Put the number qwords directly in the ops struct and don't make this
an op. Above will need N=number of qwords as well.
Regrads,
Jason
More information about the linux-arm-kernel
mailing list