On Tue, 5 Dec 2023, Federico Serafini wrote:
> Add missing parameter names to address violations of MISRA C:2012
> Rule 8.2. Furthermore, remove trailing spaces and use C standard types to
> comply with XEN coding style.
>
> No functional change.
>
> Signed-off-by: Federico Serafini <federico.seraf...@bugseng.com>
> ---
> xen/drivers/passthrough/amd/iommu.h | 17 ++++++++++-------
> xen/drivers/passthrough/amd/iommu_init.c | 24 ++++++++++++++----------
> 2 files changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/xen/drivers/passthrough/amd/iommu.h
> b/xen/drivers/passthrough/amd/iommu.h
> index d4416ebc43..1b62c083ba 100644
> --- a/xen/drivers/passthrough/amd/iommu.h
> +++ b/xen/drivers/passthrough/amd/iommu.h
> @@ -138,10 +138,12 @@ struct ivrs_mappings {
> extern unsigned int ivrs_bdf_entries;
> extern u8 ivhd_type;
>
> -struct ivrs_mappings *get_ivrs_mappings(u16 seg);
> -int iterate_ivrs_mappings(int (*)(u16 seg, struct ivrs_mappings *));
> -int iterate_ivrs_entries(int (*)(const struct amd_iommu *,
> - struct ivrs_mappings *, uint16_t));
> +struct ivrs_mappings *get_ivrs_mappings(uint16_t seg);
> +int iterate_ivrs_mappings(int (*handler)(uint16_t seg,
> + struct ivrs_mappings *map));
> +int iterate_ivrs_entries(int (*handler)(const struct amd_iommu *iommu,
> + struct ivrs_mappings *map,
> + uint16_t bdf));
>
> /* iommu tables in guest space */
> struct mmio_reg {
> @@ -226,7 +228,7 @@ struct acpi_ivrs_hardware;
> /* amd-iommu-detect functions */
> int amd_iommu_get_ivrs_dev_entries(void);
> int amd_iommu_get_supported_ivhd_type(void);
> -int amd_iommu_detect_one_acpi(const struct acpi_ivrs_hardware *);
> +int amd_iommu_detect_one_acpi(const struct acpi_ivrs_hardware *ivhd_block);
> int amd_iommu_detect_acpi(void);
> void get_iommu_features(struct amd_iommu *iommu);
>
> @@ -295,9 +297,10 @@ struct amd_iommu *find_iommu_for_device(int seg, int
> bdf);
> bool cf_check iov_supports_xt(void);
> int amd_iommu_setup_ioapic_remapping(void);
> void *amd_iommu_alloc_intremap_table(
> - const struct amd_iommu *, unsigned long **, unsigned int nr);
> + const struct amd_iommu *iommu, unsigned long **inuse_map, unsigned int
> nr);
> int cf_check amd_iommu_free_intremap_table(
> - const struct amd_iommu *, struct ivrs_mappings *, uint16_t);
> + const struct amd_iommu *iommu, struct ivrs_mappings *ivrs_mapping,
> + uint16_t bdf);
> unsigned int amd_iommu_intremap_table_order(
> const void *irt, const struct amd_iommu *iommu);
> void cf_check amd_iommu_ioapic_update_ire(
> diff --git a/xen/drivers/passthrough/amd/iommu_init.c
> b/xen/drivers/passthrough/amd/iommu_init.c
> index 5515cb70fd..62f9bfdfc8 100644
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -300,12 +300,13 @@ static void cf_check set_iommu_ppr_log_control(
> static int iommu_read_log(struct amd_iommu *iommu,
> struct ring_buffer *log,
> unsigned int entry_size,
> - void (*parse_func)(struct amd_iommu *, u32 *))
> + void (*parse_func)(struct amd_iommu *iommu,
> + uint32_t *entry))
> {
> unsigned int tail, tail_offest, head_offset;
>
> BUG_ON(!iommu || ((log != &iommu->event_log) && (log !=
> &iommu->ppr_log)));
> -
> +
> spin_lock(&log->lock);
>
> /* make sure there's an entry in the log */
> @@ -361,14 +362,15 @@ static int iommu_read_log(struct amd_iommu *iommu,
>
> out:
> spin_unlock(&log->lock);
> -
> +
> return 0;
> }
>
> /* reset event log or ppr log when overflow */
> static void iommu_reset_log(struct amd_iommu *iommu,
> struct ring_buffer *log,
> - void (*ctrl_func)(struct amd_iommu *iommu, bool))
> + void (*ctrl_func)(struct amd_iommu *iommu,
> + bool iommu_control))
instead of iommu_control it should be iommu_enable ?
> {
> unsigned int entry, run_bit, loop_count = 1000;
> bool log_run;
> @@ -1158,14 +1160,15 @@ static void __init amd_iommu_init_cleanup(void)
> iommuv2_enabled = 0;
> }
>
> -struct ivrs_mappings *get_ivrs_mappings(u16 seg)
> +struct ivrs_mappings *get_ivrs_mappings(uint16_t seg)
> {
> return radix_tree_lookup(&ivrs_maps, seg);
> }
>
> -int iterate_ivrs_mappings(int (*handler)(u16 seg, struct ivrs_mappings *))
> +int iterate_ivrs_mappings(int (*handler)(uint16_t seg,
> + struct ivrs_mappings *map))
Instead of map it should be ivrs_mappings ? Actually it reads better as
map and I know it is not a MISRA requirement to have function pointer
args match. I'll leave this one to Jan.
> {
> - u16 seg = 0;
> + uint16_t seg = 0;
> int rc = 0;
>
> do {
> @@ -1180,10 +1183,11 @@ int iterate_ivrs_mappings(int (*handler)(u16 seg,
> struct ivrs_mappings *))
> return rc;
> }
>
> -int iterate_ivrs_entries(int (*handler)(const struct amd_iommu *,
> - struct ivrs_mappings *, uint16_t
> bdf))
> +int iterate_ivrs_entries(int (*handler)(const struct amd_iommu *iommu,
> + struct ivrs_mappings *map,
same here ivrs_mappings instead of map, but actually map reads better
I checked everything else and looks OK
