[XEN PATCH 0/6] xen: address violations of MISRA C:2012 Rule 8.2

[XEN PATCH 0/6] xen: address violations of MISRA C:2012 Rule 8.2

[Federico Serafini]

This patch series adds the missing parameter names and makes some
improvements to the coding style as the removal of trailing spaces and
the use of C standard integer types over Linux typedefs.

No functional changes are introduced.

Federico Serafini (6):
  xen/acpi: address remaining violations of MISRA C:2012 Rule 8.2
  x86/mm: address violations of MISRA C:2012 Rule 8.2
  AMD/IOMMU: address violations of MISRA C:2012 Rule 8.2
  x86/page: address violations of MISRA C:2012 Rule 8.2
  x86/mce: address violations of MISRA C:2012 Rule 8.2
  xen/pci: address violations of MISRA C:2012 Rule 8.2

 xen/arch/x86/cpu/mcheck/mce.h             |  2 +-
 xen/arch/x86/include/asm/acpi.h           |  2 +-
 xen/arch/x86/include/asm/mce.h            |  4 ++--
 xen/arch/x86/include/asm/mm.h             | 20 +++++++++----------
 xen/arch/x86/include/asm/page.h           |  6 +++---
 xen/drivers/passthrough/amd/iommu.h       | 17 +++++++++-------
 xen/drivers/passthrough/amd/iommu_init.c  | 24 +++++++++++++----------
 xen/drivers/passthrough/pci.c             |  8 ++++----
 xen/include/acpi/apei.h                   |  5 +++--
 xen/include/acpi/cpufreq/cpufreq.h        |  2 +-
 xen/include/acpi/cpufreq/processor_perf.h | 16 +++++++--------
 xen/include/xen/pci.h                     |  3 ++-
 12 files changed, 59 insertions(+), 50 deletions(-)

-- 
2.34.1

[XEN PATCH 1/6] xen/acpi: address remaining violations of MISRA C:2012 Rule 8.2

[Federico Serafini]

Add missing parameter names to address violations of MISRA C:2012
Rule 8.2. Furthermore, use C standard types to comply with XEN coding style.

No functional change.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 xen/arch/x86/include/asm/acpi.h           |  2 +-
 xen/include/acpi/apei.h                   |  5 +++--
 xen/include/acpi/cpufreq/cpufreq.h        |  2 +-
 xen/include/acpi/cpufreq/processor_perf.h | 16 ++++++++--------
 4 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/include/asm/acpi.h b/xen/arch/x86/include/asm/acpi.h
index 0df92d3714..d54c105f61 100644
--- a/xen/arch/x86/include/asm/acpi.h
+++ b/xen/arch/x86/include/asm/acpi.h
@@ -81,7 +81,7 @@ extern bool acpi_lapic, acpi_ioapic, acpi_noirq;
 extern bool acpi_force, acpi_ht, acpi_disabled;
 extern u32 acpi_smi_cmd;
 extern u8 acpi_enable_value, acpi_disable_value;
-void acpi_pic_sci_set_trigger(unsigned int, u16);
+void acpi_pic_sci_set_trigger(unsigned int irq, uint16_t trigger);
 
 static inline void disable_acpi(void)
 {
diff --git a/xen/include/acpi/apei.h b/xen/include/acpi/apei.h
index eab16e2543..495819e362 100644
--- a/xen/include/acpi/apei.h
+++ b/xen/include/acpi/apei.h
@@ -12,8 +12,9 @@
 
 #define FIX_APEI_RANGE_MAX 64
 
-typedef int (*apei_hest_func_t)(const struct acpi_hest_header *, void *);
-int apei_hest_parse(apei_hest_func_t, void *);
+typedef int (*apei_hest_func_t)(const struct acpi_hest_header *hest_hdr,
+                                void *data);
+int apei_hest_parse(apei_hest_func_t func, void *data);
 
 int erst_write(const struct cper_record_header *record);
 ssize_t erst_get_record_count(void);
diff --git a/xen/include/acpi/cpufreq/cpufreq.h b/xen/include/acpi/cpufreq/cpufreq.h
index 3456d4c95f..b4685a9085 100644
--- a/xen/include/acpi/cpufreq/cpufreq.h
+++ b/xen/include/acpi/cpufreq/cpufreq.h
@@ -177,7 +177,7 @@ struct cpufreq_driver {
 
 extern struct cpufreq_driver cpufreq_driver;
 
-int cpufreq_register_driver(const struct cpufreq_driver *);
+int cpufreq_register_driver(const struct cpufreq_driver *driver_data);
 
 static inline
 void cpufreq_verify_within_limits(struct cpufreq_policy *policy,
diff --git a/xen/include/acpi/cpufreq/processor_perf.h b/xen/include/acpi/cpufreq/processor_perf.h
index 8b5a1b9bde..7cd2400619 100644
--- a/xen/include/acpi/cpufreq/processor_perf.h
+++ b/xen/include/acpi/cpufreq/processor_perf.h
@@ -10,16 +10,16 @@
 int powernow_cpufreq_init(void);
 unsigned int powernow_register_driver(void);
 unsigned int get_measured_perf(unsigned int cpu, unsigned int flag);
-void cpufreq_residency_update(unsigned int, uint8_t);
-void cpufreq_statistic_update(unsigned int, uint8_t, uint8_t);
-int  cpufreq_statistic_init(unsigned int);
-void cpufreq_statistic_exit(unsigned int);
-void cpufreq_statistic_reset(unsigned int);
+void cpufreq_residency_update(unsigned int cpu, uint8_t state);
+void cpufreq_statistic_update(unsigned int cpu, uint8_t from, uint8_t to);
+int  cpufreq_statistic_init(unsigned int cpuid);
+void cpufreq_statistic_exit(unsigned int cpuid);
+void cpufreq_statistic_reset(unsigned int cpuid);
 
-int  cpufreq_limit_change(unsigned int);
+int  cpufreq_limit_change(unsigned int cpu);
 
-int  cpufreq_add_cpu(unsigned int);
-int  cpufreq_del_cpu(unsigned int);
+int  cpufreq_add_cpu(unsigned int cpu);
+int  cpufreq_del_cpu(unsigned int cpu);
 
 struct processor_performance {
     uint32_t state;
-- 
2.34.1

Re: [XEN PATCH 1/6] xen/acpi: address remaining violations of MISRA C:2012 Rule 8.2

[Stefano Stabellini]

On Tue, 5 Dec 2023, Federico Serafini wrote:
> Add missing parameter names to address violations of MISRA C:2012
> Rule 8.2. Furthermore, use C standard types to comply with XEN coding style.
> 
> No functional change.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> ---
>  xen/arch/x86/include/asm/acpi.h           |  2 +-
>  xen/include/acpi/apei.h                   |  5 +++--
>  xen/include/acpi/cpufreq/cpufreq.h        |  2 +-
>  xen/include/acpi/cpufreq/processor_perf.h | 16 ++++++++--------
>  4 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/x86/include/asm/acpi.h b/xen/arch/x86/include/asm/acpi.h
> index 0df92d3714..d54c105f61 100644
> --- a/xen/arch/x86/include/asm/acpi.h
> +++ b/xen/arch/x86/include/asm/acpi.h
> @@ -81,7 +81,7 @@ extern bool acpi_lapic, acpi_ioapic, acpi_noirq;
>  extern bool acpi_force, acpi_ht, acpi_disabled;
>  extern u32 acpi_smi_cmd;
>  extern u8 acpi_enable_value, acpi_disable_value;
> -void acpi_pic_sci_set_trigger(unsigned int, u16);
> +void acpi_pic_sci_set_trigger(unsigned int irq, uint16_t trigger);

There is something strange about this one, I cannot find the definition
anywhere. Am I missing anything?

Everything else looks good

Re: [XEN PATCH 1/6] xen/acpi: address remaining violations of MISRA C:2012 Rule 8.2

[Jan Beulich]

On 06.12.2023 04:07, Stefano Stabellini wrote:
> On Tue, 5 Dec 2023, Federico Serafini wrote:
>> --- a/xen/arch/x86/include/asm/acpi.h
>> +++ b/xen/arch/x86/include/asm/acpi.h
>> @@ -81,7 +81,7 @@ extern bool acpi_lapic, acpi_ioapic, acpi_noirq;
>>  extern bool acpi_force, acpi_ht, acpi_disabled;
>>  extern u32 acpi_smi_cmd;
>>  extern u8 acpi_enable_value, acpi_disable_value;
>> -void acpi_pic_sci_set_trigger(unsigned int, u16);
>> +void acpi_pic_sci_set_trigger(unsigned int irq, uint16_t trigger);
> 
> There is something strange about this one, I cannot find the definition
> anywhere. Am I missing anything?

That was apparently added by mistake in 0de39e40277b ("Update ACPI headers
to Linux 2.6.24"). I wouldn't be surprised if there were more.

So yes, the line wants deleting, not updating.

Jan

[XEN PATCH 2/6] x86/mm: address violations of MISRA C:2012 Rule 8.2

[Federico Serafini]

Add missing parameter names. No functional change.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 xen/arch/x86/include/asm/mm.h | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/include/asm/mm.h b/xen/arch/x86/include/asm/mm.h
index 05dfe35502..a270f8ddd6 100644
--- a/xen/arch/x86/include/asm/mm.h
+++ b/xen/arch/x86/include/asm/mm.h
@@ -406,7 +406,7 @@ void put_page_type(struct page_info *page);
 int  get_page_type(struct page_info *page, unsigned long type);
 int  put_page_type_preemptible(struct page_info *page);
 int  get_page_type_preemptible(struct page_info *page, unsigned long type);
-int  put_old_guest_table(struct vcpu *);
+int  put_old_guest_table(struct vcpu *v);
 int  get_page_from_l1e(
     l1_pgentry_t l1e, struct domain *l1e_owner, struct domain *pg_owner);
 void put_page_from_l1e(l1_pgentry_t l1e, struct domain *l1e_owner);
@@ -557,7 +557,7 @@ void audit_domains(void);
 
 void make_cr3(struct vcpu *v, mfn_t mfn);
 pagetable_t update_cr3(struct vcpu *v);
-int vcpu_destroy_pagetables(struct vcpu *);
+int vcpu_destroy_pagetables(struct vcpu *v);
 void *do_page_walk(struct vcpu *v, unsigned long addr);
 
 /* Allocator functions for Xen pagetables. */
@@ -572,20 +572,20 @@ int __sync_local_execstate(void);
 /* Arch-specific portion of memory_op hypercall. */
 long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
 long subarch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
-int compat_arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void));
-int compat_subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void));
+int compat_arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
+int compat_subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);
 
 #define NIL(type) ((type *)-sizeof(type))
 #define IS_NIL(ptr) (!((uintptr_t)(ptr) + sizeof(*(ptr))))
 
-int create_perdomain_mapping(struct domain *, unsigned long va,
-                             unsigned int nr, l1_pgentry_t **,
-                             struct page_info **);
-void destroy_perdomain_mapping(struct domain *, unsigned long va,
+int create_perdomain_mapping(struct domain *d, unsigned long va,
+                             unsigned int nr, l1_pgentry_t **pl1tab,
+                             struct page_info **ppg);
+void destroy_perdomain_mapping(struct domain *d, unsigned long va,
                                unsigned int nr);
-void free_perdomain_mappings(struct domain *);
+void free_perdomain_mappings(struct domain *d);
 
-void __iomem *ioremap_wc(paddr_t, size_t);
+void __iomem *ioremap_wc(paddr_t pa, size_t len);
 
 extern int memory_add(unsigned long spfn, unsigned long epfn, unsigned int pxm);
 
-- 
2.34.1

Re: [XEN PATCH 2/6] x86/mm: address violations of MISRA C:2012 Rule 8.2

[Stefano Stabellini]

On Tue, 5 Dec 2023, Federico Serafini wrote:
> Add missing parameter names. No functional change.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> ---
>  xen/arch/x86/include/asm/mm.h | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/x86/include/asm/mm.h b/xen/arch/x86/include/asm/mm.h
> index 05dfe35502..a270f8ddd6 100644
> --- a/xen/arch/x86/include/asm/mm.h
> +++ b/xen/arch/x86/include/asm/mm.h
> @@ -406,7 +406,7 @@ void put_page_type(struct page_info *page);
>  int  get_page_type(struct page_info *page, unsigned long type);
>  int  put_page_type_preemptible(struct page_info *page);
>  int  get_page_type_preemptible(struct page_info *page, unsigned long type);
> -int  put_old_guest_table(struct vcpu *);
> +int  put_old_guest_table(struct vcpu *v);
>  int  get_page_from_l1e(
>      l1_pgentry_t l1e, struct domain *l1e_owner, struct domain *pg_owner);
>  void put_page_from_l1e(l1_pgentry_t l1e, struct domain *l1e_owner);
> @@ -557,7 +557,7 @@ void audit_domains(void);
>  
>  void make_cr3(struct vcpu *v, mfn_t mfn);
>  pagetable_t update_cr3(struct vcpu *v);
> -int vcpu_destroy_pagetables(struct vcpu *);
> +int vcpu_destroy_pagetables(struct vcpu *v);
>  void *do_page_walk(struct vcpu *v, unsigned long addr);
>  
>  /* Allocator functions for Xen pagetables. */
> @@ -572,20 +572,20 @@ int __sync_local_execstate(void);
>  /* Arch-specific portion of memory_op hypercall. */
>  long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
>  long subarch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
> -int compat_arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void));
> -int compat_subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void));
> +int compat_arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
> +int compat_subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);

Also for compat_subarch_memory_op I cannot find the definition

everything else looks fine

Re: [XEN PATCH 2/6] x86/mm: address violations of MISRA C:2012 Rule 8.2

[Jan Beulich]

On 06.12.2023 04:10, Stefano Stabellini wrote:
> On Tue, 5 Dec 2023, Federico Serafini wrote:
>> @@ -572,20 +572,20 @@ int __sync_local_execstate(void);
>>  /* Arch-specific portion of memory_op hypercall. */
>>  long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
>>  long subarch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
>> -int compat_arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void));
>> -int compat_subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void));
>> +int compat_arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
>> +int compat_subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);
> 
> Also for compat_subarch_memory_op I cannot find the definition

Looks like in b8a7efe8528a ("Enable compatibility mode operation for
HYPERVISOR_memory_op") I blindly added the declaration, without there
actually being a definition.

Jan

[XEN PATCH 3/6] AMD/IOMMU: address violations of MISRA C:2012 Rule 8.2

[Federico Serafini]

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.serafini@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))
 {
     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))
 {
-    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,
+                                        uint16_t bdf))
 {
-    u16 seg = 0;
+    uint16_t seg = 0;
     int rc = 0;
 
     do {
-- 
2.34.1

Re: [XEN PATCH 3/6] AMD/IOMMU: address violations of MISRA C:2012 Rule 8.2

[Stefano Stabellini]

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.serafini@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

Re: [XEN PATCH 3/6] AMD/IOMMU: address violations of MISRA C:2012 Rule 8.2

[Jan Beulich]

On 06.12.2023 04:15, Stefano Stabellini wrote:
> On Tue, 5 Dec 2023, Federico Serafini wrote:
>> --- 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));

(Note this for the comment near the end.)

>> @@ -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 ?

What purpose would "iommu_" serve? It would be actively confusing, for
colliding with the same-name global we have. Both functions passed here
use simply "enable".

>> @@ -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.

The name is entirely meaningless here (i.e. not helping with anything),
so imo "map" is not only fine but also (see above) consistent with other
code.

Jan

[XEN PATCH 4/6] x86/page: address violations of MISRA C:2012 Rule 8.2

[Federico Serafini]

Add missing parameter names. No functional change.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 xen/arch/x86/include/asm/page.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/include/asm/page.h b/xen/arch/x86/include/asm/page.h
index 93a7b368ac..350d1fb110 100644
--- a/xen/arch/x86/include/asm/page.h
+++ b/xen/arch/x86/include/asm/page.h
@@ -232,8 +232,8 @@ typedef struct { u64 pfn; } pagetable_t;
 #define pagetable_from_paddr(p) pagetable_from_pfn((p)>>PAGE_SHIFT)
 #define pagetable_null()        pagetable_from_pfn(0)
 
-void clear_page_sse2(void *);
-void copy_page_sse2(void *, const void *);
+void clear_page_sse2(void *pg);
+void copy_page_sse2(void *to, const void *from);
 
 #define clear_page(_p)      clear_page_sse2(_p)
 #define copy_page(_t, _f)   copy_page_sse2(_t, _f)
@@ -303,7 +303,7 @@ extern l3_pgentry_t l3_bootmap[L3_PAGETABLE_ENTRIES];
 extern l2_pgentry_t l2_directmap[4*L2_PAGETABLE_ENTRIES];
 extern l1_pgentry_t l1_fixmap[L1_PAGETABLE_ENTRIES];
 void paging_init(void);
-void efi_update_l4_pgtable(unsigned int l4idx, l4_pgentry_t);
+void efi_update_l4_pgtable(unsigned int l4idx, l4_pgentry_t l4e);
 #endif /* !defined(__ASSEMBLY__) */
 
 #define _PAGE_NONE     _AC(0x000,U)
-- 
2.34.1

Re: [XEN PATCH 4/6] x86/page: address violations of MISRA C:2012 Rule 8.2

[Stefano Stabellini]

On Tue, 5 Dec 2023, Federico Serafini wrote:
> Add missing parameter names. No functional change.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Re: [XEN PATCH 4/6] x86/page: address violations of MISRA C:2012 Rule 8.2

[Jan Beulich]

On 06.12.2023 04:16, Stefano Stabellini wrote:
> On Tue, 5 Dec 2023, Federico Serafini wrote:
>> Add missing parameter names. No functional change.
>>
>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Acked-by: Jan Beulich <jbeulich@suse.com>

[XEN PATCH 5/6] x86/mce: address violations of MISRA C:2012 Rule 8.2

[Federico Serafini]

Add missing parameter names. No functional change.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 xen/arch/x86/cpu/mcheck/mce.h  | 2 +-
 xen/arch/x86/include/asm/mce.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/mce.h b/xen/arch/x86/cpu/mcheck/mce.h
index b6fc9c3617..34285296b4 100644
--- a/xen/arch/x86/cpu/mcheck/mce.h
+++ b/xen/arch/x86/cpu/mcheck/mce.h
@@ -158,7 +158,7 @@ extern void mce_need_clearbank_register(mce_need_clearbank_t cbfunc);
  * MCi_STATUS value for that bank.
  */
 typedef struct mcinfo_extended *(*x86_mce_callback_t)
-    (struct mc_info *, uint16_t, uint64_t);
+    (struct mc_info *mi, uint16_t bank, uint64_t status);
 extern void x86_mce_callback_register(x86_mce_callback_t cbfunc);
 
 void *x86_mcinfo_reserve(struct mc_info *mi,
diff --git a/xen/arch/x86/include/asm/mce.h b/xen/arch/x86/include/asm/mce.h
index 2c63318c08..6ce56b5b85 100644
--- a/xen/arch/x86/include/asm/mce.h
+++ b/xen/arch/x86/include/asm/mce.h
@@ -37,8 +37,8 @@ struct domain;
 struct vcpu;
 
 /* Guest vMCE MSRs virtualization */
-extern void vmce_init_vcpu(struct vcpu *);
-extern int vmce_restore_vcpu(struct vcpu *, const struct hvm_vmce_vcpu *);
+extern void vmce_init_vcpu(struct vcpu *v);
+extern int vmce_restore_vcpu(struct vcpu *v, const struct hvm_vmce_vcpu *ctxt);
 extern int vmce_wrmsr(uint32_t msr, uint64_t val);
 extern int vmce_rdmsr(uint32_t msr, uint64_t *val);
 extern bool vmce_has_lmce(const struct vcpu *v);
-- 
2.34.1

Re: [XEN PATCH 5/6] x86/mce: address violations of MISRA C:2012 Rule 8.2

[Stefano Stabellini]

On Tue, 5 Dec 2023, Federico Serafini wrote:
> Add missing parameter names. No functional change.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Re: [XEN PATCH 5/6] x86/mce: address violations of MISRA C:2012 Rule 8.2

[Jan Beulich]

On 06.12.2023 04:18, Stefano Stabellini wrote:
> On Tue, 5 Dec 2023, Federico Serafini wrote:
>> Add missing parameter names. No functional change.
>>
>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Acked-by: Jan Beulich <jbeulich@suse.com>

[XEN PATCH 6/6] xen/pci: address violations of MISRA C:2012 Rule 8.2

[Federico Serafini]

Add missing parameter names to address violations of MISRA C:2012
Rule 8.2. Furthermore, use C standard types to comply with XEN coding style.
No functional change.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 xen/drivers/passthrough/pci.c | 8 ++++----
 xen/include/xen/pci.h         | 3 ++-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 04d00c7c37..28ed8ea817 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -104,9 +104,9 @@ static struct pci_seg *alloc_pseg(u16 seg)
 }
 
 static int pci_segments_iterate(
-    int (*handler)(struct pci_seg *, void *), void *arg)
+    int (*handler)(struct pci_seg *pseg, void *arg), void *arg)
 {
-    u16 seg = 0;
+    uint16_t seg = 0;
     int rc = 0;
 
     do {
@@ -1077,7 +1077,7 @@ int __init scan_pci_devices(void)
 
 struct setup_hwdom {
     struct domain *d;
-    int (*handler)(u8 devfn, struct pci_dev *);
+    int (*handler)(uint8_t devfn, struct pci_dev *pdev);
 };
 
 static void __hwdom_init setup_one_hwdom_device(const struct setup_hwdom *ctxt,
@@ -1157,7 +1157,7 @@ static int __hwdom_init cf_check _setup_hwdom_pci_devices(
 }
 
 void __hwdom_init setup_hwdom_pci_devices(
-    struct domain *d, int (*handler)(u8 devfn, struct pci_dev *))
+    struct domain *d, int (*handler)(uint8_t devfn, struct pci_dev *pdev))
 {
     struct setup_hwdom ctxt = { .d = d, .handler = handler };
 
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 50d7dfb2a2..a3b7434818 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -178,7 +178,8 @@ enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn);
 int find_upstream_bridge(u16 seg, u8 *bus, u8 *devfn, u8 *secbus);
 
 void setup_hwdom_pci_devices(struct domain *d,
-                            int (*handler)(u8 devfn, struct pci_dev *pdev));
+                             int (*handler)(uint8_t devfn,
+                                            struct pci_dev *pdev));
 int pci_release_devices(struct domain *d);
 void pci_segments_init(void);
 int pci_add_segment(u16 seg);
-- 
2.34.1

Re: [XEN PATCH 6/6] xen/pci: address violations of MISRA C:2012 Rule 8.2

[Stefano Stabellini]

On Tue, 5 Dec 2023, Federico Serafini wrote:
> Add missing parameter names to address violations of MISRA C:2012
> Rule 8.2. Furthermore, use C standard types to comply with XEN coding style.
> No functional change.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Re: [XEN PATCH 6/6] xen/pci: address violations of MISRA C:2012 Rule 8.2

[Jan Beulich]

On 06.12.2023 04:20, Stefano Stabellini wrote:
> On Tue, 5 Dec 2023, Federico Serafini wrote:
>> Add missing parameter names to address violations of MISRA C:2012
>> Rule 8.2. Furthermore, use C standard types to comply with XEN coding style.
>> No functional change.
>>
>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Acked-by: Jan Beulich <jbeulich@suse.com>