[XEN PATCH v2 0/5] xen: add parameter names and remove function declarations

[XEN PATCH v2 0/5] xen: add parameter names and remove function declarations

[Federico Serafini]

This patch series adds the missing parameter names and remove useless function
declarations. No functional changes are introduced.

Federico Serafini (5):
  x86/acpi: remove acpi_pic_sci_set_trigger()
  xen/acpi: address violations of MISRA C:2012 Rule 8.2
  x86/mm: remove compat_subarch_memory_op()
  x86/mm: address violations of MISRA C:2012 Rule 8.2
  AMD/IOMMU: address violations of MISRA C:2012 Rule 8.2

 xen/arch/x86/include/asm/acpi.h           |  1 -
 xen/arch/x86/include/asm/mm.h             | 19 +++++++++---------
 xen/drivers/passthrough/amd/iommu.h       | 17 +++++++++-------
 xen/drivers/passthrough/amd/iommu_init.c  | 24 +++++++++++++----------
 xen/include/acpi/apei.h                   |  5 +++--
 xen/include/acpi/cpufreq/cpufreq.h        |  2 +-
 xen/include/acpi/cpufreq/processor_perf.h | 16 +++++++--------
 7 files changed, 45 insertions(+), 39 deletions(-)

-- 
2.34.1

[XEN PATCH v2 1/5] x86/acpi: remove acpi_pic_sci_set_trigger()

[Federico Serafini]

Remove apci_pic_set_trigger() declaration: there is no definition and there are
no calls to such function in the XEN project.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 xen/arch/x86/include/asm/acpi.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/xen/arch/x86/include/asm/acpi.h b/xen/arch/x86/include/asm/acpi.h
index 0df92d3714..48b1d69946 100644
--- a/xen/arch/x86/include/asm/acpi.h
+++ b/xen/arch/x86/include/asm/acpi.h
@@ -81,7 +81,6 @@ 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);
 
 static inline void disable_acpi(void)
 {
-- 
2.34.1

Re: [XEN PATCH v2 1/5] x86/acpi: remove acpi_pic_sci_set_trigger()

[Jan Beulich]

On 07.12.2023 09:47, Federico Serafini wrote:
> Remove apci_pic_set_trigger() declaration: there is no definition and there are
> no calls to such function in the XEN project.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>

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

A reference to the offending commit would have been nice, the more that iirc
I had already gone and fished that out for you.

Jan

Re: [XEN PATCH v2 1/5] x86/acpi: remove acpi_pic_sci_set_trigger()

[Federico Serafini]

On 07/12/23 10:19, Jan Beulich wrote:
> On 07.12.2023 09:47, Federico Serafini wrote:
>> Remove apci_pic_set_trigger() declaration: there is no definition and there are
>> no calls to such function in the XEN project.
>>
>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> A reference to the offending commit would have been nice, the more that iirc
> I had already gone and fished that out for you.

Is it correct to use Fixes: <id> ("subj") even if there are other
useless entities left?
In particular, this is what I think should be removed:

Functions:
__acpi_acquire_global_lock()
__acpi_release_global_lock()
acpi_save_state{mem,disk}()
acpi_restore_state_mem()
acpi_enter_state()
{save,restore}_rest_processor_state()

Variables:
acpi_wakeup_address

Macros:
ACPI_ACQUIRE_GLOBAL_LOCK()

If you agree with me, I can propose a new patch which includes such 
removals and refers to the offending commit with a Fixes.

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)

Re: [XEN PATCH v2 1/5] x86/acpi: remove acpi_pic_sci_set_trigger()

[Jan Beulich]

On 07.12.2023 11:09, Federico Serafini wrote:
> On 07/12/23 10:19, Jan Beulich wrote:
>> On 07.12.2023 09:47, Federico Serafini wrote:
>>> Remove apci_pic_set_trigger() declaration: there is no definition and there are
>>> no calls to such function in the XEN project.
>>>
>>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
>>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>
>> A reference to the offending commit would have been nice, the more that iirc
>> I had already gone and fished that out for you.
> 
> Is it correct to use Fixes: <id> ("subj") even if there are other
> useless entities left?

It was specifically because of being uncertain in this case that I didn't
mention a possible Fixes: tag. There's no breakage from stray declarations,
so it doesn't really feel to me like a "fix".

> In particular, this is what I think should be removed:
> 
> Functions:
> __acpi_acquire_global_lock()
> __acpi_release_global_lock()
> acpi_save_state{mem,disk}()
> acpi_restore_state_mem()
> acpi_enter_state()
> {save,restore}_rest_processor_state()
> 
> Variables:
> acpi_wakeup_address
> 
> Macros:
> ACPI_ACQUIRE_GLOBAL_LOCK()
> 
> If you agree with me, I can propose a new patch which includes such 
> removals and refers to the offending commit with a Fixes.

I haven't checked the entities above, but yes, I agree other stray
declarations would want removing as well. In fact I was assuming that
removal of just one item meant no other stray ones exist (right here).

Jan

[XEN PATCH v2 2/5] xen/acpi: 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>
---
Changes in v2:
  - do not add parameters to acpi_pic_sci_set_trigger() (removed).
---
 xen/include/acpi/apei.h                   |  5 +++--
 xen/include/acpi/cpufreq/cpufreq.h        |  2 +-
 xen/include/acpi/cpufreq/processor_perf.h | 16 ++++++++--------
 3 files changed, 12 insertions(+), 11 deletions(-)

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 v2 2/5] xen/acpi: address violations of MISRA C:2012 Rule 8.2

[Stefano Stabellini]

On Thu, 7 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 v2 2/5] xen/acpi: address violations of MISRA C:2012 Rule 8.2

[Jan Beulich]

On 08.12.2023 01:43, Stefano Stabellini wrote:
> On Thu, 7 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 v2 3/5] x86/mm: remove compat_subarch_memory_op()

[Federico Serafini]

Remove remove compat_subarch_memory_op() declaration: there is no
definition and there are no calls to such function in the XEN project.

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

diff --git a/xen/arch/x86/include/asm/mm.h b/xen/arch/x86/include/asm/mm.h
index 05dfe35502..639163948e 100644
--- a/xen/arch/x86/include/asm/mm.h
+++ b/xen/arch/x86/include/asm/mm.h
@@ -573,7 +573,6 @@ int __sync_local_execstate(void);
 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));
 
 #define NIL(type) ((type *)-sizeof(type))
 #define IS_NIL(ptr) (!((uintptr_t)(ptr) + sizeof(*(ptr))))
-- 
2.34.1

Re: [XEN PATCH v2 3/5] x86/mm: remove compat_subarch_memory_op()

[Stefano Stabellini]

On Thu, 7 Dec 2023, Federico Serafini wrote:
> Remove remove compat_subarch_memory_op() declaration: there is no
> definition and there are no calls to such function in the XEN project.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>

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

Re: [XEN PATCH v2 3/5] x86/mm: remove compat_subarch_memory_op()

[Jan Beulich]

On 08.12.2023 01:34, Stefano Stabellini wrote:
> On Thu, 7 Dec 2023, Federico Serafini wrote:
>> Remove remove compat_subarch_memory_op() declaration: there is no
>> definition and there are no calls to such function in the XEN project.
>>
>> 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 v2 4/5] 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>
---
Changes in v2:
  - do not add parameters to compat_subarch_memory_op() (removed).
---
 xen/arch/x86/include/asm/mm.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/include/asm/mm.h b/xen/arch/x86/include/asm/mm.h
index 639163948e..7d26d9cd2f 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,19 +572,19 @@ 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_arch_memory_op(unsigned long cmd, 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 v2 4/5] x86/mm: address violations of MISRA C:2012 Rule 8.2

[Stefano Stabellini]

On Thu, 7 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 v2 4/5] x86/mm: address violations of MISRA C:2012 Rule 8.2

[Jan Beulich]

On 08.12.2023 01:40, Stefano Stabellini wrote:
> On Thu, 7 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 v2 5/5] 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. 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>
---
Changes in v2:
  - use "enable" instead of "iommu_control" as parameter name.
---
 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..25e62f20ae 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 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))
 {
-    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 v2 5/5] AMD/IOMMU: address violations of MISRA C:2012 Rule 8.2

[Stefano Stabellini]

On Thu, 7 Dec 2023, Federico Serafini wrote:
> Add missing parameter names to address violations of MISRA C:2012
> Rule 8.2. 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>

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

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

[Jan Beulich]

On 08.12.2023 01:39, Stefano Stabellini wrote:
> On Thu, 7 Dec 2023, Federico Serafini wrote:
>> Add missing parameter names to address violations of MISRA C:2012
>> Rule 8.2. 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>
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

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