[PATCH 0/5] types: (mostly) purge Linux-inherited s{8,16,32,64}

[PATCH 0/5] types: (mostly) purge Linux-inherited s{8,16,32,64}

[Jan Beulich]

Replace uses except where linux-compat.h, where they're moved to, is
sensible to #include.

1: x86: drop s<N>/u<N> overrides from mkelf32
2: types: replace remaining uses of s8
3: types: replace remaining uses of s16
4: types: replace remaining uses of s32
5: types: replace remaining uses of s64

Jan

[PATCH 1/5] x86: drop s<N>/u<N> overrides from mkelf32

[Jan Beulich]

Use uint<N>_t instead (s<N> were unused altogether). While adjusting
swap<N>() drop excessive casts and rename the arguments to avoid leading
underscores.

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

--- a/xen/arch/x86/boot/mkelf32.c
+++ b/xen/arch/x86/boot/mkelf32.c
@@ -17,14 +17,6 @@
 #include <unistd.h>
 #include <inttypes.h>
 
-#define u8  uint8_t
-#define u16 uint16_t
-#define u32 uint32_t
-#define u64 uint64_t
-#define s8  int8_t
-#define s16 int16_t
-#define s32 int32_t
-#define s64 int64_t
 #include "../../../include/xen/elfstructs.h"
 
 #define DYNAMICALLY_FILLED   0
@@ -72,9 +64,9 @@ static Elf32_Phdr note_phdr = {
     4                                        /* p_align */
 };
 
-static u8 out_shstrtab[] = "\0.text\0.shstrtab";
+static uint8_t out_shstrtab[] = "\0.text\0.shstrtab";
 /* If num_phdrs >= 2, we need to tack the .note. */
-static u8 out_shstrtab_extra[] = ".note\0";
+static uint8_t out_shstrtab_extra[] = ".note\0";
 
 static Elf32_Shdr out_shdr[] = {
     { 0 },
@@ -124,9 +116,9 @@ static Elf32_Shdr out_shdr_note = {
 #undef swap32
 #undef swap64
 
-#define swap16(_v) ((((u16)(_v)>>8)&0xff)|(((u16)(_v)&0xff)<<8))
-#define swap32(_v) (((u32)swap16((u16)(_v))<<16)|(u32)swap16((u32)((_v)>>16)))
-#define swap64(_v) (((u64)swap32((u32)(_v))<<32)|(u64)swap32((u32)((_v)>>32)))
+#define swap16(v) (( (uint8_t)(v)       <<  8) | (uint8_t)((v) >>  8))
+#define swap32(v) (((uint32_t)swap16(v) << 16) |    swap16((v) >> 16))
+#define swap64(v) (((uint64_t)swap32(v) << 32) |    swap32((v) >> 32))
 
 static int big_endian;
 
@@ -256,8 +248,8 @@ static void do_read(int fd, void *data,
 
 int main(int argc, char **argv)
 {
-    u64        final_exec_addr;
-    u32        loadbase, dat_siz, mem_siz, note_base, note_sz, offset;
+    uint64_t   final_exec_addr;
+    uint32_t   loadbase, dat_siz, mem_siz, note_base, note_sz, offset;
     char      *inimage, *outimage;
     int        infd, outfd;
     char       buffer[1024] = {};
@@ -302,7 +294,7 @@ int main(int argc, char **argv)
         return 1;
     }
 
-    big_endian = (*(u16 *)in32_ehdr.e_ident == ((ELFMAG0 << 8) | ELFMAG1));
+    big_endian = (*(uint16_t *)in32_ehdr.e_ident == ((ELFMAG0 << 8) | ELFMAG1));
 
     endianadjust_ehdr32(&in32_ehdr);
     if ( in32_ehdr.e_ident[EI_CLASS] != ELFCLASS64 )
@@ -345,11 +337,11 @@ int main(int argc, char **argv)
     endianadjust_phdr64(&in64_phdr);
 
     (void)lseek(infd, in64_phdr.p_offset, SEEK_SET);
-    dat_siz = (u32)in64_phdr.p_filesz;
+    dat_siz = (uint32_t)in64_phdr.p_filesz;
 
     /* Do not use p_memsz: it does not include BSS alignment padding. */
-    /*mem_siz = (u32)in64_phdr.p_memsz;*/
-    mem_siz = (u32)(final_exec_addr - in64_phdr.p_vaddr);
+    /*mem_siz = (uint32_t)in64_phdr.p_memsz;*/
+    mem_siz = (uint32_t)(final_exec_addr - in64_phdr.p_vaddr);
 
     note_sz = note_base = offset = 0;
     if ( num_phdrs > 1 )

Re: [PATCH 1/5] x86: drop s<N>/u<N> overrides from mkelf32

[Andrew Cooper]

On 29/08/2024 12:58 pm, Jan Beulich wrote:
> Use uint<N>_t instead (s<N> were unused altogether). While adjusting
> swap<N>() drop excessive casts and rename the arguments to avoid leading
> underscores.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

[PATCH 2/5] types: replace remaining uses of s8

[Jan Beulich]

... and move the type itself to linux-compat.h.

While doing so,
- convert __read_mostly to __ro_after_init for respective variables
  having their type changed (for acpi_numa add the attribute anew),
- in cpuid_hypervisor_leaves() drop a cast altogether,
- switch an adjacent struct arch_irq_desc field to bool.

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

--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -71,7 +71,7 @@ static struct {
 /*
  * Knob to control our willingness to enable the local APIC.
  */
-static s8 __initdata enable_local_apic; /* -1=force-disable, +1=force-enable */
+static int8_t __initdata enable_local_apic; /* -1=force-disable, +1=force-enable */
 
 /*
  * Debug level
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -46,7 +46,7 @@ static unsigned int __initdata opt_cpuid
 integer_param("cpuid_mask_thermal_ecx", opt_cpuid_mask_thermal_ecx);
 
 /* 1 = allow, 0 = don't allow guest creation, -1 = don't allow boot */
-s8 __read_mostly opt_allow_unsafe;
+int8_t __ro_after_init opt_allow_unsafe;
 boolean_param("allow_unsafe", opt_allow_unsafe);
 
 /* Signal whether the ACPI C1E quirk is required. */
--- a/xen/arch/x86/e820.c
+++ b/xen/arch/x86/e820.c
@@ -27,7 +27,7 @@ static unsigned long long __initdata opt
 size_param("availmem", opt_availmem);
 
 /* opt_nomtrr_check: Don't clip ram to highest cacheable MTRR. */
-static s8 __initdata e820_mtrr_clip = -1;
+static int8_t __initdata e820_mtrr_clip = -1;
 boolean_param("e820-mtrr-clip", e820_mtrr_clip);
 
 /* opt_e820_verbose: Be verbose about clipping, the original e820, &c */
--- a/xen/arch/x86/hvm/quirks.c
+++ b/xen/arch/x86/hvm/quirks.c
@@ -11,7 +11,7 @@
 #include <xen/param.h>
 #include <asm/hvm/support.h>
 
-s8 __read_mostly hvm_port80_allowed = -1;
+int8_t __ro_after_init hvm_port80_allowed = -1;
 boolean_param("hvm_port80", hvm_port80_allowed);
 
 static int __init cf_check dmi_hvm_deny_port80(const struct dmi_system_id *id)
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -59,7 +59,7 @@ static unsigned int __ro_after_init vm_n
 integer_param("vm-notify-window", vm_notify_window);
 
 static bool __read_mostly opt_ept_pml = true;
-static s8 __read_mostly opt_ept_ad = -1;
+static int8_t __ro_after_init opt_ept_ad = -1;
 int8_t __read_mostly opt_ept_exec_sp = -1;
 
 static int __init cf_check parse_ept_param(const char *s)
--- a/xen/arch/x86/include/asm/acpi.h
+++ b/xen/arch/x86/include/asm/acpi.h
@@ -100,7 +100,7 @@ extern unsigned long acpi_wakeup_address
 
 #define ARCH_HAS_POWER_INIT	1
 
-extern s8 acpi_numa;
+extern int8_t acpi_numa;
 
 extern struct acpi_sleep_info acpi_sinfo;
 #define acpi_video_flags bootsym(video_flags)
--- a/xen/arch/x86/include/asm/amd.h
+++ b/xen/arch/x86/include/asm/amd.h
@@ -160,7 +160,7 @@
 struct cpuinfo_x86;
 int cpu_has_amd_erratum(const struct cpuinfo_x86 *cpu, int osvw_id, ...);
 
-extern s8 opt_allow_unsafe;
+extern int8_t opt_allow_unsafe;
 
 void fam10h_check_enable_mmcfg(void);
 void check_enable_amd_mmconf_dmi(void);
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -241,7 +241,7 @@ struct hvm_function_table {
 
 extern struct hvm_function_table hvm_funcs;
 extern bool hvm_enabled;
-extern s8 hvm_port80_allowed;
+extern int8_t hvm_port80_allowed;
 
 extern const struct hvm_function_table *start_svm(void);
 extern const struct hvm_function_table *start_vmx(void);
--- a/xen/arch/x86/include/asm/irq.h
+++ b/xen/arch/x86/include/asm/irq.h
@@ -80,8 +80,8 @@ struct arch_irq_desc {
         cpumask_var_t pending_mask;
         vmask_t *used_vectors;
         unsigned move_cleanup_count;
-        u8 move_in_progress : 1;
-        s8 used;
+        bool move_in_progress : 1;
+        int8_t used;
         /*
          * Weak reference to domain having permission over this IRQ (which can
          * be different from the domain actually having the IRQ assigned)
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -170,7 +170,7 @@ static uint32_t __ro_after_init base_dis
       is_pv_domain(d)) ?                                        \
      L1_DISALLOW_MASK : (L1_DISALLOW_MASK & ~PAGE_CACHE_ATTRS))
 
-static s8 __read_mostly opt_mmio_relax;
+static int8_t __ro_after_init opt_mmio_relax;
 
 static int __init cf_check parse_mmio_relax(const char *s)
 {
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -35,7 +35,7 @@
 #include <xsm/xsm.h>
 #include <xen/vpci.h>
 
-static s8 __read_mostly use_msi = -1;
+static int8_t __ro_after_init use_msi = -1;
 boolean_param("msi", use_msi);
 
 static void __pci_disable_msix(struct msi_desc *entry);
--- a/xen/arch/x86/numa.c
+++ b/xen/arch/x86/numa.c
@@ -25,7 +25,7 @@ nodeid_t apicid_to_node[MAX_LOCAL_APIC]
     [0 ... MAX_LOCAL_APIC-1] = NUMA_NO_NODE
 };
 
-s8 acpi_numa = 0;
+int8_t __ro_after_init acpi_numa = 0;
 
 int __init arch_numa_setup(const char *opt)
 {
--- a/xen/arch/x86/oprofile/op_model_athlon.c
+++ b/xen/arch/x86/oprofile/op_model_athlon.c
@@ -157,7 +157,7 @@ static inline u64 op_amd_randomize_ibs_o
          * IbsOpMaxCnt must fit in the range from 0x0081 to
          * 0xff80.
          */
-        val += (s8)(random >> 4);
+        val += (int8_t)(random >> 4);
     else
         val |= (u64)(random & IBS_RANDOM_MASK) << 32;
 
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -166,7 +166,7 @@ unsigned long __read_mostly mmu_cr4_feat
 
 /* smep: Enable/disable Supervisor Mode Execution Protection */
 #define SMEP_HVM_ONLY (-2)
-static s8 __initdata opt_smep = -1;
+static int8_t __initdata opt_smep = -1;
 
 /*
  * Initial domain place holder. Needs to be global so it can be created in
@@ -203,7 +203,7 @@ custom_param("smep", parse_smep_param);
 
 /* smap: Enable/disable Supervisor Mode Access Prevention */
 #define SMAP_HVM_ONLY (-2)
-static s8 __initdata opt_smap = -1;
+static int8_t __initdata opt_smap = -1;
 
 static int __init cf_check parse_smap_param(const char *s)
 {
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1116,7 +1116,7 @@ void cpuid_hypervisor_leaves(const struc
             res->a = offset;
             res->b = offset >> 32;
             res->c = d->arch.vtsc_to_ns.mul_frac;
-            res->d = (s8)d->arch.vtsc_to_ns.shift;
+            res->d = d->arch.vtsc_to_ns.shift;
             break;
         }
 
--- a/xen/include/acpi/cpufreq/cpufreq.h
+++ b/xen/include/acpi/cpufreq/cpufreq.h
@@ -79,7 +79,7 @@ struct cpufreq_policy {
 
     bool                resume; /* flag for cpufreq 1st run
                                  * S3 wakeup, hotplug cpu, etc */
-    s8                  turbo;  /* tristate flag: 0 for unsupported
+    int8_t              turbo;  /* tristate flag: 0 for unsupported
                                  * -1 for disable, 1 for enabled
                                  * See CPUFREQ_TURBO_* below for defines */
 };
--- a/xen/include/xen/linux-compat.h
+++ b/xen/include/xen/linux-compat.h
@@ -11,7 +11,7 @@
 
 #include <xen/types.h>
 
-typedef int8_t  __s8;
+typedef int8_t  s8, __s8;
 typedef uint8_t __u8;
 typedef int16_t __s16;
 typedef int32_t __s32;
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -5,7 +5,6 @@
 #include <xen/stdint.h>
 
 /* Linux inherited types which are being phased out */
-typedef int8_t s8;
 typedef uint8_t u8;
 typedef int16_t s16;
 typedef uint16_t u16, __u16;

Re: [PATCH 2/5] types: replace remaining uses of s8

[Andrew Cooper]

On 29/08/2024 12:59 pm, Jan Beulich wrote:
> ... and move the type itself to linux-compat.h.
>
> While doing so,
> - convert __read_mostly to __ro_after_init for respective variables
>   having their type changed (for acpi_numa add the attribute anew),
> - in cpuid_hypervisor_leaves() drop a cast altogether,
> - switch an adjacent struct arch_irq_desc field to bool.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

The __read_mostly -> __ro_after_init changes are safe as far as I can tell.

use_msi is in desperate need of renaming to opt_msi.

~Andrew

[PATCH 3/5] types: replace remaining uses of s16

[Jan Beulich]

... and move the type itself to linux-compat.h.

While doing so switch an adjacent x86 struct page_info field to bool.

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

--- a/xen/arch/arm/arm32/livepatch.c
+++ b/xen/arch/arm/arm32/livepatch.c
@@ -135,7 +135,7 @@ static s32 get_addend(unsigned char type
         addend =  (*(u32 *)dest & 0x00000FFF);
         addend |= (*(u32 *)dest & 0x000F0000) >> 4;
         /* Addend is to sign-extend ([19:16],[11:0]). */
-        addend = (s16)addend;
+        addend = (int16_t)addend;
         break;
 
     case R_ARM_CALL:
--- a/xen/arch/arm/arm64/livepatch.c
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -124,7 +124,7 @@ static int reloc_data(enum aarch64_reloc
     switch ( len )
     {
     case 16:
-        *(s16 *)place = sval;
+        *(int16_t *)place = sval;
         if ( sval < INT16_MIN || sval > UINT16_MAX )
 	        return -EOVERFLOW;
         break;
--- a/xen/arch/x86/include/asm/irq.h
+++ b/xen/arch/x86/include/asm/irq.h
@@ -67,8 +67,8 @@ struct irq_desc;
  * the old destinations.
  */
 struct arch_irq_desc {
-        s16 vector;                  /* vector itself is only 8 bits, */
-        s16 old_vector;              /* but we use -1 for unassigned  */
+        int16_t vector;                  /* vector itself is only 8 bits, */
+        int16_t old_vector;              /* but we use -1 for unassigned  */
         /*
          * Except for high priority interrupts @cpu_mask may have bits set for
          * offline CPUs.  Consumers need to be careful to mask this down to
--- a/xen/arch/x86/include/asm/mm.h
+++ b/xen/arch/x86/include/asm/mm.h
@@ -286,8 +286,8 @@ struct page_info
         struct {
             u16 nr_validated_ptes:PAGETABLE_ORDER + 1;
             u16 :16 - PAGETABLE_ORDER - 1 - 1;
-            u16 partial_flags:1;
-            s16 linear_pt_count;
+            bool partial_flags:1;
+            int16_t linear_pt_count;
         };
 
         /*
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1390,7 +1390,7 @@ unmap_common(
     struct grant_table *lgt, *rgt;
     grant_ref_t ref;
     struct active_grant_entry *act;
-    s16              rc = 0;
+    int16_t          rc;
     struct grant_mapping *map;
     unsigned int flags;
     bool put_handle = false;
@@ -2580,7 +2580,7 @@ acquire_grant_for_copy(
     uint16_t trans_page_off;
     uint16_t trans_length;
     bool is_sub_page;
-    s16 rc = GNTST_okay;
+    int16_t rc = GNTST_okay;
     unsigned int pin_incr = readonly ? GNTPIN_hstr_inc : GNTPIN_hstw_inc;
 
     *page = NULL;
@@ -3416,14 +3416,14 @@ gnttab_get_version(XEN_GUEST_HANDLE_PARA
     return 0;
 }
 
-static s16
+static int16_t
 swap_grant_ref(grant_ref_t ref_a, grant_ref_t ref_b)
 {
     struct domain *d = rcu_lock_current_domain();
     struct grant_table *gt = d->grant_table;
     struct active_grant_entry *act_a = NULL;
     struct active_grant_entry *act_b = NULL;
-    s16 rc = GNTST_okay;
+    int16_t rc = GNTST_okay;
 
     grant_write_lock(gt);
 
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -43,6 +43,7 @@
 #include <xen/err.h>
 #include <xen/irq.h>
 #include <xen/lib.h>
+#include <xen/linux-compat.h>
 #include <xen/list.h>
 #include <xen/mm.h>
 #include <xen/vmap.h>
--- a/xen/include/xen/linux-compat.h
+++ b/xen/include/xen/linux-compat.h
@@ -13,7 +13,7 @@
 
 typedef int8_t  s8, __s8;
 typedef uint8_t __u8;
-typedef int16_t __s16;
+typedef int16_t s16, __s16;
 typedef int32_t __s32;
 typedef int64_t __s64;
 
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -6,7 +6,6 @@
 
 /* Linux inherited types which are being phased out */
 typedef uint8_t u8;
-typedef int16_t s16;
 typedef uint16_t u16, __u16;
 typedef int32_t s32;
 typedef uint32_t u32, __u32;

Re: [PATCH 3/5] types: replace remaining uses of s16

[Andrew Cooper]

On 29/08/2024 1:00 pm, Jan Beulich wrote:
> ... and move the type itself to linux-compat.h.
>
> While doing so switch an adjacent x86 struct page_info field to bool.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Re: [PATCH 3/5] types: replace remaining uses of s16

[Roger Pau Monné]

On Thu, Aug 29, 2024 at 02:00:20PM +0200, Jan Beulich wrote:
> ... and move the type itself to linux-compat.h.
> 
> While doing so switch an adjacent x86 struct page_info field to bool.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

[PATCH 4/5] types: replace remaining uses of s32

[Jan Beulich]

... and move the type itself to linux-compat.h.

While doing so switch a few adjacent types as well, for (a little bit
of) consistency.

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

--- a/xen/arch/arm/alternative.c
+++ b/xen/arch/arm/alternative.c
@@ -63,7 +63,7 @@ static u32 get_alt_insn(const struct alt
 
     if ( insn_is_branch_imm(insn) )
     {
-        s32 offset = insn_get_branch_offset(insn);
+        int32_t offset = insn_get_branch_offset(insn);
         unsigned long target;
 
         target = (unsigned long)altinsnptr + offset;
--- a/xen/arch/arm/arm32/livepatch.c
+++ b/xen/arch/arm/arm32/livepatch.c
@@ -32,7 +32,7 @@ void arch_livepatch_apply(const struct l
 
     if ( func->new_addr )
     {
-        s32 delta;
+        int32_t delta;
 
         /*
          * PC is current address (old_addr) + 8 bytes. The semantics for a
@@ -41,11 +41,11 @@ void arch_livepatch_apply(const struct l
          * ARM DDI 0406C.c, see A2.3 (pg 45) and A8.8.18 pg (pg 334,335)
          *
          */
-        delta = (s32)func->new_addr - (s32)(func->old_addr + 8);
+        delta = (int32_t)func->new_addr - (int32_t)(func->old_addr + 8);
 
         /* The arch_livepatch_symbol_ok should have caught it. */
-        ASSERT(delta >= -(s32)ARCH_LIVEPATCH_RANGE ||
-               delta < (s32)ARCH_LIVEPATCH_RANGE);
+        ASSERT(delta >= -(int32_t)ARCH_LIVEPATCH_RANGE ||
+               delta < (int32_t)ARCH_LIVEPATCH_RANGE);
 
         /* CPU shifts by two (left) when decoding, so we shift right by two. */
         delta = delta >> 2;
@@ -113,9 +113,9 @@ bool arch_livepatch_symbol_deny(const st
     return false;
 }
 
-static s32 get_addend(unsigned char type, void *dest)
+static int32_t get_addend(unsigned char type, void *dest)
 {
-    s32 addend = 0;
+    int32_t addend = 0;
 
     switch ( type ) {
     case R_ARM_NONE:
@@ -149,7 +149,8 @@ static s32 get_addend(unsigned char type
     return addend;
 }
 
-static int perform_rel(unsigned char type, void *dest, uint32_t val, s32 addend)
+static int perform_rel(unsigned char type, void *dest, uint32_t val,
+                       int32_t addend)
 {
 
     switch ( type ) {
@@ -203,8 +204,8 @@ static int perform_rel(unsigned char typ
          * arch_livepatch_verify_distance can't account of addend so we have
          * to do the check here as well.
          */
-        if ( (s32)val < -(s32)ARCH_LIVEPATCH_RANGE ||
-             (s32)val >= (s32)ARCH_LIVEPATCH_RANGE )
+        if ( (int32_t)val < -(int32_t)ARCH_LIVEPATCH_RANGE ||
+             (int32_t)val >= (int32_t)ARCH_LIVEPATCH_RANGE )
             return -EOVERFLOW;
 
         /* CPU always shifts insn by two, so complement it. */
@@ -234,7 +235,7 @@ int arch_livepatch_perform(struct livepa
         uint32_t val;
         void *dest;
         unsigned char type;
-        s32 addend;
+        int32_t addend;
 
         if ( use_rela )
         {
--- a/xen/arch/arm/arm64/insn.c
+++ b/xen/arch/arm/arm64/insn.c
@@ -223,9 +223,9 @@ u32 __kprobes aarch64_insn_gen_nop(void)
  * signed value (so it can be used when computing a new branch
  * target).
  */
-s32 aarch64_get_branch_offset(u32 insn)
+int32_t aarch64_get_branch_offset(uint32_t insn)
 {
-	s32 imm;
+	int32_t imm;
 
 	if (aarch64_insn_is_b(insn) || aarch64_insn_is_bl(insn)) {
 		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_26, insn);
@@ -251,7 +251,7 @@ s32 aarch64_get_branch_offset(u32 insn)
  * Encode the displacement of a branch in the imm field and return the
  * updated instruction.
  */
-u32 aarch64_set_branch_offset(u32 insn, s32 offset)
+uint32_t aarch64_set_branch_offset(uint32_t insn, int32_t offset)
 {
 	if (aarch64_insn_is_b(insn) || aarch64_insn_is_bl(insn))
 		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_26, insn,
--- a/xen/arch/arm/arm64/livepatch.c
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -130,7 +130,7 @@ static int reloc_data(enum aarch64_reloc
         break;
 
     case 32:
-        *(s32 *)place = sval;
+        *(int32_t *)place = sval;
         if ( sval < INT32_MIN || sval > UINT32_MAX )
 	        return -EOVERFLOW;
         break;
--- a/xen/arch/arm/include/asm/alternative.h
+++ b/xen/arch/arm/include/asm/alternative.h
@@ -12,11 +12,11 @@
 #include <xen/stringify.h>
 
 struct alt_instr {
-	s32 orig_offset;	/* offset to original instruction */
-	s32 repl_offset;	/* offset to replacement instruction */
-	u16 cpufeature;		/* cpufeature bit set for replacement */
-	u8  orig_len;		/* size of original instruction(s) */
-	u8  repl_len;		/* size of new instruction(s), <= orig_len */
+	int32_t  orig_offset;	/* offset to original instruction */
+	int32_t  repl_offset;	/* offset to replacement instruction */
+	uint16_t cpufeature;	/* cpufeature bit set for replacement */
+	uint8_t  orig_len;	/* size of original instruction(s) */
+	uint8_t  repl_len;	/* size of new instruction(s), <= orig_len */
 };
 
 /* Xen: helpers used by common code. */
--- a/xen/arch/arm/include/asm/arm64/insn.h
+++ b/xen/arch/arm/include/asm/arm64/insn.h
@@ -75,8 +75,8 @@ u64 aarch64_insn_decode_immediate(enum a
 u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
 				  u32 insn, u64 imm);
 
-s32 aarch64_get_branch_offset(u32 insn);
-u32 aarch64_set_branch_offset(u32 insn, s32 offset);
+int32_t aarch64_get_branch_offset(uint32_t insn);
+uint32_t aarch64_set_branch_offset(uint32_t insn, int32_t offset);
 
 u32 aarch64_insn_gen_branch_imm(unsigned long pc, unsigned long addr,
 				enum aarch64_insn_branch_type type);
@@ -89,12 +89,12 @@ static inline bool insn_is_branch_imm(u3
     return aarch64_insn_is_branch_imm(insn);
 }
 
-static inline s32 insn_get_branch_offset(u32 insn)
+static inline int32_t insn_get_branch_offset(uint32_t insn)
 {
     return aarch64_get_branch_offset(insn);
 }
 
-static inline u32 insn_set_branch_offset(u32 insn, s32 offset)
+static inline uint32_t insn_set_branch_offset(uint32_t insn, int32_t offset)
 {
     return aarch64_set_branch_offset(insn, offset);
 }
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -101,12 +101,12 @@ static void __init efi_arch_relocate_ima
     }
 }
 
-extern const s32 __trampoline_rel_start[], __trampoline_rel_stop[];
-extern const s32 __trampoline_seg_start[], __trampoline_seg_stop[];
+extern const int32_t __trampoline_rel_start[], __trampoline_rel_stop[];
+extern const int32_t __trampoline_seg_start[], __trampoline_seg_stop[];
 
 static void __init relocate_trampoline(unsigned long phys)
 {
-    const s32 *trampoline_ptr;
+    const int32_t *trampoline_ptr;
 
     trampoline_phys = phys;
 
--- a/xen/arch/x86/include/asm/uaccess.h
+++ b/xen/arch/x86/include/asm/uaccess.h
@@ -406,7 +406,7 @@ copy_from_unsafe(void *to, const void __
 
 struct exception_table_entry
 {
-	s32 addr, cont;
+	int32_t addr, cont;
 };
 extern struct exception_table_entry __start___ex_table[];
 extern struct exception_table_entry __stop___ex_table[];
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1859,8 +1859,8 @@ static void cf_check local_time_calibrat
      * This allows us to binary shift a 32-bit tsc_elapsed such that:
      * stime_elapsed < tsc_elapsed <= 2*stime_elapsed
      */
-    while ( ((u32)stime_elapsed64 != stime_elapsed64) ||
-            ((s32)stime_elapsed64 < 0) )
+    while ( ((uint32_t)stime_elapsed64 != stime_elapsed64) ||
+            ((int32_t)stime_elapsed64 < 0) )
     {
         stime_elapsed64 >>= 1;
         tsc_elapsed64   >>= 1;
--- a/xen/common/trace.c
+++ b/xen/common/trace.c
@@ -459,8 +459,8 @@ static inline bool bogus(u32 prod, u32 c
 
 static inline u32 calc_unconsumed_bytes(const struct t_buf *buf)
 {
-    u32 prod = buf->prod, cons = buf->cons;
-    s32 x;
+    uint32_t prod = buf->prod, cons = buf->cons;
+    int32_t x;
 
     barrier(); /* must read buf->prod and buf->cons only once */
     if ( bogus(prod, cons) )
@@ -478,8 +478,8 @@ static inline u32 calc_unconsumed_bytes(
 
 static inline u32 calc_bytes_to_wrap(const struct t_buf *buf)
 {
-    u32 prod = buf->prod, cons = buf->cons;
-    s32 x;
+    uint32_t prod = buf->prod, cons = buf->cons;
+    int32_t x;
 
     barrier(); /* must read buf->prod and buf->cons only once */
     if ( bogus(prod, cons) )
--- a/xen/include/acpi/actypes.h
+++ b/xen/include/acpi/actypes.h
@@ -186,8 +186,8 @@ typedef int INT32;
 
 /*! [End] no source code translation !*/
 
-typedef u32 acpi_native_uint;
-typedef s32 acpi_native_int;
+typedef uint32_t acpi_native_uint;
+typedef int32_t acpi_native_int;
 
 typedef u32 acpi_io_address;
 typedef u32 acpi_physical_address;
--- a/xen/include/xen/linux-compat.h
+++ b/xen/include/xen/linux-compat.h
@@ -14,7 +14,7 @@
 typedef int8_t  s8, __s8;
 typedef uint8_t __u8;
 typedef int16_t s16, __s16;
-typedef int32_t __s32;
+typedef int32_t s32, __s32;
 typedef int64_t __s64;
 
 typedef paddr_t phys_addr_t;
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -7,7 +7,6 @@
 /* Linux inherited types which are being phased out */
 typedef uint8_t u8;
 typedef uint16_t u16, __u16;
-typedef int32_t s32;
 typedef uint32_t u32, __u32;
 typedef int64_t s64;
 typedef uint64_t u64, __u64;

Re: [PATCH 4/5] types: replace remaining uses of s32

[Andrew Cooper]

On 29/08/2024 1:01 pm, Jan Beulich wrote:
> ... and move the type itself to linux-compat.h.
>
> While doing so switch a few adjacent types as well, for (a little bit
> of) consistency.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, with a minor
formatting request.

> --- a/xen/arch/arm/arm32/livepatch.c
> +++ b/xen/arch/arm/arm32/livepatch.c
> @@ -41,11 +41,11 @@ void arch_livepatch_apply(const struct l
>           * ARM DDI 0406C.c, see A2.3 (pg 45) and A8.8.18 pg (pg 334,335)
>           *
>           */
> -        delta = (s32)func->new_addr - (s32)(func->old_addr + 8);
> +        delta = (int32_t)func->new_addr - (int32_t)(func->old_addr + 8);
>  
>          /* The arch_livepatch_symbol_ok should have caught it. */
> -        ASSERT(delta >= -(s32)ARCH_LIVEPATCH_RANGE ||
> -               delta < (s32)ARCH_LIVEPATCH_RANGE);
> +        ASSERT(delta >= -(int32_t)ARCH_LIVEPATCH_RANGE ||
> +               delta < (int32_t)ARCH_LIVEPATCH_RANGE);

Could you vertically like this, like it is ...

> @@ -203,8 +204,8 @@ static int perform_rel(unsigned char typ
>           * arch_livepatch_verify_distance can't account of addend so we have
>           * to do the check here as well.
>           */
> -        if ( (s32)val < -(s32)ARCH_LIVEPATCH_RANGE ||
> -             (s32)val >= (s32)ARCH_LIVEPATCH_RANGE )
> +        if ( (int32_t)val < -(int32_t)ARCH_LIVEPATCH_RANGE ||
> +             (int32_t)val >= (int32_t)ARCH_LIVEPATCH_RANGE )
>              return -EOVERFLOW;

... here?

I'd argue that even this one wants one extra space in the middle, so the
'-' is further to the right of the >=.

~Andrew

Re: [PATCH 4/5] types: replace remaining uses of s32

[Jan Beulich]

On 29.08.2024 14:44, Andrew Cooper wrote:
> On 29/08/2024 1:01 pm, Jan Beulich wrote:
>> ... and move the type itself to linux-compat.h.
>>
>> While doing so switch a few adjacent types as well, for (a little bit
>> of) consistency.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>,

Thanks.

> with a minor formatting request.
> 
>> --- a/xen/arch/arm/arm32/livepatch.c
>> +++ b/xen/arch/arm/arm32/livepatch.c
>> @@ -41,11 +41,11 @@ void arch_livepatch_apply(const struct l
>>           * ARM DDI 0406C.c, see A2.3 (pg 45) and A8.8.18 pg (pg 334,335)
>>           *
>>           */
>> -        delta = (s32)func->new_addr - (s32)(func->old_addr + 8);
>> +        delta = (int32_t)func->new_addr - (int32_t)(func->old_addr + 8);
>>  
>>          /* The arch_livepatch_symbol_ok should have caught it. */
>> -        ASSERT(delta >= -(s32)ARCH_LIVEPATCH_RANGE ||
>> -               delta < (s32)ARCH_LIVEPATCH_RANGE);
>> +        ASSERT(delta >= -(int32_t)ARCH_LIVEPATCH_RANGE ||
>> +               delta < (int32_t)ARCH_LIVEPATCH_RANGE);
> 
> Could you vertically like this, like it is ...
> 
>> @@ -203,8 +204,8 @@ static int perform_rel(unsigned char typ
>>           * arch_livepatch_verify_distance can't account of addend so we have
>>           * to do the check here as well.
>>           */
>> -        if ( (s32)val < -(s32)ARCH_LIVEPATCH_RANGE ||
>> -             (s32)val >= (s32)ARCH_LIVEPATCH_RANGE )
>> +        if ( (int32_t)val < -(int32_t)ARCH_LIVEPATCH_RANGE ||
>> +             (int32_t)val >= (int32_t)ARCH_LIVEPATCH_RANGE )
>>              return -EOVERFLOW;
> 
> ... here?

If the Arm folks don't mind - sure, I can. I think though that the latter
only happens to look aligned, without there having been such an intention.
Kind of supported ...

> I'd argue that even this one wants one extra space in the middle, so the
> '-' is further to the right of the >=.

... by this observation of yours.

Jan

Re: [PATCH 4/5] types: replace remaining uses of s32

[Roger Pau Monné]

On Thu, Aug 29, 2024 at 02:01:16PM +0200, Jan Beulich wrote:
> ... and move the type itself to linux-compat.h.
> 
> While doing so switch a few adjacent types as well, for (a little bit
> of) consistency.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

Re: [PATCH 4/5] types: replace remaining uses of s32

[Jan Beulich]

On 12.09.2024 11:52, Roger Pau Monné wrote:
> On Thu, Aug 29, 2024 at 02:01:16PM +0200, Jan Beulich wrote:
>> ... and move the type itself to linux-compat.h.
>>
>> While doing so switch a few adjacent types as well, for (a little bit
>> of) consistency.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks. Andrew asked for a style adjustment, which I wasn't sure about.
I'd like to follow whatever maintainers prefer, so could you clarify
that please?

Jan

Re: [PATCH 4/5] types: replace remaining uses of s32

[Roger Pau Monné]

On Thu, Sep 12, 2024 at 12:05:23PM +0200, Jan Beulich wrote:
> On 12.09.2024 11:52, Roger Pau Monné wrote:
> > On Thu, Aug 29, 2024 at 02:01:16PM +0200, Jan Beulich wrote:
> >> ... and move the type itself to linux-compat.h.
> >>
> >> While doing so switch a few adjacent types as well, for (a little bit
> >> of) consistency.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > 
> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Thanks. Andrew asked for a style adjustment, which I wasn't sure about.
> I'd like to follow whatever maintainers prefer, so could you clarify
> that please?

Oh, sorry, I would prefer with the alignment added, as suggested by
Andrew.

Thanks, Roger.

[PATCH 5/5] types: replace remaining uses of s64

[Jan Beulich]

... and move the type itself to linux-compat.h.

While doing so
- correct the type of union uu's uq field in lib/divmod.c,
- switch a few adjacent types as well, for (a little bit of)
  consistency.

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

--- a/xen/arch/arm/arm64/cpufeature.c
+++ b/xen/arch/arm/arm64/cpufeature.c
@@ -484,8 +484,8 @@ static const struct arm64_ftr_bits ftr_r
 	ARM64_FTR_END,
 };
 
-static u64 arm64_ftr_set_value(const struct arm64_ftr_bits *ftrp, s64 reg,
-			       s64 ftr_val)
+static uint64_t arm64_ftr_set_value(const struct arm64_ftr_bits *ftrp,
+				    int64_t reg, int64_t ftr_val)
 {
 	u64 mask = arm64_ftr_mask(ftrp);
 
@@ -494,10 +494,10 @@ static u64 arm64_ftr_set_value(const str
 	return reg;
 }
 
-static s64 arm64_ftr_safe_value(const struct arm64_ftr_bits *ftrp, s64 new,
-				s64 cur)
+static int64_t arm64_ftr_safe_value(const struct arm64_ftr_bits *ftrp,
+				    int64_t new, int64_t cur)
 {
-	s64 ret = 0;
+	int64_t ret = 0;
 
 	switch (ftrp->type) {
 	case FTR_EXACT:
@@ -532,8 +532,8 @@ static void sanitize_reg(u64 *cur_reg, u
 
 	for (;ftrp->width != 0;ftrp++)
 	{
-		s64 cur_field = arm64_ftr_value(ftrp, *cur_reg);
-		s64 new_field = arm64_ftr_value(ftrp, new_reg);
+		int64_t cur_field = arm64_ftr_value(ftrp, *cur_reg);
+		int64_t new_field = arm64_ftr_value(ftrp, new_reg);
 
 		if (cur_field == new_field)
 			continue;
--- a/xen/arch/arm/arm64/livepatch.c
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -119,7 +119,7 @@ static u64 do_reloc(enum aarch64_reloc_o
 
 static int reloc_data(enum aarch64_reloc_op op, void *place, u64 val, int len)
 {
-    s64 sval = do_reloc(op, place, val);
+    int64_t sval = do_reloc(op, place, val);
 
     switch ( len )
     {
@@ -136,7 +136,7 @@ static int reloc_data(enum aarch64_reloc
         break;
 
     case 64:
-        *(s64 *)place = sval;
+        *(int64_t *)place = sval;
         break;
 
     default:
@@ -155,9 +155,9 @@ enum aarch64_insn_movw_imm_type {
 static int reloc_insn_movw(enum aarch64_reloc_op op, void *dest, u64 val,
                            int lsb, enum aarch64_insn_movw_imm_type imm_type)
 {
-    u64 imm;
-    s64 sval;
-    u32 insn = *(u32 *)dest;
+    uint64_t imm;
+    int64_t sval;
+    uint32_t insn = *(uint32_t *)dest;
 
     sval = do_reloc(op, dest, val);
     imm = sval >> lsb;
@@ -200,9 +200,9 @@ static int reloc_insn_movw(enum aarch64_
 static int reloc_insn_imm(enum aarch64_reloc_op op, void *dest, u64 val,
                           int lsb, int len, enum aarch64_insn_imm_type imm_type)
 {
-    u64 imm, imm_mask;
-    s64 sval;
-    u32 insn = *(u32 *)dest;
+    uint64_t imm, imm_mask;
+    int64_t sval;
+    uint32_t insn = *(uint32_t *)dest;
 
     /* Calculate the relocation value. */
     sval = do_reloc(op, dest, val);
@@ -220,7 +220,7 @@ static int reloc_insn_imm(enum aarch64_r
      * Extract the upper value bits (including the sign bit) and
      * shift them to bit 0.
      */
-    sval = (s64)(sval & ~(imm_mask >> 1)) >> (len - 1);
+    sval = (int64_t)(sval & ~(imm_mask >> 1)) >> (len - 1);
 
     /*
      * Overflow has occurred if the upper bits are not all equal to
--- a/xen/arch/arm/include/asm/arm64/cpufeature.h
+++ b/xen/arch/arm/include/asm/arm64/cpufeature.h
@@ -39,15 +39,15 @@ struct arm64_ftr_bits {
 	bool		visible;
 	bool		strict;	/* CPU Sanity check: strict matching required ? */
 	enum ftr_type	type;
-	u8		shift;
-	u8		width;
-	s64		safe_val; /* safe value for FTR_EXACT features */
+	uint8_t		shift;
+	uint8_t		width;
+	int64_t		safe_val; /* safe value for FTR_EXACT features */
 };
 
 static inline int __attribute_const__
 cpuid_feature_extract_signed_field_width(u64 features, int field, int width)
 {
-	return (s64)(features << (64 - width - field)) >> (64 - width);
+	return (int64_t)(features << (64 - width - field)) >> (64 - width);
 }
 
 static inline int __attribute_const__
@@ -87,9 +87,12 @@ cpuid_feature_extract_field(u64 features
 	return cpuid_feature_extract_field_width(features, field, 4, sign);
 }
 
-static inline s64 arm64_ftr_value(const struct arm64_ftr_bits *ftrp, u64 val)
+static inline int64_t arm64_ftr_value(const struct arm64_ftr_bits *ftrp,
+                                      uint64_t val)
 {
-	return (s64)cpuid_feature_extract_field_width(val, ftrp->shift, ftrp->width, ftrp->sign);
+	return (int64_t)cpuid_feature_extract_field_width(val, ftrp->shift,
+							  ftrp->width,
+							  ftrp->sign);
 }
 
 #endif /* _ASM_ARM_ARM64_CPUFEATURES_H */
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -1206,7 +1206,7 @@ void vlapic_tdt_msr_set(struct vlapic *v
     if ( value > guest_tsc )
     {
         uint64_t delta = gtsc_to_gtime(v->domain, value - guest_tsc);
-        delta = max_t(s64, delta, 0);
+        delta = max_t(int64_t, delta, 0);
 
         HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "delta[0x%016"PRIx64"]", delta);
 
--- a/xen/arch/x86/include/asm/hvm/vcpu.h
+++ b/xen/arch/x86/include/asm/hvm/vcpu.h
@@ -134,8 +134,8 @@ struct hvm_vcpu {
     unsigned long       hw_cr[5];
 
     struct vlapic       vlapic;
-    s64                 cache_tsc_offset;
-    u64                 guest_time;
+    int64_t             cache_tsc_offset;
+    uint64_t            guest_time;
 
     /* Lock and list for virtual platform timers. */
     spinlock_t          tm_lock;
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -66,10 +66,10 @@ struct cpu_time {
 struct platform_timesource {
     const char *id;
     const char *name;
-    u64 frequency;
+    uint64_t frequency;
     /* Post-init this hook may only be invoked via the read_counter() wrapper! */
-    u64 (*read_counter)(void);
-    s64 (*init)(struct platform_timesource *);
+    uint64_t (*read_counter)(void);
+    int64_t (*init)(struct platform_timesource *);
     void (*resume)(struct platform_timesource *);
     int counter_bits;
 };
@@ -368,7 +368,7 @@ static u64 cf_check read_pit_count(void)
     return count32;
 }
 
-static s64 __init cf_check init_pit(struct platform_timesource *pts)
+static int64_t __init cf_check init_pit(struct platform_timesource *pts)
 {
     u8 portb = inb(0x61);
     u64 start, end;
@@ -610,7 +610,7 @@ static u64 cf_check read_pmtimer_count(v
     return inl(pmtmr_ioport);
 }
 
-static s64 __init cf_check init_pmtimer(struct platform_timesource *pts)
+static int64_t __init cf_check init_pmtimer(struct platform_timesource *pts)
 {
     if ( !pmtmr_ioport || (pmtmr_width != 24 && pmtmr_width != 32) )
         return 0;
@@ -655,7 +655,7 @@ static unsigned int __initdata tsc_flags
  * Called in verify_tsc_reliability() under reliable TSC conditions
  * thus reusing all the checks already performed there.
  */
-static s64 __init cf_check init_tsc(struct platform_timesource *pts)
+static int64_t __init cf_check init_tsc(struct platform_timesource *pts)
 {
     u64 ret = pts->frequency;
 
@@ -1010,9 +1010,9 @@ static void __init reset_platform_timer(
     spin_unlock_irq(&platform_timer_lock);
 }
 
-static s64 __init try_platform_timer(struct platform_timesource *pts)
+static int64_t __init try_platform_timer(struct platform_timesource *pts)
 {
-    s64 rc = pts->init(pts);
+    int64_t rc = pts->init(pts);
 
     if ( rc <= 0 )
         return rc;
@@ -1046,7 +1046,7 @@ static u64 __init init_platform_timer(vo
 
     struct platform_timesource *pts = NULL;
     unsigned int i;
-    s64 rc = -1;
+    int64_t rc = -1;
 
     /* clocksource=tsc is initialized via __initcalls (when CPUs are up). */
     if ( (opt_clocksource[0] != '\0') && strcmp(opt_clocksource, "tsc") )
@@ -1837,7 +1837,7 @@ static void cf_check local_time_calibrat
      * Weirdness can happen if we lose sync with the platform timer.
      * We could be smarter here: resync platform timer with local timer?
      */
-    if ( ((s64)stime_elapsed64 < (EPOCH / 2)) )
+    if ( ((int64_t)stime_elapsed64 < (EPOCH / 2)) )
         goto out;
 
     /*
@@ -2312,7 +2312,7 @@ static void __init tsc_check_writability
 
         write_tsc(tsc | (1ULL << 32));
         tmp = rdtsc();
-        if ( ABS((s64)tmp - (s64)tmp2) < (1LL << 31) )
+        if ( ABS((int64_t)tmp - (int64_t)tmp2) < (1LL << 31) )
             what = "only partially";
     }
     else
--- a/xen/common/ubsan/ubsan.c
+++ b/xen/common/ubsan/ubsan.c
@@ -21,7 +21,6 @@ static DEFINE_PER_CPU(struct xen_ubsan[1
 #define current this_cpu(in_ubsan)
 #define dump_stack dump_execution_state
 #define u64 long long unsigned int
-#define s64 long long int
 
 #include "ubsan.h"
 
@@ -102,7 +101,7 @@ static s_max get_signed_val(struct type_
 	}
 
 	if (type_bit_width(type) == 64)
-		return *(s64 *)val;
+		return *(int64_t *)val;
 
 	return *(s_max *)val;
 }
@@ -141,10 +140,11 @@ static void val_to_string(char *str, siz
 #endif
 		} else if (type_is_signed(type)) {
 			scnprintf(str, size, "%lld",
-				(s64)get_signed_val(type, value));
+				  (long long)get_signed_val(type, value));
 		} else {
 			scnprintf(str, size, "%llu",
-				(u64)get_unsigned_val(type, value));
+				  (unsigned long long)get_unsigned_val(type,
+								       value));
 		}
 	}
 }
--- a/xen/common/ubsan/ubsan.h
+++ b/xen/common/ubsan/ubsan.h
@@ -97,8 +97,8 @@ enum {
 typedef __int128 s_max;
 typedef unsigned __int128 u_max;
 #else
-typedef s64 s_max;
-typedef u64 u_max;
+typedef int64_t s_max;
+typedef uint64_t u_max;
 #endif
 
 #endif
--- a/xen/drivers/acpi/apei/erst.c
+++ b/xen/drivers/acpi/apei/erst.c
@@ -105,7 +105,7 @@ static inline int erst_errno(int command
 
 static int erst_timedout(u64 *t, u64 spin_unit)
 {
-	if ((s64)*t < spin_unit) {
+	if ((int64_t)*t < spin_unit) {
 		printk(XENLOG_WARNING "Firmware does not respond in time\n");
 		return 1;
 	}
--- a/xen/include/acpi/actypes.h
+++ b/xen/include/acpi/actypes.h
@@ -147,8 +147,8 @@ typedef int INT32;
 
 /*! [End] no source code translation !*/
 
-typedef u64 acpi_native_uint;
-typedef s64 acpi_native_int;
+typedef uint64_t acpi_native_uint;
+typedef int64_t acpi_native_int;
 
 typedef u64 acpi_io_address;
 typedef u64 acpi_physical_address;
--- a/xen/include/xen/linux-compat.h
+++ b/xen/include/xen/linux-compat.h
@@ -15,7 +15,7 @@ typedef int8_t  s8, __s8;
 typedef uint8_t __u8;
 typedef int16_t s16, __s16;
 typedef int32_t s32, __s32;
-typedef int64_t __s64;
+typedef int64_t s64, __s64;
 
 typedef paddr_t phys_addr_t;
 
--- a/xen/include/xen/time.h
+++ b/xen/include/xen/time.h
@@ -29,7 +29,7 @@ struct vcpu;
  * of real time into system time 
  */
 
-typedef s64 s_time_t;
+typedef int64_t s_time_t;
 #define PRI_stime PRId64
 
 s_time_t get_s_time_fixed(u64 at_tick);
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -8,7 +8,6 @@
 typedef uint8_t u8;
 typedef uint16_t u16, __u16;
 typedef uint32_t u32, __u32;
-typedef int64_t s64;
 typedef uint64_t u64, __u64;
 
 #include <asm/types.h>
--- a/xen/lib/divmod.c
+++ b/xen/lib/divmod.c
@@ -46,8 +46,8 @@
  * one or more of the following formats.
  */
 union uu {
-    s64            q;              /* as a (signed) quad */
-    s64            uq;             /* as an unsigned quad */
+    int64_t        q;              /* as a (signed) quad */
+    uint64_t       uq;             /* as an unsigned quad */
     long           sl[2];          /* as two signed longs */
     unsigned long  ul[2];          /* as two unsigned longs */
 };
@@ -72,7 +72,7 @@ union uu {
  * and assembly.
  */
 #define CHAR_BIT        8               /* number of bits in a char */
-#define QUAD_BITS       (sizeof(s64) * CHAR_BIT)
+#define QUAD_BITS       (sizeof(int64_t) * CHAR_BIT)
 #define LONG_BITS       (sizeof(long) * CHAR_BIT)
 #define HALF_BITS       (sizeof(long) * CHAR_BIT / 2)
 
@@ -324,7 +324,7 @@ u64 __qdivrem(u64 uq, u64 vq, u64 *arq)
  * Divide two signed quads.
  * Truncates towards zero, as required by C99.
  */
-s64 __divdi3(s64 a, s64 b)
+int64_t __divdi3(int64_t a, int64_t b)
 {
     u64 ua, ub, uq;
     int neg = (a < 0) ^ (b < 0);
@@ -361,7 +361,7 @@ u64 __umoddi3(u64 a, u64 b)
  *  11 % -5 =  1
  * -11 % -5 = -1
  */
-s64 __moddi3(s64 a, s64 b)
+int64_t __moddi3(int64_t a, int64_t b)
 {
     u64 ua, ub, urem;
     int neg = (a < 0);
@@ -374,7 +374,7 @@ s64 __moddi3(s64 a, s64 b)
 /*
  * Quotient and remainder of unsigned long long division
  */
-s64 __ldivmod_helper(s64 a, s64 b, s64 *r)
+int64_t __ldivmod_helper(int64_t a, int64_t b, int64_t *r)
 {
     u64 ua, ub, rem, quot;
 

Re: [PATCH 5/5] types: replace remaining uses of s64

[Andrew Cooper]

On 29/08/2024 1:01 pm, Jan Beulich wrote:
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -66,10 +66,10 @@ struct cpu_time {
>  struct platform_timesource {
>      const char *id;
>      const char *name;
> -    u64 frequency;
> +    uint64_t frequency;
>      /* Post-init this hook may only be invoked via the read_counter() wrapper! */
> -    u64 (*read_counter)(void);
> -    s64 (*init)(struct platform_timesource *);
> +    uint64_t (*read_counter)(void);
> +    int64_t (*init)(struct platform_timesource *);
>      void (*resume)(struct platform_timesource *);

I'm surprised that we haven't seen MISRA complaints about this.  That,
or I've not been paying enough attention.

> --- a/xen/common/ubsan/ubsan.c
> +++ b/xen/common/ubsan/ubsan.c
> @@ -21,7 +21,6 @@ static DEFINE_PER_CPU(struct xen_ubsan[1
>  #define current this_cpu(in_ubsan)
>  #define dump_stack dump_execution_state
>  #define u64 long long unsigned int
> -#define s64 long long int

This block of defines was my magic to use ubsan.c otherwise unmodified
from Linux.

It ought to use linux-compat.h now it exists, rather than swapping away
from {u,s}64.

Everything else looks good.

~Andrew

Re: [PATCH 5/5] types: replace remaining uses of s64

[Jan Beulich]

On 29.08.2024 14:51, Andrew Cooper wrote:
> On 29/08/2024 1:01 pm, Jan Beulich wrote:
>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -66,10 +66,10 @@ struct cpu_time {
>>  struct platform_timesource {
>>      const char *id;
>>      const char *name;
>> -    u64 frequency;
>> +    uint64_t frequency;
>>      /* Post-init this hook may only be invoked via the read_counter() wrapper! */
>> -    u64 (*read_counter)(void);
>> -    s64 (*init)(struct platform_timesource *);
>> +    uint64_t (*read_counter)(void);
>> +    int64_t (*init)(struct platform_timesource *);
>>      void (*resume)(struct platform_timesource *);
> 
> I'm surprised that we haven't seen MISRA complaints about this.  That,
> or I've not been paying enough attention.

What Misra concerns do you see here?

>> --- a/xen/common/ubsan/ubsan.c
>> +++ b/xen/common/ubsan/ubsan.c
>> @@ -21,7 +21,6 @@ static DEFINE_PER_CPU(struct xen_ubsan[1
>>  #define current this_cpu(in_ubsan)
>>  #define dump_stack dump_execution_state
>>  #define u64 long long unsigned int
>> -#define s64 long long int
> 
> This block of defines was my magic to use ubsan.c otherwise unmodified
> from Linux.
> 
> It ought to use linux-compat.h now it exists, rather than swapping away
> from {u,s}64.

Except that this doesn't work without retaining the hunk adjusting
val_to_string(). I'm actually surprised Linux gets away with that, seeing
their include/uapi/asm-generic/int-l64.h (which is the model we use,
resulting in -Wformat warnings). Thoughts?

Jan