[PATCH v5 04/27] iommu/arm-smmu-v3: Add an ops indirection to the STE code
Jason Gunthorpe
jgg at nvidia.com
Mon Mar 25 07:11:32 PDT 2024
On Fri, Mar 22, 2024 at 06:14:24PM +0000, Mostafa Saleh wrote:
> > @@ -1027,57 +1038,55 @@ static void arm_smmu_get_ste_used(const struct arm_smmu_ste *ent,
> > * unused_update is an intermediate value of entry that has unused bits set to
> > * their new values.
> > */
> > -static u8 arm_smmu_entry_qword_diff(const struct arm_smmu_ste *entry,
> > - const struct arm_smmu_ste *target,
> > - struct arm_smmu_ste *unused_update)
> > +static u8 arm_smmu_entry_qword_diff(struct arm_smmu_entry_writer *writer,
> > + const __le64 *entry, const __le64 *target,
> > + __le64 *unused_update)
> > {
> > - struct arm_smmu_ste target_used = {};
> > - struct arm_smmu_ste cur_used = {};
> > + __le64 target_used[NUM_ENTRY_QWORDS] = {};
> > + __le64 cur_used[NUM_ENTRY_QWORDS] = {};
> This is confusing to me, the function was modified to be generic, so its has
> args are __le64 * instead of struct arm_smmu_ste *.
Right
> But NUM_ENTRY_QWORDS is defined as “(sizeof(struct arm_smmu_ste) / sizeof(u64))”
> and in the same function writer->ops->num_entry_qwords is used
> nterchangeably,
Right
> I understand that this not a constant and the compiler would complain.
> But since for any other num_entry_qwords larger than NUM_ENTRY_QWORDS it fails,
> and we know STEs and CDs both have the same size, we simplify the code and make
> it a constant everywhere.
So you say to get rid of num_entry_qwords and just use the constant?
> I see in the next patch, that this is redefined to be the max between STE and
> CD, but again, this hardware and it never changes, so my opinion is to simplify
> the code, as there is no need to generalize this part.
Yes, we need a constant.
It would look like this, it is a little bit simpler:
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index a54062faccde38..d015f41900d802 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -63,9 +63,9 @@ enum arm_smmu_msi_index {
ARM_SMMU_MAX_MSIS,
};
-#define NUM_ENTRY_QWORDS \
- (max(sizeof(struct arm_smmu_ste), sizeof(struct arm_smmu_cd)) / \
- sizeof(u64))
+#define NUM_ENTRY_QWORDS 8
+static_assert(sizeof(struct arm_smmu_ste) == NUM_ENTRY_QWORDS * sizeof(u64));
+static_assert(sizeof(struct arm_smmu_cd) == NUM_ENTRY_QWORDS * sizeof(u64));
static phys_addr_t arm_smmu_msi_cfg[ARM_SMMU_MAX_MSIS][3] = {
[EVTQ_MSI_INDEX] = {
@@ -1045,7 +1045,7 @@ static u8 arm_smmu_entry_qword_diff(struct arm_smmu_entry_writer *writer,
writer->ops->get_used(entry, cur_used);
writer->ops->get_used(target, target_used);
- for (i = 0; i != writer->ops->num_entry_qwords; i++) {
+ for (i = 0; i != NUM_ENTRY_QWORDS; i++) {
/*
* Check that masks are up to date, the make functions are not
* allowed to set a bit to 1 if the used function doesn't say it
@@ -1114,7 +1114,6 @@ static bool entry_set(struct arm_smmu_entry_writer *writer, __le64 *entry,
void arm_smmu_write_entry(struct arm_smmu_entry_writer *writer, __le64 *entry,
const __le64 *target)
{
- unsigned int num_entry_qwords = writer->ops->num_entry_qwords;
__le64 unused_update[NUM_ENTRY_QWORDS];
u8 used_qword_diff;
@@ -1137,9 +1136,9 @@ void arm_smmu_write_entry(struct arm_smmu_entry_writer *writer, __le64 *entry,
*/
unused_update[critical_qword_index] =
entry[critical_qword_index];
- entry_set(writer, entry, unused_update, 0, num_entry_qwords);
+ entry_set(writer, entry, unused_update, 0, NUM_ENTRY_QWORDS);
entry_set(writer, entry, target, critical_qword_index, 1);
- entry_set(writer, entry, target, 0, num_entry_qwords);
+ entry_set(writer, entry, target, 0, NUM_ENTRY_QWORDS);
} else if (used_qword_diff) {
/*
* At least two qwords need their inuse bits to be changed. This
@@ -1148,7 +1147,7 @@ void arm_smmu_write_entry(struct arm_smmu_entry_writer *writer, __le64 *entry,
*/
unused_update[0] = entry[0] & (~writer->ops->v_bit);
entry_set(writer, entry, unused_update, 0, 1);
- entry_set(writer, entry, target, 1, num_entry_qwords - 1);
+ entry_set(writer, entry, target, 1, NUM_ENTRY_QWORDS - 1);
entry_set(writer, entry, target, 0, 1);
} else {
/*
@@ -1157,7 +1156,7 @@ void arm_smmu_write_entry(struct arm_smmu_entry_writer *writer, __le64 *entry,
* compute_qword_diff().
*/
WARN_ON_ONCE(
- entry_set(writer, entry, target, 0, num_entry_qwords));
+ entry_set(writer, entry, target, 0, NUM_ENTRY_QWORDS));
}
}
@@ -1272,7 +1271,6 @@ static const struct arm_smmu_entry_writer_ops arm_smmu_cd_writer_ops = {
.sync = arm_smmu_cd_writer_sync_entry,
.get_used = arm_smmu_get_cd_used,
.v_bit = cpu_to_le64(CTXDESC_CD_0_V),
- .num_entry_qwords = sizeof(struct arm_smmu_cd) / sizeof(u64),
};
void arm_smmu_write_cd_entry(struct arm_smmu_master *master, int ssid,
@@ -1460,7 +1458,6 @@ static const struct arm_smmu_entry_writer_ops arm_smmu_ste_writer_ops = {
.sync = arm_smmu_ste_writer_sync_entry,
.get_used = arm_smmu_get_ste_used,
.v_bit = cpu_to_le64(STRTAB_STE_0_V),
- .num_entry_qwords = sizeof(struct arm_smmu_ste) / sizeof(u64),
};
static void arm_smmu_write_ste(struct arm_smmu_master *master, u32 sid,
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 8ba07b00bf6056..5936dc5f76786a 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -779,7 +779,6 @@ struct arm_smmu_entry_writer {
};
struct arm_smmu_entry_writer_ops {
- unsigned int num_entry_qwords;
__le64 v_bit;
void (*get_used)(const __le64 *entry, __le64 *used);
void (*sync)(struct arm_smmu_entry_writer *writer);
More information about the linux-arm-kernel
mailing list