[PATCH v3 2/6] xen/riscv: introduce asm/types.h header file

Oleksii Kurochko posted 6 patches 3 years ago
There is a newer version of this series
[PATCH v3 2/6] xen/riscv: introduce asm/types.h header file
Posted by Oleksii Kurochko 3 years ago
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V3:
    - Nothing changed
---
Changes in V2:
    - Remove unneeded now types from <asm/types.h>
---
 xen/arch/riscv/include/asm/types.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/types.h

diff --git a/xen/arch/riscv/include/asm/types.h b/xen/arch/riscv/include/asm/types.h
new file mode 100644
index 0000000000..fbe352ef20
--- /dev/null
+++ b/xen/arch/riscv/include/asm/types.h
@@ -0,0 +1,22 @@
+#ifndef __RISCV_TYPES_H__
+#define __RISCV_TYPES_H__
+
+#ifndef __ASSEMBLY__
+
+#if defined(__SIZE_TYPE__)
+typedef __SIZE_TYPE__ size_t;
+#else
+typedef unsigned long size_t;
+#endif
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __RISCV_TYPES_H__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.38.1
Re: [PATCH v3 2/6] xen/riscv: introduce asm/types.h header file
Posted by Jan Beulich 3 years ago
On 10.01.2023 16:17, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in V3:
>     - Nothing changed
> ---
> Changes in V2:
>     - Remove unneeded now types from <asm/types.h>

I guess you went a little too far: Types used in common code, even if you
do not build that yet, will want declaring right away imo. Of course I
should finally try and get rid of at least some of the being-phased-out
ones (s8 and s16 look to be relatively low hanging fruit, for example,
and of these only s16 looks to be used in common code) ...

Jan
Re: [PATCH v3 2/6] xen/riscv: introduce asm/types.h header file
Posted by Oleksii 3 years ago
On Tue, 2023-01-10 at 17:58 +0100, Jan Beulich wrote:
> On 10.01.2023 16:17, Oleksii Kurochko wrote:
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > ---
> > Changes in V3:
> >     - Nothing changed
> > ---
> > Changes in V2:
> >     - Remove unneeded now types from <asm/types.h>
> 
> I guess you went a little too far: Types used in common code, even if
> you
It looks then I didn't understand which one of types are needed.

In "[PATCH v1 2/8] xen/riscv: introduce asm/types.h header file" all
types were introduced as most of them are used in <xen/types.h>:
__{u|s}{8|16|32|64}. Thereby it looks like the following types in
<asm/types.h> should be present from the start:
  typedef __signed__ char __s8;
  typedef unsigned char __u8;

  typedef __signed__ short __s16;
  typedef unsigned short __u16;

  typedef __signed__ int __s32;
  typedef unsigned int __u32;

  #if defined(__GNUC__) && !defined(__STRICT_ANSI__)
  #if defined(CONFIG_RISCV_32)
    typedef __signed__ long long __s64;
    typedef unsigned long long __u64;
  #elif defined (CONFIG_RISCV_64)
    typedef __signed__ long __s64;
    typedef unsigned long __u64;
  #endif
  #endif

 Then it turns out that there is no any sense in:
  typedef signed char s8;
  typedef unsigned char u8;

  typedef signed short s16;
  typedef unsigned short u16;

  typedef signed int s32;
  typedef unsigned int u32;

  typedef signed long long s64;
  typedef unsigned long long u64;
    OR
  typedef signed long s64;
  typedef unsigned long u64;
As I understand instead of them should be used: {u|s}int<N>_t.

All other types such as {v,p}addr_t, register_t and definitions
PRIvaddr, INVALID_PADDR, PRIpaadr, PRIregister should be present in
<asm/types.h> from the start.

Am I right?
> do not build that yet, will want declaring right away imo. Of course
> I
> should finally try and get rid of at least some of the being-phased-
> out
> ones (s8 and s16 look to be relatively low hanging fruit, for
> example,
> and of these only s16 looks to be used in common code) ...
> 
> Jan
~Oleksii
Re: [PATCH v3 2/6] xen/riscv: introduce asm/types.h header file
Posted by Jan Beulich 3 years ago
On 11.01.2023 09:47, Oleksii wrote:
> On Tue, 2023-01-10 at 17:58 +0100, Jan Beulich wrote:
>> On 10.01.2023 16:17, Oleksii Kurochko wrote:
>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>> ---
>>> Changes in V3:
>>>     - Nothing changed
>>> ---
>>> Changes in V2:
>>>     - Remove unneeded now types from <asm/types.h>
>>
>> I guess you went a little too far: Types used in common code, even if
>> you
> It looks then I didn't understand which one of types are needed.
> 
> In "[PATCH v1 2/8] xen/riscv: introduce asm/types.h header file" all
> types were introduced as most of them are used in <xen/types.h>:
> __{u|s}{8|16|32|64}. Thereby it looks like the following types in
> <asm/types.h> should be present from the start:
>   typedef __signed__ char __s8;
>   typedef unsigned char __u8;
> 
>   typedef __signed__ short __s16;
>   typedef unsigned short __u16;
> 
>   typedef __signed__ int __s32;
>   typedef unsigned int __u32;
> 
>   #if defined(__GNUC__) && !defined(__STRICT_ANSI__)
>   #if defined(CONFIG_RISCV_32)
>     typedef __signed__ long long __s64;
>     typedef unsigned long long __u64;
>   #elif defined (CONFIG_RISCV_64)
>     typedef __signed__ long __s64;
>     typedef unsigned long __u64;
>   #endif
>   #endif
> 
>  Then it turns out that there is no any sense in:
>   typedef signed char s8;
>   typedef unsigned char u8;
> 
>   typedef signed short s16;
>   typedef unsigned short u16;
> 
>   typedef signed int s32;
>   typedef unsigned int u32;
> 
>   typedef signed long long s64;
>   typedef unsigned long long u64;
>     OR
>   typedef signed long s64;
>   typedef unsigned long u64;
> As I understand instead of them should be used: {u|s}int<N>_t.

Hmm, the situation is worse than I thought (recalled) it was: You're
right, xen/types.h actually uses __{u,s}<N>. So I'm sorry for mis-
guiding you; we'll need to do more cleanup first for asm/types.h to
become smaller.

Jan

Re: [PATCH v3 2/6] xen/riscv: introduce asm/types.h header file
Posted by Xenia Ragiadakou 3 years ago
On 1/11/23 11:08, Jan Beulich wrote:
> On 11.01.2023 09:47, Oleksii wrote:
>> On Tue, 2023-01-10 at 17:58 +0100, Jan Beulich wrote:
>>> On 10.01.2023 16:17, Oleksii Kurochko wrote:
>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>> ---
>>>> Changes in V3:
>>>>      - Nothing changed
>>>> ---
>>>> Changes in V2:
>>>>      - Remove unneeded now types from <asm/types.h>
>>>
>>> I guess you went a little too far: Types used in common code, even if
>>> you
>> It looks then I didn't understand which one of types are needed.
>>
>> In "[PATCH v1 2/8] xen/riscv: introduce asm/types.h header file" all
>> types were introduced as most of them are used in <xen/types.h>:
>> __{u|s}{8|16|32|64}. Thereby it looks like the following types in
>> <asm/types.h> should be present from the start:
>>    typedef __signed__ char __s8;
>>    typedef unsigned char __u8;
>>
>>    typedef __signed__ short __s16;
>>    typedef unsigned short __u16;
>>
>>    typedef __signed__ int __s32;
>>    typedef unsigned int __u32;
>>
>>    #if defined(__GNUC__) && !defined(__STRICT_ANSI__)
>>    #if defined(CONFIG_RISCV_32)
>>      typedef __signed__ long long __s64;
>>      typedef unsigned long long __u64;
>>    #elif defined (CONFIG_RISCV_64)
>>      typedef __signed__ long __s64;
>>      typedef unsigned long __u64;
>>    #endif
>>    #endif
>>
>>   Then it turns out that there is no any sense in:
>>    typedef signed char s8;
>>    typedef unsigned char u8;
>>
>>    typedef signed short s16;
>>    typedef unsigned short u16;
>>
>>    typedef signed int s32;
>>    typedef unsigned int u32;
>>
>>    typedef signed long long s64;
>>    typedef unsigned long long u64;
>>      OR
>>    typedef signed long s64;
>>    typedef unsigned long u64;
>> As I understand instead of them should be used: {u|s}int<N>_t.
> 
> Hmm, the situation is worse than I thought (recalled) it was: You're
> right, xen/types.h actually uses __{u,s}<N>. So I'm sorry for mis-
> guiding you; we'll need to do more cleanup first for asm/types.h to
> become smaller.

What is the reason for not declaring __{u,s}<N> directly in xen/types.h 
as type alias to {u,s}<N>?

IIUC __{u,s}<N> and {u,s}<N> are needed by code ported from Linux while 
Xen code should use {u|s}int<N>_t instead, right?

-- 
Xenia

Re: [PATCH v3 2/6] xen/riscv: introduce asm/types.h header file
Posted by Jan Beulich 3 years ago
On 11.01.2023 11:22, Xenia Ragiadakou wrote:
> 
> On 1/11/23 11:08, Jan Beulich wrote:
>> On 11.01.2023 09:47, Oleksii wrote:
>>> On Tue, 2023-01-10 at 17:58 +0100, Jan Beulich wrote:
>>>> On 10.01.2023 16:17, Oleksii Kurochko wrote:
>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>> ---
>>>>> Changes in V3:
>>>>>      - Nothing changed
>>>>> ---
>>>>> Changes in V2:
>>>>>      - Remove unneeded now types from <asm/types.h>
>>>>
>>>> I guess you went a little too far: Types used in common code, even if
>>>> you
>>> It looks then I didn't understand which one of types are needed.
>>>
>>> In "[PATCH v1 2/8] xen/riscv: introduce asm/types.h header file" all
>>> types were introduced as most of them are used in <xen/types.h>:
>>> __{u|s}{8|16|32|64}. Thereby it looks like the following types in
>>> <asm/types.h> should be present from the start:
>>>    typedef __signed__ char __s8;
>>>    typedef unsigned char __u8;
>>>
>>>    typedef __signed__ short __s16;
>>>    typedef unsigned short __u16;
>>>
>>>    typedef __signed__ int __s32;
>>>    typedef unsigned int __u32;
>>>
>>>    #if defined(__GNUC__) && !defined(__STRICT_ANSI__)
>>>    #if defined(CONFIG_RISCV_32)
>>>      typedef __signed__ long long __s64;
>>>      typedef unsigned long long __u64;
>>>    #elif defined (CONFIG_RISCV_64)
>>>      typedef __signed__ long __s64;
>>>      typedef unsigned long __u64;
>>>    #endif
>>>    #endif
>>>
>>>   Then it turns out that there is no any sense in:
>>>    typedef signed char s8;
>>>    typedef unsigned char u8;
>>>
>>>    typedef signed short s16;
>>>    typedef unsigned short u16;
>>>
>>>    typedef signed int s32;
>>>    typedef unsigned int u32;
>>>
>>>    typedef signed long long s64;
>>>    typedef unsigned long long u64;
>>>      OR
>>>    typedef signed long s64;
>>>    typedef unsigned long u64;
>>> As I understand instead of them should be used: {u|s}int<N>_t.
>>
>> Hmm, the situation is worse than I thought (recalled) it was: You're
>> right, xen/types.h actually uses __{u,s}<N>. So I'm sorry for mis-
>> guiding you; we'll need to do more cleanup first for asm/types.h to
>> become smaller.
> 
> What is the reason for not declaring __{u,s}<N> directly in xen/types.h 
> as type alias to {u,s}<N>?
> 
> IIUC __{u,s}<N> and {u,s}<N> are needed by code ported from Linux while 
> Xen code should use {u|s}int<N>_t instead, right?

Yes. Hence in the long run only Linux files should get to see __{u,s}<N>
and {u,s}<N>, perhaps from a new linux-types.h.

Jan

Re: [PATCH v3 2/6] xen/riscv: introduce asm/types.h header file
Posted by Xenia Ragiadakou 3 years ago
On 1/11/23 13:38, Jan Beulich wrote:
> On 11.01.2023 11:22, Xenia Ragiadakou wrote:
>>
>> On 1/11/23 11:08, Jan Beulich wrote:
>>> On 11.01.2023 09:47, Oleksii wrote:
>>>> On Tue, 2023-01-10 at 17:58 +0100, Jan Beulich wrote:
>>>>> On 10.01.2023 16:17, Oleksii Kurochko wrote:
>>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>>> ---
>>>>>> Changes in V3:
>>>>>>       - Nothing changed
>>>>>> ---
>>>>>> Changes in V2:
>>>>>>       - Remove unneeded now types from <asm/types.h>
>>>>>
>>>>> I guess you went a little too far: Types used in common code, even if
>>>>> you
>>>> It looks then I didn't understand which one of types are needed.
>>>>
>>>> In "[PATCH v1 2/8] xen/riscv: introduce asm/types.h header file" all
>>>> types were introduced as most of them are used in <xen/types.h>:
>>>> __{u|s}{8|16|32|64}. Thereby it looks like the following types in
>>>> <asm/types.h> should be present from the start:
>>>>     typedef __signed__ char __s8;
>>>>     typedef unsigned char __u8;
>>>>
>>>>     typedef __signed__ short __s16;
>>>>     typedef unsigned short __u16;
>>>>
>>>>     typedef __signed__ int __s32;
>>>>     typedef unsigned int __u32;
>>>>
>>>>     #if defined(__GNUC__) && !defined(__STRICT_ANSI__)
>>>>     #if defined(CONFIG_RISCV_32)
>>>>       typedef __signed__ long long __s64;
>>>>       typedef unsigned long long __u64;
>>>>     #elif defined (CONFIG_RISCV_64)
>>>>       typedef __signed__ long __s64;
>>>>       typedef unsigned long __u64;
>>>>     #endif
>>>>     #endif
>>>>
>>>>    Then it turns out that there is no any sense in:
>>>>     typedef signed char s8;
>>>>     typedef unsigned char u8;
>>>>
>>>>     typedef signed short s16;
>>>>     typedef unsigned short u16;
>>>>
>>>>     typedef signed int s32;
>>>>     typedef unsigned int u32;
>>>>
>>>>     typedef signed long long s64;
>>>>     typedef unsigned long long u64;
>>>>       OR
>>>>     typedef signed long s64;
>>>>     typedef unsigned long u64;
>>>> As I understand instead of them should be used: {u|s}int<N>_t.
>>>
>>> Hmm, the situation is worse than I thought (recalled) it was: You're
>>> right, xen/types.h actually uses __{u,s}<N>. So I'm sorry for mis-
>>> guiding you; we'll need to do more cleanup first for asm/types.h to
>>> become smaller.
>>
>> What is the reason for not declaring __{u,s}<N> directly in xen/types.h
>> as type alias to {u,s}<N>?
>>
>> IIUC __{u,s}<N> and {u,s}<N> are needed by code ported from Linux while
>> Xen code should use {u|s}int<N>_t instead, right?
> 
> Yes. Hence in the long run only Linux files should get to see __{u,s}<N>
> and {u,s}<N>, perhaps from a new linux-types.h.

Thanks for the clarification. Could you please help me understand also 
why __signed__ keyword is required when declaring __{u,s}<N>?
I mean why __{u,s}<N> cannot be declared using the signed type 
specifier, just like {u,s}<N>?

-- 
Xenia

Re: [PATCH v3 2/6] xen/riscv: introduce asm/types.h header file
Posted by Jan Beulich 3 years ago
On 11.01.2023 13:30, Xenia Ragiadakou wrote:
> Could you please help me understand also 
> why __signed__ keyword is required when declaring __{u,s}<N>?
> I mean why __{u,s}<N> cannot be declared using the signed type 
> specifier, just like {u,s}<N>?

I'm afraid I can't, as this looks to have been (blindly?) imported
from Linux very, very long ago. Having put time in going through
our own history, I'm afraid I don't want to go further and see
whether I could spot the reason for you by going through Linux'es.

Jan
Re: [PATCH v3 2/6] xen/riscv: introduce asm/types.h header file
Posted by Xenia Ragiadakou 3 years ago
On 1/11/23 15:17, Jan Beulich wrote:
> On 11.01.2023 13:30, Xenia Ragiadakou wrote:
>> Could you please help me understand also
>> why __signed__ keyword is required when declaring __{u,s}<N>?
>> I mean why __{u,s}<N> cannot be declared using the signed type
>> specifier, just like {u,s}<N>?
> 
> I'm afraid I can't, as this looks to have been (blindly?) imported
> from Linux very, very long ago. Having put time in going through
> our own history, I'm afraid I don't want to go further and see
> whether I could spot the reason for you by going through Linux'es.

Sorry, I was not aiming to drag you (or anyone) into spotting why Linux 
uses __signed__ when declaring __s<N>. AfAIU these types are exported 
and used in userspace and maybe the reason is to support building with 
pre-standard C compilers.
I am just wondering why Xen, that is compiled with std=c99, uses 
__signed__. If there is no reason, I think this difference between the 
declarations of __{u,s}<N> and {u,s}<N> is misleading and confusing (to 
me at least).

-- 
Xenia