| Message ID | 20241212154000.330467-2-Dave.Martin@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce flexible CLOSID/RMID translation | expand |
Hi all, On Thu, Dec 12, 2024 at 03:39:55PM +0000, Dave Martin wrote: > In mpam_apply_config(), a simple memcmp() test is used to check > whether the config passed by the caller is already installed or > not. > > This check will never find a match except (very occasionally) by > accident, since the component's version of the struct contains > things that the caller won't pass or doesn't know (such as the > garbage collection record). There might also be random padding. > > This may result in MSCs being reprogrammed unnecessarily. > > Instead, only compare fields that the caller specified. If > anything is present in the caller's config and doesn't match the > installed config, paste it across. If nothing was pasted across > then the MSC reprogramming step is skipped (as the previous code > attempted to do). > > Signed-off-by: Dave Martin <Dave.Martin@arm.com> > > --- > > There are comments in the code suggesting a change of this sort. > I may or may not have gone in the right direction with this, and > I have only tried to clean up the behaviour rather than optimising. > > No attempt is made to skip unnecessary MSC register updates if the MSC > reprogramming goes ahead. > > NOT well tested, yet. > --- Oops, git rebase didn't spot that this patch had already been applied, due to adjacent hunks inserted in the meantime from other patches. This patch can be dropped from this series. [...] Cheers ---Dave
diff --git a/drivers/platform/arm64/mpam/mpam_devices.c b/drivers/platform/arm64/mpam/mpam_devices.c index 9463045c0011..41962dd1bb68 100644 --- a/drivers/platform/arm64/mpam/mpam_devices.c +++ b/drivers/platform/arm64/mpam/mpam_devices.c @@ -3120,6 +3120,29 @@ static void mpam_extend_config(struct mpam_class *class, struct mpam_config *cfg } } +#define maybe_update_config(cfg, feature, newcfg, member, changes) do { \ + if (mpam_has_feature(feature, newcfg) && \ + (newcfg)->member != (cfg)->member) { \ + (cfg)->member = (newcfg)->member; \ + mpam_set_feature(feature, cfg); \ + \ + (changes) |= (feature); \ + } \ +} while (0) + +static mpam_features_t mpam_update_config(struct mpam_config *cfg, + const struct mpam_config *newcfg) +{ + mpam_features_t changes = 0; + + maybe_update_config(cfg, mpam_feat_cpor_part, newcfg, cpbm, changes); + maybe_update_config(cfg, mpam_feat_mbw_part, newcfg, mbw_pbm, changes); + maybe_update_config(cfg, mpam_feat_mbw_max, newcfg, mbw_max, changes); + maybe_update_config(cfg, mpam_feat_mbw_min, newcfg, mbw_min, changes); + + return changes; +} + /* TODO: split into write_config/sync_config */ /* TODO: add config_dirty bitmap to drive sync_config */ int mpam_apply_config(struct mpam_component *comp, u16 partid,
In mpam_apply_config(), a simple memcmp() test is used to check whether the config passed by the caller is already installed or not. This check will never find a match except (very occasionally) by accident, since the component's version of the struct contains things that the caller won't pass or doesn't know (such as the garbage collection record). There might also be random padding. This may result in MSCs being reprogrammed unnecessarily. Instead, only compare fields that the caller specified. If anything is present in the caller's config and doesn't match the installed config, paste it across. If nothing was pasted across then the MSC reprogramming step is skipped (as the previous code attempted to do). Signed-off-by: Dave Martin <Dave.Martin@arm.com> --- There are comments in the code suggesting a change of this sort. I may or may not have gone in the right direction with this, and I have only tried to clean up the behaviour rather than optimising. No attempt is made to skip unnecessary MSC register updates if the MSC reprogramming goes ahead. NOT well tested, yet. --- drivers/platform/arm64/mpam/mpam_devices.c | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+)