[PATCH v2 0/4] xen/public: use fixed-size integers

[PATCH v2 0/4] xen/public: use fixed-size integers

[Stefano Stabellini]

Hi all,

This short patch series replaces integers without a fixed size with
fixed-size integers in public headers.

Cheers,

Stefano

[PATCH v2 1/4] docs: add xen_ulong_t to the documented integers sizes/alignments

[Stefano Stabellini]

xen_ulong_t is widely used in public headers.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---

Given that xen_ulong_t is used in public headers there could be a better
place for documenting it but this was the most straightforward to add.
---
 docs/misra/C-language-toolchain.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/docs/misra/C-language-toolchain.rst b/docs/misra/C-language-toolchain.rst
index 5ddfe7bdbe..7a334260e6 100644
--- a/docs/misra/C-language-toolchain.rst
+++ b/docs/misra/C-language-toolchain.rst
@@ -531,6 +531,17 @@ A summary table of data types, sizes and alignment is below:
      - 64 bits 
      - x86_64, ARMv8-A AArch64, RV64, PPC64
 
+   * - xen_ulong_t
+     - 32 bits
+     - 32 bits 
+     - x86_32
+
+   * - xen_ulong_t
+     - 64 bits
+     - 64 bits 
+     - x86_64, ARMv8-A AArch64, RV64, PPC64, ARMv8-A AArch32, ARMv8-R
+       AArch32, ARMv7-A
+
    * - long long
      - 64-bit
      - 32-bit
-- 
2.25.1

[PATCH v2 2/4] public: s/int/int32_t

[Stefano Stabellini]

Straightforward int -> int32_t and unsigned int -> uint32_t replacements
in public headers. No ABI or semantic changes intended.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
Changes in v2:
- avoid changes to GUEST_HANDLEs for now (there was one GUEST_HANDLE
change in v1)
---
 xen/include/public/kexec.h   |  8 ++++----
 xen/include/public/memory.h  | 22 +++++++++++-----------
 xen/include/public/physdev.h | 18 +++++++++---------
 xen/include/public/sched.h   |  6 +++---
 xen/include/public/vcpu.h    |  2 +-
 5 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/xen/include/public/kexec.h b/xen/include/public/kexec.h
index 40d79e936b..8d2a0ef697 100644
--- a/xen/include/public/kexec.h
+++ b/xen/include/public/kexec.h
@@ -105,7 +105,7 @@ typedef struct xen_kexec_image {
  */
 #define KEXEC_CMD_kexec                 0
 typedef struct xen_kexec_exec {
-    int type;
+    int32_t type;
 } xen_kexec_exec_t;
 
 /*
@@ -116,7 +116,7 @@ typedef struct xen_kexec_exec {
 #define KEXEC_CMD_kexec_load_v1         1 /* obsolete since 0x00040400 */
 #define KEXEC_CMD_kexec_unload_v1       2 /* obsolete since 0x00040400 */
 typedef struct xen_kexec_load_v1 {
-    int type;
+    int32_t type;
     xen_kexec_image_t image;
 } xen_kexec_load_v1_t;
 
@@ -143,8 +143,8 @@ typedef struct xen_kexec_load_v1 {
  */
 #define KEXEC_CMD_kexec_get_range       3
 typedef struct xen_kexec_range {
-    int range;
-    int nr;
+    int32_t range;
+    int32_t nr;
     unsigned long size;
     unsigned long start;
 } xen_kexec_range_t;
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 5e545ae9a4..ae4a71268f 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -61,13 +61,13 @@ struct xen_memory_reservation {
 
     /* Number of extents, and size/alignment of each (2^extent_order pages). */
     xen_ulong_t    nr_extents;
-    unsigned int   extent_order;
+    uint32_t       extent_order;
 
 #if __XEN_INTERFACE_VERSION__ >= 0x00030209
     /* XENMEMF flags. */
-    unsigned int   mem_flags;
+    uint32_t       mem_flags;
 #else
-    unsigned int   address_bits;
+    uint32_t       address_bits;
 #endif
 
     /*
@@ -163,7 +163,7 @@ struct xen_machphys_mfn_list {
      * Size of the 'extent_start' array. Fewer entries will be filled if the
      * machphys table is smaller than max_extents * 2MB.
      */
-    unsigned int max_extents;
+    uint32_t max_extents;
 
     /*
      * Pointer to buffer to fill with list of extent starts. If there are
@@ -176,7 +176,7 @@ struct xen_machphys_mfn_list {
      * Number of extents written to the above array. This will be smaller
      * than 'max_extents' if the machphys table is smaller than max_e * 2MB.
      */
-    unsigned int nr_extents;
+    uint32_t nr_extents;
 };
 typedef struct xen_machphys_mfn_list xen_machphys_mfn_list_t;
 DEFINE_XEN_GUEST_HANDLE(xen_machphys_mfn_list_t);
@@ -232,7 +232,7 @@ struct xen_add_to_physmap {
     /* Number of pages to go through for gmfn_range */
     uint16_t    size;
 
-    unsigned int space; /* => enum phys_map_space */
+    uint32_t    space; /* => enum phys_map_space */
 
 #define XENMAPIDX_grant_table_status 0x80000000U
 
@@ -317,7 +317,7 @@ struct xen_memory_map {
      * return the number of entries which have been stored in
      * buffer.
      */
-    unsigned int nr_entries;
+    uint32_t nr_entries;
 
     /*
      * Entries in the buffer are in the same format as returned by the
@@ -591,7 +591,7 @@ struct xen_reserved_device_memory_map {
      * Gets set to the required number of entries when too low,
      * signaled by error code -ERANGE.
      */
-    unsigned int nr_entries;
+    uint32_t nr_entries;
     /* OUT */
     XEN_GUEST_HANDLE(xen_reserved_device_memory_t) buffer;
     /* IN */
@@ -711,9 +711,9 @@ struct xen_vnuma_topology_info {
     domid_t domid;
     uint16_t pad;
     /* IN/OUT */
-    unsigned int nr_vnodes;
-    unsigned int nr_vcpus;
-    unsigned int nr_vmemranges;
+    uint32_t nr_vnodes;
+    uint32_t nr_vcpus;
+    uint32_t nr_vmemranges;
     /* OUT */
     union {
         XEN_GUEST_HANDLE(uint) h;
diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h
index f0c0d4727c..03ccf86618 100644
--- a/xen/include/public/physdev.h
+++ b/xen/include/public/physdev.h
@@ -142,17 +142,17 @@ DEFINE_XEN_GUEST_HANDLE(physdev_irq_t);
 struct physdev_map_pirq {
     domid_t domid;
     /* IN */
-    int type;
+    int32_t type;
     /* IN (ignored for ..._MULTI_MSI) */
-    int index;
+    int32_t index;
     /* IN or OUT */
-    int pirq;
+    int32_t pirq;
     /* IN - high 16 bits hold segment for ..._MSI_SEG and ..._MULTI_MSI */
-    int bus;
+    int32_t bus;
     /* IN */
-    int devfn;
+    int32_t devfn;
     /* IN (also OUT for ..._MULTI_MSI) */
-    int entry_nr;
+    int32_t entry_nr;
     /* IN */
     uint64_t table_base;
 };
@@ -163,7 +163,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_map_pirq_t);
 struct physdev_unmap_pirq {
     domid_t domid;
     /* IN */
-    int pirq;
+    int32_t pirq;
 };
 
 typedef struct physdev_unmap_pirq physdev_unmap_pirq_t;
@@ -224,7 +224,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_op_t);
 
 #define PHYSDEVOP_setup_gsi    21
 struct physdev_setup_gsi {
-    int gsi;
+    int32_t gsi;
     /* IN */
     uint8_t triggering;
     /* IN */
@@ -242,7 +242,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_setup_gsi_t);
 #define PHYSDEVOP_get_free_pirq    23
 struct physdev_get_free_pirq {
     /* IN */
-    int type;
+    int32_t type;
     /* OUT */
     uint32_t pirq;
 };
diff --git a/xen/include/public/sched.h b/xen/include/public/sched.h
index b4362c6a1d..57ac3df47b 100644
--- a/xen/include/public/sched.h
+++ b/xen/include/public/sched.h
@@ -116,14 +116,14 @@
 /* ` } */
 
 struct sched_shutdown {
-    unsigned int reason; /* SHUTDOWN_* => enum sched_shutdown_reason */
+    uint32_t reason; /* SHUTDOWN_* => enum sched_shutdown_reason */
 };
 typedef struct sched_shutdown sched_shutdown_t;
 DEFINE_XEN_GUEST_HANDLE(sched_shutdown_t);
 
 struct sched_poll {
     XEN_GUEST_HANDLE(evtchn_port_t) ports;
-    unsigned int nr_ports;
+    uint32_t nr_ports;
     uint64_t timeout;
 };
 typedef struct sched_poll sched_poll_t;
@@ -131,7 +131,7 @@ DEFINE_XEN_GUEST_HANDLE(sched_poll_t);
 
 struct sched_remote_shutdown {
     domid_t domain_id;         /* Remote domain ID */
-    unsigned int reason;       /* SHUTDOWN_* => enum sched_shutdown_reason */
+    uint32_t reason;           /* SHUTDOWN_* => enum sched_shutdown_reason */
 };
 typedef struct sched_remote_shutdown sched_remote_shutdown_t;
 DEFINE_XEN_GUEST_HANDLE(sched_remote_shutdown_t);
diff --git a/xen/include/public/vcpu.h b/xen/include/public/vcpu.h
index f7445ac0b0..2b8df2b0da 100644
--- a/xen/include/public/vcpu.h
+++ b/xen/include/public/vcpu.h
@@ -63,7 +63,7 @@
 #define VCPUOP_get_runstate_info     4
 struct vcpu_runstate_info {
     /* VCPU's current state (RUNSTATE_*). */
-    int      state;
+    int32_t  state;
     /* When was current state entered (system time, ns)? */
     uint64_t state_entry_time;
     /*
-- 
2.25.1

[PATCH v2 3/4] xen/public: s/unsigned long/xen_ulong_t

[Stefano Stabellini]

The goal is to use only fixed-size integers in public headers, such as
uint32_t and uint64_t.

However, there are cases where the ABI changes depending on the
architecture. In those cases, adding #ifdefs might be the clearest
solution but it is also cumbersome. We already define a xen_ulong_t type
which is widely used in public headers and it is defined differently by
architecture.

Instead of unsigned long, use xen_ulong_t in public headers:
- it makes it clearer that size might change by arch
- it gets us closer to the goal of no unfixed-size integers in public
headers

Note that unsigned long and xen_ulong_t are the same thing on x86 (both
32-bit and 64-bit) but they differ on all other arches. However, all
the interfaces with xen_ulong_t or unsigned long are x86-only interfaces
today. Thus, this patch doesn't introduce any ABI or semantic changes.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
 xen/include/public/kexec.h   | 10 +++++-----
 xen/include/public/nmi.h     |  4 ++--
 xen/include/public/physdev.h |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/xen/include/public/kexec.h b/xen/include/public/kexec.h
index 8d2a0ef697..89164094d9 100644
--- a/xen/include/public/kexec.h
+++ b/xen/include/public/kexec.h
@@ -78,10 +78,10 @@
 
 typedef struct xen_kexec_image {
 #if defined(__i386__) || defined(__x86_64__)
-    unsigned long page_list[KEXEC_XEN_NO_PAGES];
+    xen_ulong_t page_list[KEXEC_XEN_NO_PAGES];
 #endif
-    unsigned long indirection_page;
-    unsigned long start_address;
+    xen_ulong_t indirection_page;
+    xen_ulong_t start_address;
 } xen_kexec_image_t;
 
 /*
@@ -145,8 +145,8 @@ typedef struct xen_kexec_load_v1 {
 typedef struct xen_kexec_range {
     int32_t range;
     int32_t nr;
-    unsigned long size;
-    unsigned long start;
+    xen_ulong_t size;
+    xen_ulong_t start;
 } xen_kexec_range_t;
 
 #if __XEN_INTERFACE_VERSION__ >= 0x00040400
diff --git a/xen/include/public/nmi.h b/xen/include/public/nmi.h
index 5900703f5f..c8c0ddafc2 100644
--- a/xen/include/public/nmi.h
+++ b/xen/include/public/nmi.h
@@ -43,8 +43,8 @@
  */
 #define XENNMI_register_callback   0
 struct xennmi_callback {
-    unsigned long handler_address;
-    unsigned long pad;
+    xen_ulong_t handler_address;
+    xen_ulong_t pad;
 };
 typedef struct xennmi_callback xennmi_callback_t;
 DEFINE_XEN_GUEST_HANDLE(xennmi_callback_t);
diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h
index 03ccf86618..5bbb809fae 100644
--- a/xen/include/public/physdev.h
+++ b/xen/include/public/physdev.h
@@ -109,7 +109,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t);
 #define PHYSDEVOP_apic_write             9
 struct physdev_apic {
     /* IN */
-    unsigned long apic_physbase;
+    xen_ulong_t apic_physbase;
     uint32_t reg;
     /* IN or OUT */
     uint32_t value;
-- 
2.25.1

[PATCH v2 4/4] xen/public: replace remaining int and long

[Stefano Stabellini]

Replace all int and long in few remaining x86-specific headers and
x86-specific hypercalls.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
 xen/include/public/arch-x86/hvm/save.h   |  4 +-
 xen/include/public/arch-x86/xen-x86_32.h | 10 ++---
 xen/include/public/arch-x86/xen-x86_64.h | 10 ++---
 xen/include/public/arch-x86/xen.h        | 50 ++++++++++++------------
 xen/include/public/xen.h                 | 24 ++++++------
 5 files changed, 49 insertions(+), 49 deletions(-)

diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
index 7ecacadde1..8e3dfcf439 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -415,7 +415,7 @@ struct hvm_hw_pci_irqs {
      * Indexed by: device*4 + INTx#.
      */
     union {
-        unsigned long i[16 / sizeof (unsigned long)]; /* DECLARE_BITMAP(i, 32*4); */
+        xen_ulong_t i[16 / sizeof (xen_ulong_t)]; /* DECLARE_BITMAP(i, 32*4); */
         uint64_t pad[2];
     };
 };
@@ -428,7 +428,7 @@ struct hvm_hw_isa_irqs {
      * Indexed by ISA IRQ (assumes no ISA-device IRQ sharing).
      */
     union {
-        unsigned long i[1];  /* DECLARE_BITMAP(i, 16); */
+        xen_ulong_t i[1];  /* DECLARE_BITMAP(i, 16); */
         uint64_t pad[1];
     };
 };
diff --git a/xen/include/public/arch-x86/xen-x86_32.h b/xen/include/public/arch-x86/xen-x86_32.h
index 9e3bf06b12..0b2671ab65 100644
--- a/xen/include/public/arch-x86/xen-x86_32.h
+++ b/xen/include/public/arch-x86/xen-x86_32.h
@@ -68,7 +68,7 @@
 #define MACH2PHYS_VIRT_END    xen_mk_ulong(__MACH2PHYS_VIRT_END)
 #define MACH2PHYS_NR_ENTRIES  ((MACH2PHYS_VIRT_END-MACH2PHYS_VIRT_START)>>2)
 #ifndef machine_to_phys_mapping
-#define machine_to_phys_mapping ((unsigned long *)MACH2PHYS_VIRT_START)
+#define machine_to_phys_mapping ((xen_ulong_t *)MACH2PHYS_VIRT_START)
 #endif
 
 /* 32-/64-bit invariability for control interfaces (domctl/sysctl). */
@@ -151,14 +151,14 @@ DEFINE_XEN_GUEST_HANDLE(cpu_user_regs_t);
 #define xen_cr3_to_pfn(cr3) (((unsigned)(cr3) >> 12) | ((unsigned)(cr3) << 20))
 
 struct arch_vcpu_info {
-    unsigned long cr2;
-    unsigned long pad[5]; /* sizeof(vcpu_info_t) == 64 */
+    xen_ulong_t cr2;
+    xen_ulong_t pad[5]; /* sizeof(vcpu_info_t) == 64 */
 };
 typedef struct arch_vcpu_info arch_vcpu_info_t;
 
 struct xen_callback {
-    unsigned long cs;
-    unsigned long eip;
+    xen_ulong_t cs;
+    xen_ulong_t eip;
 };
 typedef struct xen_callback xen_callback_t;
 
diff --git a/xen/include/public/arch-x86/xen-x86_64.h b/xen/include/public/arch-x86/xen-x86_64.h
index 43f6e3d220..bd27fc59fe 100644
--- a/xen/include/public/arch-x86/xen-x86_64.h
+++ b/xen/include/public/arch-x86/xen-x86_64.h
@@ -67,7 +67,7 @@
 #define MACH2PHYS_VIRT_END    xen_mk_ulong(__MACH2PHYS_VIRT_END)
 #define MACH2PHYS_NR_ENTRIES  ((MACH2PHYS_VIRT_END-MACH2PHYS_VIRT_START)>>3)
 #ifndef machine_to_phys_mapping
-#define machine_to_phys_mapping ((unsigned long *)HYPERVISOR_VIRT_START)
+#define machine_to_phys_mapping ((xen_ulong_t *)HYPERVISOR_VIRT_START)
 #endif
 
 /*
@@ -198,12 +198,12 @@ DEFINE_XEN_GUEST_HANDLE(cpu_user_regs_t);
 #undef __DECL_REG_LO16
 #undef __DECL_REG_HI
 
-#define xen_pfn_to_cr3(pfn) ((unsigned long)(pfn) << 12)
-#define xen_cr3_to_pfn(cr3) ((unsigned long)(cr3) >> 12)
+#define xen_pfn_to_cr3(pfn) ((xen_ulong_t)(pfn) << 12)
+#define xen_cr3_to_pfn(cr3) ((xen_ulong_t)(cr3) >> 12)
 
 struct arch_vcpu_info {
-    unsigned long cr2;
-    unsigned long pad; /* sizeof(vcpu_info_t) == 64 */
+    xen_ulong_t cr2;
+    xen_ulong_t pad; /* sizeof(vcpu_info_t) == 64 */
 };
 typedef struct arch_vcpu_info arch_vcpu_info_t;
 
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index a9a87d9b50..90710c1230 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -141,7 +141,7 @@ struct trap_info {
     uint8_t       vector;  /* exception vector                              */
     uint8_t       flags;   /* 0-3: privilege level; 4: clear event enable?  */
     uint16_t      cs;      /* code selector                                 */
-    unsigned long address; /* code offset                                   */
+    xen_ulong_t address;   /* code offset                                   */
 };
 typedef struct trap_info trap_info_t;
 DEFINE_XEN_GUEST_HANDLE(trap_info_t);
@@ -174,36 +174,36 @@ struct vcpu_guest_context {
 #define VGCF_syscall_disables_events   (1<<_VGCF_syscall_disables_events)
 #define _VGCF_online                   5
 #define VGCF_online                    (1<<_VGCF_online)
-    unsigned long flags;                    /* VGCF_* flags                 */
-    struct cpu_user_regs user_regs;         /* User-level CPU registers     */
-    struct trap_info trap_ctxt[256];        /* Virtual IDT                  */
-    unsigned long ldt_base, ldt_ents;       /* LDT (linear address, # ents) */
-    unsigned long gdt_frames[16], gdt_ents; /* GDT (machine frames, # ents) */
-    unsigned long kernel_ss, kernel_sp;     /* Virtual TSS (only SS1/SP1)   */
+    xen_ulong_t flags;                    /* VGCF_* flags                 */
+    struct cpu_user_regs user_regs;       /* User-level CPU registers     */
+    struct trap_info trap_ctxt[256];      /* Virtual IDT                  */
+    xen_ulong_t ldt_base, ldt_ents;       /* LDT (linear address, # ents) */
+    xen_ulong_t gdt_frames[16], gdt_ents; /* GDT (machine frames, # ents) */
+    xen_ulong_t kernel_ss, kernel_sp;     /* Virtual TSS (only SS1/SP1)   */
     /* NB. User pagetable on x86/64 is placed in ctrlreg[1]. */
-    unsigned long ctrlreg[8];               /* CR0-CR7 (control registers)  */
-    unsigned long debugreg[8];              /* DB0-DB7 (debug registers)    */
+    xen_ulong_t ctrlreg[8];               /* CR0-CR7 (control registers)  */
+    xen_ulong_t debugreg[8];              /* DB0-DB7 (debug registers)    */
 #ifdef __i386__
-    unsigned long event_callback_cs;        /* CS:EIP of event callback     */
-    unsigned long event_callback_eip;
-    unsigned long failsafe_callback_cs;     /* CS:EIP of failsafe callback  */
-    unsigned long failsafe_callback_eip;
+    xen_ulong_t event_callback_cs;        /* CS:EIP of event callback     */
+    xen_ulong_t event_callback_eip;
+    xen_ulong_t failsafe_callback_cs;     /* CS:EIP of failsafe callback  */
+    xen_ulong_t failsafe_callback_eip;
 #else
-    unsigned long event_callback_eip;
-    unsigned long failsafe_callback_eip;
+    xen_ulong_t event_callback_eip;
+    xen_ulong_t failsafe_callback_eip;
 #ifdef __XEN__
     union {
-        unsigned long syscall_callback_eip;
+        xen_ulong_t syscall_callback_eip;
         struct {
-            unsigned int event_callback_cs;    /* compat CS of event cb     */
-            unsigned int failsafe_callback_cs; /* compat CS of failsafe cb  */
+            uint32_t event_callback_cs;    /* compat CS of event cb     */
+            uint32_t failsafe_callback_cs; /* compat CS of failsafe cb  */
         };
     };
 #else
-    unsigned long syscall_callback_eip;
+    xen_ulong_t syscall_callback_eip;
 #endif
 #endif
-    unsigned long vm_assist;                /* VMASST_TYPE_* bitmap */
+    xen_ulong_t vm_assist;                /* VMASST_TYPE_* bitmap */
 #ifdef __x86_64__
     /* Segment base addresses. */
     uint64_t      fs_base;
@@ -219,7 +219,7 @@ struct arch_shared_info {
      * Number of valid entries in the p2m table(s) anchored at
      * pfn_to_mfn_frame_list_list and/or p2m_vaddr.
      */
-    unsigned long max_pfn;
+    xen_ulong_t max_pfn;
     /*
      * Frame containing list of mfns containing list of mfns containing p2m.
      * A value of 0 indicates it has not yet been set up, ~0 indicates it has
@@ -228,7 +228,7 @@ struct arch_shared_info {
      * is to be used.
      */
     xen_pfn_t     pfn_to_mfn_frame_list_list;
-    unsigned long nmi_reason;
+    xen_ulong_t nmi_reason;
     /*
      * Following three fields are valid if p2m_cr3 contains a value different
      * from 0.
@@ -247,9 +247,9 @@ struct arch_shared_info {
      * Modifying a p2m element in the linear p2m list is allowed via an atomic
      * write only.
      */
-    unsigned long p2m_cr3;         /* cr3 value of the p2m address space */
-    unsigned long p2m_vaddr;       /* virtual address of the p2m list */
-    unsigned long p2m_generation;  /* generation count of p2m mapping */
+    xen_ulong_t p2m_cr3;         /* cr3 value of the p2m address space */
+    xen_ulong_t p2m_vaddr;       /* virtual address of the p2m list */
+    xen_ulong_t p2m_generation;  /* generation count of p2m mapping */
 #ifdef __i386__
     /* There's no room for this field in the generic structure. */
     uint32_t wc_sec_hi;
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index b47d48d0e2..2444e752ab 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -430,17 +430,17 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
 
 #ifndef __ASSEMBLY__
 struct mmuext_op {
-    unsigned int cmd; /* => enum mmuext_cmd */
+    uint32_t cmd; /* => enum mmuext_cmd */
     union {
         /* [UN]PIN_TABLE, NEW_BASEPTR, NEW_USER_BASEPTR
          * CLEAR_PAGE, COPY_PAGE, [UN]MARK_SUPER */
         xen_pfn_t     mfn;
         /* INVLPG_LOCAL, INVLPG_ALL, SET_LDT */
-        unsigned long linear_addr;
+        xen_ulong_t linear_addr;
     } arg1;
     union {
         /* SET_LDT */
-        unsigned int nr_ents;
+        uint32_t nr_ents;
         /* TLB_FLUSH_MULTI, INVLPG_MULTI */
 #if __XEN_INTERFACE_VERSION__ >= 0x00030205
         XEN_GUEST_HANDLE(const_void) vcpumask;
@@ -832,8 +832,8 @@ typedef struct shared_info shared_info_t;
 struct start_info {
     /* THE FOLLOWING ARE FILLED IN BOTH ON INITIAL BOOT AND ON RESUME.    */
     char magic[32];             /* "xen-<version>-<platform>".            */
-    unsigned long nr_pages;     /* Total pages allocated to this domain.  */
-    unsigned long shared_info;  /* MACHINE address of shared info struct. */
+    xen_ulong_t nr_pages;     /* Total pages allocated to this domain.  */
+    xen_ulong_t shared_info;  /* MACHINE address of shared info struct. */
     uint32_t flags;             /* SIF_xxx flags.                         */
     xen_pfn_t store_mfn;        /* MACHINE page number of shared page.    */
     uint32_t store_evtchn;      /* Event channel for store communication. */
@@ -848,18 +848,18 @@ struct start_info {
         } dom0;
     } console;
     /* THE FOLLOWING ARE ONLY FILLED IN ON INITIAL BOOT (NOT RESUME).     */
-    unsigned long pt_base;      /* VIRTUAL address of page directory.     */
-    unsigned long nr_pt_frames; /* Number of bootstrap p.t. frames.       */
-    unsigned long mfn_list;     /* VIRTUAL address of page-frame list.    */
-    unsigned long mod_start;    /* VIRTUAL address of pre-loaded module   */
+    xen_ulong_t pt_base;      /* VIRTUAL address of page directory.     */
+    xen_ulong_t nr_pt_frames; /* Number of bootstrap p.t. frames.       */
+    xen_ulong_t mfn_list;     /* VIRTUAL address of page-frame list.    */
+    xen_ulong_t mod_start;    /* VIRTUAL address of pre-loaded module   */
                                 /* (PFN of pre-loaded module if           */
                                 /*  SIF_MOD_START_PFN set in flags).      */
-    unsigned long mod_len;      /* Size (bytes) of pre-loaded module.     */
+    xen_ulong_t mod_len;      /* Size (bytes) of pre-loaded module.     */
 #define MAX_GUEST_CMDLINE 1024
     int8_t cmd_line[MAX_GUEST_CMDLINE];
     /* The pfn range here covers both page table and p->m table frames.   */
-    unsigned long first_p2m_pfn;/* 1st pfn forming initial P->M table.    */
-    unsigned long nr_p2m_frames;/* # of pfns forming initial P->M table.  */
+    xen_ulong_t first_p2m_pfn;/* 1st pfn forming initial P->M table.    */
+    xen_ulong_t nr_p2m_frames;/* # of pfns forming initial P->M table.  */
 };
 typedef struct start_info start_info_t;
 
-- 
2.25.1

Re: [PATCH v2 1/4] docs: add xen_ulong_t to the documented integers sizes/alignments

[Julien Grall]

[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]

On Wed, 10 Apr 2024 at 19:47, Stefano Stabellini <stefano.stabellini@amd.com>
wrote:

> xen_ulong_t is widely used in public headers.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> ---
>
> Given that xen_ulong_t is used in public headers there could be a better
> place for documenting it but this was the most straightforward to add.
> ---
>  docs/misra/C-language-toolchain.rst | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/docs/misra/C-language-toolchain.rst
> b/docs/misra/C-language-toolchain.rst
> index 5ddfe7bdbe..7a334260e6 100644
> --- a/docs/misra/C-language-toolchain.rst
> +++ b/docs/misra/C-language-toolchain.rst
> @@ -531,6 +531,17 @@ A summary table of data types, sizes and alignment is
> below:
>       - 64 bits
>       - x86_64, ARMv8-A AArch64, RV64, PPC64
>
> +   * - xen_ulong_t
> +     - 32 bits
> +     - 32 bits
> +     - x86_32
> +
> +   * - xen_ulong_t
> +     - 64 bits
> +     - 64 bits
> +     - x86_64, ARMv8-A AArch64, RV64, PPC64, ARMv8-A AArch32, ARMv8-R
> +       AArch32, ARMv7-A


We support neither ARMv8-R nor ARMv8-A Aarch32.

I could possibly accept the latter because it works to. But the former is
so far misleading.

Cheers,


> +
>     * - long long
>       - 64-bit
>       - 32-bit
> --
> 2.25.1
>
>

[-- Attachment #2: Type: text/html, Size: 2201 bytes --]

Re: [PATCH v2 1/4] docs: add xen_ulong_t to the documented integers sizes/alignments

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 2011 bytes --]

On Wed, 10 Apr 2024, Julien Grall wrote:
> On Wed, 10 Apr 2024 at 19:47, Stefano Stabellini <stefano.stabellini@amd.com> wrote:
>       xen_ulong_t is widely used in public headers.
> 
>       Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>       ---
> 
>       Given that xen_ulong_t is used in public headers there could be a better
>       place for documenting it but this was the most straightforward to add.
>       ---
>        docs/misra/C-language-toolchain.rst | 11 +++++++++++
>        1 file changed, 11 insertions(+)
> 
>       diff --git a/docs/misra/C-language-toolchain.rst b/docs/misra/C-language-toolchain.rst
>       index 5ddfe7bdbe..7a334260e6 100644
>       --- a/docs/misra/C-language-toolchain.rst
>       +++ b/docs/misra/C-language-toolchain.rst
>       @@ -531,6 +531,17 @@ A summary table of data types, sizes and alignment is below:
>             - 64 bits
>             - x86_64, ARMv8-A AArch64, RV64, PPC64
> 
>       +   * - xen_ulong_t
>       +     - 32 bits
>       +     - 32 bits
>       +     - x86_32
>       +
>       +   * - xen_ulong_t
>       +     - 64 bits
>       +     - 64 bits
>       +     - x86_64, ARMv8-A AArch64, RV64, PPC64, ARMv8-A AArch32, ARMv8-R
>       +       AArch32, ARMv7-A
> 
> 
> We support neither ARMv8-R nor ARMv8-A Aarch32.
> 
> I could possibly accept the latter because it works to. But the former is so far misleading.

Yes I think you are right. Moreover this document
(C-language-toolchain.rst) is meant for the Xen build. While this patch
is trying to document the types used in the public headers for the
external-facing ABI.

I'll move the information this patch is adding to a separate document,
specific to the public headers. I will only add the architectures
currently working: I'll add ARMv8-A Aarch32 because although it is
unsupported it is interesting to know the size of xen_ulong_t for
aarch32 in the public headers. I will remove ARMv8-R as it is not
available upstream.

Re: [PATCH v2 1/4] docs: add xen_ulong_t to the documented integers sizes/alignments

[Julien Grall]

[-- Attachment #1: Type: text/plain, Size: 2316 bytes --]

On Thu, 11 Apr 2024 at 18:08, Stefano Stabellini <sstabellini@kernel.org>
wrote:

> On Wed, 10 Apr 2024, Julien Grall wrote:
> > On Wed, 10 Apr 2024 at 19:47, Stefano Stabellini <
> stefano.stabellini@amd.com> wrote:
> >       xen_ulong_t is widely used in public headers.
> >
> >       Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> >       ---
> >
> >       Given that xen_ulong_t is used in public headers there could be a
> better
> >       place for documenting it but this was the most straightforward to
> add.
> >       ---
> >        docs/misra/C-language-toolchain.rst | 11 +++++++++++
> >        1 file changed, 11 insertions(+)
> >
> >       diff --git a/docs/misra/C-language-toolchain.rst
> b/docs/misra/C-language-toolchain.rst
> >       index 5ddfe7bdbe..7a334260e6 100644
> >       --- a/docs/misra/C-language-toolchain.rst
> >       +++ b/docs/misra/C-language-toolchain.rst
> >       @@ -531,6 +531,17 @@ A summary table of data types, sizes and
> alignment is below:
> >             - 64 bits
> >             - x86_64, ARMv8-A AArch64, RV64, PPC64
> >
> >       +   * - xen_ulong_t
> >       +     - 32 bits
> >       +     - 32 bits
> >       +     - x86_32
> >       +
> >       +   * - xen_ulong_t
> >       +     - 64 bits
> >       +     - 64 bits
> >       +     - x86_64, ARMv8-A AArch64, RV64, PPC64, ARMv8-A AArch32,
> ARMv8-R
> >       +       AArch32, ARMv7-A
> >
> >
> > We support neither ARMv8-R nor ARMv8-A Aarch32.
> >
> > I could possibly accept the latter because it works to. But the former
> is so far misleading.
>
> Yes I think you are right. Moreover this document
> (C-language-toolchain.rst) is meant for the Xen build. While this patch
> is trying to document the types used in the public headers for the
> external-facing ABI.
>
> I'll move the information this patch is adding to a separate document,
> specific to the public headers. I will only add the architectures
> currently working: I'll add ARMv8-A Aarch32 because although it is
> unsupported it is interesting to know the size of xen_ulong_t for
> aarch32 in the public headers. I will remove ARMv8-R as it is not
> available upstream.


Thinking a bit more. What about Armv9? Rather than listing each version,
should we instead use ARMv7-A aarch32 and later, ARMv8-A aarch64 and later?

>

[-- Attachment #2: Type: text/html, Size: 3273 bytes --]

Re: [PATCH v2 1/4] docs: add xen_ulong_t to the documented integers sizes/alignments

[Bertrand Marquis]

Hi Julien,

> On 12 Apr 2024, at 15:53, Julien Grall <julien.grall.oss@gmail.com> wrote:
> 
> 
> 
> On Thu, 11 Apr 2024 at 18:08, Stefano Stabellini <sstabellini@kernel.org> wrote:
> On Wed, 10 Apr 2024, Julien Grall wrote:
> > On Wed, 10 Apr 2024 at 19:47, Stefano Stabellini <stefano.stabellini@amd.com> wrote:
> >       xen_ulong_t is widely used in public headers.
> > 
> >       Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> >       ---
> > 
> >       Given that xen_ulong_t is used in public headers there could be a better
> >       place for documenting it but this was the most straightforward to add.
> >       ---
> >        docs/misra/C-language-toolchain.rst | 11 +++++++++++
> >        1 file changed, 11 insertions(+)
> > 
> >       diff --git a/docs/misra/C-language-toolchain.rst b/docs/misra/C-language-toolchain.rst
> >       index 5ddfe7bdbe..7a334260e6 100644
> >       --- a/docs/misra/C-language-toolchain.rst
> >       +++ b/docs/misra/C-language-toolchain.rst
> >       @@ -531,6 +531,17 @@ A summary table of data types, sizes and alignment is below:
> >             - 64 bits
> >             - x86_64, ARMv8-A AArch64, RV64, PPC64
> > 
> >       +   * - xen_ulong_t
> >       +     - 32 bits
> >       +     - 32 bits
> >       +     - x86_32
> >       +
> >       +   * - xen_ulong_t
> >       +     - 64 bits
> >       +     - 64 bits
> >       +     - x86_64, ARMv8-A AArch64, RV64, PPC64, ARMv8-A AArch32, ARMv8-R
> >       +       AArch32, ARMv7-A
> > 
> > 
> > We support neither ARMv8-R nor ARMv8-A Aarch32.
> > 
> > I could possibly accept the latter because it works to. But the former is so far misleading.
> 
> Yes I think you are right. Moreover this document
> (C-language-toolchain.rst) is meant for the Xen build. While this patch
> is trying to document the types used in the public headers for the
> external-facing ABI.
> 
> I'll move the information this patch is adding to a separate document,
> specific to the public headers. I will only add the architectures
> currently working: I'll add ARMv8-A Aarch32 because although it is
> unsupported it is interesting to know the size of xen_ulong_t for
> aarch32 in the public headers. I will remove ARMv8-R as it is not
> available upstream.
> 
> Thinking a bit more. What about Armv9? Rather than listing each version, should we instead use ARMv7-A aarch32 and later, ARMv8-A aarch64 and later?

Definitely you are right here but as for Armv8-R, Armv9 is not something that we explicitely support right now (even though it should work).

Cheers
Bertrand

Re: [PATCH v2 1/4] docs: add xen_ulong_t to the documented integers sizes/alignments

[Julien Grall]

[-- Attachment #1: Type: text/plain, Size: 3125 bytes --]

On Fri, 12 Apr 2024 at 11:30, Bertrand Marquis <Bertrand.Marquis@arm.com>
wrote:

> Hi Julien,
>
> > On 12 Apr 2024, at 15:53, Julien Grall <julien.grall.oss@gmail.com>
> wrote:
> >
> >
> >
> > On Thu, 11 Apr 2024 at 18:08, Stefano Stabellini <sstabellini@kernel.org>
> wrote:
> > On Wed, 10 Apr 2024, Julien Grall wrote:
> > > On Wed, 10 Apr 2024 at 19:47, Stefano Stabellini <
> stefano.stabellini@amd.com> wrote:
> > >       xen_ulong_t is widely used in public headers.
> > >
> > >       Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> > >       ---
> > >
> > >       Given that xen_ulong_t is used in public headers there could be
> a better
> > >       place for documenting it but this was the most straightforward
> to add.
> > >       ---
> > >        docs/misra/C-language-toolchain.rst | 11 +++++++++++
> > >        1 file changed, 11 insertions(+)
> > >
> > >       diff --git a/docs/misra/C-language-toolchain.rst
> b/docs/misra/C-language-toolchain.rst
> > >       index 5ddfe7bdbe..7a334260e6 100644
> > >       --- a/docs/misra/C-language-toolchain.rst
> > >       +++ b/docs/misra/C-language-toolchain.rst
> > >       @@ -531,6 +531,17 @@ A summary table of data types, sizes and
> alignment is below:
> > >             - 64 bits
> > >             - x86_64, ARMv8-A AArch64, RV64, PPC64
> > >
> > >       +   * - xen_ulong_t
> > >       +     - 32 bits
> > >       +     - 32 bits
> > >       +     - x86_32
> > >       +
> > >       +   * - xen_ulong_t
> > >       +     - 64 bits
> > >       +     - 64 bits
> > >       +     - x86_64, ARMv8-A AArch64, RV64, PPC64, ARMv8-A AArch32,
> ARMv8-R
> > >       +       AArch32, ARMv7-A
> > >
> > >
> > > We support neither ARMv8-R nor ARMv8-A Aarch32.
> > >
> > > I could possibly accept the latter because it works to. But the former
> is so far misleading.
> >
> > Yes I think you are right. Moreover this document
> > (C-language-toolchain.rst) is meant for the Xen build. While this patch
> > is trying to document the types used in the public headers for the
> > external-facing ABI.
> >
> > I'll move the information this patch is adding to a separate document,
> > specific to the public headers. I will only add the architectures
> > currently working: I'll add ARMv8-A Aarch32 because although it is
> > unsupported it is interesting to know the size of xen_ulong_t for
> > aarch32 in the public headers. I will remove ARMv8-R as it is not
> > available upstream.
> >
> > Thinking a bit more. What about Armv9? Rather than listing each version,
> should we instead use ARMv7-A aarch32 and later, ARMv8-A aarch64 and later?
>
> Definitely you are right here but as for Armv8-R, Armv9 is not something
> that we explicitely support right now (even though it should work).


I am confused with the comparison. I thought you can’t boot Xen at all on
Armv8-R. But you can on Armv9-A as this just Armv8-A + features the
software don’t need to use.

Did you intend to draw the comparison with Armv8-A Aarch32?

Cheers,


>
> Cheers
> Bertrand
>
>
>

[-- Attachment #2: Type: text/html, Size: 4587 bytes --]

Re: [PATCH v2 1/4] docs: add xen_ulong_t to the documented integers sizes/alignments

[Bertrand Marquis]

Hi Julien,

> On 12 Apr 2024, at 19:01, Julien Grall <julien.grall.oss@gmail.com> wrote:
> 
> 
> 
> On Fri, 12 Apr 2024 at 11:30, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote:
> Hi Julien,
> 
> > On 12 Apr 2024, at 15:53, Julien Grall <julien.grall.oss@gmail.com> wrote:
> > 
> > 
> > 
> > On Thu, 11 Apr 2024 at 18:08, Stefano Stabellini <sstabellini@kernel.org> wrote:
> > On Wed, 10 Apr 2024, Julien Grall wrote:
> > > On Wed, 10 Apr 2024 at 19:47, Stefano Stabellini <stefano.stabellini@amd.com> wrote:
> > >       xen_ulong_t is widely used in public headers.
> > > 
> > >       Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> > >       ---
> > > 
> > >       Given that xen_ulong_t is used in public headers there could be a better
> > >       place for documenting it but this was the most straightforward to add.
> > >       ---
> > >        docs/misra/C-language-toolchain.rst | 11 +++++++++++
> > >        1 file changed, 11 insertions(+)
> > > 
> > >       diff --git a/docs/misra/C-language-toolchain.rst b/docs/misra/C-language-toolchain.rst
> > >       index 5ddfe7bdbe..7a334260e6 100644
> > >       --- a/docs/misra/C-language-toolchain.rst
> > >       +++ b/docs/misra/C-language-toolchain.rst
> > >       @@ -531,6 +531,17 @@ A summary table of data types, sizes and alignment is below:
> > >             - 64 bits
> > >             - x86_64, ARMv8-A AArch64, RV64, PPC64
> > > 
> > >       +   * - xen_ulong_t
> > >       +     - 32 bits
> > >       +     - 32 bits
> > >       +     - x86_32
> > >       +
> > >       +   * - xen_ulong_t
> > >       +     - 64 bits
> > >       +     - 64 bits
> > >       +     - x86_64, ARMv8-A AArch64, RV64, PPC64, ARMv8-A AArch32, ARMv8-R
> > >       +       AArch32, ARMv7-A
> > > 
> > > 
> > > We support neither ARMv8-R nor ARMv8-A Aarch32.
> > > 
> > > I could possibly accept the latter because it works to. But the former is so far misleading.
> > 
> > Yes I think you are right. Moreover this document
> > (C-language-toolchain.rst) is meant for the Xen build. While this patch
> > is trying to document the types used in the public headers for the
> > external-facing ABI.
> > 
> > I'll move the information this patch is adding to a separate document,
> > specific to the public headers. I will only add the architectures
> > currently working: I'll add ARMv8-A Aarch32 because although it is
> > unsupported it is interesting to know the size of xen_ulong_t for
> > aarch32 in the public headers. I will remove ARMv8-R as it is not
> > available upstream.
> > 
> > Thinking a bit more. What about Armv9? Rather than listing each version, should we instead use ARMv7-A aarch32 and later, ARMv8-A aarch64 and later?
> 
> Definitely you are right here but as for Armv8-R, Armv9 is not something that we explicitely support right now (even though it should work).
> 
> I am confused with the comparison. I thought you can’t boot Xen at all on Armv8-R. But you can on Armv9-A as this just Armv8-A + features the software don’t need to use.
> 
> Did you intend to draw the comparison with Armv8-A Aarch32?

Yes in my mind armv9 even if currently working it is not something officially supported so it is in the same state as armv8 aarch32.

Armv8-R currently cannot work at all so it is a different state.

Cheers
Bertrand

> 
> Cheers,
> 
> 
> 
> Cheers
> Bertrand

Re: [PATCH v2 1/4] docs: add xen_ulong_t to the documented integers sizes/alignments

[Julien Grall]

Hi Bertrand,

On 15/04/2024 08:48, Bertrand Marquis wrote:
> Hi Julien,
> 
>> On 12 Apr 2024, at 19:01, Julien Grall <julien.grall.oss@gmail.com> wrote:
>>
>>
>>
>> On Fri, 12 Apr 2024 at 11:30, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote:
>> Hi Julien,
>>
>>> On 12 Apr 2024, at 15:53, Julien Grall <julien.grall.oss@gmail.com> wrote:
>>>
>>>
>>>
>>> On Thu, 11 Apr 2024 at 18:08, Stefano Stabellini <sstabellini@kernel.org> wrote:
>>> On Wed, 10 Apr 2024, Julien Grall wrote:
>>>> On Wed, 10 Apr 2024 at 19:47, Stefano Stabellini <stefano.stabellini@amd.com> wrote:
>>>>        xen_ulong_t is widely used in public headers.
>>>>
>>>>        Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>>>        ---
>>>>
>>>>        Given that xen_ulong_t is used in public headers there could be a better
>>>>        place for documenting it but this was the most straightforward to add.
>>>>        ---
>>>>         docs/misra/C-language-toolchain.rst | 11 +++++++++++
>>>>         1 file changed, 11 insertions(+)
>>>>
>>>>        diff --git a/docs/misra/C-language-toolchain.rst b/docs/misra/C-language-toolchain.rst
>>>>        index 5ddfe7bdbe..7a334260e6 100644
>>>>        --- a/docs/misra/C-language-toolchain.rst
>>>>        +++ b/docs/misra/C-language-toolchain.rst
>>>>        @@ -531,6 +531,17 @@ A summary table of data types, sizes and alignment is below:
>>>>              - 64 bits
>>>>              - x86_64, ARMv8-A AArch64, RV64, PPC64
>>>>
>>>>        +   * - xen_ulong_t
>>>>        +     - 32 bits
>>>>        +     - 32 bits
>>>>        +     - x86_32
>>>>        +
>>>>        +   * - xen_ulong_t
>>>>        +     - 64 bits
>>>>        +     - 64 bits
>>>>        +     - x86_64, ARMv8-A AArch64, RV64, PPC64, ARMv8-A AArch32, ARMv8-R
>>>>        +       AArch32, ARMv7-A
>>>>
>>>>
>>>> We support neither ARMv8-R nor ARMv8-A Aarch32.
>>>>
>>>> I could possibly accept the latter because it works to. But the former is so far misleading.
>>>
>>> Yes I think you are right. Moreover this document
>>> (C-language-toolchain.rst) is meant for the Xen build. While this patch
>>> is trying to document the types used in the public headers for the
>>> external-facing ABI.
>>>
>>> I'll move the information this patch is adding to a separate document,
>>> specific to the public headers. I will only add the architectures
>>> currently working: I'll add ARMv8-A Aarch32 because although it is
>>> unsupported it is interesting to know the size of xen_ulong_t for
>>> aarch32 in the public headers. I will remove ARMv8-R as it is not
>>> available upstream.
>>>
>>> Thinking a bit more. What about Armv9? Rather than listing each version, should we instead use ARMv7-A aarch32 and later, ARMv8-A aarch64 and later?
>>
>> Definitely you are right here but as for Armv8-R, Armv9 is not something that we explicitely support right now (even though it should work).
>>
>> I am confused with the comparison. I thought you can’t boot Xen at all on Armv8-R. But you can on Armv9-A as this just Armv8-A + features the software don’t need to use.
>>
>> Did you intend to draw the comparison with Armv8-A Aarch32?
> 
> Yes in my mind armv9 even if currently working it is not something officially supported so it is in the same state as armv8 aarch32.

AFAICT, Stefano said he will add ARMv8-A AArch32, so we should be 
consistent and add Armv9-A in the list.

Cheers,

-- 
Julien Grall

Re: [PATCH v2 1/4] docs: add xen_ulong_t to the documented integers sizes/alignments

[Bertrand Marquis]

Hi Julien,

> On 15 Apr 2024, at 12:08, Julien Grall <julien@xen.org> wrote:
> 
> Hi Bertrand,
> 
> On 15/04/2024 08:48, Bertrand Marquis wrote:
>> Hi Julien,
>>> On 12 Apr 2024, at 19:01, Julien Grall <julien.grall.oss@gmail.com> wrote:
>>> 
>>> 
>>> 
>>> On Fri, 12 Apr 2024 at 11:30, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote:
>>> Hi Julien,
>>> 
>>>> On 12 Apr 2024, at 15:53, Julien Grall <julien.grall.oss@gmail.com> wrote:
>>>> 
>>>> 
>>>> 
>>>> On Thu, 11 Apr 2024 at 18:08, Stefano Stabellini <sstabellini@kernel.org> wrote:
>>>> On Wed, 10 Apr 2024, Julien Grall wrote:
>>>>> On Wed, 10 Apr 2024 at 19:47, Stefano Stabellini <stefano.stabellini@amd.com> wrote:
>>>>>       xen_ulong_t is widely used in public headers.
>>>>> 
>>>>>       Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>>>>       ---
>>>>> 
>>>>>       Given that xen_ulong_t is used in public headers there could be a better
>>>>>       place for documenting it but this was the most straightforward to add.
>>>>>       ---
>>>>>        docs/misra/C-language-toolchain.rst | 11 +++++++++++
>>>>>        1 file changed, 11 insertions(+)
>>>>> 
>>>>>       diff --git a/docs/misra/C-language-toolchain.rst b/docs/misra/C-language-toolchain.rst
>>>>>       index 5ddfe7bdbe..7a334260e6 100644
>>>>>       --- a/docs/misra/C-language-toolchain.rst
>>>>>       +++ b/docs/misra/C-language-toolchain.rst
>>>>>       @@ -531,6 +531,17 @@ A summary table of data types, sizes and alignment is below:
>>>>>             - 64 bits
>>>>>             - x86_64, ARMv8-A AArch64, RV64, PPC64
>>>>> 
>>>>>       +   * - xen_ulong_t
>>>>>       +     - 32 bits
>>>>>       +     - 32 bits
>>>>>       +     - x86_32
>>>>>       +
>>>>>       +   * - xen_ulong_t
>>>>>       +     - 64 bits
>>>>>       +     - 64 bits
>>>>>       +     - x86_64, ARMv8-A AArch64, RV64, PPC64, ARMv8-A AArch32, ARMv8-R
>>>>>       +       AArch32, ARMv7-A
>>>>> 
>>>>> 
>>>>> We support neither ARMv8-R nor ARMv8-A Aarch32.
>>>>> 
>>>>> I could possibly accept the latter because it works to. But the former is so far misleading.
>>>> 
>>>> Yes I think you are right. Moreover this document
>>>> (C-language-toolchain.rst) is meant for the Xen build. While this patch
>>>> is trying to document the types used in the public headers for the
>>>> external-facing ABI.
>>>> 
>>>> I'll move the information this patch is adding to a separate document,
>>>> specific to the public headers. I will only add the architectures
>>>> currently working: I'll add ARMv8-A Aarch32 because although it is
>>>> unsupported it is interesting to know the size of xen_ulong_t for
>>>> aarch32 in the public headers. I will remove ARMv8-R as it is not
>>>> available upstream.
>>>> 
>>>> Thinking a bit more. What about Armv9? Rather than listing each version, should we instead use ARMv7-A aarch32 and later, ARMv8-A aarch64 and later?
>>> 
>>> Definitely you are right here but as for Armv8-R, Armv9 is not something that we explicitely support right now (even though it should work).
>>> 
>>> I am confused with the comparison. I thought you can’t boot Xen at all on Armv8-R. But you can on Armv9-A as this just Armv8-A + features the software don’t need to use.
>>> 
>>> Did you intend to draw the comparison with Armv8-A Aarch32?
>> Yes in my mind armv9 even if currently working it is not something officially supported so it is in the same state as armv8 aarch32.
> 
> AFAICT, Stefano said he will add ARMv8-A AArch32, so we should be consistent and add Armv9-A in the list.

Yes that makes sense, I agree.

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall

Re: [PATCH v2 2/4] public: s/int/int32_t

[Jan Beulich]

On 11.04.2024 01:47, Stefano Stabellini wrote:
> Straightforward int -> int32_t and unsigned int -> uint32_t replacements
> in public headers. No ABI or semantic changes intended.

I understand there is a limited in scope goal here, but once doing type
adjustments I think it ought to at least be considered to go one step
further and ...

> --- a/xen/include/public/kexec.h
> +++ b/xen/include/public/kexec.h
> @@ -105,7 +105,7 @@ typedef struct xen_kexec_image {
>   */
>  #define KEXEC_CMD_kexec                 0
>  typedef struct xen_kexec_exec {
> -    int type;
> +    int32_t type;
>  } xen_kexec_exec_t;
>  
>  /*
> @@ -116,7 +116,7 @@ typedef struct xen_kexec_exec {
>  #define KEXEC_CMD_kexec_load_v1         1 /* obsolete since 0x00040400 */
>  #define KEXEC_CMD_kexec_unload_v1       2 /* obsolete since 0x00040400 */
>  typedef struct xen_kexec_load_v1 {
> -    int type;
> +    int32_t type;
>      xen_kexec_image_t image;
>  } xen_kexec_load_v1_t;
>  
> @@ -143,8 +143,8 @@ typedef struct xen_kexec_load_v1 {
>   */
>  #define KEXEC_CMD_kexec_get_range       3
>  typedef struct xen_kexec_range {
> -    int range;
> -    int nr;
> +    int32_t range;
> +    int32_t nr;
>      unsigned long size;
>      unsigned long start;
>  } xen_kexec_range_t;

... change e.g. all of the ones touched here to uint32_t. None have a
purpose of permitting negative values afaict.

> --- a/xen/include/public/physdev.h
> +++ b/xen/include/public/physdev.h
> @@ -142,17 +142,17 @@ DEFINE_XEN_GUEST_HANDLE(physdev_irq_t);
>  struct physdev_map_pirq {
>      domid_t domid;
>      /* IN */
> -    int type;
> +    int32_t type;
>      /* IN (ignored for ..._MULTI_MSI) */
> -    int index;
> +    int32_t index;
>      /* IN or OUT */
> -    int pirq;
> +    int32_t pirq;
>      /* IN - high 16 bits hold segment for ..._MSI_SEG and ..._MULTI_MSI */
> -    int bus;
> +    int32_t bus;
>      /* IN */
> -    int devfn;
> +    int32_t devfn;
>      /* IN (also OUT for ..._MULTI_MSI) */
> -    int entry_nr;
> +    int32_t entry_nr;
>      /* IN */
>      uint64_t table_base;
>  };

Whereas iirc at least "index" here would need keeping as a signed quantity.
It may therefore make sense to split the patch (I'd be fine ack-ing the
memory.h and sched.h changes right away).

Jan

Re: [PATCH v2 3/4] xen/public: s/unsigned long/xen_ulong_t

[Jan Beulich]

On 11.04.2024 01:47, Stefano Stabellini wrote:
> The goal is to use only fixed-size integers in public headers, such as
> uint32_t and uint64_t.
> 
> However, there are cases where the ABI changes depending on the
> architecture. In those cases, adding #ifdefs might be the clearest
> solution but it is also cumbersome. We already define a xen_ulong_t type
> which is widely used in public headers and it is defined differently by
> architecture.
> 
> Instead of unsigned long, use xen_ulong_t in public headers:
> - it makes it clearer that size might change by arch
> - it gets us closer to the goal of no unfixed-size integers in public
> headers
> 
> Note that unsigned long and xen_ulong_t are the same thing on x86 (both
> 32-bit and 64-bit) but they differ on all other arches. However, all
> the interfaces with xen_ulong_t or unsigned long are x86-only interfaces
> today. Thus, this patch doesn't introduce any ABI or semantic changes.

And it is presumably because of this that the conversion wasn't done for
these when it was done elsewhere for the Arm port, many years ago.

> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>

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

Jan

Re: [PATCH v2 4/4] xen/public: replace remaining int and long

[Jan Beulich]

On 11.04.2024 01:47, Stefano Stabellini wrote:
> --- a/xen/include/public/arch-x86/hvm/save.h
> +++ b/xen/include/public/arch-x86/hvm/save.h
> @@ -415,7 +415,7 @@ struct hvm_hw_pci_irqs {
>       * Indexed by: device*4 + INTx#.
>       */
>      union {
> -        unsigned long i[16 / sizeof (unsigned long)]; /* DECLARE_BITMAP(i, 32*4); */
> +        xen_ulong_t i[16 / sizeof (xen_ulong_t)]; /* DECLARE_BITMAP(i, 32*4); */
>          uint64_t pad[2];
>      };
>  };
> @@ -428,7 +428,7 @@ struct hvm_hw_isa_irqs {
>       * Indexed by ISA IRQ (assumes no ISA-device IRQ sharing).
>       */
>      union {
> -        unsigned long i[1];  /* DECLARE_BITMAP(i, 16); */
> +        xen_ulong_t i[1];  /* DECLARE_BITMAP(i, 16); */
>          uint64_t pad[1];
>      };
>  };

Taking just this as example: Why is this not part of patch 3? If there was
"x86" somewhere in the title, I could probably see a reason for such
separation.

> --- a/xen/include/public/arch-x86/xen-x86_32.h
> +++ b/xen/include/public/arch-x86/xen-x86_32.h
> @@ -68,7 +68,7 @@
>  #define MACH2PHYS_VIRT_END    xen_mk_ulong(__MACH2PHYS_VIRT_END)
>  #define MACH2PHYS_NR_ENTRIES  ((MACH2PHYS_VIRT_END-MACH2PHYS_VIRT_START)>>2)
>  #ifndef machine_to_phys_mapping
> -#define machine_to_phys_mapping ((unsigned long *)MACH2PHYS_VIRT_START)
> +#define machine_to_phys_mapping ((xen_ulong_t *)MACH2PHYS_VIRT_START)

While making this (and the corresponding 64-bit) adjustment, we ought to
at least consider to also add const. The overall change is at risk of being
disruptive to consumers anyway, in case they use less lax compilers than
e.g. gcc and Clang are: "unsigned int" and "uint32_t", even if both 32 bits
wide, don't necessarily need to be treated as the same type by the compiler.
In fact portability of code would be increased if compilers insisted on
clearly distinguishing the two.

In fact because of this it may want considering to split the conversions of
"long" from those of "int", just to cover the remote possibility of the
latter needing reverting from some reason.

> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -141,7 +141,7 @@ struct trap_info {
>      uint8_t       vector;  /* exception vector                              */
>      uint8_t       flags;   /* 0-3: privilege level; 4: clear event enable?  */
>      uint16_t      cs;      /* code selector                                 */
> -    unsigned long address; /* code offset                                   */
> +    xen_ulong_t address;   /* code offset                                   */
>  };

Nit: Please add two padding blanks to retain columnar arrangement of field
names.

> @@ -174,36 +174,36 @@ struct vcpu_guest_context {
>  #define VGCF_syscall_disables_events   (1<<_VGCF_syscall_disables_events)
>  #define _VGCF_online                   5
>  #define VGCF_online                    (1<<_VGCF_online)
> -    unsigned long flags;                    /* VGCF_* flags                 */
> -    struct cpu_user_regs user_regs;         /* User-level CPU registers     */
> -    struct trap_info trap_ctxt[256];        /* Virtual IDT                  */
> -    unsigned long ldt_base, ldt_ents;       /* LDT (linear address, # ents) */
> -    unsigned long gdt_frames[16], gdt_ents; /* GDT (machine frames, # ents) */
> -    unsigned long kernel_ss, kernel_sp;     /* Virtual TSS (only SS1/SP1)   */
> +    xen_ulong_t flags;                    /* VGCF_* flags                 */
> +    struct cpu_user_regs user_regs;       /* User-level CPU registers     */
> +    struct trap_info trap_ctxt[256];      /* Virtual IDT                  */

Please can these two lines remain untouched, and other lines instead have
padding blanks added to keep comments aligned?

> @@ -228,7 +228,7 @@ struct arch_shared_info {
>       * is to be used.
>       */
>      xen_pfn_t     pfn_to_mfn_frame_list_list;
> -    unsigned long nmi_reason;
> +    xen_ulong_t nmi_reason;

Nit: Here there again want to be two padding blanks.

Jan