| Message ID | 20210629133322.19193-4-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: sie and uv cleanups | expand |
On Tue, Jun 29 2021, Janosch Frank <frankja@linux.ibm.com> wrote: > These structs have largely been copied from the kernel so they still > have the old uint short types which we want to avoid in favor of the > uint*_t ones. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > lib/s390x/asm/uv.h | 142 +++++++++++++++++++++++---------------------- > 1 file changed, 72 insertions(+), 70 deletions(-) Reviewed-by: Cornelia Huck <cohuck@redhat.com>
On 29/06/2021 15.33, Janosch Frank wrote: > These structs have largely been copied from the kernel so they still > have the old uint short types which we want to avoid in favor of the > uint*_t ones. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > lib/s390x/asm/uv.h | 142 +++++++++++++++++++++++---------------------- > 1 file changed, 72 insertions(+), 70 deletions(-) > > diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h > index dc3e02d..96a2a7e 100644 > --- a/lib/s390x/asm/uv.h > +++ b/lib/s390x/asm/uv.h > @@ -12,6 +12,8 @@ > #ifndef _ASMS390X_UV_H_ > #define _ASMS390X_UV_H_ > > +#include <stdint.h> > + > #define UVC_RC_EXECUTED 0x0001 > #define UVC_RC_INV_CMD 0x0002 > #define UVC_RC_INV_STATE 0x0003 > @@ -68,73 +70,73 @@ enum uv_cmds_inst { > }; > > struct uv_cb_header { > - u16 len; > - u16 cmd; /* Command Code */ > - u16 rc; /* Response Code */ > - u16 rrc; /* Return Reason Code */ > + uint16_t len; > + uint16_t cmd; /* Command Code */ > + uint16_t rc; /* Response Code */ > + uint16_t rrc; /* Return Reason Code */ > } __attribute__((packed)) __attribute__((aligned(8))); Hmm, for files that are more or less a copy from the corresponding kernel header, I'm not sure whether it makes sense to convert them to the stdint.h types? It might be better to keep the kernel types so that updates to this header can be ported more easily to the kvm-unit-tests later? Thomas
On 7/4/21 9:51 AM, Thomas Huth wrote: > On 29/06/2021 15.33, Janosch Frank wrote: >> These structs have largely been copied from the kernel so they still >> have the old uint short types which we want to avoid in favor of the >> uint*_t ones. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> lib/s390x/asm/uv.h | 142 +++++++++++++++++++++++---------------------- >> 1 file changed, 72 insertions(+), 70 deletions(-) >> >> diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h >> index dc3e02d..96a2a7e 100644 >> --- a/lib/s390x/asm/uv.h >> +++ b/lib/s390x/asm/uv.h >> @@ -12,6 +12,8 @@ >> #ifndef _ASMS390X_UV_H_ >> #define _ASMS390X_UV_H_ >> >> +#include <stdint.h> >> + >> #define UVC_RC_EXECUTED 0x0001 >> #define UVC_RC_INV_CMD 0x0002 >> #define UVC_RC_INV_STATE 0x0003 >> @@ -68,73 +70,73 @@ enum uv_cmds_inst { >> }; >> >> struct uv_cb_header { >> - u16 len; >> - u16 cmd; /* Command Code */ >> - u16 rc; /* Response Code */ >> - u16 rrc; /* Return Reason Code */ >> + uint16_t len; >> + uint16_t cmd; /* Command Code */ >> + uint16_t rc; /* Response Code */ >> + uint16_t rrc; /* Return Reason Code */ >> } __attribute__((packed)) __attribute__((aligned(8))); > > Hmm, for files that are more or less a copy from the corresponding kernel > header, I'm not sure whether it makes sense to convert them to the stdint.h > types? It might be better to keep the kernel types so that updates to this > header can be ported more easily to the kvm-unit-tests later? sie.h contents are 90% sblk which came directly from KVM... Do you really want to have exceptions for one file? Because if that's the case then I see no sense in changing other things over since I prefer using short types. > > Thomas >
On 05/07/2021 11.33, Janosch Frank wrote: > On 7/4/21 9:51 AM, Thomas Huth wrote: >> On 29/06/2021 15.33, Janosch Frank wrote: >>> These structs have largely been copied from the kernel so they still >>> have the old uint short types which we want to avoid in favor of the >>> uint*_t ones. >>> >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>> --- >>> lib/s390x/asm/uv.h | 142 +++++++++++++++++++++++---------------------- >>> 1 file changed, 72 insertions(+), 70 deletions(-) >>> >>> diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h >>> index dc3e02d..96a2a7e 100644 >>> --- a/lib/s390x/asm/uv.h >>> +++ b/lib/s390x/asm/uv.h >>> @@ -12,6 +12,8 @@ >>> #ifndef _ASMS390X_UV_H_ >>> #define _ASMS390X_UV_H_ >>> >>> +#include <stdint.h> >>> + >>> #define UVC_RC_EXECUTED 0x0001 >>> #define UVC_RC_INV_CMD 0x0002 >>> #define UVC_RC_INV_STATE 0x0003 >>> @@ -68,73 +70,73 @@ enum uv_cmds_inst { >>> }; >>> >>> struct uv_cb_header { >>> - u16 len; >>> - u16 cmd; /* Command Code */ >>> - u16 rc; /* Response Code */ >>> - u16 rrc; /* Return Reason Code */ >>> + uint16_t len; >>> + uint16_t cmd; /* Command Code */ >>> + uint16_t rc; /* Response Code */ >>> + uint16_t rrc; /* Return Reason Code */ >>> } __attribute__((packed)) __attribute__((aligned(8))); >> >> Hmm, for files that are more or less a copy from the corresponding kernel >> header, I'm not sure whether it makes sense to convert them to the stdint.h >> types? It might be better to keep the kernel types so that updates to this >> header can be ported more easily to the kvm-unit-tests later? > > sie.h contents are 90% sblk which came directly from KVM... > Do you really want to have exceptions for one file? Because if that's > the case then I see no sense in changing other things over since I > prefer using short types. Completely inaccurate checks with the lib directory of the kvm-unit-tests: $ grep -r u64 lib/ | wc -l 234 $ grep -r uint64 lib/ | wc -l 245 $ grep -r u8 lib/ | wc -l 137 $ grep -r uint8 lib/ | wc -l 193 ... I guess that's an indication that we do not really have a prevailing style here? I personally prefer the stdint.h types, I'm just not sure whether it makes sense to keep some headers close to the kernel or not...? Thomas
On Mon, 5 Jul 2021 11:41:57 +0200 Thomas Huth <thuth@redhat.com> wrote: [snip] > > Completely inaccurate checks with the lib directory of the > kvm-unit-tests: > > $ grep -r u64 lib/ | wc -l > 234 > $ grep -r uint64 lib/ | wc -l > 245 > > $ grep -r u8 lib/ | wc -l > 137 > $ grep -r uint8 lib/ | wc -l > 193 > > ... I guess that's an indication that we do not really have a > prevailing style here? > I personally prefer the stdint.h types, I'm just not sure whether it > makes sense to keep some headers close to the kernel or not...? > > Thomas > I agree, the project as a whole needs to decide the policy regarding stdint.h types. Do we want them always? only for stuff that doesn't need synchronization with the kernel? or maybe we just don't care? I don't care which way we go, but I think we need to decide on one way to go.
diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h index dc3e02d..96a2a7e 100644 --- a/lib/s390x/asm/uv.h +++ b/lib/s390x/asm/uv.h @@ -12,6 +12,8 @@ #ifndef _ASMS390X_UV_H_ #define _ASMS390X_UV_H_ +#include <stdint.h> + #define UVC_RC_EXECUTED 0x0001 #define UVC_RC_INV_CMD 0x0002 #define UVC_RC_INV_STATE 0x0003 @@ -68,73 +70,73 @@ enum uv_cmds_inst { }; struct uv_cb_header { - u16 len; - u16 cmd; /* Command Code */ - u16 rc; /* Response Code */ - u16 rrc; /* Return Reason Code */ + uint16_t len; + uint16_t cmd; /* Command Code */ + uint16_t rc; /* Response Code */ + uint16_t rrc; /* Return Reason Code */ } __attribute__((packed)) __attribute__((aligned(8))); struct uv_cb_init { struct uv_cb_header header; - u64 reserved08[2]; - u64 stor_origin; - u64 stor_len; - u64 reserved28[4]; + uint64_t reserved08[2]; + uint64_t stor_origin; + uint64_t stor_len; + uint64_t reserved28[4]; } __attribute__((packed)) __attribute__((aligned(8))); struct uv_cb_qui { struct uv_cb_header header; - u64 reserved08; - u64 inst_calls_list[4]; - u64 reserved30[2]; - u64 uv_base_stor_len; - u64 reserved48; - u64 conf_base_phys_stor_len; - u64 conf_base_virt_stor_len; - u64 conf_virt_var_stor_len; - u64 cpu_stor_len; - u32 reserved70[3]; - u32 max_num_sec_conf; - u64 max_guest_stor_addr; - u8 reserved88[158 - 136]; - u16 max_guest_cpus; - u8 reserveda0[200 - 160]; + uint64_t reserved08; + uint64_t inst_calls_list[4]; + uint64_t reserved30[2]; + uint64_t uv_base_stor_len; + uint64_t reserved48; + uint64_t conf_base_phys_stor_len; + uint64_t conf_base_virt_stor_len; + uint64_t conf_virt_var_stor_len; + uint64_t cpu_stor_len; + uint32_t reserved70[3]; + uint32_t max_num_sec_conf; + uint64_t max_guest_stor_addr; + uint8_t reserved88[158 - 136]; + uint16_t max_guest_cpus; + uint8_t reserveda0[200 - 160]; } __attribute__((packed)) __attribute__((aligned(8))); struct uv_cb_cgc { struct uv_cb_header header; - u64 reserved08[2]; - u64 guest_handle; - u64 conf_base_stor_origin; - u64 conf_var_stor_origin; - u64 reserved30; - u64 guest_stor_origin; - u64 guest_stor_len; - u64 guest_sca; - u64 guest_asce; - u64 reserved60[5]; + uint64_t reserved08[2]; + uint64_t guest_handle; + uint64_t conf_base_stor_origin; + uint64_t conf_var_stor_origin; + uint64_t reserved30; + uint64_t guest_stor_origin; + uint64_t guest_stor_len; + uint64_t guest_sca; + uint64_t guest_asce; + uint64_t reserved60[5]; } __attribute__((packed)) __attribute__((aligned(8))); struct uv_cb_csc { struct uv_cb_header header; - u64 reserved08[2]; - u64 cpu_handle; - u64 guest_handle; - u64 stor_origin; - u8 reserved30[6]; - u16 num; - u64 state_origin; - u64 reserved[4]; + uint64_t reserved08[2]; + uint64_t cpu_handle; + uint64_t guest_handle; + uint64_t stor_origin; + uint8_t reserved30[6]; + uint16_t num; + uint64_t state_origin; + uint64_t reserved[4]; } __attribute__((packed)) __attribute__((aligned(8))); struct uv_cb_unp { struct uv_cb_header header; - u64 reserved08[2]; - u64 guest_handle; - u64 gaddr; - u64 tweak[2]; - u64 reserved38[3]; + uint64_t reserved08[2]; + uint64_t guest_handle; + uint64_t gaddr; + uint64_t tweak[2]; + uint64_t reserved38[3]; } __attribute__((packed)) __attribute__((aligned(8))); /* @@ -144,42 +146,42 @@ struct uv_cb_unp { */ struct uv_cb_nodata { struct uv_cb_header header; - u64 reserved08[2]; - u64 handle; - u64 reserved20[4]; + uint64_t reserved08[2]; + uint64_t handle; + uint64_t reserved20[4]; } __attribute__((packed)) __attribute__((aligned(8))); struct uv_cb_share { struct uv_cb_header header; - u64 reserved08[3]; - u64 paddr; - u64 reserved28; + uint64_t reserved08[3]; + uint64_t paddr; + uint64_t reserved28; } __attribute__((packed)) __attribute__((aligned(8))); /* Convert to Secure */ struct uv_cb_cts { struct uv_cb_header header; - u64 reserved08[2]; - u64 guest_handle; - u64 gaddr; + uint64_t reserved08[2]; + uint64_t guest_handle; + uint64_t gaddr; } __attribute__((packed)) __attribute__((aligned(8))); /* Convert from Secure / Pin Page Shared */ struct uv_cb_cfs { struct uv_cb_header header; - u64 reserved08[2]; - u64 paddr; + uint64_t reserved08[2]; + uint64_t paddr; } __attribute__((packed)) __attribute__((aligned(8))); /* Set Secure Config Parameter */ struct uv_cb_ssc { struct uv_cb_header header; - u64 reserved08[2]; - u64 guest_handle; - u64 sec_header_origin; - u32 sec_header_len; - u32 reserved2c; - u64 reserved30[4]; + uint64_t reserved08[2]; + uint64_t guest_handle; + uint64_t sec_header_origin; + uint32_t sec_header_len; + uint32_t reserved2c; + uint64_t reserved30[4]; } __attribute__((packed)) __attribute__((aligned(8))); static inline int uv_call_once(unsigned long r1, unsigned long r2) @@ -211,7 +213,7 @@ static inline int uv_call(unsigned long r1, unsigned long r2) return cc; } -static inline int share(unsigned long addr, u16 cmd) +static inline int share(unsigned long addr, uint16_t cmd) { struct uv_cb_share uvcb = { .header.cmd = cmd, @@ -220,7 +222,7 @@ static inline int share(unsigned long addr, u16 cmd) }; int cc; - cc = uv_call(0, (u64)&uvcb); + cc = uv_call(0, (uint64_t)&uvcb); if (!cc && uvcb.header.rc == UVC_RC_EXECUTED) return 0; @@ -252,11 +254,11 @@ static inline int uv_remove_shared(unsigned long addr) struct uv_cb_cpu_set_state { struct uv_cb_header header; - u64 reserved08[2]; - u64 cpu_handle; - u8 reserved20[7]; - u8 state; - u64 reserved28[5]; + uint64_t reserved08[2]; + uint64_t cpu_handle; + uint8_t reserved20[7]; + uint8_t state; + uint64_t reserved28[5]; }; #define PV_CPU_STATE_OPR 1
These structs have largely been copied from the kernel so they still have the old uint short types which we want to avoid in favor of the uint*_t ones. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- lib/s390x/asm/uv.h | 142 +++++++++++++++++++++++---------------------- 1 file changed, 72 insertions(+), 70 deletions(-)