Re: [PATCH] linux-user: syscall: ioctls: support DRM_IOCTL_I915_GETPARAM
On 2020/7/14 上午2:46, Laurent Vivier wrote: >> +gparam->value = lock_user(VERIFY_WRITE, target_gparam->value, >> + sizeof(*gparam->value), 0); > > I don't think you should use directly the guest memory. > You should have something like that: > > int value; > > gparam->value = > >> +if (!gparam->value) { >> +unlock_user_struct(target_gparam, arg, 0); >> +return -TARGET_EFAULT; >> +} >> + >> +ret = get_errno(safe_ioctl(fd, ie->host_cmd, gparam)); > > and then: > > put_user_s32(value, target_gparam->value); > OK, thanks. It will be better. >> + >> +unlock_user(gparam->value, target_gparam->value, >> sizeof(*gparam->value)); >> +unlock_user_struct(target_gparam, arg, 0); >> +return ret; >> +} >> + >> +static abi_long do_ioctl_drm_i915(const IOCTLEntry *ie, uint8_t *buf_temp, >> + int fd, int cmd, abi_long arg) >> +{ >> +switch (ie->host_cmd) { >> +case DRM_IOCTL_I915_GETPARAM: >> +return do_ioctl_drm_i915_getparam(ie, >> + (struct drm_i915_getparam >> *)buf_temp, >> + fd, arg); >> +default: >> +return -TARGET_ENOSYS; >> +} >> +} > > There is a better way to register a struct with convertion functions: > you might use STRUCT_SPECIAL() to declare the structure rather than > IOCTL_SPECIAL() to declare the ioctl command. > (I think STRUCT_SPECIAL() could also be used with DRM_IOCTL_VERSION) > For me, STRUCT_SPECIAL sounds good for the complex structures, but for drm_i915_getparam which is simple enough, it is not quite suitable. For me, STRUCT_SPECIAL is much suitable for DRM_IOCTL_VERSION. Welcome your additional ideas. >> >> static IOCTLEntry ioctl_entries[] = { >> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h >> index 3c261cff0e..9082f6c2bc 100644 >> --- a/linux-user/syscall_defs.h >> +++ b/linux-user/syscall_defs.h >> @@ -1170,6 +1170,9 @@ struct target_rtc_pll_info { >> /* drm ioctls */ >> #define TARGET_DRM_IOCTL_VERSION TARGET_IOWRU('d', 0x00) >> >> +/* drm i915 ioctls */ >> +#define TARGET_DRM_IOCTL_I915_GETPARAM TARGET_IOWRU('d', 0x46) > > why do you use the U variant? > > TARGET_IOWR('d', 0x46, struct target_drm_i915_getparam) > Because qemu will automatically set the size with the target structure size in syscall_init(). It'll be more easier. e.g. usb ioctls and device mapper ioctls also do like this.
Re: [PATCH v7] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION
I shall try to send another DRM_IOCTL_* patches within this weekend. Thanks. On 2020/6/29 下午7:05, Laurent Vivier wrote: > Le 05/06/2020 à 03:32, cheng...@emindsoft.com.cn a écrit : >> From: Chen Gang >> >> Another DRM_IOCTL_* commands will be done later. >> >> Signed-off-by: Chen Gang >> ... > > Applied to my branch linux-user-for-5.1 > > Thanks, > Laurent >
Re: [PATCH] [PATCH v6] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION
That sounds good, I'll send patch v7, thanks. On 2020/6/4 下午5:10, Laurent Vivier wrote: > Le 04/06/2020 à 03:45, cheng...@emindsoft.com.cn a écrit : >> From: Chen Gang >> >> Another DRM_IOCTL_* commands will be done later. >> >> Signed-off-by: Chen Gang >> --- >> configure | 10 >> linux-user/ioctls.h| 5 ++ >> linux-user/syscall.c | 95 ++ >> linux-user/syscall_defs.h | 15 ++ >> linux-user/syscall_types.h | 11 + >> 5 files changed, 136 insertions(+) >> >> diff --git a/configure b/configure >> index f087d2bcd1..389dbb1d09 100755 >> --- a/configure >> +++ b/configure >> @@ -3136,6 +3136,13 @@ if ! check_include "ifaddrs.h" ; then >>have_ifaddrs_h=no >> fi >> >> +# >> +# libdrm check >> +have_drm_h=no >> +if check_include "libdrm/drm.h" ; then >> +have_drm_h=yes >> +fi >> + >> ## >> # VTE probe >> >> @@ -7127,6 +7134,9 @@ fi >> if test "$have_ifaddrs_h" = "yes" ; then >> echo "HAVE_IFADDRS_H=y" >> $config_host_mak >> fi >> +if test "$have_drm_h" = "yes" ; then >> + echo "HAVE_DRM_H=y" >> $config_host_mak >> +fi >> if test "$have_broken_size_max" = "yes" ; then >> echo "HAVE_BROKEN_SIZE_MAX=y" >> $config_host_mak >> fi >> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h >> index 0defa1d8c1..f2e2fa9c87 100644 >> --- a/linux-user/ioctls.h >> +++ b/linux-user/ioctls.h >> @@ -574,6 +574,11 @@ >>IOCTL_SPECIAL(SIOCDELRT, IOC_W, do_ioctl_rt, >> MK_PTR(MK_STRUCT(STRUCT_rtentry))) >> >> +#ifdef HAVE_DRM_H >> + IOCTL_SPECIAL(DRM_IOCTL_VERSION, IOC_RW, do_ioctl_drm, >> +MK_PTR(MK_STRUCT(STRUCT_drm_version))) >> +#endif >> + >> #ifdef TARGET_TIOCSTART >>IOCTL_IGNORE(TIOCSTART) >>IOCTL_IGNORE(TIOCSTOP) >> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >> index 7f6700c54e..1744e7acc7 100644 >> --- a/linux-user/syscall.c >> +++ b/linux-user/syscall.c >> @@ -112,6 +112,9 @@ >> #include >> #include >> #include >> +#ifdef HAVE_DRM_H >> +#include >> +#endif >> #include "linux_loop.h" >> #include "uname.h" >> >> @@ -5276,6 +5279,98 @@ static abi_long do_ioctl_tiocgptpeer(const IOCTLEntry >> *ie, uint8_t *buf_temp, >> } >> #endif >> >> +#ifdef HAVE_DRM_H >> + >> +static void unlock_drm_version(struct drm_version *host_ver) >> +{ >> +unlock_user(host_ver->name, 0UL, 0); >> +unlock_user(host_ver->date, 0UL, 0); >> +unlock_user(host_ver->desc, 0UL, 0); >> +} > > This is correct but I would prefer to not have this kind of function > with 0UL parameter. > > You can use target_ver for the guest pointer and host_ver for the len, > guest pointer will be ignored if host pointer is NULL. > > You can have a generic function that not copies data in case of error: > > static void unlock_drm_strings(struct drm_version *host_ver, >struct target_drm_version *target_ver, >int copy) > { > unlock_user(host_ver->name, target_ver->name, > copy ? host_ver->name_len : 0); > ... > >> + >> +static inline abi_long target_to_host_drmversion(struct drm_version >> *host_ver, >> + struct target_drm_version >> *target_ver) >> +{ >> +memset(host_ver, 0, sizeof(*host_ver)); >> + >> +__get_user(host_ver->name_len, _ver->name_len); >> +if (host_ver->name_len) { >> +host_ver->name = lock_user(VERIFY_WRITE, target_ver->name, >> + target_ver->name_len, 0); >> +if (!host_ver->name) { >> +goto err; >> +} >> +} >> + >> +__get_user(host_ver->date_len, _ver->date_len); >> +if (host_ver->date_len) { >> +host_ver->date = lock_user(VERIFY_WRITE, target_ver->date, >> + target_ver->date_len, 0); >> +if (!host_ver->date) { >> +goto err; >> +} >> +} >> + >> +__get_user(host_ver->
Re: [PATCH v5] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION
OK, thanks. I'll send patch v6. :) On 2020/6/3 下午8:03, Laurent Vivier wrote: > Le 03/06/2020 à 13:05, Chen Gang a écrit : >> On 2020/6/3 下午5:49, Laurent Vivier wrote: >>> Le 03/06/2020 à 03:08, cheng...@emindsoft.com.cn a écrit : >>>> +#ifdef HAVE_DRM_H >>>> + >>>> +static void unlock_drm_version(struct drm_version *host_ver) >>>> +{ >>>> +if (host_ver->name) { >>>> +unlock_user(host_ver->name, 0UL, 0); >>> >>> unlock_user() allows to have a NULL host pointer parameter, so you don't >>> need to check. But you must provide the target pointer, with the length. >>> The same below. >>> >> >> As far as I know, the unlock_user is defined in >> include/exec/softmmu-semi.h, which only checks the len before calling >> cpu_memory_rw_debug, and only calls free() for the host pointer. >> >> So we have to be sure that the host pointer must be valid. When we pass >> 0 length to unlock_user, we want it to free host pointer only. > > No, it is defined in our case in linux-user/qemu.h, and associated > comment is: > > /* Unlock an area of guest memory. The first LEN bytes must be >flushed back to guest memory. host_ptr = NULL is explicitly >allowed and does nothing. */ > >> >>>> +if (host_ver->desc_len) { >>>> +host_ver->desc = lock_user(VERIFY_WRITE, target_ver->desc, >>>> + target_ver->desc_len, 0); >>>> +if (!host_ver->desc) { >>>> +goto err; >>>> +} >>>> +} >>>> + >>>> +unlock_user_struct(target_ver, target_addr, 0); >>>> +return 0; >>>> +err: >>>> +unlock_drm_version(host_ver); >>>> +unlock_user_struct(target_ver, target_addr, 0); >>>> +return -ENOMEM; >>> >>> In fact it should be -TARGET_EFAULT: it has failed because of access rights. >>> >> >> As far as I know, the lock_user is defined in >> include/exec/softmmu-semi.h. If the parameter 'copy' is 0 (in our case), >> lock_user will only malloc a host pointer and return it. > > No, in linux-user/qemu.h: > > /* Lock an area of guest memory into the host. If copy is true then the >host area will have the same contents as the guest. */ > >> In our case, I guess the only failure from malloc() is "no memory". > > See use-cases in syscall.c, they all fail with -TARGET_EFAULT.
Re: [PATCH v5] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION
On 2020/6/3 下午5:49, Laurent Vivier wrote: > Le 03/06/2020 à 03:08, cheng...@emindsoft.com.cn a écrit : >> +#ifdef HAVE_DRM_H >> + >> +static void unlock_drm_version(struct drm_version *host_ver) >> +{ >> +if (host_ver->name) { >> +unlock_user(host_ver->name, 0UL, 0); > > unlock_user() allows to have a NULL host pointer parameter, so you don't > need to check. But you must provide the target pointer, with the length. > The same below. > As far as I know, the unlock_user is defined in include/exec/softmmu-semi.h, which only checks the len before calling cpu_memory_rw_debug, and only calls free() for the host pointer. So we have to be sure that the host pointer must be valid. When we pass 0 length to unlock_user, we want it to free host pointer only. >> +if (host_ver->desc_len) { >> +host_ver->desc = lock_user(VERIFY_WRITE, target_ver->desc, >> + target_ver->desc_len, 0); >> +if (!host_ver->desc) { >> +goto err; >> +} >> +} >> + >> +unlock_user_struct(target_ver, target_addr, 0); >> +return 0; >> +err: >> +unlock_drm_version(host_ver); >> +unlock_user_struct(target_ver, target_addr, 0); >> +return -ENOMEM; > > In fact it should be -TARGET_EFAULT: it has failed because of access rights. > As far as I know, the lock_user is defined in include/exec/softmmu-semi.h. If the parameter 'copy' is 0 (in our case), lock_user will only malloc a host pointer and return it. In our case, I guess the only failure from malloc() is "no memory". >> +} >> >> +static inline abi_long host_to_target_drmversion(abi_ulong target_addr, >> + struct drm_version >> *host_ver) >> +{ >> +struct target_drm_version *target_ver; >> + >> +if (!lock_user_struct(VERIFY_WRITE, target_ver, target_addr, 0)) { > > I think you should not unlock_struct() in target_to_host_drmversion() so > you don't have to lock it again here. > OK, thanks. >> +static abi_long do_ioctl_drm(const IOCTLEntry *ie, uint8_t *buf_temp, >> + int fd, int cmd, abi_long arg) >> +{ >> +struct drm_version *ver; >> +abi_long ret; >> + >> +switch (ie->host_cmd) { >> +case DRM_IOCTL_VERSION: >> +ver = (struct drm_version *)buf_temp; > > you should lock the structure here (rather than in > target_to_host_drmversion())... > OK, thanks. >> +ret = target_to_host_drmversion(ver, arg); >> +if (is_error(ret)) { >> +return ret; >> +} >> +ret = get_errno(safe_ioctl(fd, ie->host_cmd, ver)); >> +if (is_error(ret)) { >> +unlock_drm_version(ver); >> +return ret; >> +} >> +return host_to_target_drmversion(arg, ver); > > and unlock the structure here (rather than in host_to_target_drmversion()). > > You should return "ret" too. > OK, thanks. >> +} >> +return -TARGET_EFAULT; > > Why -TARGET_EFAULT? -TARGET_ENOSYS would be better. > OK, thanks. Chen Gang.
Re: [PATCH v4] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION
On 2020/6/2 下午9:40, Laurent Vivier wrote: >> +static inline abi_long target_to_host_drmversion(struct drm_version >> *host_ver, >> +abi_long target_addr) >> +{ >> +struct target_drm_version *target_ver; >> + >> +if (!lock_user_struct(VERIFY_READ, target_ver, target_addr, 0)) { >> +return -TARGET_EFAULT; >> +} >> +__get_user(host_ver->name_len, _ver->name_len); >> +host_ver->name = host_ver->name_len ? g2h(target_ver->name) : NULL; >> +__get_user(host_ver->date_len, _ver->date_len); >> +host_ver->date = host_ver->date_len ? g2h(target_ver->date) : NULL; >> +__get_user(host_ver->desc_len, _ver->desc_len); >> +host_ver->desc = host_ver->desc_len ? g2h(target_ver->desc) : NULL; > > but I think the string buffers must be locked and access rights must be > checked. > > So I think you should have something like: > > host_ver->name = lock_user(VERIFY_WRITE, target_ver->name, >target_ver->name_len, 0); > ... > OK, thanks. >> +unlock_user_struct(target_ver, target_addr, 0); >> +return 0; >> +} >> + >> +static inline abi_long host_to_target_drmversion(abi_ulong target_addr, >> + struct drm_version >> *host_ver) >> +{ >> +struct target_drm_version *target_ver; >> + >> +if (!lock_user_struct(VERIFY_WRITE, target_ver, target_addr, 0)) { >> +return -TARGET_EFAULT; >> +} >> +__put_user(host_ver->version_major, _ver->version_major); >> +__put_user(host_ver->version_minor, _ver->version_minor); >> +__put_user(host_ver->version_patchlevel, >> _ver->version_patchlevel); >> +__put_user(host_ver->name_len, _ver->name_len); > > unlock_user(host_ver->name, target_ver->name, host_ver->name_len); > ... > OK, thanks. I'll send patch v5.
Re: [PATCH v3] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION
On 2020/5/28 下午6:25, Laurent Vivier wrote: >> diff --git a/configure b/configure >> index e225a1e3ff..2c2c489d1e 100755 >> --- a/configure >> +++ b/configure >> @@ -3912,6 +3912,24 @@ EOF >> fi >> fi >> >> +# >> +# libdrm check >> + >> +cat > $TMPC << EOF >> +#include >> +#include >> +int main(void) >> +{ >> +struct drm_version version; >> +ioctl(-1, DRM_IOCTL_VERSION, ); >> +return 0; >> +} >> +EOF >> +if ! compile_prog "" ; then >> +error_exit "libdrm check failed" \ >> +"Make sure to have the libdrm/drm.h installed." >> +fi > > You break the build of qemu if libdrm is not available, not a good idea. > > In fact, you should only check for the include with something like > "check_include libdrm/drm.h" and then define a HAVE_DRM_H to use it > around the new code: > > #ifdef HAVE_DRM_H > #include > #endif > > ... > #ifdef HAVE_DRM_H > static inline abi_long target_to_host_drmversion(... > ... > #endif > ... > > #ifdef HAVE_DRM_H > IOCTL_SPECIAL(DRM_IOCTL_VERSION,... > ... > #endif > OK, thanks. I'll send patch v4 Chen Gang
Re: [PATCH v2] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION
On 2020/5/12 上午2:43, Laurent Vivier wrote: >> >> + IOCTL_SPECIAL(DRM_IOCTL_VERSION, IOC_RW, do_ioctl_drm, >> +MK_PTR(MK_STRUCT(STRUCT_drm_version))) > > Add a blank line here. > OK, thanks. >> #ifdef TARGET_TIOCSTART >>IOCTL_IGNORE(TIOCSTART) >>IOCTL_IGNORE(TIOCSTOP) >> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >> index 8d27d10807..2eb7c91ab4 100644 >> --- a/linux-user/syscall.c >> +++ b/linux-user/syscall.c >> @@ -112,6 +112,7 @@ >> #include >> #include >> #include >> +#include > > I think you should check in configure that this file is available on the > system. > OK, I'll check in configure in patch v3. >> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h >> index 152ec637cb..3c261cff0e 100644 >> --- a/linux-user/syscall_defs.h >> +++ b/linux-user/syscall_defs.h >> @@ -1167,6 +1167,9 @@ struct target_rtc_pll_info { >> #define TARGET_DM_TARGET_MSG TARGET_IOWRU(0xfd, 0x0e) >> #define TARGET_DM_DEV_SET_GEOMETRYTARGET_IOWRU(0xfd, 0x0f) >> >> +/* drm ioctls */ >> +#define TARGET_DRM_IOCTL_VERSION TARGET_IOWRU('d', 0x00) > > Why do you use the TARGET_IOWRU variant? > > Can't you use TARGET_IOWR('d', 0x00, struct target_drm_version)? > Because qemu will automatically set the size with the target structure size in syscall_init(). It'll be more easier. e.g. usb ioctls and device mapper ioctls also do like this. Thanks.
Re: [PATCH] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION
Thank you very much for your reply. :) DRM_IOCTL_VERSION has to be a special one, but my patch really need be improved. The string pointers should be translated, but the new string buffers need not be allocated. I will try to send patch v2 within this week end. DRM_IOCTL_* really have many commands, at present, I have implemented about 20+ commands for i965 gpu using, although some of them are not generic enough. I will try to send all of them when I have free time. Thanks. On 2020/3/11 下午3:35, Laurent Vivier wrote: > Le 10/03/2020 à 21:16, Laurent Vivier a écrit : >> Le 26/02/2020 à 12:38, cheng...@emindsoft.com.cn a écrit : >>> From: Chen Gang >>> >>> The other DRM_IOCTL_* commands will be done later. >>> >>> Signed-off-by: Chen Gang >>> --- >>> linux-user/ioctls.h| 3 + >>> linux-user/syscall.c | 134 + >>> linux-user/syscall_defs.h | 16 + >>> linux-user/syscall_types.h | 12 >>> 4 files changed, 165 insertions(+) >>> >>> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h >>> index 0defa1d8c1..c2294b48a0 100644 >>> --- a/linux-user/ioctls.h >>> +++ b/linux-user/ioctls.h >>> @@ -574,6 +574,9 @@ >>>IOCTL_SPECIAL(SIOCDELRT, IOC_W, do_ioctl_rt, >>> MK_PTR(MK_STRUCT(STRUCT_rtentry))) >>> >>> + IOCTL_SPECIAL(DRM_IOCTL_VERSION, IOC_RW, do_ioctl_drm, >>> +MK_PTR(MK_STRUCT(STRUCT_drm_version))) >>> + >> >> Rather than adding a specific function to process the structure, perhaps >> we can add this in a generic way? >> >> The problem with drm_version structure is the pointers to the strings. >> >> Did you try to add a TYPE_STRING in >> thunk_type_size()/thunk_type_align()/think_convert()/do_ioctl() to do that? > > In fact we can't do that because we need to know the size of the buffer > to allocate and it is provided by another field. It cannot be generic, > so I think what you do is the best we can do. > > Thanks, > LAurent > > > > >
Re: [PATCH] target: i386: Check float overflow about register stack
On 2020/2/24 下午8:43, Paolo Bonzini wrote: > On 22/02/20 13:25, Chen Gang wrote: >> On 2020/2/22 下午3:37, Paolo Bonzini wrote: >>> On 22/02/20 03:10, Chen Gang wrote: >>>> Set C1 to 1 if stack overflow occurred; set to 0 otherwise". >>>> >>>> In helper_fxam_ST0, I guess, we need "env->fpus |= 0x200" (but I don't >>>> know wheter it will be conflict with SIGND(temp)). And we have to still >>>> need foverflow, because all env->fptags being 0 doesn't mean overflow. >>> >>> No, you need to add "env->fpus |= 0x200" and "env->fpus &= ~0x200" >>> directly to fpush, fpop, etc. >>> >> >> OK. The content below is my next TODO, welcome your opinions. >> >> When overflow occurs, for me, we need keep everything no touch except >> set C1 flag. > > No, push will overwrite the top entry if there is overflow. > >> In fxam, we don't clear C1, but keep no touch for clearning >> C1 in another places. > > FXAM is neither push nor pop, it just detects an empty slot via fptags. > FXAM should be okay with my patch. > OK. I am not quite clear about it, but it fixes the current issues at least. Please apply your patch. Thanks.
Re: [PATCH] target: i386: Check float overflow about register stack
On 2020/2/22 下午3:37, Paolo Bonzini wrote: > On 22/02/20 03:10, Chen Gang wrote: >> Set C1 to 1 if stack overflow occurred; set to 0 otherwise". >> >> In helper_fxam_ST0, I guess, we need "env->fpus |= 0x200" (but I don't >> know wheter it will be conflict with SIGND(temp)). And we have to still >> need foverflow, because all env->fptags being 0 doesn't mean overflow. > > No, you need to add "env->fpus |= 0x200" and "env->fpus &= ~0x200" > directly to fpush, fpop, etc. > OK. The content below is my next TODO, welcome your opinions. When overflow occurs, for me, we need keep everything no touch except set C1 flag. In fxam, we don't clear C1, but keep no touch for clearning C1 in another places. When underflow occurs, for me, we need keep everything no touch except set env->fpstt 8, so the next consecutive fpop/f[i]stp* can be checked easier, and the next fpush/f[i]ld* can work well in normal. For fxam, we check env->fpstt == 8 and env->fptags for empty. And when env->fpstt is 8, it need be set 7 before used in fincstp and ffree_STN. Thanks.
Re: [PATCH] target: i386: Check float overflow about register stack
On 2020/2/22 上午10:10, Chen Gang wrote: > On 2020/2/22 上午12:18, Paolo Bonzini wrote: >> On 21/02/20 15:09, Chen Gang wrote: >>>> -/* XXX: test fptags too */ >>>> +if (env->fptags[env->fpstt]) { >>>> +env->fpus |= 0x4100; /* Empty */ >>>> +return; >>>> +} >>>> + >>> For fpop overflow, this fix is enough, but for me, we still need >>> foverflow to check fpush/fld*_ST0 overflow. >>> >>> Don't you think we need check fpush/f[i]ld*_ST0 overflow? >> >> After fld/fild or any other push, FXAM ST0 should not return empty in my >> opinion. >> > > OK, it sounds reasonable. > > After check the intel document for f[i]ld* instructions, it says: > > "Set C1 to 1 if stack overflow occurred; set to 0 otherwise". > > In helper_fxam_ST0, I guess, we need "env->fpus |= 0x200" (but I don't > know wheter it will be conflict with SIGND(temp)). And we have to still > need foverflow, because all env->fptags being 0 doesn't mean overflow. > After read the document more about fstp* and fld* instructions, I find that fstp* are also related with C1, and fld* will generate IS exception when stack underflow or overflow occurred. It seems more modifications should be done (but they will be several patches). Thanks.
Re: [PATCH] target: i386: Check float overflow about register stack
On 2020/2/22 上午12:18, Paolo Bonzini wrote: > On 21/02/20 15:09, Chen Gang wrote: >>> -/* XXX: test fptags too */ >>> +if (env->fptags[env->fpstt]) { >>> +env->fpus |= 0x4100; /* Empty */ >>> +return; >>> +} >>> + >> For fpop overflow, this fix is enough, but for me, we still need >> foverflow to check fpush/fld*_ST0 overflow. >> >> Don't you think we need check fpush/f[i]ld*_ST0 overflow? > > After fld/fild or any other push, FXAM ST0 should not return empty in my > opinion. > OK, it sounds reasonable. After check the intel document for f[i]ld* instructions, it says: "Set C1 to 1 if stack overflow occurred; set to 0 otherwise". In helper_fxam_ST0, I guess, we need "env->fpus |= 0x200" (but I don't know wheter it will be conflict with SIGND(temp)). And we have to still need foverflow, because all env->fptags being 0 doesn't mean overflow. Thanks.
Re: [PATCH] target: i386: Check float overflow about register stack
On 2020/2/21 下午4:58, Paolo Bonzini wrote: > On 21/02/20 04:45, cheng...@emindsoft.com.cn wrote: >> static inline void fpush(CPUX86State *env) >> { >> -env->fpstt = (env->fpstt - 1) & 7; >> -env->fptags[env->fpstt] = 0; /* validate stack entry */ >> +set_fpstt(env, env->fpstt - 1, false, true); > > On overflow fpstt is ~0, so this does: > > env->foverflow = true; > env->fpstt = 7; > env->fptags[7] = 0; /* validate stack entry */ > > Is this correct? You are going to set ST0 so the register should not be > marked empty. > Originally, I wanted to add foverflow to mark the stack overflow only, and kept another things no touch. But I think what you said above is correct, for me, if fpush/f[i]ld*_STO are overflow, the env->fpstt, env->fpregs and env->fptags should be kept no touch, and foverflow is set true, so there is no negative effect. Welcome your idea. >> void helper_fdecstp(CPUX86State *env) >> { >> -env->fpstt = (env->fpstt - 1) & 7; >> +set_fpstt(env, env->fpstt - 1, false, false); > > This is clearing env->foverflow. But after 8 consecutive fdecstp or > fincstp the result of FXAM should not change. > >> env->fpus &= ~0x4700; >> } >> >> void helper_fincstp(CPUX86State *env) >> { >> -env->fpstt = (env->fpstt + 1) & 7; >> +set_fpstt(env, env->fpstt + 1, true, false); > > Same here. > OK. thanks. Now if foverflow is only for fpush/f[i]ld*_ST0, I guess fincstp/fdecstp can clear foverflow. The env->fptags are only for fpop, which keep no touch in fincstp/fdecstp. > The actual bug is hinted in helper_fxam_ST0: > > /* XXX: test fptags too */ > > I think the correct fix should be something like > > diff --git a/target/i386/fpu_helper.c b/target/i386/fpu_helper.c > index 99f28f267f..792a128a6d 100644 > --- a/target/i386/fpu_helper.c > +++ b/target/i386/fpu_helper.c > @@ -991,7 +991,11 @@ void helper_fxam_ST0(CPUX86State *env) > env->fpus |= 0x200; /* C1 <-- 1 */ > } > > -/* XXX: test fptags too */ > +if (env->fptags[env->fpstt]) { > +env->fpus |= 0x4100; /* Empty */ > +return; > +} > + For fpop overflow, this fix is enough, but for me, we still need foverflow to check fpush/fld*_ST0 overflow. Don't you think we need check fpush/f[i]ld*_ST0 overflow? Thanks
Re: [Qemu-devel] Deprecate tilegx ?
On 2/28/18 20:41, Bastian Koppelmann wrote: > On 02/28/2018 07:11 AM, Thomas Huth wrote: >> On 27.02.2018 12:51, Peter Maydell wrote: >> >>> Possibly there are other target architectures we could reasonably >>> deprecate-and-remove (though none of the other ones Linux is dropping >>> in this round are ones we support)... >> >> I'd vote for marking tilegx as deprecated, too, since we even do not >> have an active maintainer for that CPU core (at least I did not spot one >> in our MAINTAINERS file). Opinions? > > I always saw it as a big plus that QEMU supports nearly any > architecture, no matter how obscure it is. So I'm a bit more hesitant on > dropping architectures quickly. > Firstly, sorry for too long time to no response about tilegx. During latest 2 years, I turned to android development (custom aosp with java/c++/c, merge android and linux together), and revert engineering about android (java/c). At present, sorry, I really have no time for tilegx any more. :( For me, tilegx is still a good platform, and we have done much things, now, so if we can find another maintainers/volunteers for it, I hope qemu can still support tilegx. By the way, for floating point instructions patches, I have sent before (2 years ago), it seems I did not get any reply. I hope they are useful for tilegx. Thanks. -- Chen Gang (陈刚)
Re: [Qemu-devel] [PATCH 16/19] target-tilegx: move cpu_exec_realize() to realize function
On 10/6/16 03:38, Laurent Vivier wrote: > I've removed the cannot_destroy_with_object_finalize_yet field as > cpu_exec_init() is not called by tilegx_cpu_initfn() anymore > (not tested with QOM command as tilegx is only available in linux-user > mode) > For master branch, I can not find cpu_exec_realize in total source code, I am not quite sure whether it is ok or not. And in honest, now, tilegx-linux-user is not implemented completely, the floating point insns are not supported by our main branch (which should be one of main feature for tilegx) -- which I should try. Thanks. > CC: Chen Gang <gang.chen.5...@gmail.com> > Signed-off-by: Laurent Vivier <lviv...@redhat.com> > --- > target-tilegx/cpu.c | 15 +++ > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/target-tilegx/cpu.c b/target-tilegx/cpu.c > index f7ec920..6be69ef 100644 > --- a/target-tilegx/cpu.c > +++ b/target-tilegx/cpu.c > @@ -92,6 +92,13 @@ static void tilegx_cpu_realizefn(DeviceState *dev, Error > **errp) > { > CPUState *cs = CPU(dev); > TileGXCPUClass *tcc = TILEGX_CPU_GET_CLASS(dev); > +Error *local_err = NULL; > + > +cpu_exec_realize(cs, _err); > +if (local_err != NULL) { > +error_propagate(errp, local_err); > +return; > +} > > cpu_reset(cs); > qemu_init_vcpu(cs); > @@ -108,7 +115,6 @@ static void tilegx_cpu_initfn(Object *obj) > > cs->env_ptr = env; > cpu_exec_init(cs, _abort); > -cpu_exec_realize(cs, _abort); > > if (tcg_enabled() && !tcg_initialized) { > tcg_initialized = true; > @@ -163,13 +169,6 @@ static void tilegx_cpu_class_init(ObjectClass *oc, void > *data) > cc->set_pc = tilegx_cpu_set_pc; > cc->handle_mmu_fault = tilegx_cpu_handle_mmu_fault; > cc->gdb_num_core_regs = 0; > - > -/* > - * Reason: tilegx_cpu_initfn() calls cpu_exec_init(), which saves > - * the object in cpus -> dangling pointer after final > - * object_unref(). > - */ > -dc->cannot_destroy_with_object_finalize_yet = true; > } > > static const TypeInfo tilegx_cpu_type_info = { > -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [Qemu-devel] [PATCH v6 0/5] target-tilegx: Implement floating point instructions
Hello Maintainers: Please help check this patch when you have time. Thanks. On 5/15/16 07:40, cheng...@emindsoft.com.cn wrote: > From: Chen Gang <cheng...@emindsoft.com.cn> > > These patches are the normal floating point implementation, instead of > the original temporary one. > > It passes building, and gcc testsuite. > > > Chen Gang (5): > fpu: softfloat: Add normalize_roundpack_float[32|64] functions > target-tilegx/helper-fshared.h: Add floating point shared function > target-tilegx/helper-fsingle.c: Implement single floating point > target-tilegx/helper-fdouble.c: Implement double floating point > target-tilegx: Integrate floating pointer implementation > > fpu/softfloat.c| 65 > include/fpu/softfloat.h| 15 ++ > target-tilegx/Makefile.objs| 3 +- > target-tilegx/helper-fdouble.c | 365 > + > target-tilegx/helper-fshared.h | 56 +++ > target-tilegx/helper-fsingle.c | 201 +++ > target-tilegx/helper.h | 12 ++ > target-tilegx/translate.c | 66 +++- > 8 files changed, 773 insertions(+), 10 deletions(-) > create mode 100644 target-tilegx/helper-fdouble.c > create mode 100644 target-tilegx/helper-fshared.h > create mode 100644 target-tilegx/helper-fsingle.c > -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [Qemu-devel] [PATCH] linux-user/signal.c: Use target address instead of host address for microblaze restorer
On 5/6/16 00:11, Edgar E. Iglesias wrote: > On Thu, May 05, 2016 at 10:48:57PM +0800, Chen Gang wrote: >> On 5/5/16 00:05, Peter Maydell wrote: >>> On 29 March 2016 at 15:13, <cheng...@emindsoft.com.cn> wrote: >>>> From: Chen Gang <cheng...@emindsoft.com.cn> >>>> >>>> The return address is in target space, so the restorer address needs to >>>> be target space, too. >>>> >>>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >>>> --- >>>> linux-user/signal.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/linux-user/signal.c b/linux-user/signal.c >>>> index 4157154..c0a6f7e 100644 >>>> --- a/linux-user/signal.c >>>> +++ b/linux-user/signal.c >>>> @@ -3532,7 +3532,8 @@ static void setup_frame(int sig, struct >>>> target_sigaction *ka, >>>> >>>> /* Return from sighandler will jump to the tramp. >>>> Negative 8 offset because return is rtsd r15, 8 */ >>>> -env->regs[15] = ((unsigned long)frame->tramp) - 8; >>>> +env->regs[15] = frame_addr + offsetof(struct target_signal_frame, >>>> tramp) >>>> + - 8; >>>> } >>>> >>>> /* Set up registers for signal handler */ >>> >>> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> >>> >> >> Thank all of you for the 2 patches reviewing. >> >> I guess, this month, I may have free time (at least, will not be as busy >> as the previous month), I shall finish tilegx floating point insns (it >> has been delayed too long). > > Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com> > OK, thank you for your work. And now (finally), I guess, I should have free time for open source, and I should try to finish tilegx floating point insns within this month. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [Qemu-devel] [PATCH] linux-user/signal.c: Use target address instead of host address for microblaze restorer
On 5/5/16 00:05, Peter Maydell wrote: > On 29 March 2016 at 15:13, <cheng...@emindsoft.com.cn> wrote: >> From: Chen Gang <cheng...@emindsoft.com.cn> >> >> The return address is in target space, so the restorer address needs to >> be target space, too. >> >> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >> --- >> linux-user/signal.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/linux-user/signal.c b/linux-user/signal.c >> index 4157154..c0a6f7e 100644 >> --- a/linux-user/signal.c >> +++ b/linux-user/signal.c >> @@ -3532,7 +3532,8 @@ static void setup_frame(int sig, struct >> target_sigaction *ka, >> >> /* Return from sighandler will jump to the tramp. >> Negative 8 offset because return is rtsd r15, 8 */ >> -env->regs[15] = ((unsigned long)frame->tramp) - 8; >> +env->regs[15] = frame_addr + offsetof(struct target_signal_frame, >> tramp) >> + - 8; >> } >> >> /* Set up registers for signal handler */ > > Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> > Thank all of you for the 2 patches reviewing. I guess, this month, I may have free time (at least, will not be as busy as the previous month), I shall finish tilegx floating point insns (it has been delayed too long). Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [Qemu-devel] [PATCH v2] Use frame->retcode instead of frame address for alpha target restorer
Hello all: I am very sorry for delay finishing tilegx floating points insns. The reason is that during these days (I guess, until 2016-05-01), I have no any free time. - During these days, I am often not at home (sleep in a hotel near the office), and often work in the weekend. - It is about the android frameworks/base, which is mostly written in Java. It is of cause very simple to me, but really quite much work need be done. - And I guess, after 2016-05-01, I have enough free time resources on our tilegx floating point insns. And sorry again for my delaying, if we can not bear the time point, please help implement the tilegx floating point insns. Thanks. On 3/31/16 21:57, Chen Gang wrote: > > On 3/31/16 00:09, Laurent Vivier wrote: >> >> Le 30/03/2016 17:42, cheng...@emindsoft.com.cn a écrit : >>> From: Chen Gang <cheng...@emindsoft.com.cn> >>> >>> The restorer needs the return code address which is frame->retcode, not >>> frame itself. >>> >>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >> >> Reviewed-by: Laurent Vivier <laur...@vivier.eu> >> > > Thank all of you for your reviewing. > > Next, I shall try to finish tilegx floating point insns. > > Thanks. > -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [Qemu-devel] [PATCH v2] Use frame->retcode instead of frame address for alpha target restorer
On 3/31/16 00:09, Laurent Vivier wrote: > > Le 30/03/2016 17:42, cheng...@emindsoft.com.cn a écrit : >> From: Chen Gang <cheng...@emindsoft.com.cn> >> >> The restorer needs the return code address which is frame->retcode, not >> frame itself. >> >> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> > > Reviewed-by: Laurent Vivier <laur...@vivier.eu> > Thank all of you for your reviewing. Next, I shall try to finish tilegx floating point insns. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [Qemu-devel] [PATCH] linux-user/signal.c: Use frame->retcode instead of frame address for alpha target restorer
On 3/29/16 22:25, Laurent Vivier wrote: > Le 29/03/2016 16:01, cheng...@emindsoft.com.cn a écrit : >> The restorer needs the return code address which is frame->retcode, not >> frame itself. >> >> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >> --- >> linux-user/signal.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/linux-user/signal.c b/linux-user/signal.c >> index e487f9e..4157154 100644 >> --- a/linux-user/signal.c >> +++ b/linux-user/signal.c >> @@ -5455,7 +5455,7 @@ static void setup_rt_frame(int sig, struct >> target_sigaction *ka, >> >retcode[1]); >> __put_user(INSN_CALLSYS, >retcode[2]); >> /* imb(); */ >> -r26 = frame_addr; >> +r26 = frame_addr + offsetof(struct target_rt_sigframe, retcode); >> } >> >> if (err) { >> > > If you change setup_rt_frame(), you must update setup_frame() too. > Oh, yes, thanks. > It seems correct. > > Richard, as you have written the original code, could you check this is > correct? > Please give a check when you have time. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [Qemu-devel] [PATCH] linux-user/signal.c: Generate opcode data for restorer in setup_rt_frame
On 3/29/16 06:57, Chen Gang wrote: > On 3/29/16 06:17, Laurent Vivier wrote: >> >> The address of retcode in host and guest can differ. >> You need something like: >> >> restorer = (unsigned long)(frame_addr + offsetof(struct >> target_rt_sigframe, retcode)); >> >> I've experienced this on sh4 (see commit 2a0fa68) >> > > OK, thanks. What you said above sounds reasonable to me. :-) > > I shall send patch v2 for it (although tilegx is a pure 64-bit target, > with this patch, I guess, tilegx target should still run correctly under > 32-bit host). > > By the way, it looks that s390x and microblaze targets also have the > same issue. It looks the alpha target may also need consider about retcode, for "r26 = frame_addr" in setup_rt_frame(). Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [Qemu-devel] [PATCH] linux-user/signal.c: Generate opcode data for restorer in setup_rt_frame
On 3/29/16 06:17, Laurent Vivier wrote: >> On 3/15/16 05:51, cheng...@emindsoft.com.cn wrote: >>> >>> Original implementation uses do_rt_sigreturn directly in host space, >>> when a guest program is in unwind procedure in guest space, it will get >>> an incorrect restore address, then causes unwind failure. >>> >>> Also cleanup the original incorrect indentation. >>> >>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >>> --- >>> linux-user/signal.c | 12 ++-- >>> 1 file changed, 10 insertions(+), 2 deletions(-) >>> >>> diff --git a/linux-user/signal.c b/linux-user/signal.c >>> index 919aa83..0e3b1c6 100644 >>> --- a/linux-user/signal.c >>> +++ b/linux-user/signal.c >>> @@ -5566,8 +5566,13 @@ struct target_rt_sigframe { >>> unsigned char save_area[16]; /* caller save area */ >>> struct target_siginfo info; >>> struct target_ucontext uc; >>> +abi_ulong retcode[2]; >>> }; >>> >>> +#define INSN_MOVELI_R10_139 0x00045fe551483000ULL /* { moveli r10, 139 } >>> */ >>> +#define INSN_SWINT1 0x286b180051485000ULL /* { swint1 } */ >>> + >>> + >>> static void setup_sigcontext(struct target_sigcontext *sc, >>> CPUArchState *env, int signo) >>> { >>> @@ -5643,9 +5648,12 @@ static void setup_rt_frame(int sig, struct >>> target_sigaction *ka, >>> __put_user(target_sigaltstack_used.ss_size, >>> >uc.tuc_stack.ss_size); >>> setup_sigcontext(>uc.tuc_mcontext, env, info->si_signo); >>> >>> -restorer = (unsigned long) do_rt_sigreturn; >>> if (ka->sa_flags & TARGET_SA_RESTORER) { >>> -restorer = (unsigned long) ka->sa_restorer; >>> +restorer = (unsigned long) ka->sa_restorer; >>> +} else { >>> +__put_user(INSN_MOVELI_R10_139, >retcode[0]); >>> +__put_user(INSN_SWINT1, >retcode[1]); >>> +restorer = (unsigned long)frame->retcode; > > The address of retcode in host and guest can differ. > You need something like: > > restorer = (unsigned long)(frame_addr + offsetof(struct > target_rt_sigframe, retcode)); > > I've experienced this on sh4 (see commit 2a0fa68) > OK, thanks. What you said above sounds reasonable to me. :-) I shall send patch v2 for it (although tilegx is a pure 64-bit target, with this patch, I guess, tilegx target should still run correctly under 32-bit host). By the way, it looks that s390x and microblaze targets also have the same issue. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [Qemu-devel] [PATCH] target-tilegx: Support dumping working flow instructions and registers
Hello all: Is this patch helpful? If it shouldn't be merged into our master branch, please let me know, I shall treat it as my own local patch (when I update my local master branch, I'll merge it at last). Thanks. On 3/16/16 23:51, Chen Gang wrote: > Hello all: > > It is only for analyzing issues, I guess it is helpful, so I send it, > welcome any other ideas, suggestions, and completions. > > And next, I shall continue to improve the floating point instruction > features: "remove (u)int64_to_float64 from fdouble implementation". > > Thanks. > > On 3/16/16 23:38, cheng...@emindsoft.com.cn wrote: >> From: Chen Gang <gang.chen.5...@gmail.com> >> >> It is only for debug and analyzing tilegx qemu related issues. >> >> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >> --- >> target-tilegx/helper.c| 34 ++ >> target-tilegx/helper.h| 2 ++ >> target-tilegx/translate.c | 37 + >> 3 files changed, 73 insertions(+) >> >> diff --git a/target-tilegx/helper.c b/target-tilegx/helper.c >> index 616c5c7..c7cfe66 100644 >> --- a/target-tilegx/helper.c >> +++ b/target-tilegx/helper.c >> @@ -160,3 +160,37 @@ uint64_t helper_cmul2(uint64_t srca, uint64_t srcb, int >> shift, int round) >> >> return deposit32(realr >> shift, 16, 16, imagr >> shift); >> } >> + >> +extern char **debug_buf; >> + >> +#define DEBUG_DUMP_REGS >> + >> +static void dump_regs(CPUTLGState *env, int idx) >> +{ >> +#ifdef DEBUG_DUMP_REGS >> +int i; >> + >> +if (debug_buf[idx][2] == '\n') { >> +for (i = 0; i < TILEGX_R_COUNT; i++) { >> +if (!(i % 4)) { >> +fprintf(stderr, "\n"); >> +} >> +fprintf(stderr, "r%2.2u = %16.16lx, ", i, env->regs[i]); >> +} >> +fprintf(stderr, "\n"); >> +for (i = 0; i < TILEGX_SPR_COUNT; i++) { >> +if (!(i % 8)) { >> +fprintf(stderr, "\n"); >> +} >> +fprintf(stderr, "s%2.2u = %16.16lx, ", i, env->spregs[i]); >> +} >> +fprintf(stderr, "\n"); >> +} >> +#endif >> +} >> + >> +void helper_print_step(CPUTLGState *env, int idx) >> +{ >> +fprintf(stderr, "%s", debug_buf[idx]); >> +dump_regs(env, idx); >> +} >> diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h >> index 9281d0f..74f796a 100644 >> --- a/target-tilegx/helper.h >> +++ b/target-tilegx/helper.h >> @@ -24,3 +24,5 @@ DEF_HELPER_FLAGS_2(v1shrs, TCG_CALL_NO_RWG_SE, i64, i64, >> i64) >> DEF_HELPER_FLAGS_2(v2shl, TCG_CALL_NO_RWG_SE, i64, i64, i64) >> DEF_HELPER_FLAGS_2(v2shru, TCG_CALL_NO_RWG_SE, i64, i64, i64) >> DEF_HELPER_FLAGS_2(v2shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64) >> + >> +DEF_HELPER_2(print_step, void, env, int) >> diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c >> index 03918eb..8489494 100644 >> --- a/target-tilegx/translate.c >> +++ b/target-tilegx/translate.c >> @@ -47,6 +47,43 @@ static const char * const reg_names[64] = { >> "sn", "idn0", "idn1", "udn0", "udn1", "udn2", "udn2", "zero" >> }; >> >> +#define STATIC_DEBUG >> + >> +char **debug_buf; >> + >> +#ifdef STATIC_DEBUG >> + >> +#include >> + >> +#define qemu_log_mask(flag, fmt, ...) debug_print(flag, fmt, ##__VA_ARGS__) >> + >> +static int debug_idx; >> + >> +extern void debug_print(int flag, const char *fmt, ...); >> + >> +void debug_print(int flag, const char *fmt, ...) >> +{ >> +va_list args; >> +char buf[0x400]; >> +TCGv_i32 tmp; >> + >> +va_start(args, fmt); >> +vsprintf(buf, fmt, args); >> +va_end(args); >> + >> +if (!debug_buf) { >> +debug_buf = malloc(sizeof(char *) * 0x40); >> +} >> +debug_buf[debug_idx] = malloc(strlen(buf) + 1); >> +strcpy(debug_buf[debug_idx], buf); >> + >> +tmp = tcg_const_i32(debug_idx++); >> +gen_helper_print_step(cpu_env, tmp); >> +tcg_temp_free_i32(tmp); >> +} >> + >> +#endif >> + >> /* Modified registers are cached in temporaries until the end of the >> bundle. */ >> typedef struct { >> unsigned reg; >> > -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [Qemu-devel] [PATCH] linux-user/signal.c: Generate opcode data for restorer in setup_rt_frame
Hello All: Please help check this patch when you have time. After this patch, we can let gcc testsuite cleanup-10 run successfully. Next, I shall continue to implement floating point instructions: remove (u)int64_to_float64 from fdouble implementation. Thanks. On 3/15/16 05:51, cheng...@emindsoft.com.cn wrote: > From: Chen Gang <cheng...@emindsoft.com.cn> > > Original implementation uses do_rt_sigreturn directly in host space, > when a guest program is in unwind procedure in guest space, it will get > an incorrect restore address, then causes unwind failure. > > Also cleanup the original incorrect indentation. > > Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> > --- > linux-user/signal.c | 12 ++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/linux-user/signal.c b/linux-user/signal.c > index 919aa83..0e3b1c6 100644 > --- a/linux-user/signal.c > +++ b/linux-user/signal.c > @@ -5566,8 +5566,13 @@ struct target_rt_sigframe { > unsigned char save_area[16]; /* caller save area */ > struct target_siginfo info; > struct target_ucontext uc; > +abi_ulong retcode[2]; > }; > > +#define INSN_MOVELI_R10_139 0x00045fe551483000ULL /* { moveli r10, 139 } */ > +#define INSN_SWINT1 0x286b180051485000ULL /* { swint1 } */ > + > + > static void setup_sigcontext(struct target_sigcontext *sc, > CPUArchState *env, int signo) > { > @@ -5643,9 +5648,12 @@ static void setup_rt_frame(int sig, struct > target_sigaction *ka, > __put_user(target_sigaltstack_used.ss_size, > >uc.tuc_stack.ss_size); > setup_sigcontext(>uc.tuc_mcontext, env, info->si_signo); > > -restorer = (unsigned long) do_rt_sigreturn; > if (ka->sa_flags & TARGET_SA_RESTORER) { > -restorer = (unsigned long) ka->sa_restorer; > +restorer = (unsigned long) ka->sa_restorer; > +} else { > +__put_user(INSN_MOVELI_R10_139, >retcode[0]); > +__put_user(INSN_SWINT1, >retcode[1]); > + restorer = (unsigned long)frame->retcode; > } > env->pc = (unsigned long) ka->_sa_handler; > env->regs[TILEGX_R_SP] = (unsigned long) frame; > -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [Qemu-devel] [PATCH] target-tilegx: Support dumping working flow instructions and registers
Hello all: It is only for analyzing issues, I guess it is helpful, so I send it, welcome any other ideas, suggestions, and completions. And next, I shall continue to improve the floating point instruction features: "remove (u)int64_to_float64 from fdouble implementation". Thanks. On 3/16/16 23:38, cheng...@emindsoft.com.cn wrote: > From: Chen Gang <gang.chen.5...@gmail.com> > > It is only for debug and analyzing tilegx qemu related issues. > > Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> > --- > target-tilegx/helper.c| 34 ++ > target-tilegx/helper.h| 2 ++ > target-tilegx/translate.c | 37 + > 3 files changed, 73 insertions(+) > > diff --git a/target-tilegx/helper.c b/target-tilegx/helper.c > index 616c5c7..c7cfe66 100644 > --- a/target-tilegx/helper.c > +++ b/target-tilegx/helper.c > @@ -160,3 +160,37 @@ uint64_t helper_cmul2(uint64_t srca, uint64_t srcb, int > shift, int round) > > return deposit32(realr >> shift, 16, 16, imagr >> shift); > } > + > +extern char **debug_buf; > + > +#define DEBUG_DUMP_REGS > + > +static void dump_regs(CPUTLGState *env, int idx) > +{ > +#ifdef DEBUG_DUMP_REGS > +int i; > + > +if (debug_buf[idx][2] == '\n') { > +for (i = 0; i < TILEGX_R_COUNT; i++) { > +if (!(i % 4)) { > +fprintf(stderr, "\n"); > +} > +fprintf(stderr, "r%2.2u = %16.16lx, ", i, env->regs[i]); > +} > +fprintf(stderr, "\n"); > +for (i = 0; i < TILEGX_SPR_COUNT; i++) { > +if (!(i % 8)) { > +fprintf(stderr, "\n"); > +} > +fprintf(stderr, "s%2.2u = %16.16lx, ", i, env->spregs[i]); > +} > +fprintf(stderr, "\n"); > +} > +#endif > +} > + > +void helper_print_step(CPUTLGState *env, int idx) > +{ > +fprintf(stderr, "%s", debug_buf[idx]); > +dump_regs(env, idx); > +} > diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h > index 9281d0f..74f796a 100644 > --- a/target-tilegx/helper.h > +++ b/target-tilegx/helper.h > @@ -24,3 +24,5 @@ DEF_HELPER_FLAGS_2(v1shrs, TCG_CALL_NO_RWG_SE, i64, i64, > i64) > DEF_HELPER_FLAGS_2(v2shl, TCG_CALL_NO_RWG_SE, i64, i64, i64) > DEF_HELPER_FLAGS_2(v2shru, TCG_CALL_NO_RWG_SE, i64, i64, i64) > DEF_HELPER_FLAGS_2(v2shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64) > + > +DEF_HELPER_2(print_step, void, env, int) > diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c > index 03918eb..8489494 100644 > --- a/target-tilegx/translate.c > +++ b/target-tilegx/translate.c > @@ -47,6 +47,43 @@ static const char * const reg_names[64] = { > "sn", "idn0", "idn1", "udn0", "udn1", "udn2", "udn2", "zero" > }; > > +#define STATIC_DEBUG > + > +char **debug_buf; > + > +#ifdef STATIC_DEBUG > + > +#include > + > +#define qemu_log_mask(flag, fmt, ...) debug_print(flag, fmt, ##__VA_ARGS__) > + > +static int debug_idx; > + > +extern void debug_print(int flag, const char *fmt, ...); > + > +void debug_print(int flag, const char *fmt, ...) > +{ > +va_list args; > +char buf[0x400]; > +TCGv_i32 tmp; > + > +va_start(args, fmt); > +vsprintf(buf, fmt, args); > +va_end(args); > + > +if (!debug_buf) { > +debug_buf = malloc(sizeof(char *) * 0x40); > +} > +debug_buf[debug_idx] = malloc(strlen(buf) + 1); > +strcpy(debug_buf[debug_idx], buf); > + > +tmp = tcg_const_i32(debug_idx++); > +gen_helper_print_step(cpu_env, tmp); > +tcg_temp_free_i32(tmp); > +} > + > +#endif > + > /* Modified registers are cached in temporaries until the end of the bundle. > */ > typedef struct { > unsigned reg; > -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [Qemu-devel] [PATCH v2 1/3] linux-user/mmap.c: Set prot page flags for the correct region in mmap_frag()
On 2016年01月28日 22:54, Peter Maydell wrote: > On 27 January 2016 at 01:37, Chen Gang <cheng...@emindsoft.com.cn> wrote: >> Within one single call to target_mmap(), it should be OK. >> >> But multiple call to target_mmap(), may call mmap_frag() multiple times >> for the same host page (also for the same target page). In our case: >> >> - 4600 >> mmap2(0x0034,135168,PROT_READ,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED,-1,0) >> = 0x0034 >> >>It will call mmap_frag() with start address 0x0034 + 128KB, and >>set the target page with PAGE_VALID. But left the half below host >>page without PAGE_VALID. > > So, just to put some numbers in here: > > 0x34 .. 0x34 0x35 .. 0x35 0x36 .. 0x360fff >(64k, first host page) (64k, second host page) (4k guest page) > > and we call mmap_frag() once for that last 4K fragment. It should: > * allocate a host page (since none is there yet) > * return to target_mmap, which will mark the range >0x3f .. 0x360fff as PROT_VALID (together with the other >read/write/etc permissions) > > I think this part is definitely correct. > Yes to me. >> - 4600 mmap2(0x0034,135168,PROT_READ,MAP_SHARED|MAP_FIXED,8,0) = >> 0x0034 >> >>It will call mmap_frag() with start address 0x0034 + 128KB, and >>check the half below host page which has no PAGE_VALID, then "prot1 >>== 0", mmap_frag() thinks "no page was there, so we allocate one". > > On the second call, we again call mmap_frag for that last 4K. > We check for any other valid guest pages in the 64k host page, > and there aren't any. This will indeed cause us to mmap() again, > which ideally we would not. But: > OK. > (1) Is this actually causing anything to fail? Calling host > mmap() again is ever so slightly inefficient, but I don't think > that it causes the guest to see anything wrong. > For me, something may be a little complex (assume 8KB host page, 4KB guest page): - 1st mmap2() is for MAP_PRIVATE, 2nd mmap2() is for MAP_SHARED. - So, if 2nd call mmap_frag() with the same start address only calls mprotect(), doesn't call mmap2() again, the target page will be still MAP_PRIVATE? (but caller wants it to be MAP_SHARED). And theoretically, if the caller wants the 2 target pages within a host page have different mapping attributes (e.g. half top host page is MAP_SHARED, but half bottom host page is MAP_PRIVATE): - I guess, our current softmmu can not do that (we have to implment softmmu again, just like rth said originally). - But lucky to me, Wine will manage the whole memory by its own, and also windows its own also manage its whole memory. They try to be as simple as they can. So I guess, current softmmu is enough to me. > (2) If we do want to fix this, your fix is doing the wrong thing. > It is correct that we don't mark the areas outside the guest page > as PROT_VALID, because they are not valid guest memory. If you > want to avoid the mmap() you need to change the condition we're using > to decide whether to mmap() a fresh host page (it would need to > look at the PROT_VALID bits within the new guest mapping, not just > the ones outside it). Something like: > > /* get the protection of the target pages outside the mapping, > * and check whether we already have a host page allocated > */ > prot1 = 0; > havevalid = 0; > for(addr = real_start; addr < real_end; addr++) { > int pageprot = page_get_flags(addr); > if (addr < start || addr >= end) { > prot1 |= pageprot; > } > havevalid |= pageprot; > } > > if (!havevalid) { > /* no page was there, so ... */ > ... > } > What you said above sounds OK to me, if we don't consider about MAP_PRIVATE or MAP_SHARED. After think of again, for me: we need keep the current code no touch, but the related comments "/* no page was there, so ... */" need be improved, I guess, it should be: - If there is no host page or only one target page, we need call mmap2 again, which will satisfy the parameter 'flags' (e.g. MAP_PRIVATE or MAP_SHARED), else FIXME: at present, the 'flags' has to be skipped. > But I think it's only worth making this change if we're fixing > a real bug where the guest behaves wrongly. > It sounds OK to me. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v2 1/3] linux-user/mmap.c: Set prot page flags for the correct region in mmap_frag()
On 2016年01月26日 17:11, Peter Maydell wrote: > On 26 January 2016 at 02:58, Chen Gang <cheng...@emindsoft.com.cn> wrote: >> The related comments for "if (prot1 == 0)" code block is "no page was >> there, so we allocate one". >> >> So I guess this code block is not only allocate page for guest, but also >> for host. So prot1 is not only for the guest page, but also for host >> page. > > The comment means specifically "allocate a host page". > OK, thanks. >> If we do not page_set_flags with PAGE_VALID, The next call >> in mmap_frag for the same area will let prot1 be 0, so still >> fall into "if (prot1 == 0)" code block. > > But in what case will we call mmap_frag() again before we > call page_set_flags() at the bottom of target_mmap()? > That is what is not clear to me, and why I asked you to describe > what the case is that you're seeing problems with. > When I run WeChat.exe with i386 wine with qemu-i386 under sw_64 arch. - The related command: "./i386-linux-user/qemu-i386 -strace -L /upstream/i386_wine /upstream/i386_wine/usr/local/bin/wine "C:\\Program Files\\Tencent\\WeChat\\WeChat.exe" > ana/try/info-strace.log 2>&1" - The related output (no any munmap, 135168 = 128KB + 4KB): 4600 mmap2(0x0034,135168,PROT_READ,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED,-1,0) = 0x0034 4600 mmap2(0x0034,135168,PROT_READ,MAP_SHARED|MAP_FIXED,8,0) = 0x0034 4600 rt_sigprocmask(SIG_SETMASK,0x0033f574,NULL) = 0 4600 rt_sigprocmask(SIG_BLOCK,0x7bced7e0,0x0033f5d0) = 0 4600 write(3,0x33f6cc,64) = 64 4600 read(4,0x33f6cc,64) = 1 4600 rt_sigprocmask(SIG_SETMASK,0x0033f5d0,NULL) = 0 4600 close(8) = 0 4600 rt_sigprocmask(SIG_BLOCK,0x7bced7e0,0x0033f674) = 0 4600 mprotect(0x0016,65536,PROT_READ|PROT_WRITE) = 0 4600 rt_sigprocmask(SIG_SETMASK,0x0033f674,NULL) = 0 4600 rt_sigprocmask(SIG_BLOCK,0x7bced7e0,0x0033f990) = 0 4600 mmap2(0x0034,135168,PROT_NONE,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED|MAP_NORESERVE,-1,0) = 0x0034 wine often does like above, map the same position multiple times. > Reading the target_mmap() code, its intention seems to be: > (a) if the whole allocation fits in one host page, call > mmap_frag() once and then "goto the_end1" Also yes to me. > (b) otherwise, we'll call mmap_frag() once for the start > of the guest mapping, and once for the end, which must > be two different host pages > Also yes to me. > So if you're seeing mmap_frag() called twice for the same > host page then something is going wrong, but I'm not sure what. > For the case I provide above, it can call mmap_frag() twice for the same host page. By the way, after have a full test again, all related issues are OK, it seems we need not this patch to fix current issues, it is really very strange to me!(maybe it is fixed by my other patches? I don't know) At present, our sw_64 qemu-i386 status: - Can run notepad.exe correctly, can run acdsee5.0.exe setup program successfully. - The performance is acceptable, after optimize the wine code (simply use 32 split times instead of 2 for reserve_area recursion), the initialization speed is really quick enough. :-) - When run WeChat.exe, it can popup connection GUI box, but will quit under sw_64. But for x86_64 qemu-i386, it can run WeChat.exe correctly (although after enter main gui, it is not stable enough). Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v2 1/3] linux-user/mmap.c: Set prot page flags for the correct region in mmap_frag()
On 2016年01月26日 18:26, Peter Maydell wrote: > On 26 January 2016 at 10:19, Chen Gang <cheng...@emindsoft.com.cn> wrote: >> When I run WeChat.exe with i386 wine with qemu-i386 under sw_64 arch. >> >> - The related command: >> >>"./i386-linux-user/qemu-i386 -strace -L /upstream/i386_wine >> /upstream/i386_wine/usr/local/bin/wine "C:\\Program >> Files\\Tencent\\WeChat\\WeChat.exe" > ana/try/info-strace.log 2>&1" >> >> - The related output (no any munmap, 135168 = 128KB + 4KB): >> >>4600 >> mmap2(0x0034,135168,PROT_READ,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED,-1,0) >> = 0x0034 >>4600 mmap2(0x0034,135168,PROT_READ,MAP_SHARED|MAP_FIXED,8,0) = >> 0x0034 >>4600 rt_sigprocmask(SIG_SETMASK,0x0033f574,NULL) = 0 >>4600 rt_sigprocmask(SIG_BLOCK,0x7bced7e0,0x0033f5d0) = 0 >>4600 write(3,0x33f6cc,64) = 64 >>4600 read(4,0x33f6cc,64) = 1 >>4600 rt_sigprocmask(SIG_SETMASK,0x0033f5d0,NULL) = 0 >>4600 close(8) = 0 >>4600 rt_sigprocmask(SIG_BLOCK,0x7bced7e0,0x0033f674) = 0 >>4600 mprotect(0x0016,65536,PROT_READ|PROT_WRITE) = 0 >>4600 rt_sigprocmask(SIG_SETMASK,0x0033f674,NULL) = 0 >>4600 rt_sigprocmask(SIG_BLOCK,0x7bced7e0,0x0033f990) = 0 >>4600 >> mmap2(0x0034,135168,PROT_NONE,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED|MAP_NORESERVE,-1,0) >> = 0x0034 >> >> wine often does like above, map the same position multiple times. > > That output seems to show all the mmap calls working fine, though. > OK, thanks. >> >> For the case I provide above, it can call mmap_frag() twice for the same >> host page. > > For the same single call to target_mmap() ? What is the code flow > within QEMU that causes this? > Within one single call to target_mmap(), it should be OK. But multiple call to target_mmap(), may call mmap_frag() multiple times for the same host page (also for the same target page). In our case: - 4600 mmap2(0x0034,135168,PROT_READ,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED,-1,0) = 0x0034 It will call mmap_frag() with start address 0x0034 + 128KB, and set the target page with PAGE_VALID. But left the half below host page without PAGE_VALID. - 4600 mmap2(0x0034,135168,PROT_READ,MAP_SHARED|MAP_FIXED,8,0) = 0x0034 It will call mmap_frag() with start address 0x0034 + 128KB, and check the half below host page which has no PAGE_VALID, then "prot1 == 0", mmap_frag() thinks "no page was there, so we allocate one". - But in fact, the first mmap_frag() has already allocated one page at 0x0034 + 128KB. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v2 1/3] linux-user/mmap.c: Set prot page flags for the correct region in mmap_frag()
Firstly, thank you very much to still focus on this patch, which will be much helpful for my current work! And I modified some code, but did not send patches to upstream, e.g. sw_64 related code, use TARGET_PAGE_SIZE instead of HOST_PAGE_SIZE ...). I skipped MAP_SHARED with PROT_WRITE check to skip some another issues, (since I guess for running wine, it should be OK). maybe it be related with this issue? diff --git a/linux-user/mmap.c b/linux-user/mmap.c index 86c270b..a76450a 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -170,12 +170,13 @@ static int mmap_frag(abi_ulong real_start, prot_new = prot | prot1; if (!(flags & MAP_ANONYMOUS)) { +#if 0 /* msync() won't work here, so we return an error if write is possible while it is a shared mapping */ if ((flags & MAP_TYPE) == MAP_SHARED && (prot & PROT_WRITE)) return -1; - +#endif /* adjust protection to be able to read */ if (!(prot1 & PROT_WRITE)) mprotect(host_start, qemu_host_page_size, prot1 | PROT_WRITE); And the other related reply for this thread is at the bottom of this mail, please check. On 2016年01月25日 23:07, Peter Maydell wrote: > On 11 January 2016 at 09:01, <cheng...@emindsoft.com.cn> wrote: >> From: Chen Gang <cheng...@emindsoft.com.cn> >> >> mmap() size in mmap_frag() is qemu_host_page_size, but the outside calls >> page_set_flags() may be not with qemu_host_page_size. So after mmap(), >> call page_set_flags() in time. >> >> After this fix, for the next call for the same region, prot1 will be >> PAGE_VALID (not 0), so can avoid to enter "if (prot1 == 0)" case, again. >> >> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >> --- >> linux-user/mmap.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/linux-user/mmap.c b/linux-user/mmap.c >> index 445e8c6..7807ed0 100644 >> --- a/linux-user/mmap.c >> +++ b/linux-user/mmap.c >> @@ -162,6 +162,8 @@ static int mmap_frag(abi_ulong real_start, >> flags | MAP_ANONYMOUS, -1, 0); >> if (p == MAP_FAILED) >> return -1; >> +page_set_flags(real_start, real_start + qemu_host_page_size, >> + PAGE_VALID); >> prot1 = prot; >> } >> prot1 &= PAGE_BITS; > > I'm confused about this change, because as far as I can see > page_set_flags/page_get_flags work on guest pages, not host pages. > So setting the flags for the whole of the host page to PAGE_VALID > doesn't seem right -- the other guest pages within this host page > are not valid. And the VALID bit should be set for the guest page > that is within the mapping as part of the call to page_set_flags() > at the bottom of target_mmap(). > The related comments for "if (prot1 == 0)" code block is "no page was there, so we allocate one". So I guess this code block is not only allocate page for guest, but also for host. So prot1 is not only for the guest page, but also for host page. If we do not page_set_flags with PAGE_VALID, The next call in mmap_frag for the same area will let prot1 be 0, so still fall into "if (prot1 == 0)" code block. > It would help if you could explain what the failure case is that > you're trying to fix, and how the current code fails. > I shall give a full test again, and provide the test result later. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Let lv always match val in do_getsockopt()
On 2016年01月14日 17:41, Laurent Vivier wrote: > > Le 14/01/2016 10:37, Chen Gang a écrit : >> >> On 2016年01月14日 17:10, Laurent Vivier wrote: >>> >>> Le 14/01/2016 10:01, Chen Gang a écrit : >>>> >>>> I am not quite sure whether kernel always returns sizeof(int) (I guess, >>>> it should be). >>> >>> it can be 1 only if len is 1, but this is managed below. >>> >> >> Excuse me, I do not quite understand your meaning. >> >> For me, if we are sure lv is always 4 for SO_TYPE, we don't need this >> patch. >> >> If lv may be 1 for SO_TYPE (it means val >> 8 may be zero), after call >> host_to_target_sock_type, val >> 8 may be non-zero, theoretically. In >> this case, we need modify lv to 4. >> >>>> For me, if you are sure, we can skip this patch. >>> >>> Does it fix something ? >>> >> >> It is only theoretical, not real world, at present. > > So I think we can skip it. > It is OK to me. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] translate-all.c: Remove writable protection feature for tb_alloc_page()
On 2016年01月14日 18:30, Peter Maydell wrote: > On 14 January 2016 at 10:26, Chen Gang <cheng...@emindsoft.com.cn> wrote: >> On 2016年01月14日 18:05, Peter Maydell wrote: >>> If we don't mark the page as non-writeable when we generate a TB >>> from it, how do we detect when guest code later writes to that >>> page (which means we need to invalidate the TB) ? >>> >> >> For me, what you said above sounds reasonable, at present, that's really >> valuable to me :-) >> >> I guess, you also mean: our qemu will catch the host page fault signal >> and invalidate the TB. > > Yes, this is how it works for user-mode. (For softmmu we can catch > writes and send them via the slow path which does the check for > whether TBs need to be invalidated; for linux-user we have no > emulated MMU so we must rely on the host kernel sending us the > SIGSEGV.) The bit of code that does this is at the top of > handle_cpu_signal(): > > if (is_write && h2g_valid(address) > && page_unprotect(h2g(address), pc, puc)) { > return 1; > } > OK, thank you very much! :-) Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Let lv always match val in do_getsockopt()
On 2016年01月14日 17:10, Laurent Vivier wrote: > > Le 14/01/2016 10:01, Chen Gang a écrit : >> >> I am not quite sure whether kernel always returns sizeof(int) (I guess, >> it should be). > > it can be 1 only if len is 1, but this is managed below. > Excuse me, I do not quite understand your meaning. For me, if we are sure lv is always 4 for SO_TYPE, we don't need this patch. If lv may be 1 for SO_TYPE (it means val >> 8 may be zero), after call host_to_target_sock_type, val >> 8 may be non-zero, theoretically. In this case, we need modify lv to 4. >> For me, if you are sure, we can skip this patch. > > Does it fix something ? > It is only theoretical, not real world, at present. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Let lv always match val in do_getsockopt()
On 2016年01月14日 16:15, Laurent Vivier wrote: > Le 14/01/2016 07:24, cheng...@emindsoft.com.cn a écrit : >> From: Chen Gang <cheng...@emindsoft.com.cn> >> >> After host_to_target_sock_type(), the length of val may be changed, so >> calculate the related lv, too. >> >> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >> --- >> linux-user/syscall.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >> index fcdca2a..0e95f35 100644 >> --- a/linux-user/syscall.c >> +++ b/linux-user/syscall.c >> @@ -1841,6 +1841,7 @@ static abi_long do_getsockopt(int sockfd, int level, >> int optname, >> return ret; >> if (optname == SO_TYPE) { >> val = host_to_target_sock_type(val); >> +lv = (val >> 8) ? 4 : 1; > > It seems the kernel always returns sizeof(int) (for all archs), what is > the aim of reducing the size ? > I am not quite sure whether kernel always returns sizeof(int) (I guess, it should be). For me, if you are sure, we can skip this patch. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] translate-all.c: Remove writable protection feature for tb_alloc_page()
On 2016年01月14日 18:05, Peter Maydell wrote: > On 14 January 2016 at 06:03, <cheng...@emindsoft.com.cn> wrote: >> From: Chen Gang <cheng...@emindsoft.com.cn> >> >> Guest may allocate a readable, writable, and executable page, then write >> data on the page, and execute data as code on the page too, then write >> anther data still within the page. >> >> So remove this feature from linux-user: it not only consumes a little >> performance, but also causes issue with the old Linux kernel under some >> of architectures (they will directly generate segment fault for it). > > If we don't mark the page as non-writeable when we generate a TB > from it, how do we detect when guest code later writes to that > page (which means we need to invalidate the TB) ? > For me, what you said above sounds reasonable, at present, that's really valuable to me :-) I guess, you also mean: our qemu will catch the host page fault signal and invalidate the TB. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v2] linux-user/syscall.c: Add SO_RCVTIMEO and SO_SNDTIMEO for getsockopt
On 1/11/16 19:04, Peter Maydell wrote: > On 11 January 2016 at 10:31, Chen Gang <cheng...@emindsoft.com.cn> wrote: > [various things about a patch] > > Why have we dropped qemu-devel from this email thread? > Oh, sorry, I did not notice about it. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Add SO_RCVTIMEO and SO_SNDTIMEO for getsockopt
Oh, sorry, after check again, I guess, we need continue to discuss. On 2016年01月08日 17:40, Chen Gang wrote: > > On 2016年01月08日 16:25, Laurent Vivier wrote: >> >>> +if (optlen < sizeof(struct target_timeval)) { >>> +return -TARGET_EINVAL; >>> +} >> >> You don't have to check the len (kernel doesn't), EINVAL is not listed >> in the getsockopt() error cases, it should be an EFAULT, and this will >> be managed by copy_to_user_timeval(). >> After "man getsockopt", there is EINVAL in its error cases. > OK. > >>> +lv = sizeof(tv); >>> +ret = get_errno(getsockopt(sockfd, level, optname, , )); >>> +if (ret < 0) { >>> +return ret; >>> +} >> >> if (len > lv) >> len = lv; >> For me, len is for target, lv is for host, they cann't be compared with each other. > > OK. > >>> +if (copy_to_user_timeval(optval_addr, )) { >>> +return -TARGET_EFAULT; >>> +} >>> +if (put_user_u32(sizeof(struct target_timeval), optlen)) { >>> +return -TARGET_EFAULT; >>> +} >> >> if (put_user_u32(len, optlen)) >> return -TARGET_EFAULT; >> For me, we still need put sizeof(struct target_timeval) to optlen, since the host lengh should be sizeof(struct timeval). > > OK. I shall send patch v2 for it, in the next week. > > Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Let tilegx support sigaltstack
On 1/10/16 18:43, Laurent Vivier wrote: > Michael Tokarev has already enable sigaltstack() for all architecture: > > https://patchwork.ozlabs.org/patch/561514/ > > It is in the "-for-upstream" tree of Riku. > OK, thanks. I shall continue solving existing issues for qemu tilegx, during my free time. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Add SO_RCVTIMEO and SO_SNDTIMEO for getsockopt
Firstly, thank you very much for your careful work. On 2016年01月08日 16:25, Laurent Vivier wrote: > > > Le 08/01/2016 02:59, cheng...@emindsoft.com.cn a écrit : [...] >> @@ -1692,10 +1693,30 @@ static abi_long do_getsockopt(int sockfd, int level, >> int optname, >> switch (optname) { >> /* These don't just return a single integer */ >> case TARGET_SO_LINGER: >> -case TARGET_SO_RCVTIMEO: >> -case TARGET_SO_SNDTIMEO: >> case TARGET_SO_PEERNAME: >> goto unimplemented; >> + > > useless blank line > OK. >> +case TARGET_SO_RCVTIMEO: >> +optname = SO_RCVTIMEO; >> +goto time_case; >> +case TARGET_SO_SNDTIMEO: >> +optname = SO_SNDTIMEO; >> +time_case: > > Something like in "int_case", I think optlen is a pointer, not the length: > > if (get_user_u32(len, optlen)) > return -TARGET_EFAULT; > if (len < 0) > return -TARGET_EINVAL; > OK. > >> +if (optlen < sizeof(struct target_timeval)) { >> +return -TARGET_EINVAL; >> +} > > You don't have to check the len (kernel doesn't), EINVAL is not listed > in the getsockopt() error cases, it should be an EFAULT, and this will > be managed by copy_to_user_timeval(). > OK. >> +lv = sizeof(tv); >> +ret = get_errno(getsockopt(sockfd, level, optname, , )); >> +if (ret < 0) { >> +return ret; >> +} > > if (len > lv) > len = lv; > OK. >> +if (copy_to_user_timeval(optval_addr, )) { >> +return -TARGET_EFAULT; >> +} >> +if (put_user_u32(sizeof(struct target_timeval), optlen)) { >> +return -TARGET_EFAULT; >> +} > > if (put_user_u32(len, optlen)) > return -TARGET_EFAULT; > OK. I shall send patch v2 for it, in the next week. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user: syscall: Add SO_LINGER for setsockopt
On 2016年01月08日 17:57, Laurent Vivier wrote: > >>>> +return get_errno(setsockopt(sockfd, SOL_SOCKET, optname, >>>> + , sizeof(lg))); >>> >>> Why do you use "SOL_SOCKET" instead of "level" ? >>> >> >> At present, level is TARGET_SOL_SOCKET, but we need SOL_SOCKET. > > Yes, you're right... so there is a bug in TARGET_SO_BINDTODEVICE which > is using "level" :) > OK, thanks. I shall send patch v2 for it, next week. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user: syscall: Add SO_LINGER for setsockopt
On 2016年01月08日 16:38, Laurent Vivier wrote: > >> +if (!lock_user_struct(VERIFY_READ, tlg, optval_addr, 1)) { >> +return -TARGET_EFAULT; >> +} >> +__get_user(lg.l_onoff, >l_onoff); >> +__get_user(lg.l_linger, >l_linger); >> +unlock_user_struct(tlg, optval_addr, 0); > > You can't unlock the structure you're going to use. > OK, thanks. >> +return get_errno(setsockopt(sockfd, SOL_SOCKET, optname, >> + , sizeof(lg))); > > Why do you use "SOL_SOCKET" instead of "level" ? > At present, level is TARGET_SOL_SOCKET, but we need SOL_SOCKET. >> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h >> index 9d3c537..5a4d565 100644 >> --- a/linux-user/syscall_defs.h >> +++ b/linux-user/syscall_defs.h >> @@ -165,6 +165,11 @@ struct target_ip_mreq_source { >> uint32_t imr_sourceaddr; >> }; >> >> +struct target_linger { >> +int l_onoff;/* Linger active*/ >> +int l_linger; /* How long to linger for */ >> +}; >> + > > Must be "abi_int" to force good alignment for the target. > OK, thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v2] linux-user/mmap.c: Set prot page flags for the correct region in mmap_frag()
On 2016年01月08日 03:35, Peter Maydell wrote: > > Please don't do multiple things in a single patch. This patch > has all of: > * a fix for an unnecessary inefficiency > * a coding style change with no functional effects > * a bug fix > > Mixing them up together like this makes it harder to evaluate > the bug fix, which is the important part of the change. > OK, thanks. >> diff --git a/linux-user/mmap.c b/linux-user/mmap.c >> index 445e8c6..7920c5e 100644 >> --- a/linux-user/mmap.c >> +++ b/linux-user/mmap.c >> @@ -151,17 +151,19 @@ static int mmap_frag(abi_ulong real_start, >> >> /* get the protection of the target pages outside the mapping */ >> prot1 = 0; >> -for(addr = real_start; addr < real_end; addr++) { >> +for (addr = real_start; addr < real_end; addr += TARGET_PAGE_SIZE) { >> if (addr < start || addr >= end) >> prot1 |= page_get_flags(addr); >> } >> >> if (prot1 == 0) { >> /* no page was there, so we allocate one */ >> -void *p = mmap(host_start, qemu_host_page_size, prot, >> - flags | MAP_ANONYMOUS, -1, 0); >> -if (p == MAP_FAILED) >> +if (mmap(host_start, qemu_host_page_size, prot, flags | >> MAP_ANONYMOUS, >> + -1, 0) == MAP_FAILED) { >> return -1; >> +} >> +page_set_flags(real_start, real_start + qemu_host_page_size, >> + prot | PAGE_VALID); > > I'm not convinced this is right -- it will mean that we set > the page flags for every target page in this host page to > be the same thing (the ORed together result we calculated). > I don't think we want to update the page flags like that -- > if one target page was read-only and the other read-write then > we need to make the whole host page read-write (since it's > bigger and covers both target pages), but we don't want to > incorrectly record that the first target-page is read-write. > OK, thanks. It looks this patch is not quite precise. I guess, we need use PAGE_VALID instead of "prot | PAGE_VALID" for page_set_flags() after mmap() in mmap_frag(). So when the next call for the same location, prot1 will be PAGE_VALID ( now, it may be 0), then can protect to enter "if (prot1 == 0)", again. > I don't really understand this mmap code, though -- that's just > the result of looking at it for fifteen minutes or so. > OK, I can understand. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v5 1/5] fpu: softfloat: Add normalize_roundpack_float32 function
For sig == 0 case, the original implementation is incorrect (although it passes gcc testsuite), it needs to consider about sign for float_zero. The related fix diff for it is below. After patches v5 are finished reviewing, I shall merge the fix diff below to patch v6, next. Thanks. diff --git a/fpu/softfloat.c b/fpu/softfloat.c index dba8566..5ad8bb5 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -7098,19 +7098,15 @@ float32 normalize_roundpack_float32(flag sign, int_fast16_t exp, uint32_t sig, return packFloat32(sign, 0, sig); } +if (!sig) { +return float32_set_sign(float32_zero, sign); +} + if (sign) { if (sig & 0x7FFF) { return normalizeRoundAndPackFloat32(1, exp - 2, sig, status); } -if (sig) { -return packFloat32(1, exp, 0); -} else { -return float32_zero; -} -} - -if (!sig) { -return float32_zero; +return packFloat32(1, exp, 0); } scount = countLeadingZeros64(absa) - 40; On 1/3/16 06:25, cheng...@emindsoft.com.cn wrote: > From: Chen Gang <gang.chen.5...@gmail.com> > > It is based on (u)int32_to_float32 function to support float32 packing. > > Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> > --- > fpu/softfloat.c | 55 > + > include/fpu/softfloat.h | 8 +++ > 2 files changed, 63 insertions(+) > > diff --git a/fpu/softfloat.c b/fpu/softfloat.c > index f1170fe..dba8566 100644 > --- a/fpu/softfloat.c > +++ b/fpu/softfloat.c > @@ -7080,6 +7080,61 @@ float64 uint32_to_float64(uint32_t a, float_status > *status) > return int64_to_float64(a, status); > } > > +/* > + * The mantissa contents the hide bit, e.g. exp: 0x9e with sig: 1 means 1.0f. > + * > + * It references from int32_to_float32() and uint32_to_float32() > + */ > +float32 normalize_roundpack_float32(flag sign, int_fast16_t exp, uint32_t > sig, > +float_status *status) > +{ > +uint64_t absa = sig; > +int8_t scount; > + > +if (exp >= 0xff) { > +return packFloat32(sign, 0xFF, 0); > +} else if (exp <= 0) { > +shift32RightJamming(sig, 0 - exp, ); > +return packFloat32(sign, 0, sig); > +} > + > +if (sign) { > +if (sig & 0x7FFF) { > +return normalizeRoundAndPackFloat32(1, exp - 2, sig, status); > +} > +if (sig) { > +return packFloat32(1, exp, 0); > +} else { > +return float32_zero; > +} > +} > + > +if (!sig) { > +return float32_zero; > +} > + > +scount = countLeadingZeros64(absa) - 40; > +if (scount >= 0) { > +exp -= 7 + scount + 2; > +if (exp <= 0) { > +return packFloat32(0, 0, absa); > +} > +return packFloat32(0, exp, absa << scount); > +} > + > +scount += 7; > +exp -= scount + 2; > +if (exp <= 0) { > +return packFloat32(0, 0, absa); > +} > +if (scount < 0) { > +shift64RightJamming(absa, 0 - scount, ); > +} else { > +absa <<= scount; > +} > +return roundAndPackFloat32(0, exp, absa, status); > +} > + > uint32 float32_to_uint32(float32 a, float_status *status) > { > int64_t v; > diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h > index ded34eb..4995a15 100644 > --- a/include/fpu/softfloat.h > +++ b/include/fpu/softfloat.h > @@ -422,6 +422,14 @@ int float32_is_signaling_nan( float32 ); > float32 float32_maybe_silence_nan( float32 ); > float32 float32_scalbn(float32, int, float_status *status); > > +/* > + * The mantissa contents the hide bit, e.g. exp: 0x9e with sig: 1 means 1.0f. > + * > + * It references from int32_to_float32() and uint32_to_float32() > + */ > +float32 normalize_roundpack_float32(flag sign, int_fast16_t exp, uint32_t > sig, > +float_status *status); > + > static inline float32 float32_abs(float32 a) > { > /* Note that abs does *not* handle NaN specially, nor does > -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user/mmap.c: Support shared memory mapping in mmap_frag()
Oh, sorry, please skip this patch (or discussion), I have send a new fix patch for the related issue, so let the old ACDSee.exe and the latest WinRAR.exe setup programs run successfully under real WinXP system dlls. The related root cause is not related with shared memory, so at least now, we can skip this patch (or discussion). Thanks. On 2015年12月30日 09:35, Chen Gang wrote: > Hello all: > > This patch is only for a discussion, I guess this patch is valuable for > i386 wine running Windows. > > Theoretically, this patch is incorrect, we have to implement softmmu to > support different host and target pages (e.g. host 8KB, target 4KB): > > - If host 8KB is mapped as PRIVATE | FIXED, and runs a while (trigger >copy on write). > > - Then map its low 4KB as SHARED | FIXED. > > - It has to fail. > > So this patch is only a temporary fix: > > - It looks Windows maps shared memory with 64KB alignments (at least, >at present, it is true). > > - OS will manage the process address area: let most shared memory area >nearby. > > - After this patch, we can run Windows Notepad.exe and ACDSee5.0 setup >program successfully. Next, I will run wine with the true windows >system dlls for a test (e.g. windows own kernel32.dll). > > I guess, we can use a switch macro or a new input parameter to enable or > disable the related code (if they are really valuable enough for some > using cases). > > Thanks. > > On 2015年12月30日 09:11, cheng...@emindsoft.com.cn wrote: >> From: Chen Gang <cheng...@emindsoft.com.cn> >> >> It is a temporary fix for i386 target system running Windows. >> >> Also remove useless variable 'p'. >> >> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >> --- >> linux-user/mmap.c | 22 +++--- >> 1 files changed, 19 insertions(+), 3 deletions(-) >> >> diff --git a/linux-user/mmap.c b/linux-user/mmap.c >> index 445e8c6..07758d4 100644 >> --- a/linux-user/mmap.c >> +++ b/linux-user/mmap.c >> @@ -156,12 +156,28 @@ static int mmap_frag(abi_ulong real_start, >> prot1 |= page_get_flags(addr); >> } >> >> +/* >> + * It is a temporary fix. Normally, target system will let shared memory >> + * aligned with 64KB, and allocate them near with each other, no any >> other >> + * kinds of mapping regions nearby. e.g. Windows on i386. >> + */ >> +if ((start == real_start) && (flags & MAP_SHARED)) { >> +if (prot1) { >> +munmap(host_start, qemu_host_page_size); >> +} >> +if (mmap(host_start, qemu_host_page_size, prot, flags, fd, offset) >> +== MAP_FAILED) { >> +return -1; >> +} >> +return 0; >> +} >> + >> if (prot1 == 0) { >> /* no page was there, so we allocate one */ >> -void *p = mmap(host_start, qemu_host_page_size, prot, >> - flags | MAP_ANONYMOUS, -1, 0); >> -if (p == MAP_FAILED) >> +if (mmap(host_start, qemu_host_page_size, prot, flags | >> MAP_ANONYMOUS, >> + -1, 0) == MAP_FAILED) { >> return -1; >> +} >> prot1 = prot; >> } >> prot1 &= PAGE_BITS; >> > -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user/mmap.c: Set prot page flags for the correct region in mmap_frag()
Oh, sorry, for the coding style issues. Also the typo in the comments. I shall send patch v2, next. This fix patch can let the old ACDSee.exe, and the latest WinRAR.exe setup programs run successfully under real WinXP system dlls. Thanks. On 2015年12月31日 15:14, cheng...@emindsoft.com.cn wrote: > From: Chen Gang <cheng...@emindsoft.com.cn> > > mmap() size in mmap_frag() is qemu_host_page_size, but the outside calls > page_set_flags() may be not with qemu_host_page_size. So after mmap(), > call page_set_flags() in time. > > Also let addr increasing step be TARGET_PAGE_SIZE, just like another > areas have done. > > Also remote redundant varialbe p. > > Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> > --- > linux-user/mmap.c | 10 ++ > 1 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/linux-user/mmap.c b/linux-user/mmap.c > index 445e8c6..7920c5e 100644 > --- a/linux-user/mmap.c > +++ b/linux-user/mmap.c > @@ -151,17 +151,19 @@ static int mmap_frag(abi_ulong real_start, > > /* get the protection of the target pages outside the mapping */ > prot1 = 0; > -for(addr = real_start; addr < real_end; addr++) { > +for(addr = real_start; addr < real_end; addr += TARGET_PAGE_SIZE) { > if (addr < start || addr >= end) > prot1 |= page_get_flags(addr); > } > > if (prot1 == 0) { > /* no page was there, so we allocate one */ > -void *p = mmap(host_start, qemu_host_page_size, prot, > - flags | MAP_ANONYMOUS, -1, 0); > -if (p == MAP_FAILED) > +if (mmap(host_start, qemu_host_page_size, prot, flags | > MAP_ANONYMOUS, > + -1, 0) == MAP_FAILED) { > return -1; > +} > +page_set_flags(real_start, real_start + qemu_host_page_size, > + prot | PAGE_VALID); > prot1 = prot; > } > prot1 &= PAGE_BITS; > -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v3] linux-user/mmap.c: Always zero MAP_ANONYMOUS memory in mmap_frag()
On 12/30/15 09:14, Laurent Vivier wrote: > > Le 30/12/2015 02:10, cheng...@emindsoft.com.cn a écrit : >> From: Chen Gang <cheng...@emindsoft.com.cn> >> >> When mapping MAP_ANONYMOUS memory fragments, still need notice about to >> set it zero, or it will cause issues. >> >> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >> --- >> linux-user/mmap.c |4 +++- >> 1 files changed, 3 insertions(+), 1 deletions(-) >> >> diff --git a/linux-user/mmap.c b/linux-user/mmap.c >> index 7b459d5..c6c478e 100644 >> --- a/linux-user/mmap.c >> +++ b/linux-user/mmap.c >> @@ -186,10 +186,12 @@ static int mmap_frag(abi_ulong real_start, >> if (prot_new != (prot1 | PROT_WRITE)) >> mprotect(host_start, qemu_host_page_size, prot_new); >> } else { >> -/* just update the protection */ >> if (prot_new != prot1) { >> mprotect(host_start, qemu_host_page_size, prot_new); >> } >> +if (prot_new & PROT_WRITE) { >> +memset(g2h(start), 0, end - start); >> +} >> } >> return 0; >> } >> > > Reviewed-by: Laurent Vivier <laur...@vivier.eu> > Thank you for your work. Happy New Year! :-) -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user/mmap.c: Support shared memory mapping in mmap_frag()
Hello all: This patch is only for a discussion, I guess this patch is valuable for i386 wine running Windows. Theoretically, this patch is incorrect, we have to implement softmmu to support different host and target pages (e.g. host 8KB, target 4KB): - If host 8KB is mapped as PRIVATE | FIXED, and runs a while (trigger copy on write). - Then map its low 4KB as SHARED | FIXED. - It has to fail. So this patch is only a temporary fix: - It looks Windows maps shared memory with 64KB alignments (at least, at present, it is true). - OS will manage the process address area: let most shared memory area nearby. - After this patch, we can run Windows Notepad.exe and ACDSee5.0 setup program successfully. Next, I will run wine with the true windows system dlls for a test (e.g. windows own kernel32.dll). I guess, we can use a switch macro or a new input parameter to enable or disable the related code (if they are really valuable enough for some using cases). Thanks. On 2015年12月30日 09:11, cheng...@emindsoft.com.cn wrote: > From: Chen Gang <cheng...@emindsoft.com.cn> > > It is a temporary fix for i386 target system running Windows. > > Also remove useless variable 'p'. > > Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> > --- > linux-user/mmap.c | 22 +++--- > 1 files changed, 19 insertions(+), 3 deletions(-) > > diff --git a/linux-user/mmap.c b/linux-user/mmap.c > index 445e8c6..07758d4 100644 > --- a/linux-user/mmap.c > +++ b/linux-user/mmap.c > @@ -156,12 +156,28 @@ static int mmap_frag(abi_ulong real_start, > prot1 |= page_get_flags(addr); > } > > +/* > + * It is a temporary fix. Normally, target system will let shared memory > + * aligned with 64KB, and allocate them near with each other, no any > other > + * kinds of mapping regions nearby. e.g. Windows on i386. > + */ > +if ((start == real_start) && (flags & MAP_SHARED)) { > +if (prot1) { > +munmap(host_start, qemu_host_page_size); > +} > +if (mmap(host_start, qemu_host_page_size, prot, flags, fd, offset) > +== MAP_FAILED) { > +return -1; > +} > +return 0; > +} > + > if (prot1 == 0) { > /* no page was there, so we allocate one */ > -void *p = mmap(host_start, qemu_host_page_size, prot, > - flags | MAP_ANONYMOUS, -1, 0); > -if (p == MAP_FAILED) > +if (mmap(host_start, qemu_host_page_size, prot, flags | > MAP_ANONYMOUS, > + -1, 0) == MAP_FAILED) { > return -1; > +} > prot1 = prot; > } > prot1 &= PAGE_BITS; > -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v4 2/4] target-tilegx: Add single floating point implementation
On 12/24/15 23:52, Chen Gang wrote: > On 12/24/15 07:07, Richard Henderson wrote: > >> Moreover, I thought we agreed to do away with that CALC bit. >> After check again, I guess, we can stil reserve CALC bit: - Then we can remove float32_to_sfmt (use high 32-bit to save float32 directly). And in helper_fsingle_pack2, for CALC, we can return high 32-bit directly. Only for NCALC, we need process it in details. - I guess, most cases are for CALC, so it will let the performance a little better (need float32_to_sfmt, then sfmt_to_float32 again). Then we can only focus on NCAL in helper_fsingle_pack2. Thanks. > > OK, I will try, next. > > I will copy and reconstruct related code from qemu fpu implementation > instead of (u)int32/64_to_float32/64 functions (just like you said, I > guess). > > Hope I can finish within 2015-12-31. > > Thanks. > -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v4 1/4] target-tilegx: Add floating point shared functions
On 12/25/15 04:01, Richard Henderson wrote: > On 12/24/2015 07:38 AM, Chen Gang wrote: >> >> OK, thanks. Since fp_status need to be initialized to be 0, so I will >> declared it statically, too (need we consider about thread safe for it? >> I guess not). > > While qemu is not currently thread-safe, there's work going on to make that > happen. There is no need to exacerbate the problem. > OK, thanks. > Also, I think using an on-stack automatic variable, initialized each time, > emphasizes the fact there there is no state that is preserved across > operations. > > This should really be as simple as > > float_status fp_status = { > .float_rounding_mode = float_round_nearest_even > }; > > (I realize float_round_nearest_even is *also* zero, but humor me. At least > the other members are either flags or booleans.) > OK, thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user/mmap.c: Use end instead of real_end in target_mmap
On 12/24/15 17:54, Laurent Vivier wrote: > > Le 24/12/2015 02:07, cheng...@emindsoft.com.cn a écrit : >> From: Chen Gang <cheng...@emindsoft.com.cn> >> >> In this case, real_end is larger than end, which may cause mmap_frag >> process the incorrect memory region. >> >> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >> --- >> linux-user/mmap.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/linux-user/mmap.c b/linux-user/mmap.c >> index 7b459d5..57b0361 100644 >> --- a/linux-user/mmap.c >> +++ b/linux-user/mmap.c >> @@ -536,7 +536,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int >> prot, >> /* handle the end of the mapping */ >> if (end < real_end) { >> ret = mmap_frag(real_end - qemu_host_page_size, >> -real_end - qemu_host_page_size, real_end, >> +real_end - qemu_host_page_size, end, >> prot, flags, fd, >> offset + real_end - qemu_host_page_size - >> start); >> if (ret == -1) >> > > The fragment must effectively be mapped only to "end" not to "real_end" > (which is a host page aligned address, and thus this is not a fragment). > It is consistent with what it is done in the case of one single page. > > Reviewed-by: Laurent Vivier <laur...@vivier.eu> Thank you for your comments. I only met this issue, and knew it should be fixed in this way, but really don't know the related details. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v4 1/4] target-tilegx: Add floating point shared functions
On 12/24/15 06:51, Richard Henderson wrote: > On 12/23/2015 01:48 PM, cheng...@emindsoft.com.cn wrote: >> +extern float_status fp_status; > > No. Locally declared in e.g. main_calc. > OK, thanks. Since fp_status need to be initialized to be 0, so I will declared it statically, too (need we consider about thread safe for it? I guess not). Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v4 2/4] target-tilegx: Add single floating point implementation
On 12/24/15 07:07, Richard Henderson wrote: > On 12/23/2015 01:48 PM, cheng...@emindsoft.com.cn wrote: >> +static float32 sfmt_to_float32(uint64_t sfmt) >> +{ >> +uint32_t sign = get_fsingle_sign(sfmt); >> +uint32_t man = get_fsingle_man(sfmt); >> +uint32_t exp = get_fsingle_exp(sfmt); >> +float32 f; >> + >> +if (get_fsingle_calc(sfmt) == TILEGX_F_CALC_CVT) { >> +if (sign) { >> +f = int32_to_float32(0 - man, _status); >> +} else { >> +f = uint32_to_float32(man, _status); >> +} >> +exp += get_f32_exp(f) - 0x9e; >> +if ((int32_t) exp < 0) { >> +return float32_infinity | float32_set_sign(float32_zero, sign); >> +} else if (exp >= 0xff) { >> +return float32_zero | float32_set_sign(float32_zero, sign); >> +} else { >> +set_f32_exp(, exp); >> +} > > > What in the world are you attempting to do here? > This is not normalization. This is not even remotely > correct with respect to zero or infinity. > For fdouble, I use almost the same way, but can get the correct result ( pass gcc testsuite for fdouble, include inf and zero tests). But tests will never enough, we can not say the fdouble implementation must be OK. So please help check the fdouble implementation in details, again, when you have time, it may still have issues. > Moreover, I thought we agreed to do away with that CALC bit. > OK, I will try, next. I will copy and reconstruct related code from qemu fpu implementation instead of (u)int32/64_to_float32/64 functions (just like you said, I guess). Hope I can finish within 2015-12-31. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user/mmap.c: Use end instead of real_end in target_mmap
Hello all: After this patch, the i386 wine notepad.exe can be bootup and run under sw_64 host. - The initialization is very very slow (about 10 minutes), it is mainly running in find_vma_reserved (consume more than 95% time resource). I guess, it can be optimized. - After initialization, for real using, the performance is acceptable!! :-) - Next, I shall try the real Windows XP notepad.exe. I guess, we need not softmmu, at least, for wine, it is true (wine will mmap all memory areas during initializing, and manage them by itself when real running). Welcome any suggestions, ideas, and completions. BTW: Merry Christmas! :-) Thanks. On 2015年12月24日 09:07, cheng...@emindsoft.com.cn wrote: > From: Chen Gang <cheng...@emindsoft.com.cn> > > In this case, real_end is larger than end, which may cause mmap_frag > process the incorrect memory region. > > Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> > --- > linux-user/mmap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/linux-user/mmap.c b/linux-user/mmap.c > index 7b459d5..57b0361 100644 > --- a/linux-user/mmap.c > +++ b/linux-user/mmap.c > @@ -536,7 +536,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int > prot, > /* handle the end of the mapping */ > if (end < real_end) { > ret = mmap_frag(real_end - qemu_host_page_size, > -real_end - qemu_host_page_size, real_end, > +real_end - qemu_host_page_size, end, > prot, flags, fd, > offset + real_end - qemu_host_page_size - start); > if (ret == -1) > -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v3 2/4] target-tilegx: Add single floating point implementation
On 12/11/15 06:15, Chen Gang wrote: > > On 12/11/15 04:18, Richard Henderson wrote: >> On 12/10/2015 09:15 AM, Richard Henderson wrote: >>> d = (uint64_t)sign << 63; >>> d = deposit64(d, 53, 11, exp); >>> d = deposit64(d, 21, 32, man); >>> return float64_to_float32(d, fp_status); >> >> Hmm. Actually, this incorrectly adds the implicit bit. We'd actually need >> to >> steal portions of softfloat.c to do this properly. Which still isn't that >> difficult. >> Oh, sorry, I misunderstood this reply. > > Yes, thanks. > Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v3 2/4] target-tilegx: Add single floating point implementation
On 12/21/15 23:01, Richard Henderson wrote: > On 12/20/2015 07:30 AM, Chen Gang wrote: >> And we have to still check TILEGX_F_CALC_CVT, for they are really two >> different format: TILEGX_F_CALC_CVT has no HBIT, but TILEGX_F_CALC_NCVT >> has HBIT (which we need process it specially). > > The both do, in that you re-normalize to produce that HBIT. > That's the whole point. > Oh, yes. But all together, we want to normalize the float value in fsingle_pack2, so we can not use float64_to_float32(), it assumes the input is already normalized (if we can let the input normalized, we will return directly). Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v2] linux-user/mmap.c: Always zero MAP_ANONYMOUS memory inmmap_frag()
> From: "Laurent Vivier";<laur...@vivier.eu>; > > Le 21/12/2015 03:33, cheng...@emindsoft.com.cn a écrit : >> From: Chen Gang <cheng...@emindsoft.com.cn> >> >> When mapping MAP_ANONYMOUS memory fragments, still need notice about to >> set it zero, or it will cause issues. > > Perhaps you can explain in the commit message why this page is not > already filled by zeros ? > In fact, I don't know. when I debug related issues under sw_64 host ( almost the same as alpha) for i386 target, I found it. The host is 8KB page, but the guest is 4KB page. So I guess: - Firstly, qemu allocate one !MAP_ANONYMOUS 8KB page, so it is not zeroed. - Then qemu want one MAP_ANONYMOUS fragment, just can use the the left room of the page above. It merges and sets the related prot, but forgot to reset zero for MAP_ANONYMOUS fragment. - For normal host 4KB and guest also 4KB page, it is rarely happen ( may never happen, I guess). But for host 8KB and guest 4KB pages, it often occurs. Our qemu, at present, let i386 use 8KB page by force, but it can not work. Our qemu members told me to use softmmu, or we would meet many various issues. But at present, I do not meet softmmu related issues: - I can chroot with binfmt_misc successfully by using sw_64 host i386 static qemu, and most i386 programs (e.g. gcc, vi, xclac...) are OK (I build i386 wine under sw_64 host i386 chroot environments). - After fix this issue, some wine programs can run successfully, e.g. cmd.exe, clock.exe, winver.exe (they almost like windows own exe), but initialization is very slow in qemu mmap_find_vma_reserved(). - Now, I am just analyzing and fixing the issue about notepad.exe, is it related with softmmu? I am not sure. When I really meet softmmu related issue, I have to solve it, although it may spend much time resources. >> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >> --- >> linux-user/mmap.c |4 +++- >> 1 files changed, 3 insertions(+), 1 deletions(-) >> >> diff --git a/linux-user/mmap.c b/linux-user/mmap.c >> index 7b459d5..29fe646 100644 >> --- a/linux-user/mmap.c >> +++ b/linux-user/mmap.c >> @@ -186,10 +186,12 @@ static int mmap_frag(abi_ulong real_start, >> if (prot_new != (prot1 | PROT_WRITE)) >> mprotect(host_start, qemu_host_page_size, prot_new); >> } else { >> -/* just update the protection */ >> if (prot_new != prot1) { >> mprotect(host_start, qemu_host_page_size, prot_new); >> } >> +if ((prot_new & PROT_WRITE) && ((flags & MAP_PRIVATE) || (fd == >> -1))) { > > According to manpage, for MAP_ANONYMOUS, fd can be ignored. > Why do you check if the page is MAP_PRIVATE or not ? > For me, we can remove them, originally I only worry about MAP_SHARED and fd according to the man page (it mentions MAP_SHARED and fd): MAP_ANONYMOUS The mapping is not backed by any file; its contents are initialized to zero. The fd and offset arguments are ignored; however, some implementations require fd to be -1 if MAP_ANONYMOUS (or MAP_ANON) is specified, and portable applications should ensure this. The use of MAP_ANONYMOUS in conjunction with MAP_SHARED is supported on Linux only since kernel 2.4. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
On 2015年12月18日 18:47, Chen Gang wrote: > > On 2015年12月18日 17:41, Laurent Vivier wrote: >> >> Le 18/12/2015 07:51, Chen Gang a écrit : [...] >>> >>> +++ b/linux-user/mmap.c >>> @@ -567,6 +567,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, >>> int prot, >>> printf("\n"); >>> #endif >>> tb_invalidate_phys_range(start, start + len); >>> +if ((prot & PROT_WRITE) && (flags & MAP_ANONYMOUS) >>> +&& ((flags & MAP_PRIVATE) || (fd == -1))) { >>> +memset(g2h(start), 0, len); >>> +} >> >> IMHO, their kernel needs a fix, mmap(2): >> >> MAP_ANONYMOUS >> The mapping is not backed by any file; its contents are initial‐ >> ized to zero. >> Oh, sorry, after check again, it is not kernel's issue: mmap() will always zero the ANONYMOUS memory for kernel 3.8 in sw_64 arch. It is still our qemu's issue in mmap_frag (do not zero ANONYMOUS memory fragments), so I sent patch v2 for it, please help check. Thanks. > > OK, Thanks. > > -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v3 2/4] target-tilegx: Add single floating point implementation
After tried, I guess, this way below is incorrect: float64_to_float32() assumes the input 'd' is already a standard (packed) float64 variable. But in fact, it is not (e.g. the input from floatsisf2). And we have to still check TILEGX_F_CALC_CVT, for they are really two different format: TILEGX_F_CALC_CVT has no HBIT, but TILEGX_F_CALC_NCVT has HBIT (which we need process it specially). For me, the way like helper_fdouble_pack2 (the double implementation) is OK to TILEGX_F_CALC_NCVT format, too. - Shift left to get HBIT, and change the related vexp (use vexp instead of exp to process overflow cases -- like double implementation does). - Use (u)int32_to_float32 for the mantissa. - Then process exp again. Thanks. On 12/11/15 06:14, Chen Gang wrote: >> In particular, if gcc decided to optimize fractional fixed-point types, it >> > would do something very similar to the current floatsisf2 code sequence, >> > except >> > that it wouldn't use 0x9e as the exponent; it would use something smaller, >> > so >> > that some number of low bits of the mantessa would be below the radix >> > point. >> > > Oh, really. > >> > Therefore, I think that fsingle_pack2 should do the following: Take the >> > (sign,exp,man) tuple and slot them into a double -- recall that a single >> > only >> > has 23 bits in its mantessa, and this temp format has 32 -- then convert >> > the >> > double to a single. Pre-rounded single results from fsingle_* will be >> > unchanged, while integer data that gcc has constructed will be properly >> > rounded. >> > >> > E.g. >> > >> > uint32_t sign = get_fsingle_sign(sfmt); >> > uint32_t exp = get_fsingle_exp(sfmt); >> > uint32_t man = get_fsingle_man(sfmt); >> > uint64_t d; >> > >> > /* Adjust the exponent for double precision, preserving Inf/NaN. */ >> > if (exp == 0xff) { >> > exp = 0x7ff; >> > } else { >> > exp += 1023 - 127; >> > } >> > >> > d = (uint64_t)sign << 63; >> > d = deposit64(d, 53, 11, exp); >> > d = deposit64(d, 21, 32, man); >> > return float64_to_float32(d, fp_status); >> > >> > Note that this does require float32_to_sfmt to store the mantissa >> > left-justified. That is, not in bits [54-32] as you're doing now, but in >> > bits >> > [63-41]. >> > > For me, it is a good idea! :-) > > -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
On 2015年12月18日 17:41, Laurent Vivier wrote: > > > Le 18/12/2015 07:51, Chen Gang a écrit : >> >> I found this issue during my working time, it is about sw_64 (almost the >> same as alpha) host running i386 wine programs. >> >> I also found another issue, but I am not quite sure whether it is worth >> enough for our upstream: The related fix patch is below, which will let >> the initialization slower, but for most archs, they have no this issue. >> >> linux-user/mmap.c: Always zero MAP_ANONYMOUS memory in target_mmap() >> >> In some architectures, they have no policy to zero MAP_ANONYMOUS memory, >> which will cause issue for qemu target_mmap. >> >> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >> >> diff --git a/linux-user/mmap.c b/linux-user/mmap.c >> index 7b459d5..9c9152d 100644 >> --- a/linux-user/mmap.c >> +++ b/linux-user/mmap.c >> @@ -567,6 +567,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, >> int prot, >> printf("\n"); >> #endif >> tb_invalidate_phys_range(start, start + len); >> +if ((prot & PROT_WRITE) && (flags & MAP_ANONYMOUS) >> +&& ((flags & MAP_PRIVATE) || (fd == -1))) { >> +memset(g2h(start), 0, len); >> +} > > IMHO, their kernel needs a fix, mmap(2): > > MAP_ANONYMOUS > The mapping is not backed by any file; its contents are initial‐ > ized to zero. > OK, Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
On 12/18/15 17:37, Laurent Vivier wrote: > > Le 18/12/2015 07:26, Chen Gang a écrit : >> >> For fcntl, it always needs to notice about it, just like do_fcntl() has >> done, or it will cause issue (e.g. alpha host run i386 guest). >> >> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >> --- >> linux-user/syscall.c | 18 -- >> 1 files changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >> index 0f8adeb..1a60e6f 100644 >> --- a/linux-user/syscall.c >> +++ b/linux-user/syscall.c >> @@ -9007,7 +9007,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long >> arg1, >> if (((CPUARMState *)cpu_env)->eabi) { >> if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) >> goto efault; >> -fl.l_type = tswap16(target_efl->l_type); >> +fl.l_type = >> target_to_host_bitmask(tswap16(target_fl->l_type), >> + flock_tbl); >> fl.l_whence = tswap16(target_efl->l_whence); >> fl.l_start = tswap64(target_efl->l_start); >> fl.l_len = tswap64(target_efl->l_len); >> @@ -9018,7 +9019,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long >> arg1, >> { >> if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) >> goto efault; >> -fl.l_type = tswap16(target_fl->l_type); >> +fl.l_type = >> target_to_host_bitmask(tswap16(target_fl->l_type), >> + flock_tbl); >> fl.l_whence = tswap16(target_fl->l_whence); >> fl.l_start = tswap64(target_fl->l_start); >> fl.l_len = tswap64(target_fl->l_len); >> @@ -9031,7 +9033,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long >> arg1, >> if (((CPUARMState *)cpu_env)->eabi) { >> if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, >> 0)) >> goto efault; >> -target_efl->l_type = tswap16(fl.l_type); >> +target_efl->l_type = host_to_target_bitmask( >> + tswap16(fl.l_type), >> flock_tbl); >> target_efl->l_whence = tswap16(fl.l_whence); >> target_efl->l_start = tswap64(fl.l_start); >> target_efl->l_len = tswap64(fl.l_len); >> @@ -9042,7 +9045,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long >> arg1, >> { >> if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, >> 0)) >> goto efault; >> -target_fl->l_type = tswap16(fl.l_type); >> +target_fl->l_type = host_to_target_bitmask( >> + tswap16(fl.l_type), >> flock_tbl); >> target_fl->l_whence = tswap16(fl.l_whence); >> target_fl->l_start = tswap64(fl.l_start); >> target_fl->l_len = tswap64(fl.l_len); >> @@ -9058,7 +9062,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long >> arg1, >> if (((CPUARMState *)cpu_env)->eabi) { >> if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) >> goto efault; >> -fl.l_type = tswap16(target_efl->l_type); >> +fl.l_type = >> target_to_host_bitmask(tswap16(target_fl->l_type), >> + flock_tbl); >> fl.l_whence = tswap16(target_efl->l_whence); >> fl.l_start = tswap64(target_efl->l_start); >> fl.l_len = tswap64(target_efl->l_len); >> @@ -9069,7 +9074,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long >> arg1, >> { >> if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) >> goto efault; >> -fl.l_type = tswap16(target_fl->l_type); >> +fl.l_type = >> target_to_host_bitmask(tswap16(target_fl->l_type), >> + flock_tbl); >> fl.l_whence = tswap16(target_fl->l_whence); >> fl.l_start
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
On 12/19/15 06:15, Laurent Vivier wrote: > > Le 18/12/2015 23:12, Chen Gang a écrit : >> [...] >> >> OK, thank you very much. I shall config my email client again to notice >> about it. > > You should not use your email client to send patches, you should use > "git send-email": > > http://wiki.qemu.org/Contribute/SubmitAPatch > OK, thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
On 12/19/15 05:58, Laurent Vivier wrote: > > Le 18/12/2015 22:40, Chen Gang a écrit : >> [...] >> I did not get any script/checkpatch.pl complains, originally. >> >> Does my email client configuration is incorrect, then cause incorrect >> mail format? I guess not. The related reason is below. >> >> - I copy your full reply mail contents to a new file (diff.patch). > > If you copy, you loose the special characters. I do a "Save as". > >> >> - Remove all '> ' in vi editor (1,% s/^> //g) (so get the original >>patch contents). >> >> - ./script/checkpatch.pl diff.patch, it has no any complains. > > If I run "file" on the saved file, I have: > > $ file orig.eml > orig.eml: SMTP mail, ASCII text, with CRLF line terminators > > I can convert it with "tr": > > $ tr "\r" "\n" < orig.eml > new.eml > > $ file new.eml > new.eml: SMTP mail, ASCII text > > $ ./scripts/checkpatch.pl new.eml > total: 0 errors, 0 warnings, 54 lines checked > > new.eml has no obvious style problems and is ready for submission. > > Laurent > OK, thank you very much. I shall config my email client again to notice about it. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed.
[Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
For fcntl, it always needs to notice about it, just like do_fcntl() has done, or it will cause issue (e.g. alpha host run i386 guest). Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> --- linux-user/syscall.c | 18 -- 1 files changed, 12 insertions(+), 6 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 0f8adeb..1a60e6f 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -9007,7 +9007,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, if (((CPUARMState *)cpu_env)->eabi) { if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) goto efault; -fl.l_type = tswap16(target_efl->l_type); +fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type), + flock_tbl); fl.l_whence = tswap16(target_efl->l_whence); fl.l_start = tswap64(target_efl->l_start); fl.l_len = tswap64(target_efl->l_len); @@ -9018,7 +9019,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, { if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) goto efault; -fl.l_type = tswap16(target_fl->l_type); +fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type), + flock_tbl); fl.l_whence = tswap16(target_fl->l_whence); fl.l_start = tswap64(target_fl->l_start); fl.l_len = tswap64(target_fl->l_len); @@ -9031,7 +9033,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, if (((CPUARMState *)cpu_env)->eabi) { if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, 0)) goto efault; -target_efl->l_type = tswap16(fl.l_type); +target_efl->l_type = host_to_target_bitmask( + tswap16(fl.l_type), flock_tbl); target_efl->l_whence = tswap16(fl.l_whence); target_efl->l_start = tswap64(fl.l_start); target_efl->l_len = tswap64(fl.l_len); @@ -9042,7 +9045,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, { if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, 0)) goto efault; -target_fl->l_type = tswap16(fl.l_type); +target_fl->l_type = host_to_target_bitmask( + tswap16(fl.l_type), flock_tbl); target_fl->l_whence = tswap16(fl.l_whence); target_fl->l_start = tswap64(fl.l_start); target_fl->l_len = tswap64(fl.l_len); @@ -9058,7 +9062,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, if (((CPUARMState *)cpu_env)->eabi) { if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) goto efault; -fl.l_type = tswap16(target_efl->l_type); +fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type), + flock_tbl); fl.l_whence = tswap16(target_efl->l_whence); fl.l_start = tswap64(target_efl->l_start); fl.l_len = tswap64(target_efl->l_len); @@ -9069,7 +9074,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, { if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) goto efault; -fl.l_type = tswap16(target_fl->l_type); +fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type), + flock_tbl); fl.l_whence = tswap16(target_fl->l_whence); fl.l_start = tswap64(target_fl->l_start); fl.l_len = tswap64(target_fl->l_len); -- 1.7.3.4
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
I found this issue during my working time, it is about sw_64 (almost the same as alpha) host running i386 wine programs. I also found another issue, but I am not quite sure whether it is worth enough for our upstream: The related fix patch is below, which will let the initialization slower, but for most archs, they have no this issue. linux-user/mmap.c: Always zero MAP_ANONYMOUS memory in target_mmap() In some architectures, they have no policy to zero MAP_ANONYMOUS memory, which will cause issue for qemu target_mmap. Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> diff --git a/linux-user/mmap.c b/linux-user/mmap.c index 7b459d5..9c9152d 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -567,6 +567,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot, printf("\n"); #endif tb_invalidate_phys_range(start, start + len); +if ((prot & PROT_WRITE) && (flags & MAP_ANONYMOUS) +&& ((flags & MAP_PRIVATE) || (fd == -1))) { +memset(g2h(start), 0, len); +} mmap_unlock(); return start; fail: Thanks. On 2015年12月18日 14:26, Chen Gang wrote: > > For fcntl, it always needs to notice about it, just like do_fcntl() has > done, or it will cause issue (e.g. alpha host run i386 guest). > > Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> > --- > linux-user/syscall.c | 18 -- > 1 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 0f8adeb..1a60e6f 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -9007,7 +9007,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long > arg1, > if (((CPUARMState *)cpu_env)->eabi) { > if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) > goto efault; > -fl.l_type = tswap16(target_efl->l_type); > +fl.l_type = > target_to_host_bitmask(tswap16(target_fl->l_type), > + flock_tbl); > fl.l_whence = tswap16(target_efl->l_whence); > fl.l_start = tswap64(target_efl->l_start); > fl.l_len = tswap64(target_efl->l_len); > @@ -9018,7 +9019,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long > arg1, > { > if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) > goto efault; > -fl.l_type = tswap16(target_fl->l_type); > +fl.l_type = > target_to_host_bitmask(tswap16(target_fl->l_type), > + flock_tbl); > fl.l_whence = tswap16(target_fl->l_whence); > fl.l_start = tswap64(target_fl->l_start); > fl.l_len = tswap64(target_fl->l_len); > @@ -9031,7 +9033,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long > arg1, > if (((CPUARMState *)cpu_env)->eabi) { > if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, > 0)) > goto efault; > -target_efl->l_type = tswap16(fl.l_type); > +target_efl->l_type = host_to_target_bitmask( > + tswap16(fl.l_type), > flock_tbl); > target_efl->l_whence = tswap16(fl.l_whence); > target_efl->l_start = tswap64(fl.l_start); > target_efl->l_len = tswap64(fl.l_len); > @@ -9042,7 +9045,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long > arg1, > { > if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, 0)) > goto efault; > -target_fl->l_type = tswap16(fl.l_type); > +target_fl->l_type = host_to_target_bitmask( > + tswap16(fl.l_type), > flock_tbl); > target_fl->l_whence = tswap16(fl.l_whence); > target_fl->l_start = tswap64(fl.l_start); > target_fl->l_len = tswap64(fl.l_len); > @@ -9058,7 +9062,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long > arg1, > if (((CPUARMState *)cpu_env)->eabi) { > if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) > goto efault; > -fl.l_type = tswap16(target_efl->l_type); > +fl.l_type = > target_to_host_bitmask(tswap16(target_fl->l_type), > + flock_tbl); >
Re: [Qemu-devel] [PATCH v3 3/4] target-tilegx: Add double floating point implementation
On 12/11/15 05:17, Richard Henderson wrote: > On 12/10/2015 06:15 AM, Chen Gang wrote: >> +#define TILEGX_F_MAN_HBIT (1ULL << 59) > ... >> +static uint64_t fr_to_man(float64 d) >> +{ >> +uint64_t val = get_f64_man(d) << 7; >> + >> +if (get_f64_exp(d)) { >> +val |= TILEGX_F_MAN_HBIT; >> +} >> + >> +return val; >> +} > > One presumes that "HBIT" is the ieee implicit one bit. > A better name or better comments would help there. > OK, thanks. And after think of again, I guess, the real hardware does not use HBIT internally (use the full 64 bits as mantissa without HBIT). But what I have done is still OK (use 59 bits + 1 HBIT as mantissa), for 59 bits are enough for double mantissa (52 bits). It makes the overflow processing easier, but has to process mul operation specially. It we have to try to match the real hardware have done, I shall rewrite the related code for mantissa. (I guess, we need to match the real hardware have done). > Do we know for sure that "7" is the correct number of guard bits? From the > gcc > implementation of floatsidf, I might guess that the correct number is "4". > According to floatsidf, it seems "4", but after I expanded the bits, I guess, it is "7". /* * Double exp analyzing: (0x21b00 << 1) - 0x37(55) = 0x3ff * * 17 16 15 14 13 12 11 10 9 8 76 5 4 3 2 1 0 * *1 0 0 0 0 1 1 0 1 1 00 0 0 0 0 0 0 * *0 0 0 0 0 1 1 0 1 1 1=> 0x37(55) * *0 1 1 1 1 1 1 1 1 1 1=> 0x3ff * */ I guess, I need restore this comments in helper_fdouble.c. >> +static uint32_t get_fdouble_vexp(uint64_t n) >> +{ >> +return extract32(n, 7, 13); >> +} > > What's a "vexp"? > It is exp + overflow bit + underflow bit. We can use vexp for internal calculation, directly, and check uv and ov for the result. I guess the real hardware will do like this. The full description of the format is: typedef union TileGXFPDFmtF { struct { uint64_t unknown0 : 7;/* unknown */ uint64_t vexp : 13; /* vexp = exp | ov | uv */ #if 0 /* it is only the explanation for vexp above */ uint64_t exp : 11;/* exp, 0x21b << 1: 55 + TILEGX_F_EXP_DZERO */ uint64_t ov : 1; /* overflow for mul, low priority */ uint64_t uv : 1; /* underflow for mul, high priority */ #endif uint64_t sign : 1;/* Sign bit for the total value */ uint64_t calc: 2; /* absolute add, sub, or mul */ uint64_t inf: 1; /* infinit */ uint64_t nan: 1; /* nan */ /* Come from TILE-Gx ISA document, Table 7-2 for floating point */ uint64_t unordered : 1; /* The two are unordered */ uint64_t lt : 1; /* 1st is less than 2nd */ uint64_t le : 1; /* 1st is less than or equal to 2nd */ uint64_t gt : 1; /* 1st is greater than 2nd */ uint64_t ge : 1; /* 1st is greater than or equal to 2nd */ uint64_t eq : 1; /* The two operands are equal */ uint64_t neq : 1; /* The two operands are not equal */ uint64_t unknown1 : 32; /* unknown */ } fmt; uint64_t ll; /* only for easy using */ } TileGXFPDFmtF; >> +uint64_t helper_fdouble_unpack_min(CPUTLGState *env, >> + uint64_t srca, uint64_t srcb) >> +{ >> +uint64_t v = 0; >> +uint32_t expa = get_f64_exp(srca); >> +uint32_t expb = get_f64_exp(srcb); >> + >> +if (float64_is_any_nan(srca) || float64_is_any_nan(srcb) >> +|| float64_is_infinity(srca) || float64_is_infinity(srcb)) { >> +return 0; >> +} else if (expa > expb) { >> +if (expa - expb < 64) { >> +set_fdouble_man(, fr_to_man(srcb) >> (expa - expb)); >> +} else { >> +return 0; >> +} >> +} else if (expa < expb) { >> +if (expb - expa < 64) { >> +set_fdouble_man(, fr_to_man(srca) >> (expb - expa)); > > I very sincerely doubt that a simple right-shift is correct. In order to > obtain proper rounding for real computation, a sticky bit is required. That > is, set bit 0 if any bits are shifted out. See the implementation of > shift64RightJamming in fpu/softfloat-macros.h. > Oh, really, thanks. >> +uint64_t helper_fdouble_addsub(CPUTLGState *env, >> + uint64_t dest, uint64_t srca, uint64_t srcb) >> +{ >> +if (get_fdouble_calc(srcb) == TILEGX_
Re: [Qemu-devel] [PATCH v3 3/4] target-tilegx: Add double floating point implementation
On 12/12/15 08:41, Richard Henderson wrote: > On 12/11/2015 03:38 PM, Chen Gang wrote: >> >> On 12/11/15 05:17, Richard Henderson wrote: >>> On 12/10/2015 06:15 AM, Chen Gang wrote: >>>> +#define TILEGX_F_MAN_HBIT (1ULL << 59) >>> ... >>>> +static uint64_t fr_to_man(float64 d) >>>> +{ >>>> +uint64_t val = get_f64_man(d) << 7; >>>> + >>>> +if (get_f64_exp(d)) { >>>> +val |= TILEGX_F_MAN_HBIT; >>>> +} >>>> + >>>> +return val; >>>> +} >>> >>> One presumes that "HBIT" is the ieee implicit one bit. >>> A better name or better comments would help there. >>> >> >> OK, thanks. And after think of again, I guess, the real hardware does >> not use HBIT internally (use the full 64 bits as mantissa without HBIT). > > It must do. Otherwise the arithmetic doesn't work out. > Oh, yes, and we have to use my original implementation (60 for mantissa, 4 bits for other using). >> But what I have done is still OK (use 59 bits + 1 HBIT as mantissa), for >> 59 bits are enough for double mantissa (52 bits). It makes the overflow >> processing easier, but has to process mul operation specially. > > What you have works. But the mul operation isn't as special as you make it > out -- aside from requiring at least 104 bits as intermediate -- in that when > one implements what the hardware does, subtraction also may require > significant normalization. > I guess, you misunderstood what I said (my English is not quite well). For mul, at least, it needs (104 - 1) bits, At present, we have 120 bits for it (in fact, our mul generates 119 bits result). So it is enough. >> According to floatsidf, it seems "4", but after I expanded the bits, I >> guess, it is "7". >> >> /* >> * Double exp analyzing: (0x21b00 << 1) - 0x37(55) = 0x3ff >> * >> * 17 16 15 14 13 12 11 10 9 8 76 5 4 3 2 1 0 >> * >> *1 0 0 0 0 1 1 0 1 1 00 0 0 0 0 0 0 >> * >> *0 0 0 0 0 1 1 0 1 1 1=> 0x37(55) >> * >> *0 1 1 1 1 1 1 1 1 1 1=> 0x3ff >> * >> */ > > That's the exponent within the flags temporary. It has nothing to do with > the position of the extracted mantissa. > 0x37(55) + 4 (guard bits) + 1 (HBIT) = 60 bits. So, if the above is correct, the mantissa is 60 bits (with HBIT), and bit 18 in flags for overflow, bit 19 for underflow (bit 20 must be for sign). > FWIW, the minimum shift would be 3, in order to properly implement rounding; > if the hardware uses a shift of 4, that's fine too. > I guess, so it uses 4 guard bits. > What I would love to know is if the shift present in floatsidf is not really > required; equally valid to adjust 0x21b00 by 4. Meaning normalization would > do a proper job with the entire given mantissa. This would require better > documentation, or access to hardware to verify. > I guess, before call any fdouble insns, we can use the low 4 bits as mantissa (e.g. calc mul), but when call any fdouble insn, we can not use the lower 4 guard bits, so floatsidf has to shift 4 bits left. >>>> +uint64_t helper_fdouble_addsub(CPUTLGState >> And for my current implementation (I guess, it should be correct): >> >> typedef union TileGXFPDFmtV { >> struct { >> uint64_t mantissa : 60; /* mantissa */ >> uint64_t overflow : 1;/* carry/overflow bit for absolute >> add/mul */ >> uint64_t unknown1 : 3;/* unknown */ > > I personally like to call all 4 of the top bits overflow. But I have no idea > what the real hardware actually does. > >> In helper_fdouble_addsub(), both dest and srca are unpacked, so they are >> within 60 bits. So one time absolute add are within 61 bits, so let bit >> 61 as overflow bit is enough. > > True. But if all 4 top bits are considered overflow, then one could > implement floatdidf fairly easily. But I suspect that real hw doesn't work > that way, or it would have already been done. > So, I only assumed bit 60 is for overflow, the high 3 bits are unknown. For me, if one bit for overflow is enough, the hardware will save the other bits for another using (or are reserved for future). Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
[Qemu-devel] [PATCH v3 2/4] target-tilegx: Add single floating point implementation
It passes gcc testsuite. Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> --- target-tilegx/helper-fsingle.c | 212 + 1 file changed, 212 insertions(+) create mode 100644 target-tilegx/helper-fsingle.c diff --git a/target-tilegx/helper-fsingle.c b/target-tilegx/helper-fsingle.c new file mode 100644 index 000..a33837e --- /dev/null +++ b/target-tilegx/helper-fsingle.c @@ -0,0 +1,212 @@ +/* + * QEMU TILE-Gx helpers + * + * Copyright (c) 2015 Chen Gang + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see + * <http://www.gnu.org/licenses/lgpl-2.1.html> + */ + +#include "cpu.h" +#include "qemu-common.h" +#include "exec/helper-proto.h" +#include "fpu/softfloat.h" + +#include "helper-fshared.c" + +/* + * FSingle instructions implemenation: + * + * fsingle_add1 ; calc srca and srcb, + * ; convert float_32 to TileGXFPSFmt result. + * ; move TileGXFPSFmt result to dest. + * + * fsingle_sub1 ; calc srca and srcb. + * ; convert float_32 to TileGXFPSFmt result. + * ; move TileGXFPSFmt result to dest. + * + * fsingle_addsub2 ; nop. + * + * fsingle_mul1 ; calc srca and srcb. + * ; convert float_32 value to TileGXFPSFmt result. + * ; move TileGXFPSFmt result to dest. + * + * fsingle_mul2 ; move srca to dest. + * + * fsingle_pack1; nop + * + * fsingle_pack2; treate srca as TileGXFPSFmt result. + * ; convert TileGXFPSFmt result to float_32 value. + * ; move float_32 value to dest. + */ + +#define TILEGX_F_CALC_CVT 0 /* convert int to fsingle */ +#define TILEGX_F_CALC_NCVT 1 /* Not convertion */ + +static uint32_t get_f32_exp(float32 f) +{ +return extract32(float32_val(f), 23, 8); +} + +static void set_f32_exp(float32 *f, uint32_t exp) +{ +*f = make_float32(deposit32(float32_val(*f), 23, 8, exp)); +} + +static uint32_t get_f32_man(float32 f) +{ +return float32_val(f) & 0x7f; +} + +static float32 create_f32_man(uint32_t man) +{ + return make_float32(man & 0x7f); +} + +static inline uint32_t get_fsingle_exp(uint64_t n) +{ +return n & 0xff; +} + +static inline uint64_t create_fsingle_exp(uint32_t exp) +{ +return exp & 0xff; +} + +static inline uint32_t get_fsingle_sign(uint64_t n) +{ +return test_bit(10, ); +} + +static inline void set_fsingle_sign(uint64_t *n) +{ +set_bit(10, n); +} + +static inline unsigned int get_fsingle_calc(uint64_t n) +{ +return test_bit(11, ); +} + +static inline void set_fsingle_calc(uint64_t *n, uint32_t calc) +{ +set_bit(11, n); +} + +static inline unsigned int get_fsingle_man(uint64_t n) +{ +return n >> 32; +} + +static inline uint64_t create_fsingle_man(uint32_t man) +{ +return (uint64_t)man << 32; +} + +static uint64_t float32_to_sfmt(float32 f) +{ +uint64_t sfmt = 0; + +if (float32_is_neg(f)) { +set_fsingle_sign(); +} +sfmt |= create_fsingle_exp(get_f32_exp(f)); +sfmt |= create_fsingle_man((get_f32_man(f) << 8) | (1 << 31)); + +return sfmt; +} + +static float32 sfmt_to_float32(uint64_t sfmt, float_status *fp_status) +{ +float32 f; +uint32_t sign = get_fsingle_sign(sfmt); +uint32_t man = get_fsingle_man(sfmt); + +if (get_fsingle_calc(sfmt) == TILEGX_F_CALC_CVT) { +if (sign) { +return int32_to_float32(0 - man, fp_status); +} else { +return uint32_to_float32(man, fp_status); +} +} else { +f = float32_set_sign(float32_zero, sign); +f |= create_f32_man(man >> 8); +set_f32_exp(, get_fsingle_exp(sfmt)); +} + +return f; +} + +uint64_t helper_fsingle_pack2(CPUTLGState *env, uint64_t srca) +{ +return float32_val(sfmt_to_float32(srca, >fp_status)); +} + +static void ana_bits(float_status *fp_status, + float32 fsrca, float32 fsrcb, uint64_t *sfmt) +{ +if (float32_eq(fsrca, fsrcb, fp_status)) { +*sfmt |= create_fsfd_flag_eq(); +} else { +*sfmt |= create_fsfd_flag_ne(); +} + +if (float32_lt(fsrca, fsrcb, fp_status)) { +*sfmt |= create_fsfd_flag_lt(); +} +if (float32_le
[Qemu-devel] [PATCH v3 1/4] target-tilegx: Add floating point shared functions
They are used by fsingle and fdouble helpers. Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> --- target-tilegx/helper-fshared.c | 53 ++ 1 file changed, 53 insertions(+) create mode 100644 target-tilegx/helper-fshared.c diff --git a/target-tilegx/helper-fshared.c b/target-tilegx/helper-fshared.c new file mode 100644 index 000..d669f58 --- /dev/null +++ b/target-tilegx/helper-fshared.c @@ -0,0 +1,53 @@ +/* + * TILE-Gx virtual Floating point shared functions + * + * Copyright (c) 2015 Chen Gang + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see <http://www.gnu.org/licenses/>. + */ + +static inline uint64_t create_fsfd_flag_un(void) +{ +return 1 << 25; +} + +static inline uint64_t create_fsfd_flag_lt(void) +{ +return 1 << 26; +} + +static inline uint64_t create_fsfd_flag_le(void) +{ +return 1 << 27; +} + +static inline uint64_t create_fsfd_flag_gt(void) +{ +return 1 << 28; +} + +static inline uint64_t create_fsfd_flag_ge(void) +{ +return 1 << 29; +} + +static inline uint64_t create_fsfd_flag_eq(void) +{ +return 1 << 30; +} + +static inline uint64_t create_fsfd_flag_ne(void) +{ +return 1ULL << 31; +} -- 1.9.3
Re: [Qemu-devel] [PATCH v3 4/4] target-tilegx: Integrate floating pointer implementation
On 12/11/15 05:37, Richard Henderson wrote: > On 12/10/2015 06:16 AM, Chen Gang wrote: [...] >> >> diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h >> index 03df107..445a606 100644 >> --- a/target-tilegx/cpu.h >> +++ b/target-tilegx/cpu.h >> @@ -88,6 +88,8 @@ typedef struct CPUTLGState { >> uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside >> */ >> uint64_t pc; /* Current pc */ >> >> +float_status fp_status;/* floating point status */ > > As mentioned elsewhere, this is pointless. > OK, thanks. >> +case OE_RR_X0(FSINGLE_PACK1): >> +case OE_RR_Y0(FSINGLE_PACK1): >> +mnemonic = "fsingle_pack1"; >> +goto done2; > > This could use a comment that we're "copying" dest to dest. > OK, thanks. >> @@ -742,13 +745,21 @@ static TileExcp gen_rr_opcode(DisasContext *dc, >> unsigned opext, >> static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext, >> unsigned dest, unsigned srca, unsigned srcb) >> { >> -TCGv tdest = dest_gr(dc, dest); >> -TCGv tsrca = load_gr(dc, srca); >> -TCGv tsrcb = load_gr(dc, srcb); >> +TCGv tdest, tsrca, tsrcb; >> TCGv t0; >> const char *mnemonic; >> >> switch (opext) { >> +case OE_RRR(FSINGLE_ADDSUB2, 0, X0): >> +mnemonic = "fsingle_addsub2"; >> +goto done2; >> +} > > Likewise. > Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v3 2/4] target-tilegx: Add single floating point implementation
On 12/11/15 04:18, Richard Henderson wrote: > On 12/10/2015 09:15 AM, Richard Henderson wrote: >> d = (uint64_t)sign << 63; >> d = deposit64(d, 53, 11, exp); >> d = deposit64(d, 21, 32, man); >> return float64_to_float32(d, fp_status); > > Hmm. Actually, this incorrectly adds the implicit bit. We'd actually need to > steal portions of softfloat.c to do this properly. Which still isn't that > difficult. > Yes, thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v3 2/4] target-tilegx: Add single floating point implementation
On 12/11/15 01:15, Richard Henderson wrote: > On 12/10/2015 06:15 AM, Chen Gang wrote: >> +#define TILEGX_F_CALC_CVT 0 /* convert int to fsingle */ >> +#define TILEGX_F_CALC_NCVT 1 /* Not convertion */ >> + >> +static uint32_t get_f32_exp(float32 f) >> +{ >> +return extract32(float32_val(f), 23, 8); >> +} >> + >> +static void set_f32_exp(float32 *f, uint32_t exp) >> +{ >> +*f = make_float32(deposit32(float32_val(*f), 23, 8, exp)); >> +} > > Why take a pointer instead of returning the new value? > I referenced set_* functions' declarations in "include/fpu/softfloat.h", originally. >> +static inline uint32_t get_fsingle_sign(uint64_t n) >> +{ >> +return test_bit(10, ); >> +} >> + >> +static inline void set_fsingle_sign(uint64_t *n) >> +{ >> +set_bit(10, n); >> +} > > Why are you using test_bit and set_bit here, rather than continuing to use > deposit and extract? > It is really only for one bit test and set, so test_bit/set_bit are simpler and clearer than deposit/extract. >> +static float32 sfmt_to_float32(uint64_t sfmt, float_status *fp_status) >> +{ >> +float32 f; >> +uint32_t sign = get_fsingle_sign(sfmt); >> +uint32_t man = get_fsingle_man(sfmt); >> + >> +if (get_fsingle_calc(sfmt) == TILEGX_F_CALC_CVT) { >> +if (sign) { >> +return int32_to_float32(0 - man, fp_status); >> +} else { >> +return uint32_to_float32(man, fp_status); >> +} >> +} else { >> +f = float32_set_sign(float32_zero, sign); >> +f |= create_f32_man(man >> 8); >> +set_f32_exp(, get_fsingle_exp(sfmt)); >> +} > > I'm not especially keen on this calc bit. I'd much rather that we always pack > and round properly. > OK. > In particular, if gcc decided to optimize fractional fixed-point types, it > would do something very similar to the current floatsisf2 code sequence, > except > that it wouldn't use 0x9e as the exponent; it would use something smaller, so > that some number of low bits of the mantessa would be below the radix point. > Oh, really. > Therefore, I think that fsingle_pack2 should do the following: Take the > (sign,exp,man) tuple and slot them into a double -- recall that a single only > has 23 bits in its mantessa, and this temp format has 32 -- then convert the > double to a single. Pre-rounded single results from fsingle_* will be > unchanged, while integer data that gcc has constructed will be properly > rounded. > > E.g. > > uint32_t sign = get_fsingle_sign(sfmt); > uint32_t exp = get_fsingle_exp(sfmt); > uint32_t man = get_fsingle_man(sfmt); > uint64_t d; > > /* Adjust the exponent for double precision, preserving Inf/NaN. */ > if (exp == 0xff) { > exp = 0x7ff; > } else { > exp += 1023 - 127; > } > > d = (uint64_t)sign << 63; > d = deposit64(d, 53, 11, exp); > d = deposit64(d, 21, 32, man); > return float64_to_float32(d, fp_status); > > Note that this does require float32_to_sfmt to store the mantissa > left-justified. That is, not in bits [54-32] as you're doing now, but in bits > [63-41]. > For me, it is a good idea! :-) >> +static void ana_bits(float_status *fp_status, >> + float32 fsrca, float32 fsrcb, uint64_t *sfmt) > > Is "ana" supposed to be short for "analyze"? > Yes. >> +{ >> +if (float32_eq(fsrca, fsrcb, fp_status)) { >> +*sfmt |= create_fsfd_flag_eq(); >> +} else { >> +*sfmt |= create_fsfd_flag_ne(); >> +} >> + >> +if (float32_lt(fsrca, fsrcb, fp_status)) { >> +*sfmt |= create_fsfd_flag_lt(); >> +} >> +if (float32_le(fsrca, fsrcb, fp_status)) { >> +*sfmt |= create_fsfd_flag_le(); >> +} >> + >> +if (float32_lt(fsrcb, fsrca, fp_status)) { >> +*sfmt |= create_fsfd_flag_gt(); >> +} >> +if (float32_le(fsrcb, fsrca, fp_status)) { >> +*sfmt |= create_fsfd_flag_ge(); >> +} >> + >> +if (float32_unordered(fsrca, fsrcb, fp_status)) { >> +*sfmt |= create_fsfd_flag_un(); >> +} >> +} > > Again, I think it's better to return the new sfmt value than modify a pointer. > Oh, I guess, we can inline ana_bits() to main_calc(), for they are both simple short functions, and ana_bits() is only called by main_calc(). Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v3 0/4] target-tilegx: Implement floating point instructions
Hello all: After communicated with my company, I am permitted to use my company email to send patches to open source community. My company supports what I have done for open source community, but at present, I still use my personal mail id as Signed-of-by (make and send patches mainly during my free time). And thank my company for the support. :-) Thanks. On 12/10/15 22:12, Chen Gang wrote: > > These patches are the normal floating point implementation, instead of > the original temporary one. > > It passes building, and gcc testsuite. > > Chen Gang (4): > target-tilegx: Add floating point shared functions > target-tilegx: Add single floating point implementation > target-tilegx: Add double floating point implementation > target-tilegx: Integrate floating pointer implementation > > target-tilegx/Makefile.objs| 3 +- > target-tilegx/cpu.h| 2 + > target-tilegx/helper-fdouble.c | 400 > + > target-tilegx/helper-fshared.c | 53 ++ > target-tilegx/helper-fsingle.c | 212 ++ > target-tilegx/helper.h | 12 ++ > target-tilegx/translate.c | 68 ++- > 7 files changed, 740 insertions(+), 10 deletions(-) > create mode 100644 target-tilegx/helper-fdouble.c > create mode 100644 target-tilegx/helper-fshared.c > create mode 100644 target-tilegx/helper-fsingle.c > -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
[Qemu-devel] [PATCH v3 3/4] target-tilegx: Add double floating point implementation
It passes gcc testsuite. Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> --- target-tilegx/helper-fdouble.c | 400 + 1 file changed, 400 insertions(+) create mode 100644 target-tilegx/helper-fdouble.c diff --git a/target-tilegx/helper-fdouble.c b/target-tilegx/helper-fdouble.c new file mode 100644 index 000..3b824f7 --- /dev/null +++ b/target-tilegx/helper-fdouble.c @@ -0,0 +1,400 @@ +/* + * QEMU TILE-Gx helpers + * + * Copyright (c) 2015 Chen Gang + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see + * <http://www.gnu.org/licenses/lgpl-2.1.html> + */ + +#include "cpu.h" +#include "qemu-common.h" +#include "exec/helper-proto.h" +#include "fpu/softfloat.h" + +#include "helper-fshared.c" + +/* + * FDouble instructions implemenation: + * + * fdouble_unpack_min ; srca and srcb are float_64 value. + * ; get the min absolute value's mantissa. + * ; move "mantissa >> (exp_max - exp_min)" to dest. + * + * fdouble_unpack_max ; srca and srcb are float_64 value. + * ; get the max absolute value's mantissa. + * ; move mantissa to dest. + * + * fdouble_add_flags; srca and srcb are float_64 value. + * ; calc exp (exp_max), sign, and comp bits for flags. + * ; set addsub bit to flags and move flags to dest. + * + * fdouble_sub_flags; srca and srcb are float_64 value. + * ; calc exp (exp_max), sign, and comp bits for flags. + * ; set addsub bit to flags and move flags to dest. + * + * fdouble_addsub: ; dest, srca (max, min mantissa), and srcb (flags). + * ; "dest +/- srca" depend on the add/sub bit of flags. + * ; move result mantissa to dest. + * + * fdouble_mul_flags: ; srca and srcb are float_64 value. + * ; calc sign (xor), exp (min + max), and comp bits. + * ; mix sign, exp, and comp bits as flags to dest. + * + * fdouble_pack1; move srcb (flags) to dest. + * + * fdouble_pack2; srca, srcb (high, low mantissa), and dest (flags) + * ; normalize and pack result from srca, srcb, and dest. + * ; move result to dest. + */ + +#define TILEGX_F_EXP_DZERO 0x3ff /* Zero exp for double 11-bits */ +#define TILEGX_F_EXP_DMAX 0x7fe /* max exp for double 11-bits */ +#define TILEGX_F_EXP_DUF0x1000/* underflow exp bit for double */ + +#define TILEGX_F_MAN_HBIT (1ULL << 59) + +#define TILEGX_F_CALC_ADD 1 /* Perform absolute add operation */ +#define TILEGX_F_CALC_SUB 2 /* Perform absolute sub operation */ +#define TILEGX_F_CALC_MUL 3 /* Perform absolute mul operation */ + +static uint32_t get_f64_exp(float64 d) +{ +return extract64(float64_val(d), 52, 11); +} + +static void set_f64_exp(float64 *d, uint32_t exp) +{ +*d = make_float64(deposit64(float64_val(*d), 52, 11, exp)); +} + +static uint64_t get_f64_man(float64 d) +{ +return extract64(float64_val(d), 0, 52); +} + +static uint64_t fr_to_man(float64 d) +{ +uint64_t val = get_f64_man(d) << 7; + +if (get_f64_exp(d)) { +val |= TILEGX_F_MAN_HBIT; +} + +return val; +} + +static uint64_t get_fdouble_man(uint64_t n) +{ +return extract64(n, 0, 60); +} + +static void set_fdouble_man(uint64_t *n, uint64_t man) +{ +*n = deposit64(*n, 0, 60, man); +} + +static uint64_t get_fdouble_man_of(uint64_t n) +{ +return test_bit(60, ); +} + +static void clear_fdouble_man_of(uint64_t *n) +{ +return clear_bit(60, n); +} + +static uint32_t get_fdouble_nan(uint64_t n) +{ +return test_bit(24, ); +} + +static void set_fdouble_nan(uint64_t *n) +{ +set_bit(24, n); +} + +static uint32_t get_fdouble_inf(uint64_t n) +{ +return test_bit(23, ); +} + +static void set_fdouble_inf(uint64_t *n) +{ +set_bit(23, n); +} + +static uint32_t get_fdouble_calc(uint64_t n) +{ +return extract32(n, 21, 2); +} + +static void set_fdouble_calc(uint64_t *n, uint32_t calc) +{ +*n = deposit64(*n, 21, 2, calc); +} + +static uint32_t get_fdouble_sign(uint64_t n) +{ +return test_bit(20, ); +} + +static void set_fdouble_sign(uint64_t *n) +{ +set_bit(20, n); +} +
[Qemu-devel] [PATCH v3 4/4] target-tilegx: Integrate floating pointer implementation
It passes normal building, and gcc testsuite. Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> --- target-tilegx/Makefile.objs | 3 +- target-tilegx/cpu.h | 2 ++ target-tilegx/helper.h | 12 target-tilegx/translate.c | 68 +++-- 4 files changed, 75 insertions(+), 10 deletions(-) diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs index 0db778f..136ad60 100644 --- a/target-tilegx/Makefile.objs +++ b/target-tilegx/Makefile.objs @@ -1 +1,2 @@ -obj-y += cpu.o translate.o helper.o simd_helper.o +obj-y += cpu.o translate.o helper.o simd_helper.o \ + helper-fsingle.o helper-fdouble.o diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h index 03df107..445a606 100644 --- a/target-tilegx/cpu.h +++ b/target-tilegx/cpu.h @@ -88,6 +88,8 @@ typedef struct CPUTLGState { uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside */ uint64_t pc; /* Current pc */ +float_status fp_status;/* floating point status */ + #if defined(CONFIG_USER_ONLY) uint64_t excaddr; /* exception address */ uint64_t atomic_srca; /* Arguments to atomic "exceptions" */ diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h index 9281d0f..b785bf2 100644 --- a/target-tilegx/helper.h +++ b/target-tilegx/helper.h @@ -24,3 +24,15 @@ DEF_HELPER_FLAGS_2(v1shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64) DEF_HELPER_FLAGS_2(v2shl, TCG_CALL_NO_RWG_SE, i64, i64, i64) DEF_HELPER_FLAGS_2(v2shru, TCG_CALL_NO_RWG_SE, i64, i64, i64) DEF_HELPER_FLAGS_2(v2shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64) + +DEF_HELPER_3(fsingle_add1, i64, env, i64, i64) +DEF_HELPER_3(fsingle_sub1, i64, env, i64, i64) +DEF_HELPER_3(fsingle_mul1, i64, env, i64, i64) +DEF_HELPER_2(fsingle_pack2, i64, env, i64) +DEF_HELPER_3(fdouble_unpack_min, i64, env, i64, i64) +DEF_HELPER_3(fdouble_unpack_max, i64, env, i64, i64) +DEF_HELPER_3(fdouble_add_flags, i64, env, i64, i64) +DEF_HELPER_3(fdouble_sub_flags, i64, env, i64, i64) +DEF_HELPER_4(fdouble_addsub, i64, env, i64, i64, i64) +DEF_HELPER_3(fdouble_mul_flags, i64, env, i64, i64) +DEF_HELPER_4(fdouble_pack2, i64, env, i64, i64, i64) diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c index 354f25a..5c2a98d 100644 --- a/target-tilegx/translate.c +++ b/target-tilegx/translate.c @@ -597,6 +597,11 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext, } qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s", mnemonic, reg_names[srca]); return ret; + +case OE_RR_X0(FSINGLE_PACK1): +case OE_RR_Y0(FSINGLE_PACK1): +mnemonic = "fsingle_pack1"; +goto done2; } tdest = dest_gr(dc, dest); @@ -613,9 +618,6 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext, gen_helper_cnttz(tdest, tsrca); mnemonic = "cnttz"; break; -case OE_RR_X0(FSINGLE_PACK1): -case OE_RR_Y0(FSINGLE_PACK1): -return TILEGX_EXCP_OPCODE_UNIMPLEMENTED; case OE_RR_X1(LD1S): memop = MO_SB; mnemonic = "ld1s"; /* prefetch_l1_fault */ @@ -734,6 +736,7 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext, return TILEGX_EXCP_OPCODE_UNKNOWN; } +done2: qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s, %s", mnemonic, reg_names[dest], reg_names[srca]); return ret; @@ -742,13 +745,21 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext, static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext, unsigned dest, unsigned srca, unsigned srcb) { -TCGv tdest = dest_gr(dc, dest); -TCGv tsrca = load_gr(dc, srca); -TCGv tsrcb = load_gr(dc, srcb); +TCGv tdest, tsrca, tsrcb; TCGv t0; const char *mnemonic; switch (opext) { +case OE_RRR(FSINGLE_ADDSUB2, 0, X0): +mnemonic = "fsingle_addsub2"; +goto done2; +} + +tdest = dest_gr(dc, dest); +tsrca = load_gr(dc, srca); +tsrcb = load_gr(dc, srcb); + +switch (opext) { case OE_RRR(ADDXSC, 0, X0): case OE_RRR(ADDXSC, 0, X1): gen_saturate_op(tdest, tsrca, tsrcb, tcg_gen_add_tl); @@ -906,14 +917,39 @@ static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext, mnemonic = "exch"; break; case OE_RRR(FDOUBLE_ADDSUB, 0, X0): +gen_helper_fdouble_addsub(tdest, cpu_env, + load_gr(dc, dest), tsrca, tsrcb); +mnemonic = "fdouble_addsub"; +break; case OE_RRR(FDOUBLE_ADD_FLAGS, 0, X0): +gen_helper_fdouble_add_flags(tdest, cpu_env, tsrca, tsrcb); +mnemonic = "fdouble_add_flags"; +break; case OE_RRR(FDOUBLE_MUL_FLAGS, 0, X0): +gen_helper_fdouble_mul_flags(tdest, cpu_env, tsrca, tsrcb); +mnemon
Re: [Qemu-devel] [PATCH v2 0/4] target-tilegx: Implement floating point instructions
Hello Maintainers: Please help check these patches when you have time. If it is necessary to send patch v3 for it, please let me know. Thanks. On 11/17/15 03:37, Chen Gang wrote: > From d0f0e0a78e81f9589d25b0a2b4ad826d6e55257d Mon Sep 17 00:00:00 2001 > From: Chen Gang <gang.chen.5...@gmail.com> > Date: Tue, 17 Nov 2015 03:13:54 +0800 > Subject: [PATCH v2 0/4] target-tilegx: Implement floating point instructions > > These patches are the normal floating point implementation, instead of > the original temporary one. > > It passes building, and gcc testsuite. > > Chen Gang (4): > target-tilegx: Add floating point shared functions > target-tilegx: Add single floating point implementation > target-tilegx: Add double floating point implementation > target-tilegx: Integrate floating pointer implementation > > target-tilegx/Makefile.objs | 3 +- > target-tilegx/cpu.h | 2 + > target-tilegx/helper-fdouble.c | 400 + > target-tilegx/helper-fshared.c | 53 ++ > target-tilegx/helper-fsingle.c | 212 ++ > target-tilegx/helper.h | 12 ++ > target-tilegx/translate.c | 68 ++- > 7 files changed, 740 insertions(+), 10 deletions(-) > create mode 100644 target-tilegx/helper-fdouble.c > create mode 100644 target-tilegx/helper-fshared.c > create mode 100644 target-tilegx/helper-fsingle.c > > -- > 1.9.3 > -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v2 3/4] target-tilegx: Add double floating point implementation
Oh, sorry again, the original attachment is incorrect, either. The attachment in this reply is the correct one. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed > From: xili_gchen_5...@hotmail.com > To: r...@twiddle.net; peter.mayd...@linaro.org; cmetc...@ezchip.com > CC: qemu-devel@nongnu.org > Subject: Re: [PATCH v2 3/4] target-tilegx: Add double floating point > implementation > Date: Tue, 17 Nov 2015 03:57:28 +0800 > > > Excuse me, I copied/pasted the patch to the website, which may generate > the incorrect coding styles. > > Please check the attachment in this reply mail for the correct coding > styles (and sorry, the original mail's attachment which I chose is > incorrect). > > On 11/17/15 03:41, Chen Gang wrote: >> From db171c94e8c9df446c091d9f42004c80ed8c6ccc Mon Sep 17 00:00:00 2001 >> From: Chen Gang <gang.chen.5...@gmail.com> >> Date: Tue, 17 Nov 2015 03:08:38 +0800 >> Subject: [PATCH v2 3/4] target-tilegx: Add double floating point >> implementation >> >> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >> --- >> target-tilegx/helper-fdouble.c | 400 >> + >> 1 file changed, 400 insertions(+) >> create mode 100644 target-tilegx/helper-fdouble.c >> >> diff --git a/target-tilegx/helper-fdouble.c b/target-tilegx/helper-fdouble.c >> new file mode 100644 >> index 000..f005c80 >> --- /dev/null >> +++ b/target-tilegx/helper-fdouble.c >> @@ -0,0 +1,400 @@ >> +/* >> + * QEMU TILE-Gx helpers >> + * >> + * Copyright (c) 2015 Chen Gang >> + * >> + * This library is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU Lesser General Public >> + * License as published by the Free Software Foundation; either >> + * version 2.1 of the License, or (at your option) any later version. >> + * >> + * This library is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + * Lesser General Public License for more details. >> + * >> + * You should have received a copy of the GNU Lesser General Public >> + * License along with this library; if not, see >> + * <http://www.gnu.org/licenses/lgpl-2.1.html> >> + */ >> + >> +#include "cpu.h" >> +#include "qemu-common.h" >> +#include "exec/helper-proto.h" >> +#include "fpu/softfloat.h" >> + >> +#include "helper-fshared.c" >> + >> +/* >> + * FDouble instructions implemenation: >> + * >> + * fdouble_unpack_min ; srca and srcb are float_64 value. >> + * ; get the min absolute value's mantissa. >> + * ; move "mantissa>> (exp_max - exp_min)" to dest. >> + * >> + * fdouble_unpack_max ; srca and srcb are float_64 value. >> + * ; get the max absolute value's mantissa. >> + * ; move mantissa to dest. >> + * >> + * fdouble_add_flags ; srca and srcb are float_64 value. >> + * ; calc exp (exp_max), sign, and comp bits for flags. >> + * ; set addsub bit to flags and move flags to dest. >> + * >> + * fdouble_sub_flags ; srca and srcb are float_64 value. >> + * ; calc exp (exp_max), sign, and comp bits for flags. >> + * ; set addsub bit to flags and move flags to dest. >> + * >> + * fdouble_addsub: ; dest, srca (max, min mantissa), and srcb (flags). >> + * ; "dest +/- srca" depend on the add/sub bit of flags. >> + * ; move result mantissa to dest. >> + * >> + * fdouble_mul_flags: ; srca and srcb are float_64 value. >> + * ; calc sign (xor), exp (min + max), and comp bits. >> + * ; mix sign, exp, and comp bits as flags to dest. >> + * >> + * fdouble_pack1 ; move srcb (flags) to dest. >> + * >> + * fdouble_pack2 ; srca, srcb (high, low mantissa), and dest (flags) >> + * ; normalize and pack result from srca, srcb, and dest. >> + * ; move result to dest. >> + */ >> + >> +#define TILEGX_F_EXP_DZERO 0x3ff /* Zero exp for double 11-bits */ >> +#define TILEGX_F_EXP_DMAX 0x7fe /* max exp for double 11-bits */ >> +#define TILEGX_F_EXP_DUF 0x1000/* underflow exp bit for double */ >> + >> +#define TILEGX_F_MAN_HBIT (1ULL << 59) >> + >> +#define TILEGX_F_CALC_ADD 1 /* Perform absolute add operation */ >> +#define TILEGX_F_CALC_SUB 2 /* Perform absolute sub operation */ >> +#define TILEGX_F_CALC_MUL 3 /* Perform ab
[Qemu-devel] [PATCH v2 2/4] target-tilegx: Add single floating point implementation
>From 1c4bb91e72995a1675d3aa0f911c534a3e8db749 Mon Sep 17 00:00:00 2001 From: Chen Gang <gang.chen.5...@gmail.com> Date: Tue, 17 Nov 2015 03:07:33 +0800 Subject: [PATCH v2 2/4] target-tilegx: Add single floating point implementation Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> --- target-tilegx/helper-fsingle.c | 212 + 1 file changed, 212 insertions(+) create mode 100644 target-tilegx/helper-fsingle.c diff --git a/target-tilegx/helper-fsingle.c b/target-tilegx/helper-fsingle.c new file mode 100644 index 000..a33837e --- /dev/null +++ b/target-tilegx/helper-fsingle.c @@ -0,0 +1,212 @@ +/* + * QEMU TILE-Gx helpers + * + * Copyright (c) 2015 Chen Gang + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see + * <http://www.gnu.org/licenses/lgpl-2.1.html> + */ + +#include "cpu.h" +#include "qemu-common.h" +#include "exec/helper-proto.h" +#include "fpu/softfloat.h" + +#include "helper-fshared.c" + +/* + * FSingle instructions implemenation: + * + * fsingle_add1 ; calc srca and srcb, + * ; convert float_32 to TileGXFPSFmt result. + * ; move TileGXFPSFmt result to dest. + * + * fsingle_sub1 ; calc srca and srcb. + * ; convert float_32 to TileGXFPSFmt result. + * ; move TileGXFPSFmt result to dest. + * + * fsingle_addsub2 ; nop. + * + * fsingle_mul1 ; calc srca and srcb. + * ; convert float_32 value to TileGXFPSFmt result. + * ; move TileGXFPSFmt result to dest. + * + * fsingle_mul2 ; move srca to dest. + * + * fsingle_pack1 ; nop + * + * fsingle_pack2 ; treate srca as TileGXFPSFmt result. + * ; convert TileGXFPSFmt result to float_32 value. + * ; move float_32 value to dest. + */ + +#define TILEGX_F_CALC_CVT 0 /* convert int to fsingle */ +#define TILEGX_F_CALC_NCVT 1 /* Not convertion */ + +static uint32_t get_f32_exp(float32 f) +{ + return extract32(float32_val(f), 23, 8); +} + +static void set_f32_exp(float32 *f, uint32_t exp) +{ + *f = make_float32(deposit32(float32_val(*f), 23, 8, exp)); +} + +static uint32_t get_f32_man(float32 f) +{ + return float32_val(f) & 0x7f; +} + +static float32 create_f32_man(uint32_t man) +{ + return make_float32(man & 0x7f); +} + +static inline uint32_t get_fsingle_exp(uint64_t n) +{ + return n & 0xff; +} + +static inline uint64_t create_fsingle_exp(uint32_t exp) +{ + return exp & 0xff; +} + +static inline uint32_t get_fsingle_sign(uint64_t n) +{ + return test_bit(10, ); +} + +static inline void set_fsingle_sign(uint64_t *n) +{ + set_bit(10, n); +} + +static inline unsigned int get_fsingle_calc(uint64_t n) +{ + return test_bit(11, ); +} + +static inline void set_fsingle_calc(uint64_t *n, uint32_t calc) +{ + set_bit(11, n); +} + +static inline unsigned int get_fsingle_man(uint64_t n) +{ + return n>> 32; +} + +static inline uint64_t create_fsingle_man(uint32_t man) +{ + return (uint64_t)man << 32; +} + +static uint64_t float32_to_sfmt(float32 f) +{ + uint64_t sfmt = 0; + + if (float32_is_neg(f)) { + set_fsingle_sign(); + } + sfmt |= create_fsingle_exp(get_f32_exp(f)); + sfmt |= create_fsingle_man((get_f32_man(f) << 8) | (1 << 31)); + + return sfmt; +} + +static float32 sfmt_to_float32(uint64_t sfmt, float_status *fp_status) +{ + float32 f; + uint32_t sign = get_fsingle_sign(sfmt); + uint32_t man = get_fsingle_man(sfmt); + + if (get_fsingle_calc(sfmt) == TILEGX_F_CALC_CVT) { + if (sign) { + return int32_to_float32(0 - man, fp_status); + } else { + return uint32_to_float32(man, fp_status); + } + } else { + f = float32_set_sign(float32_zero, sign); + f |= create_f32_man(man>> 8); + set_f32_exp(, get_fsingle_exp(sfmt)); + } + + return f; +} + +uint64_t helper_fsingle_pack2(CPUTLGState *env, uint64_t srca) +{ + return float32_val(sfmt_to_float32(srca, >fp_status)); +} + +static void ana_bits(float_status *fp_status, + float32 fsrca, float32 fsrcb, uint64_t *sfmt) +{ + if (float32_eq(fsrca, fsrcb, fp_status)) { + *
Re: [Qemu-devel] [PATCH v2 4/4] target-tilegx: Integrate floating pointer implementation
Excuse me, I copied/pasted the patch to the website, which may generate the incorrect coding styles. Please check the attachment in this reply mail for the correct coding styles (and sorry, the original mail's attachment which I chose is incorrect). On 11/17/15 03:43, Chen Gang wrote: > From d0f0e0a78e81f9589d25b0a2b4ad826d6e55257d Mon Sep 17 00:00:00 2001 > From: Chen Gang <gang.chen.5...@gmail.com> > Date: Tue, 17 Nov 2015 03:09:18 +0800 > Subject: [PATCH v2 4/4] target-tilegx: Integrate floating pointer > implementation > > It passes normal building, and gcc testsuite. > > Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> > --- > target-tilegx/Makefile.objs | 3 +- > target-tilegx/cpu.h | 2 ++ > target-tilegx/helper.h | 12 > target-tilegx/translate.c | 68 +++-- > 4 files changed, 75 insertions(+), 10 deletions(-) > > diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs > index 0db778f..136ad60 100644 > --- a/target-tilegx/Makefile.objs > +++ b/target-tilegx/Makefile.objs > @@ -1 +1,2 @@ > -obj-y += cpu.o translate.o helper.o simd_helper.o > +obj-y += cpu.o translate.o helper.o simd_helper.o \ > + helper-fsingle.o helper-fdouble.o > diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h > index 03df107..445a606 100644 > --- a/target-tilegx/cpu.h > +++ b/target-tilegx/cpu.h > @@ -88,6 +88,8 @@ typedef struct CPUTLGState { > uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside */ > uint64_t pc; /* Current pc */ > > + float_status fp_status; /* floating point status */ > + > #if defined(CONFIG_USER_ONLY) > uint64_t excaddr; /* exception address */ > uint64_t atomic_srca; /* Arguments to atomic "exceptions" */ > diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h > index 9281d0f..b785bf2 100644 > --- a/target-tilegx/helper.h > +++ b/target-tilegx/helper.h > @@ -24,3 +24,15 @@ DEF_HELPER_FLAGS_2(v1shrs, TCG_CALL_NO_RWG_SE, i64, i64, > i64) > DEF_HELPER_FLAGS_2(v2shl, TCG_CALL_NO_RWG_SE, i64, i64, i64) > DEF_HELPER_FLAGS_2(v2shru, TCG_CALL_NO_RWG_SE, i64, i64, i64) > DEF_HELPER_FLAGS_2(v2shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64) > + > +DEF_HELPER_3(fsingle_add1, i64, env, i64, i64) > +DEF_HELPER_3(fsingle_sub1, i64, env, i64, i64) > +DEF_HELPER_3(fsingle_mul1, i64, env, i64, i64) > +DEF_HELPER_2(fsingle_pack2, i64, env, i64) > +DEF_HELPER_3(fdouble_unpack_min, i64, env, i64, i64) > +DEF_HELPER_3(fdouble_unpack_max, i64, env, i64, i64) > +DEF_HELPER_3(fdouble_add_flags, i64, env, i64, i64) > +DEF_HELPER_3(fdouble_sub_flags, i64, env, i64, i64) > +DEF_HELPER_4(fdouble_addsub, i64, env, i64, i64, i64) > +DEF_HELPER_3(fdouble_mul_flags, i64, env, i64, i64) > +DEF_HELPER_4(fdouble_pack2, i64, env, i64, i64, i64) > diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c > index b8ca401..4d74b1d 100644 > --- a/target-tilegx/translate.c > +++ b/target-tilegx/translate.c > @@ -597,6 +597,11 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned > opext, > } > qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s", mnemonic, reg_names[srca]); > return ret; > + > + case OE_RR_X0(FSINGLE_PACK1): > + case OE_RR_Y0(FSINGLE_PACK1): > + mnemonic = "fsingle_pack1"; > + goto done2; > } > > tdest = dest_gr(dc, dest); > @@ -613,9 +618,6 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned > opext, > gen_helper_cnttz(tdest, tsrca); > mnemonic = "cnttz"; > break; > - case OE_RR_X0(FSINGLE_PACK1): > - case OE_RR_Y0(FSINGLE_PACK1): > - return TILEGX_EXCP_OPCODE_UNIMPLEMENTED; > case OE_RR_X1(LD1S): > memop = MO_SB; > mnemonic = "ld1s"; /* prefetch_l1_fault */ > @@ -734,6 +736,7 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned > opext, > return TILEGX_EXCP_OPCODE_UNKNOWN; > } > > +done2: > qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s, %s", mnemonic, > reg_names[dest], reg_names[srca]); > return ret; > @@ -742,13 +745,21 @@ static TileExcp gen_rr_opcode(DisasContext *dc, > unsigned opext, > static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext, > unsigned dest, unsigned srca, unsigned srcb) > { > - TCGv tdest = dest_gr(dc, dest); > - TCGv tsrca = load_gr(dc, srca); > - TCGv tsrcb = load_gr(dc, srcb); > + TCGv tdest, tsrca, tsrcb; > TCGv t0; > const char *mnemonic; > > switch (opext) { > + case OE_RRR(FSINGLE_ADDSUB2, 0, X0): > + mnemonic = "fsingle_addsub2"; > + goto done2; > + } > + > + tdest = dest_gr(dc, dest); > + tsrca = load_gr(dc, srca); > + tsrcb = load_gr(dc, srcb); > + > + switch (opext) { > case OE_RRR(ADDXSC, 0, X0): > case OE_RRR(ADDXSC, 0, X1): >
[Qemu-devel] [PATCH v2 4/4] target-tilegx: Integrate floating pointer implementation
>From d0f0e0a78e81f9589d25b0a2b4ad826d6e55257d Mon Sep 17 00:00:00 2001 From: Chen Gang <gang.chen.5...@gmail.com> Date: Tue, 17 Nov 2015 03:09:18 +0800 Subject: [PATCH v2 4/4] target-tilegx: Integrate floating pointer implementation It passes normal building, and gcc testsuite. Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> --- target-tilegx/Makefile.objs | 3 +- target-tilegx/cpu.h | 2 ++ target-tilegx/helper.h | 12 target-tilegx/translate.c | 68 +++-- 4 files changed, 75 insertions(+), 10 deletions(-) diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs index 0db778f..136ad60 100644 --- a/target-tilegx/Makefile.objs +++ b/target-tilegx/Makefile.objs @@ -1 +1,2 @@ -obj-y += cpu.o translate.o helper.o simd_helper.o +obj-y += cpu.o translate.o helper.o simd_helper.o \ + helper-fsingle.o helper-fdouble.o diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h index 03df107..445a606 100644 --- a/target-tilegx/cpu.h +++ b/target-tilegx/cpu.h @@ -88,6 +88,8 @@ typedef struct CPUTLGState { uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside */ uint64_t pc; /* Current pc */ + float_status fp_status; /* floating point status */ + #if defined(CONFIG_USER_ONLY) uint64_t excaddr; /* exception address */ uint64_t atomic_srca; /* Arguments to atomic "exceptions" */ diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h index 9281d0f..b785bf2 100644 --- a/target-tilegx/helper.h +++ b/target-tilegx/helper.h @@ -24,3 +24,15 @@ DEF_HELPER_FLAGS_2(v1shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64) DEF_HELPER_FLAGS_2(v2shl, TCG_CALL_NO_RWG_SE, i64, i64, i64) DEF_HELPER_FLAGS_2(v2shru, TCG_CALL_NO_RWG_SE, i64, i64, i64) DEF_HELPER_FLAGS_2(v2shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64) + +DEF_HELPER_3(fsingle_add1, i64, env, i64, i64) +DEF_HELPER_3(fsingle_sub1, i64, env, i64, i64) +DEF_HELPER_3(fsingle_mul1, i64, env, i64, i64) +DEF_HELPER_2(fsingle_pack2, i64, env, i64) +DEF_HELPER_3(fdouble_unpack_min, i64, env, i64, i64) +DEF_HELPER_3(fdouble_unpack_max, i64, env, i64, i64) +DEF_HELPER_3(fdouble_add_flags, i64, env, i64, i64) +DEF_HELPER_3(fdouble_sub_flags, i64, env, i64, i64) +DEF_HELPER_4(fdouble_addsub, i64, env, i64, i64, i64) +DEF_HELPER_3(fdouble_mul_flags, i64, env, i64, i64) +DEF_HELPER_4(fdouble_pack2, i64, env, i64, i64, i64) diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c index b8ca401..4d74b1d 100644 --- a/target-tilegx/translate.c +++ b/target-tilegx/translate.c @@ -597,6 +597,11 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext, } qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s", mnemonic, reg_names[srca]); return ret; + + case OE_RR_X0(FSINGLE_PACK1): + case OE_RR_Y0(FSINGLE_PACK1): + mnemonic = "fsingle_pack1"; + goto done2; } tdest = dest_gr(dc, dest); @@ -613,9 +618,6 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext, gen_helper_cnttz(tdest, tsrca); mnemonic = "cnttz"; break; - case OE_RR_X0(FSINGLE_PACK1): - case OE_RR_Y0(FSINGLE_PACK1): - return TILEGX_EXCP_OPCODE_UNIMPLEMENTED; case OE_RR_X1(LD1S): memop = MO_SB; mnemonic = "ld1s"; /* prefetch_l1_fault */ @@ -734,6 +736,7 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext, return TILEGX_EXCP_OPCODE_UNKNOWN; } +done2: qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s, %s", mnemonic, reg_names[dest], reg_names[srca]); return ret; @@ -742,13 +745,21 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext, static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext, unsigned dest, unsigned srca, unsigned srcb) { - TCGv tdest = dest_gr(dc, dest); - TCGv tsrca = load_gr(dc, srca); - TCGv tsrcb = load_gr(dc, srcb); + TCGv tdest, tsrca, tsrcb; TCGv t0; const char *mnemonic; switch (opext) { + case OE_RRR(FSINGLE_ADDSUB2, 0, X0): + mnemonic = "fsingle_addsub2"; + goto done2; + } + + tdest = dest_gr(dc, dest); + tsrca = load_gr(dc, srca); + tsrcb = load_gr(dc, srcb); + + switch (opext) { case OE_RRR(ADDXSC, 0, X0): case OE_RRR(ADDXSC, 0, X1): gen_saturate_op(tdest, tsrca, tsrcb, tcg_gen_add_tl); @@ -906,14 +917,39 @@ static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext, mnemonic = "exch"; break; case OE_RRR(FDOUBLE_ADDSUB, 0, X0): + gen_helper_fdouble_addsub(tdest, cpu_env, + load_gr(dc, dest),tsrca, tsrcb); + mnemonic = "fdouble_addsub"; + break; case OE_RRR(FDOUBLE_ADD_FLAGS, 0, X0): + gen_helper
[Qemu-devel] [PATCH v2 3/4] target-tilegx: Add double floating point implementation
>From db171c94e8c9df446c091d9f42004c80ed8c6ccc Mon Sep 17 00:00:00 2001 From: Chen Gang <gang.chen.5...@gmail.com> Date: Tue, 17 Nov 2015 03:08:38 +0800 Subject: [PATCH v2 3/4] target-tilegx: Add double floating point implementation Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> --- target-tilegx/helper-fdouble.c | 400 + 1 file changed, 400 insertions(+) create mode 100644 target-tilegx/helper-fdouble.c diff --git a/target-tilegx/helper-fdouble.c b/target-tilegx/helper-fdouble.c new file mode 100644 index 000..f005c80 --- /dev/null +++ b/target-tilegx/helper-fdouble.c @@ -0,0 +1,400 @@ +/* + * QEMU TILE-Gx helpers + * + * Copyright (c) 2015 Chen Gang + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see + * <http://www.gnu.org/licenses/lgpl-2.1.html> + */ + +#include "cpu.h" +#include "qemu-common.h" +#include "exec/helper-proto.h" +#include "fpu/softfloat.h" + +#include "helper-fshared.c" + +/* + * FDouble instructions implemenation: + * + * fdouble_unpack_min ; srca and srcb are float_64 value. + * ; get the min absolute value's mantissa. + * ; move "mantissa>> (exp_max - exp_min)" to dest. + * + * fdouble_unpack_max ; srca and srcb are float_64 value. + * ; get the max absolute value's mantissa. + * ; move mantissa to dest. + * + * fdouble_add_flags ; srca and srcb are float_64 value. + * ; calc exp (exp_max), sign, and comp bits for flags. + * ; set addsub bit to flags and move flags to dest. + * + * fdouble_sub_flags ; srca and srcb are float_64 value. + * ; calc exp (exp_max), sign, and comp bits for flags. + * ; set addsub bit to flags and move flags to dest. + * + * fdouble_addsub: ; dest, srca (max, min mantissa), and srcb (flags). + * ; "dest +/- srca" depend on the add/sub bit of flags. + * ; move result mantissa to dest. + * + * fdouble_mul_flags: ; srca and srcb are float_64 value. + * ; calc sign (xor), exp (min + max), and comp bits. + * ; mix sign, exp, and comp bits as flags to dest. + * + * fdouble_pack1 ; move srcb (flags) to dest. + * + * fdouble_pack2 ; srca, srcb (high, low mantissa), and dest (flags) + * ; normalize and pack result from srca, srcb, and dest. + * ; move result to dest. + */ + +#define TILEGX_F_EXP_DZERO 0x3ff /* Zero exp for double 11-bits */ +#define TILEGX_F_EXP_DMAX 0x7fe /* max exp for double 11-bits */ +#define TILEGX_F_EXP_DUF 0x1000/* underflow exp bit for double */ + +#define TILEGX_F_MAN_HBIT (1ULL << 59) + +#define TILEGX_F_CALC_ADD 1 /* Perform absolute add operation */ +#define TILEGX_F_CALC_SUB 2 /* Perform absolute sub operation */ +#define TILEGX_F_CALC_MUL 3 /* Perform absolute mul operation */ + +static uint32_t get_f64_exp(float64 d) +{ + return extract64(float64_val(d), 52, 11); +} + +static void set_f64_exp(float64 *d, uint32_t exp) +{ + *d = make_float64(deposit64(float64_val(*d), 52, 11, exp)); +} + +static uint64_t get_f64_man(float64 d) +{ + return extract64(float64_val(d), 0, 52); +} + +static uint64_t fr_to_man(float64 d) +{ + uint64_t val = get_f64_man(d) << 7; + + if (get_f64_exp(d)) { + val |= TILEGX_F_MAN_HBIT; + } + + return val; +} + +static uint64_t get_fdouble_man(uint64_t n) +{ + return extract64(n, 0, 60); +} + +static void set_fdouble_man(uint64_t *n, uint64_t man) +{ + *n = deposit64(*n, 0, 60, man); +} + +static uint64_t get_fdouble_man_of(uint64_t n) +{ + return test_bit(60, ); +} + +static void clear_fdouble_man_of(uint64_t *n) +{ + return clear_bit(60, n); +} + +static uint32_t get_fdouble_nan(uint64_t n) +{ + return test_bit(24, ); +} + +static void set_fdouble_nan(uint64_t *n) +{ + set_bit(24, n); +} + +static uint32_t get_fdouble_inf(uint64_t n) +{ + return test_bit(23, ); +} + +static void set_fdouble_inf(uint64_t *n) +{ + set_bit(23, n); +} + +static uint32_t get_fdouble_calc(uint64_t n) +{ + return extract32(n, 21, 2); +} + +static void set_fdouble_calc(uint64_t
[Qemu-devel] [PATCH v2 0/4] target-tilegx: Implement floating point instructions
>From d0f0e0a78e81f9589d25b0a2b4ad826d6e55257d Mon Sep 17 00:00:00 2001 From: Chen Gang <gang.chen.5...@gmail.com> Date: Tue, 17 Nov 2015 03:13:54 +0800 Subject: [PATCH v2 0/4] target-tilegx: Implement floating point instructions These patches are the normal floating point implementation, instead of the original temporary one. It passes building, and gcc testsuite. Chen Gang (4): target-tilegx: Add floating point shared functions target-tilegx: Add single floating point implementation target-tilegx: Add double floating point implementation target-tilegx: Integrate floating pointer implementation target-tilegx/Makefile.objs | 3 +- target-tilegx/cpu.h | 2 + target-tilegx/helper-fdouble.c | 400 + target-tilegx/helper-fshared.c | 53 ++ target-tilegx/helper-fsingle.c | 212 ++ target-tilegx/helper.h | 12 ++ target-tilegx/translate.c | 68 ++- 7 files changed, 740 insertions(+), 10 deletions(-) create mode 100644 target-tilegx/helper-fdouble.c create mode 100644 target-tilegx/helper-fshared.c create mode 100644 target-tilegx/helper-fsingle.c -- 1.9.3 v2--cover-letter.patch Description: Binary data
[Qemu-devel] [PATCH v2 1/4] target-tilegx: Add floating point shared functions
>From 40c3e3f79b206b9506e0d6679e301885bb3ee277 Mon Sep 17 00:00:00 2001 From: Chen Gang <gang.chen.5...@gmail.com> Date: Tue, 17 Nov 2015 03:04:50 +0800 Subject: [PATCH v2 1/4] target-tilegx: Add floating point shared functions They are used by fsingle and fdouble helpers. Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> --- target-tilegx/helper-fshared.c | 53 ++ 1 file changed, 53 insertions(+) create mode 100644 target-tilegx/helper-fshared.c diff --git a/target-tilegx/helper-fshared.c b/target-tilegx/helper-fshared.c new file mode 100644 index 000..d669f58 --- /dev/null +++ b/target-tilegx/helper-fshared.c @@ -0,0 +1,53 @@ +/* + * TILE-Gx virtual Floating point shared functions + * + * Copyright (c) 2015 Chen Gang + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see <http://www.gnu.org/licenses/>. + */ + +static inline uint64_t create_fsfd_flag_un(void) +{ + return 1 << 25; +} + +static inline uint64_t create_fsfd_flag_lt(void) +{ + return 1 << 26; +} + +static inline uint64_t create_fsfd_flag_le(void) +{ + return 1 << 27; +} + +static inline uint64_t create_fsfd_flag_gt(void) +{ + return 1 << 28; +} + +static inline uint64_t create_fsfd_flag_ge(void) +{ + return 1 << 29; +} + +static inline uint64_t create_fsfd_flag_eq(void) +{ + return 1 << 30; +} + +static inline uint64_t create_fsfd_flag_ne(void) +{ + return 1ULL << 31; +} -- 1.9.3 v2-0001-target-tilegx-Add-floating-point-shared-functions.patch Description: Binary data
Re: [Qemu-devel] [PATCH v2 3/4] target-tilegx: Add double floating point implementation
Excuse me, I copied/pasted the patch to the website, which may generate the incorrect coding styles. Please check the attachment in this reply mail for the correct coding styles (and sorry, the original mail's attachment which I chose is incorrect). On 11/17/15 03:41, Chen Gang wrote: > From db171c94e8c9df446c091d9f42004c80ed8c6ccc Mon Sep 17 00:00:00 2001 > From: Chen Gang <gang.chen.5...@gmail.com> > Date: Tue, 17 Nov 2015 03:08:38 +0800 > Subject: [PATCH v2 3/4] target-tilegx: Add double floating point > implementation > > Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> > --- > target-tilegx/helper-fdouble.c | 400 + > 1 file changed, 400 insertions(+) > create mode 100644 target-tilegx/helper-fdouble.c > > diff --git a/target-tilegx/helper-fdouble.c b/target-tilegx/helper-fdouble.c > new file mode 100644 > index 000..f005c80 > --- /dev/null > +++ b/target-tilegx/helper-fdouble.c > @@ -0,0 +1,400 @@ > +/* > + * QEMU TILE-Gx helpers > + * > + * Copyright (c) 2015 Chen Gang > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see > + * <http://www.gnu.org/licenses/lgpl-2.1.html> > + */ > + > +#include "cpu.h" > +#include "qemu-common.h" > +#include "exec/helper-proto.h" > +#include "fpu/softfloat.h" > + > +#include "helper-fshared.c" > + > +/* > + * FDouble instructions implemenation: > + * > + * fdouble_unpack_min ; srca and srcb are float_64 value. > + * ; get the min absolute value's mantissa. > + * ; move "mantissa>> (exp_max - exp_min)" to dest. > + * > + * fdouble_unpack_max ; srca and srcb are float_64 value. > + * ; get the max absolute value's mantissa. > + * ; move mantissa to dest. > + * > + * fdouble_add_flags ; srca and srcb are float_64 value. > + * ; calc exp (exp_max), sign, and comp bits for flags. > + * ; set addsub bit to flags and move flags to dest. > + * > + * fdouble_sub_flags ; srca and srcb are float_64 value. > + * ; calc exp (exp_max), sign, and comp bits for flags. > + * ; set addsub bit to flags and move flags to dest. > + * > + * fdouble_addsub: ; dest, srca (max, min mantissa), and srcb (flags). > + * ; "dest +/- srca" depend on the add/sub bit of flags. > + * ; move result mantissa to dest. > + * > + * fdouble_mul_flags: ; srca and srcb are float_64 value. > + * ; calc sign (xor), exp (min + max), and comp bits. > + * ; mix sign, exp, and comp bits as flags to dest. > + * > + * fdouble_pack1 ; move srcb (flags) to dest. > + * > + * fdouble_pack2 ; srca, srcb (high, low mantissa), and dest (flags) > + * ; normalize and pack result from srca, srcb, and dest. > + * ; move result to dest. > + */ > + > +#define TILEGX_F_EXP_DZERO 0x3ff /* Zero exp for double 11-bits */ > +#define TILEGX_F_EXP_DMAX 0x7fe /* max exp for double 11-bits */ > +#define TILEGX_F_EXP_DUF 0x1000/* underflow exp bit for double */ > + > +#define TILEGX_F_MAN_HBIT (1ULL << 59) > + > +#define TILEGX_F_CALC_ADD 1 /* Perform absolute add operation */ > +#define TILEGX_F_CALC_SUB 2 /* Perform absolute sub operation */ > +#define TILEGX_F_CALC_MUL 3 /* Perform absolute mul operation */ > + > +static uint32_t get_f64_exp(float64 d) > +{ > + return extract64(float64_val(d), 52, 11); > +} > + > +static void set_f64_exp(float64 *d, uint32_t exp) > +{ > + *d = make_float64(deposit64(float64_val(*d), 52, 11, exp)); > +} > + > +static uint64_t get_f64_man(float64 d) > +{ > + return extract64(float64_val(d), 0, 52); > +} > + > +static uint64_t fr_to_man(float64 d) > +{ > + uint64_t val = get_f64_man(d) << 7; > + > + if (get_f64_exp(d)) { > + val |= TILEGX_F_MAN_HBIT; > + } > + > + return val; > +} > + > +static uint64_t get_fdouble_man(uint64_t n) > +{ > + return extract64(n, 0, 60); > +} > + > +static void set_fdouble_man(uint64_t *n, uint64_t man) > +{ > + *n = deposit64(*n, 0, 60, man); > +} > + > +static uint64_t get_fdouble_man_of(uint64_t n) > +{ > + return test_
Re: [Qemu-devel] [PATCH 1/4] target-tilegx: Add fpu header file
On 11/12/15 22:34, Richard Henderson wrote: > On 11/08/2015 06:43 AM, Chen Gang wrote: > >> +#if !defined(HOST_WORDS_BIGENDIAN) >> +/* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md */ >> +uint64_t exp : 8; /* exp, 0x9e: 31 + TILEGX_F_EXP_FZERO */ >> +uint64_t uiknown0 : 2;/* unknown */ > > I would really rather you didn't use bitfields, because of exactly this sort > of endianness problem. Because, really, you can't trust this layout. But I > won't press this point, because it is complicated enough already. > Because of endianess issues, for me, I don't like bit fields either. But I can not find any other simpler ways than current. >> +#endif >> +} TileGXFPSFmt; >> +/* > > Watch your spacing. > OK, thanks. And I also shall let the related comments above the structure. >> + * Double exp analyzing: (0x21b00 << 1) - 0x36(54) = 0x400 Oh, it should be (0x21b00 << 1) - 0x37(55) = 0x3ff >> + * >> + * 17 16 15 14 13 12 11 10 9 8 76 5 4 3 2 1 0 >> + * >> + *1 0 0 0 0 1 1 0 1 1 00 0 0 0 0 0 0 >> + * >> + *0 0 0 0 0 1 1 0 1 1 1=> 0x37(55) >> + * >> + *0 1 1 1 1 1 1 1 1 1 1=> 0x3ff > > What is this table supposed to mean? > I want to use this table to explain my guess: at first, we don't know the internal exp position, neither know the internal mantasa bits. We have to guess both of them: - the real exp should not be less than 11 bits (IEEE double exp is 11 bits). - Since IEEE double exp 0 is 0x3ff, probobly, for tilegx internally, 0x3ff shoul be '0', too (don't move mantissa). - After analyzing the data from floatunssidf2 in gcc tilegx.md (the table above is part for it), I found that "(0x21b00 << 1) - 0x37 = 0x3ff" is the best to match "all things". So, I guess: the double exp is from bit 7 to bit 17, then the mantissa is 60 bits (55 + 1 + 4), it from bit 0 to bit 59. We can use 7 - 19 bits for normal exp calculation, then can get the real exp (7 - 17 bits) with the overflow and underflow (so, I guess 18 bit is for overflow, and 19 bit is for underflow). >> +#if 0 >> +uint64_t exp : 11;/* exp, 0x21b << 1: 55 + >> TILEGX_F_EXP_DZERO */ >> +uint64_t ov : 1; /* overflow for mul, low priority */ >> +uint64_t uv : 1; /* underflow for mul, high priority */ >> +#endif > > > No if 0. > OK, I shall remove them. >> +#pragma pack(pop) > > Huh? What are you attempting to do here? > It is for matching "#pragma pack(push, 1)" which is above all related struct/unions in this header file. For the bit fields, we use uint64_t, but gcc still treate it as two uint32_t, so for safety reason, I use pragma pack for our structures. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH 2/4] target-tilegx: Implement fpu single floating point
On 11/13/15 00:18, Richard Henderson wrote: > On 11/12/2015 05:12 PM, Chen Gang wrote: >> On 11/12/15 22:36, Richard Henderson wrote: >>>> +if (sfmt.calc == TILEGX_F_CALC_CVT) { >>>> +if (sfmt.sign) >>>> +f.f = int32_to_float32(0 - sfmt.mantissa, fp_status); >>>> +else >>>> +f.f = uint32_to_float32(sfmt.mantissa, fp_status); >>>> +} else { >>> >>> Formatting. >>> >>> You really should know better by now. >>> I'm not even going to look at the rest. >>> >> >> Excuse me, my English is not quite well, I am not quite understand your >> meaning. >> >> Does the code above have issues? >> > > Please read ./CODING_STYLE, and use ./scripts/checkpatch.pl. > Oh, sorry. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH 2/4] target-tilegx: Implement fpu single floating point
On 11/12/15 22:36, Richard Henderson wrote: >> +if (sfmt.calc == TILEGX_F_CALC_CVT) { >> +if (sfmt.sign) >> +f.f = int32_to_float32(0 - sfmt.mantissa, fp_status); >> +else >> +f.f = uint32_to_float32(sfmt.mantissa, fp_status); >> +} else { > > Formatting. > > You really should know better by now. > I'm not even going to look at the rest. > Excuse me, my English is not quite well, I am not quite understand your meaning. Does the code above have issues? Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH 1/4] target-tilegx: Add fpu header file
On 11/13/15 00:10, Peter Maydell wrote: > On 12 November 2015 at 16:04, Chen Gang <xili_gchen_5...@hotmail.com> wrote: >> On 11/12/15 22:34, Richard Henderson wrote: >>> On 11/08/2015 06:43 AM, Chen Gang wrote: >>> >>>> +#if !defined(HOST_WORDS_BIGENDIAN) >>>> +/* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md >>>> */ >>>> +uint64_t exp : 8; /* exp, 0x9e: 31 + TILEGX_F_EXP_FZERO */ >>>> +uint64_t uiknown0 : 2;/* unknown */ >>> >>> I would really rather you didn't use bitfields, because of exactly this >>> sort of endianness problem. Because, really, you can't trust this layout. >>> But I won't press this point, because it is complicated enough already. >>> >> >> Because of endianess issues, for me, I don't like bit fields either. But >> I can not find any other simpler ways than current. > >> OK, I shall remove them. >> >>>> +#pragma pack(pop) >>> >>> Huh? What are you attempting to do here? >>> >> >> It is for matching "#pragma pack(push, 1)" which is above all related >> struct/unions in this header file. > > Please don't use 'pragma pack' or bitfields. If you need to pack > and unpack things from a target-CPU defined field use bit operations > and/or extract32/deposit32/extract64/deposit64. > OK, thanks, although it means I have to rewrite most of code, and test them again. I shall try to send patch v2 within the next week. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
[Qemu-devel] [PATCH 3/4] target-tilegx: Implement fpu fdouble floating point
>From c467db1c0a5f4c6560b8b2115732aa718c4b Mon Sep 17 00:00:00 2001 From: Chen Gang <gang.chen.5...@gmail.com> Date: Sun, 8 Nov 2015 09:10:17 +0800 Subject: [PATCH 3/4] target-tilegx: Implement fpu fdouble floating point instructions Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> --- target-tilegx/fdouble_helper.c | 290 + 1 file changed, 290 insertions(+) create mode 100644 target-tilegx/fdouble_helper.c diff --git a/target-tilegx/fdouble_helper.c b/target-tilegx/fdouble_helper.c new file mode 100644 index 000..7ba0583 --- /dev/null +++ b/target-tilegx/fdouble_helper.c @@ -0,0 +1,290 @@ +/* + * QEMU TILE-Gx helpers + * + * Copyright (c) 2015 Chen Gang + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see + * <http://www.gnu.org/licenses/lgpl-2.1.html> + */ + +#include "cpu.h" +#include "qemu-common.h" +#include "exec/helper-proto.h" +#include "fpu/softfloat.h" + +#include "fpu.h" + +#define TILEGX_F_MAN_HBIT (1ULL << 59) + +#pragma pack(push, 1) +typedef union F64Fmt { + float64 d; + struct { +#if defined(HOST_WORDS_BIGENDIAN) + uint64_t sign : 1; + uint64_t exp : 11; + uint64_t frac : 52; +#else + uint64_t frac : 52; + uint64_t exp : 11; + uint64_t sign : 1; +#endif + } bits; +} F64Fmt; +#pragma pack(pop) + +static uint64_t fr_to_man(F64Fmt v) +{ + uint64_t val = (uint64_t)v.bits.frac << 7; + + if (v.bits.exp) + val |= TILEGX_F_MAN_HBIT; + + return val; +} + +uint64_t helper_fdouble_unpack_min(CPUTLGState *env, + uint64_t srca, uint64_t srcb) +{ + F64Fmt va, vb; + TileGXFPDFmtV v; + + va.d = make_float64(srca); + vb.d = make_float64(srcb); + v.ll = 0; /* also cause v.fmt.overflow = 0 */ + + if (float64_is_any_nan(srca) || float64_is_any_nan(srcb) + || float64_is_infinity(srca) || float64_is_infinity(srcb)) + ; + else if (va.bits.exp> vb.bits.exp) { + if (va.bits.exp - vb.bits.exp < 64) + v.fmt.mantissa = fr_to_man(vb)>> (va.bits.exp - vb.bits.exp); + } else if (va.bits.exp < vb.bits.exp) { + if (vb.bits.exp - va.bits.exp < 64) + v.fmt.mantissa = fr_to_man(va)>> (vb.bits.exp - va.bits.exp); + } else if (va.bits.frac> vb.bits.frac) + v.fmt.mantissa = fr_to_man(vb); + else + v.fmt.mantissa = fr_to_man(va); + + return v.ll; +} + +uint64_t helper_fdouble_unpack_max(CPUTLGState *env, + uint64_t srca, uint64_t srcb) +{ + F64Fmt va, vb; + TileGXFPDFmtV v; + + va.d = make_float64(srca); + vb.d = make_float64(srcb); + v.ll = 0; /* also cause v.fmt.overflow = 0 */ + + if (float64_is_any_nan(srca) || float64_is_any_nan(srcb) + || float64_is_infinity(srca) || float64_is_infinity(srcb)) + ; + else if (va.bits.exp> vb.bits.exp) + v.fmt.mantissa = fr_to_man(va); + else if (va.bits.exp < vb.bits.exp) + v.fmt.mantissa = fr_to_man(vb); + else if (va.bits.frac> vb.bits.frac) + v.fmt.mantissa = fr_to_man(va); + else + v.fmt.mantissa = fr_to_man(vb); + + return v.ll; +} + +uint64_t helper_fdouble_addsub(CPUTLGState *env, + uint64_t dest, uint64_t srca, uint64_t srcb) +{ + TileGXFPDFmtF flags; + TileGXFPDFmtV v; + + flags.ll = srcb; + if (flags.fmt.calc == TILEGX_F_CALC_ADD) { + v.ll = dest + srca; /* maybe set addsub overflow bit */ + } else + v.ll = dest - srca; + + return v.ll; +} + +/* absolute-add/mul may cause add/mul carry or overflow */ +static bool proc_oflow(TileGXFPDFmtF *flags, TileGXFPDFmtV *v, uint64_t *srcb) +{ + if (v->fmt.overflow) { + flags->fmt.vexp++; + *srcb>>= 1; + *srcb |= (uint64_t)v->ll << 63; + v->ll>>= 1; + v->fmt.overflow = 0; + } + return flags->fmt.vexp> TILEGX_F_EXP_DMAX; +} + +uint64_t helper_fdouble_pack2(CPUTLGState *env, + uint64_t dest, uint64_t srca, uint64_t srcb) +{ + TileGXFPDFmtF flags; + TileGXFPDFmtV v; + F64Fmt d; + + flags.ll = dest; + v.ll = srca; + + d.d = 0; + d.bits.sign = flags.fmt.sign; + + /* + * fdouble
[Qemu-devel] [PATCH 4/4] target-tilegx: Let fpu implementation code can be built and used
>From 8fab455a5ac5508d06cc69f778e926ad098fbe5b Mon Sep 17 00:00:00 2001 From: Chen Gang <gang.chen.5...@gmail.com> Date: Sun, 8 Nov 2015 09:11:36 +0800 Subject: [PATCH 4/4] target-tilegx: Let fpu implementation code can be built and used It passes gcc testsuite: it can get the same result as the original fpu implementation have done. Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> --- target-tilegx/Makefile.objs | 3 +- target-tilegx/cpu.h | 2 ++ target-tilegx/helper.h | 12 target-tilegx/translate.c | 68 +++-- 4 files changed, 75 insertions(+), 10 deletions(-) diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs index 0db778f..c2cf2f1 100644 --- a/target-tilegx/Makefile.objs +++ b/target-tilegx/Makefile.objs @@ -1 +1,2 @@ -obj-y += cpu.o translate.o helper.o simd_helper.o +obj-y += cpu.o translate.o helper.o simd_helper.o \ + fsingle_helper.o fdouble_helper.o diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h index 03df107..445a606 100644 --- a/target-tilegx/cpu.h +++ b/target-tilegx/cpu.h @@ -88,6 +88,8 @@ typedef struct CPUTLGState { uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside */ uint64_t pc; /* Current pc */ + float_status fp_status; /* floating point status */ + #if defined(CONFIG_USER_ONLY) uint64_t excaddr; /* exception address */ uint64_t atomic_srca; /* Arguments to atomic "exceptions" */ diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h index 9281d0f..b785bf2 100644 --- a/target-tilegx/helper.h +++ b/target-tilegx/helper.h @@ -24,3 +24,15 @@ DEF_HELPER_FLAGS_2(v1shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64) DEF_HELPER_FLAGS_2(v2shl, TCG_CALL_NO_RWG_SE, i64, i64, i64) DEF_HELPER_FLAGS_2(v2shru, TCG_CALL_NO_RWG_SE, i64, i64, i64) DEF_HELPER_FLAGS_2(v2shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64) + +DEF_HELPER_3(fsingle_add1, i64, env, i64, i64) +DEF_HELPER_3(fsingle_sub1, i64, env, i64, i64) +DEF_HELPER_3(fsingle_mul1, i64, env, i64, i64) +DEF_HELPER_2(fsingle_pack2, i64, env, i64) +DEF_HELPER_3(fdouble_unpack_min, i64, env, i64, i64) +DEF_HELPER_3(fdouble_unpack_max, i64, env, i64, i64) +DEF_HELPER_3(fdouble_add_flags, i64, env, i64, i64) +DEF_HELPER_3(fdouble_sub_flags, i64, env, i64, i64) +DEF_HELPER_4(fdouble_addsub, i64, env, i64, i64, i64) +DEF_HELPER_3(fdouble_mul_flags, i64, env, i64, i64) +DEF_HELPER_4(fdouble_pack2, i64, env, i64, i64, i64) diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c index b8ca401..4d74b1d 100644 --- a/target-tilegx/translate.c +++ b/target-tilegx/translate.c @@ -597,6 +597,11 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext, } qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s", mnemonic, reg_names[srca]); return ret; + + case OE_RR_X0(FSINGLE_PACK1): + case OE_RR_Y0(FSINGLE_PACK1): + mnemonic = "fsingle_pack1"; + goto done2; } tdest = dest_gr(dc, dest); @@ -613,9 +618,6 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext, gen_helper_cnttz(tdest, tsrca); mnemonic = "cnttz"; break; - case OE_RR_X0(FSINGLE_PACK1): - case OE_RR_Y0(FSINGLE_PACK1): - return TILEGX_EXCP_OPCODE_UNIMPLEMENTED; case OE_RR_X1(LD1S): memop = MO_SB; mnemonic = "ld1s"; /* prefetch_l1_fault */ @@ -734,6 +736,7 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext, return TILEGX_EXCP_OPCODE_UNKNOWN; } +done2: qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s, %s", mnemonic, reg_names[dest], reg_names[srca]); return ret; @@ -742,13 +745,21 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext, static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext, unsigned dest, unsigned srca, unsigned srcb) { - TCGv tdest = dest_gr(dc, dest); - TCGv tsrca = load_gr(dc, srca); - TCGv tsrcb = load_gr(dc, srcb); + TCGv tdest, tsrca, tsrcb; TCGv t0; const char *mnemonic; switch (opext) { + case OE_RRR(FSINGLE_ADDSUB2, 0, X0): + mnemonic = "fsingle_addsub2"; + goto done2; + } + + tdest = dest_gr(dc, dest); + tsrca = load_gr(dc, srca); + tsrcb = load_gr(dc, srcb); + + switch (opext) { case OE_RRR(ADDXSC, 0, X0): case OE_RRR(ADDXSC, 0, X1): gen_saturate_op(tdest, tsrca, tsrcb, tcg_gen_add_tl); @@ -906,14 +917,39 @@ static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext, mnemonic = "exch"; break; case OE_RRR(FDOUBLE_ADDSUB, 0, X0): + gen_helper_fdouble_addsub(tdest, cpu_env, + load_gr(dc, dest),tsrca, tsrcb); + mnemonic = "fdouble_addsub"; + break;
[Qemu-devel] [PATCH 1/4] target-tilegx: Add fpu header file
>From 91a3d7d591cac6c4a39d4dbc5c3ffe8c17b10b6a Mon Sep 17 00:00:00 2001 From: Chen Gang <gang.chen.5...@gmail.com> Date: Sun, 8 Nov 2015 09:05:29 +0800 Subject: [PATCH 1/4] target-tilegx: Add fpu header file It defines the main data structures for tilegx fpu. Also it provides the summary description for each floating point instructions impmementation. Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> --- target-tilegx/fpu.h | 228 1 file changed, 228 insertions(+) create mode 100644 target-tilegx/fpu.h diff --git a/target-tilegx/fpu.h b/target-tilegx/fpu.h new file mode 100644 index 000..adfa1f5 --- /dev/null +++ b/target-tilegx/fpu.h @@ -0,0 +1,228 @@ +/* + * TILE-Gx virtual FPU header + * + * Copyright (c) 2015 Chen Gang + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see <http://www.gnu.org/licenses/>. + */ +#ifndef FPU_TILEGX_H +#define FPU_TILEGX_H + +/* + * From IEEE standard, exp of float is 8-bits, exp of double is 11-bits. + */ +#define TILEGX_F_EXP_FZERO 0x7f /* Zero exp for single 8-bits */ +#define TILEGX_F_EXP_FMAX 0xfe /* max exp for single 8-bits */ +#define TILEGX_F_EXP_DZERO 0x3ff /* Zero exp for double 11-bits */ +#define TILEGX_F_EXP_DMAX 0x7fe /* max exp for double 11-bits */ +#define TILEGX_F_EXP_DUF 0x1000/* underflow exp bit for double */ + +/* + * For fdouble absolute cacluation flag + */ +#define TILEGX_F_CALC_CVT 0 /* Perform int to fsingle/fdouble */ +#define TILEGX_F_CALC_ADD 1 /* Perform absolute add operation */ +#define TILEGX_F_CALC_SUB 2 /* Perform absolute sub operation */ +#define TILEGX_F_CALC_MUL 3 /* Perform absolute mul operation */ + +#pragma pack(push, 1) + +/* + * Single format, it is 64-bit. + * + * Single exp analyzing: 0x9e - 0x1e(30) = 0x80 + * + * 7 6 5 4 3 2 1 0 + * + * 1 0 0 1 1 1 1 0 + * + * 0 0 0 1 1 1 1 1 => 0x1f(31) + * + * 0 1 1 1 1 1 1 1 => 0x7f + */ +typedef struct TileGXFPSFmt { + +#if !defined(HOST_WORDS_BIGENDIAN) + /* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md */ + uint64_t exp : 8; /* exp, 0x9e: 31 + TILEGX_F_EXP_FZERO */ + uint64_t uiknown0 : 2; /* unknown */ + uint64_t sign : 1; /* Sign bit for the total value */ + uint64_t calc: 2; /* calculation flag */ + uint64_t unknown1 : 12; /* unknown */ + + /* Come from TILE-Gx ISA document, Table 7-2 for floating point */ + uint64_t unordered : 1; /* The two are unordered */ + uint64_t lt : 1; /* 1st is less than 2nd */ + uint64_t le : 1; /* 1st is less than or equal to 2nd */ + uint64_t gt : 1; /* 1st is greater than 2nd */ + uint64_t ge : 1; /* 1st is greater than or equal to 2nd */ + uint64_t eq : 1; /* The two operands are equal */ + uint64_t neq : 1; /* The two operands are not equal */ + + /* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md */ + uint64_t mantissa : 32; /* mantissa */ +#else + uint64_t mantissa : 32; /* mantissa */ + uint64_t neq : 1; /* The two operands are not equal */ + uint64_t eq : 1; /* The two operands are equal */ + uint64_t ge : 1; /* 1st is greater than or equal to 2nd */ + uint64_t gt : 1; /* 1st is greater than 2nd */ + uint64_t le : 1; /* 1st is less than or equal to 2nd */ + uint64_t lt : 1; /* 1st is less than 2nd */ + uint64_t unordered : 1; /* The two are unordered */ + uint64_t unknown1 : 12; /* unknown */ + uint64_t calc: 2; /* calculation flag */ + uint64_t sign : 1; /* Sign bit for the total value */ + uint64_t unknown0 : 2; /* unknown */ + uint64_t exp : 8; /* exp, 0x9e: 31 + TILEGX_F_EXP_FZERO */ +#endif +} TileGXFPSFmt; +/* + * FSingle instructions implemenation: + * + * fsingle_add1 ; calc srca and srcb, + * ; convert float_32 to TileGXFPSFmt result. + * ; move TileGXFPSFmt result to dest. + * + * fsingle_sub1 ; calc srca and srcb. + * ; convert float_32 to TileGXFPSFmt result. + * ; move
[Qemu-devel] [PATCH 2/4] target-tilegx: Implement fpu single floating point
>From 3270485ebd56429f6f62f0a4f967009d8a1b14d6 Mon Sep 17 00:00:00 2001 From: Chen Gang <gang.chen.5...@gmail.com> Date: Sun, 8 Nov 2015 09:08:40 +0800 Subject: [PATCH 2/4] target-tilegx: Implement fpu single floating point instructions Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> --- target-tilegx/fsingle_helper.c | 168 + 1 file changed, 168 insertions(+) create mode 100644 target-tilegx/fsingle_helper.c diff --git a/target-tilegx/fsingle_helper.c b/target-tilegx/fsingle_helper.c new file mode 100644 index 000..f8f6b69 --- /dev/null +++ b/target-tilegx/fsingle_helper.c @@ -0,0 +1,168 @@ +/* + * QEMU TILE-Gx helpers + * + * Copyright (c) 2015 Chen Gang + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see + * <http://www.gnu.org/licenses/lgpl-2.1.html> + */ + +#include "cpu.h" +#include "qemu-common.h" +#include "exec/helper-proto.h" +#include "fpu/softfloat.h" + +#include "fpu.h" + +#pragma pack(push, 1) +typedef union F32Fmt { + uint32_t f; + struct { +#if defined(HOST_WORDS_BIGENDIAN) + uint32_t sign : 1; + uint32_t exp : 8; + uint32_t frac : 23; +#else + uint32_t frac : 23; + uint32_t exp : 8; + uint32_t sign : 1; +#endif + } bits; +} F32Fmt; +#pragma pack(pop) + +static uint64_t sfmt_to_uint64(TileGXFPSFmt a) +{ + union { + TileGXFPSFmt a; + uint64_t v; + } t; + t.a = a; + return t.v; +} + +static TileGXFPSFmt uint64_to_sfmt(uint64 v) +{ + union { + TileGXFPSFmt a; + uint64_t v; + } t; + t.v = v; + return t.a; +} + +static TileGXFPSFmt float32_to_sfmt(float32 f) +{ + F32Fmt tmp = {f}; + union { + TileGXFPSFmt fmt; + uint64_t v; + } sfmt; + + sfmt.v = 0; + sfmt.fmt.sign = tmp.bits.sign; + sfmt.fmt.exp = tmp.bits.exp; + sfmt.fmt.mantissa = (tmp.bits.frac << 8) | (1 << 31); + + return sfmt.fmt; +} + +static float32 sfmt_to_float32(TileGXFPSFmt sfmt, float_status *fp_status) +{ + F32Fmt f; + + if (sfmt.calc == TILEGX_F_CALC_CVT) { + if (sfmt.sign) + f.f = int32_to_float32(0 - sfmt.mantissa, fp_status); + else + f.f = uint32_to_float32(sfmt.mantissa, fp_status); + } else { + f.bits.sign = sfmt.sign; + f.bits.exp = sfmt.exp; + f.bits.frac = sfmt.mantissa>> 8; + } + + return f.f; +} + +uint64_t helper_fsingle_pack2(CPUTLGState *env, uint64_t srca) +{ + return float32_val(sfmt_to_float32(uint64_to_sfmt(srca), >fp_status)); +} + +static void ana_bits(float_status *fp_status, + float32 fsrca, float32 fsrcb, TileGXFPSFmt *sfmt) +{ + if (float32_eq(fsrca, fsrcb, fp_status)) { + sfmt->eq = 1; + } else { + sfmt->neq = 1; + } + + if (float32_lt(fsrca, fsrcb, fp_status)) { + sfmt->lt = 1; + } + if (float32_le(fsrca, fsrcb, fp_status)) { + sfmt->le = 1; + } + + if (float32_lt(fsrcb, fsrca, fp_status)) { + sfmt->gt = 1; + } + if (float32_le(fsrcb, fsrca, fp_status)) { + sfmt->ge = 1; + } + + if (float32_unordered(fsrca, fsrcb, fp_status)) { + sfmt->unordered = 1; + } +} + +static uint64_t main_calc(float_status *fp_status, + float32 fsrca, float32 fsrcb, + float32 (*calc)(float32, float32, float_status *)) +{ + TileGXFPSFmt sfmt = float32_to_sfmt(calc(fsrca, fsrcb, fp_status)); + + ana_bits(fp_status, fsrca, fsrcb, ); + + if (calc == float32_add) + sfmt.calc = TILEGX_F_CALC_ADD; + else if (calc == float32_sub) + sfmt.calc = TILEGX_F_CALC_SUB; + else + sfmt.calc = TILEGX_F_CALC_MUL; + + sfmt_to_float32(sfmt, fp_status); + + return sfmt_to_uint64(sfmt); +} + +uint64_t helper_fsingle_add1(CPUTLGState *env, uint64_t srca, uint64_t srcb) +{ + return main_calc(>fp_status, + make_float32(srca), make_float32(srcb), float32_add); +} + +uint64_t helper_fsingle_sub1(CPUTLGState *env, uint64_t srca, uint64_t srcb) +{ + return main_calc(>fp_status, + make_float32(srca), make_float32(srcb), float32_sub); +} + +uint64_t helper_fsingle_mul1(CPUTLGState *env, uint64_t srca, uint64_t srcb) +{ +
[Qemu-devel] [PATCH 0/4] Implment fpu floating point instructions
>From 8fab455a5ac5508d06cc69f778e926ad098fbe5b Mon Sep 17 00:00:00 2001 From: Chen Gang <gang.chen.5...@gmail.com> Date: Sun, 8 Nov 2015 09:28:16 +0800 Subject: [PATCH 0/4] Implment fpu floating point instructions It passes gcc testsuite: get the same result like the original floating point implementation has done (original implementation is temporary). Chen Gang (4): target-tilegx: Add fpu header file target-tilegx: Implement fpu single floating point instructions target-tilegx: Implement fpu fdouble floating point instructions target-tilegx: Let fpu implementation code can be built and used target-tilegx/Makefile.objs | 3 +- target-tilegx/cpu.h | 2 + target-tilegx/fdouble_helper.c | 290 + target-tilegx/fpu.h | 228 target-tilegx/fsingle_helper.c | 168 target-tilegx/helper.h | 12 ++ target-tilegx/translate.c | 68 -- 7 files changed, 761 insertions(+), 10 deletions(-) create mode 100644 target-tilegx/fdouble_helper.c create mode 100644 target-tilegx/fpu.h create mode 100644 target-tilegx/fsingle_helper.c -- 1.9.3
Re: [Qemu-devel] [PATCH] target-tilegx: Implement floating point instructions
Sorry, based on this patch, after fix 5 issues, it still has issues. The issues which I have fixed are: - Fix floating point fdouble multiply related instructions issues - Process every overflow cases. - Process absolute multiply carrying just like absolute add has done. - Process the nearest skipped bit carrying for fdouble. - Process the nearest skipped bit carrying for fsingle. I shall try to fix the left issues all within this week (2015-11-08). Thanks. On 11/1/15 12:30, Chen Gang wrote: > > Oh, sorry, it can not pass gcc testsuite: the fdouble mul insns have > issues (original temporary implementation had no this cases -- it > skipped the outside mul operation). > > After it passes gcc testtsuite, I shall split it into several small > patches, and send patch v2 (I shall try to finish today). > > Thanks. > > > On 11/1/15 00:59, Chen Gang wrote: >> From 42733d085bfcb4882cfa4eb25a9387e3d953a64f Mon Sep 17 00:00:00 2001 >> From: Chen Gang <gang.chen.5...@gmail.com> >> Date: Sun, 1 Nov 2015 00:50:33 +0800 >> Subject: [PATCH] target-tilegx: Implement floating point instructions >> >> It is implenented in a normal way, and passed unit tests (8 test cases). >> >> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >> --- >> target-tilegx/Makefile.objs | 3 +- >> target-tilegx/cpu.h | 2 + >> target-tilegx/fdouble_helper.c | 234 >> + >> target-tilegx/fpu.h | 217 ++ >> target-tilegx/fsingle_helper.c | 157 +++ >> target-tilegx/helper.h | 12 +++ >> target-tilegx/translate.c | 68 ++-- >> 7 files changed, 683 insertions(+), 10 deletions(-) >> create mode 100644 target-tilegx/fdouble_helper.c >> create mode 100644 target-tilegx/fpu.h >> create mode 100644 target-tilegx/fsingle_helper.c >> >> diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs >> index 0db778f..c2cf2f1 100644 >> --- a/target-tilegx/Makefile.objs >> +++ b/target-tilegx/Makefile.objs >> @@ -1 +1,2 @@ >> -obj-y += cpu.o translate.o helper.o simd_helper.o >> +obj-y += cpu.o translate.o helper.o simd_helper.o \ >> + fsingle_helper.o fdouble_helper.o >> diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h >> index 03df107..445a606 100644 >> --- a/target-tilegx/cpu.h >> +++ b/target-tilegx/cpu.h >> @@ -88,6 +88,8 @@ typedef struct CPUTLGState { >> uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside */ >> uint64_t pc; /* Current pc */ >> >> + float_status fp_status; /* floating point status */ >> + >> #if defined(CONFIG_USER_ONLY) >> uint64_t excaddr; /* exception address */ >> uint64_t atomic_srca; /* Arguments to atomic "exceptions" */ >> diff --git a/target-tilegx/fdouble_helper.c b/target-tilegx/fdouble_helper.c >> new file mode 100644 >> index 000..b3b0588 >> --- /dev/null >> +++ b/target-tilegx/fdouble_helper.c >> @@ -0,0 +1,234 @@ >> +/* >> + * QEMU TILE-Gx helpers >> + * >> + * Copyright (c) 2015 Chen Gang >> + * >> + * This library is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU Lesser General Public >> + * License as published by the Free Software Foundation; either >> + * version 2.1 of the License, or (at your option) any later version. >> + * >> + * This library is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + * Lesser General Public License for more details. >> + * >> + * You should have received a copy of the GNU Lesser General Public >> + * License along with this library; if not, see >> + * <http://www.gnu.org/licenses/lgpl-2.1.html> >> + */ >> + >> +#include "cpu.h" >> +#include "qemu-common.h" >> +#include "exec/helper-proto.h" >> +#include "fpu/softfloat.h" >> + >> +#include "fpu.h" >> + >> +#define TILEGX_F_MAN_HBIT (1ULL << 59) >> + >> +#pragma pack(push, 1) >> +typedef union F64Fmt { >> + float64 d; >> + struct { >> +#if defined(HOST_WORDS_BIGENDIAN) >> + uint64_t sign : 1; >> + uint64_t exp : 11; >> + uint64_t frac : 52; >> +#else >> + uint64_t frac : 52; >> + uint64_t exp : 11; >> + uint64_t sign : 1; >> +#endif >> + } bits; >> +} F64Fmt; >> +#pragma pack(pop) >> + >> +static uint64_t fr_t
Re: [Qemu-devel] [PATCH] target-tilegx: Implement floating point instructions
Oh, sorry, it can not pass gcc testsuite: the fdouble mul insns have issues (original temporary implementation had no this cases -- it skipped the outside mul operation). After it passes gcc testtsuite, I shall split it into several small patches, and send patch v2 (I shall try to finish today). Thanks. On 11/1/15 00:59, Chen Gang wrote: > From 42733d085bfcb4882cfa4eb25a9387e3d953a64f Mon Sep 17 00:00:00 2001 > From: Chen Gang <gang.chen.5...@gmail.com> > Date: Sun, 1 Nov 2015 00:50:33 +0800 > Subject: [PATCH] target-tilegx: Implement floating point instructions > > It is implenented in a normal way, and passed unit tests (8 test cases). > > Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> > --- > target-tilegx/Makefile.objs| 3 +- > target-tilegx/cpu.h| 2 + > target-tilegx/fdouble_helper.c | 234 > + > target-tilegx/fpu.h| 217 ++ > target-tilegx/fsingle_helper.c | 157 +++ > target-tilegx/helper.h | 12 +++ > target-tilegx/translate.c | 68 ++-- > 7 files changed, 683 insertions(+), 10 deletions(-) > create mode 100644 target-tilegx/fdouble_helper.c > create mode 100644 target-tilegx/fpu.h > create mode 100644 target-tilegx/fsingle_helper.c > > diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs > index 0db778f..c2cf2f1 100644 > --- a/target-tilegx/Makefile.objs > +++ b/target-tilegx/Makefile.objs > @@ -1 +1,2 @@ > -obj-y += cpu.o translate.o helper.o simd_helper.o > +obj-y += cpu.o translate.o helper.o simd_helper.o \ > + fsingle_helper.o fdouble_helper.o > diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h > index 03df107..445a606 100644 > --- a/target-tilegx/cpu.h > +++ b/target-tilegx/cpu.h > @@ -88,6 +88,8 @@ typedef struct CPUTLGState { > uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside > */ > uint64_t pc; /* Current pc */ > > +float_status fp_status;/* floating point status */ > + > #if defined(CONFIG_USER_ONLY) > uint64_t excaddr; /* exception address */ > uint64_t atomic_srca; /* Arguments to atomic "exceptions" */ > diff --git a/target-tilegx/fdouble_helper.c b/target-tilegx/fdouble_helper.c > new file mode 100644 > index 000..b3b0588 > --- /dev/null > +++ b/target-tilegx/fdouble_helper.c > @@ -0,0 +1,234 @@ > +/* > + * QEMU TILE-Gx helpers > + * > + * Copyright (c) 2015 Chen Gang > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see > + * <http://www.gnu.org/licenses/lgpl-2.1.html> > + */ > + > +#include "cpu.h" > +#include "qemu-common.h" > +#include "exec/helper-proto.h" > +#include "fpu/softfloat.h" > + > +#include "fpu.h" > + > +#define TILEGX_F_MAN_HBIT(1ULL << 59) > + > +#pragma pack(push, 1) > +typedef union F64Fmt { > +float64 d; > +struct { > +#if defined(HOST_WORDS_BIGENDIAN) > +uint64_t sign : 1; > +uint64_t exp : 11; > +uint64_t frac : 52; > +#else > +uint64_t frac : 52; > +uint64_t exp : 11; > +uint64_t sign : 1; > +#endif > +} bits; > +} F64Fmt; > +#pragma pack(pop) > + > +static uint64_t fr_to_man(F64Fmt v) > +{ > +uint64_t val = (uint64_t)v.bits.frac << 7; > + > +if (v.bits.exp) > +val |= TILEGX_F_MAN_HBIT; > + > +return val; > +} > + > +uint64_t helper_fdouble_unpack_min(CPUTLGState *env, > + uint64_t srca, uint64_t srcb) > +{ > +F64Fmt va, vb; > +TileGXFPDFmtV v; > + > +va.d = make_float64(srca); > +vb.d = make_float64(srcb); > +v.ll = 0; /* also cause v.fmt.overflow = 0 */ > + > +if (va.bits.exp> vb.bits.exp) > +v.fmt.mantissa = fr_to_man(vb)>> (va.bits.exp - vb.bits.exp); > +else if (va.bits.exp < vb.bits.exp) > +
Re: [Qemu-devel] [PULL] tilegx queued patch
On 10/31/15 04:44, Richard Henderson wrote: > Just one mergable patch since the last update. > I guess this will be it before hard-freeze. > Thank you for your work. I have finished coding for the implementation of tilegx floating point instructions, at present, I am just testing and fixing the related issues. Hope I can finish today and send patches for it (although I am not quite sure). Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
[Qemu-devel] [PATCH] target-tilegx: Implement floating point instructions
>From 42733d085bfcb4882cfa4eb25a9387e3d953a64f Mon Sep 17 00:00:00 2001 From: Chen Gang <gang.chen.5...@gmail.com> Date: Sun, 1 Nov 2015 00:50:33 +0800 Subject: [PATCH] target-tilegx: Implement floating point instructions It is implenented in a normal way, and passed unit tests (8 test cases). Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> --- target-tilegx/Makefile.objs | 3 +- target-tilegx/cpu.h | 2 + target-tilegx/fdouble_helper.c | 234 + target-tilegx/fpu.h | 217 ++ target-tilegx/fsingle_helper.c | 157 +++ target-tilegx/helper.h | 12 +++ target-tilegx/translate.c | 68 ++-- 7 files changed, 683 insertions(+), 10 deletions(-) create mode 100644 target-tilegx/fdouble_helper.c create mode 100644 target-tilegx/fpu.h create mode 100644 target-tilegx/fsingle_helper.c diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs index 0db778f..c2cf2f1 100644 --- a/target-tilegx/Makefile.objs +++ b/target-tilegx/Makefile.objs @@ -1 +1,2 @@ -obj-y += cpu.o translate.o helper.o simd_helper.o +obj-y += cpu.o translate.o helper.o simd_helper.o \ + fsingle_helper.o fdouble_helper.o diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h index 03df107..445a606 100644 --- a/target-tilegx/cpu.h +++ b/target-tilegx/cpu.h @@ -88,6 +88,8 @@ typedef struct CPUTLGState { uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside */ uint64_t pc; /* Current pc */ + float_status fp_status; /* floating point status */ + #if defined(CONFIG_USER_ONLY) uint64_t excaddr; /* exception address */ uint64_t atomic_srca; /* Arguments to atomic "exceptions" */ diff --git a/target-tilegx/fdouble_helper.c b/target-tilegx/fdouble_helper.c new file mode 100644 index 000..b3b0588 --- /dev/null +++ b/target-tilegx/fdouble_helper.c @@ -0,0 +1,234 @@ +/* + * QEMU TILE-Gx helpers + * + * Copyright (c) 2015 Chen Gang + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see + * <http://www.gnu.org/licenses/lgpl-2.1.html> + */ + +#include "cpu.h" +#include "qemu-common.h" +#include "exec/helper-proto.h" +#include "fpu/softfloat.h" + +#include "fpu.h" + +#define TILEGX_F_MAN_HBIT (1ULL << 59) + +#pragma pack(push, 1) +typedef union F64Fmt { + float64 d; + struct { +#if defined(HOST_WORDS_BIGENDIAN) + uint64_t sign : 1; + uint64_t exp : 11; + uint64_t frac : 52; +#else + uint64_t frac : 52; + uint64_t exp : 11; + uint64_t sign : 1; +#endif + } bits; +} F64Fmt; +#pragma pack(pop) + +static uint64_t fr_to_man(F64Fmt v) +{ + uint64_t val = (uint64_t)v.bits.frac << 7; + + if (v.bits.exp) + val |= TILEGX_F_MAN_HBIT; + + return val; +} + +uint64_t helper_fdouble_unpack_min(CPUTLGState *env, + uint64_t srca, uint64_t srcb) +{ + F64Fmt va, vb; + TileGXFPDFmtV v; + + va.d = make_float64(srca); + vb.d = make_float64(srcb); + v.ll = 0; /* also cause v.fmt.overflow = 0 */ + + if (va.bits.exp> vb.bits.exp) + v.fmt.mantissa = fr_to_man(vb)>> (va.bits.exp - vb.bits.exp); + else if (va.bits.exp < vb.bits.exp) + v.fmt.mantissa = fr_to_man(va)>> (vb.bits.exp - va.bits.exp); + else if (va.bits.frac> vb.bits.frac) + v.fmt.mantissa = fr_to_man(vb); + else + v.fmt.mantissa = fr_to_man(va); + + return v.ll; +} + +uint64_t helper_fdouble_unpack_max(CPUTLGState *env, + uint64_t srca, uint64_t srcb) +{ + F64Fmt va, vb; + TileGXFPDFmtV v; + + va.d = make_float64(srca); + vb.d = make_float64(srcb); + v.ll = 0; /* also cause v.fmt.overflow = 0 */ + + if (va.bits.exp> vb.bits.exp) + v.fmt.mantissa = fr_to_man(va); + else if (va.bits.exp < vb.bits.exp) + v.fmt.mantissa = fr_to_man(vb); + else if (va.bits.frac> vb.bits.frac) + v.fmt.mantissa = fr_to_man(va); + else + v.fmt.mantissa = fr_to_man(vb); + + return v.ll; +} + +uint64_t helper_fdouble_addsub(CPUTLGState *env, + uint64_t dest, uint64_
Re: [Qemu-devel] [Consult] tilegx: About floating point instructions
gs ; srca and srcb are float_64 value. * ; calc exp (exp_min), sign, and comp bits for flags. * ; set addsub bit to flags and move flags to dest. * * fdouble_sub_flags ; srca and srcb are float_64 value. * ; calc exp (exp_min), sign, and comp bits for flags. * ; set addsub bit to flags and move flags to dest. * * fdouble_addsub: ; dest, srca (max, min mantissa), and srcb (flags). * ; "dest +/- srca" depend on the add/sub bit of flags. * ; move result mantissa to dest. * * fdouble_mul_flags: ; srca and srcb are float_64 value. * ; calc sign (xor), exp (exp_min + exp_max), and comp bits. * ; mix sign, exp, and comp bits as flags to dest. * * fdouble_pack1 ; move srcb (flags) to dest. * * fdouble_pack2 ; srca, srcb (high, low mantissa), and dest (flags) * ; normalize and pack result from srca, srcb, and dest. * ; move result to dest. */ #pragma pack(pop) Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [Consult] tilegx: About floating point instructions
I guess, for sign number, the highest bit will not be used, but for unsigned number, the highest bit will be used (then can let sign and unsigned number can use the same format contents). On 10/25/15 23:38, Chen Gang wrote: > > /* > * Single format, it is 64-bit. > */ > typedef struct TileGXFPSFmt { > > /* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md */ > uint64_t exp : 8; /* exp, 0x9e: 30 + TILEGX_F_EXP_FZERO */ > uint64_t unknown0 : 1;/* unknown */ > uint64_t sign : 1;/* Sign bit for the total value */ > uint64_t unknown1 : 15; /* unknown */ > > /* Come from TILE-Gx ISA document, Table 7-2 for floating point */ > uint64_t unordered : 1; /* The two are unordered */ > uint64_t lt : 1; /* 1st is less than 2nd */ > uint64_t le : 1; /* 1st is less than or equal to 2nd */ > uint64_t gt : 1; /* 1st is greater than 2nd */ > uint64_t ge : 1; /* 1st is greater than or equal to 2nd */ > uint64_t eq : 1; /* The two operands are equal */ > uint64_t neq : 1; /* The two operands are not equal */ > > /* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md */ > uint64_t mantissa : 31; /* mantissa */ > uint64_t unknown2 : 1;/* unknown */ It is not unknown2, it means 0x8000 when sign is 0, and meaningless when sign is 1. > } TileGXFPSFmt; > [...] > > /* > * Dobule format, value, 64-bit. > */ > typedef struct TileGXFPDFmtV { > uint64_t unknown0 : 4;/* unknown */ > uint64_t mantissa : 55; /* mantissa */ > uint64_t unknown1 : 5;/* unknown */ I guess, unknow1 is 4 bits, and the other 1 bit is for (1ULL << 55) when sign is 0, and meaningless when sign is 1. > } TileGXFPDFmtV; > > #pragma pack(pop) > > Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [Consult] tilegx: About floating point instructions
On 8/18/15 01:31, Richard Henderson wrote: > > For single-precision it appears that the format is > > 63 31 24 10 9 0 > [ mantissa with implicit and guard bits | cmp flags | ?? | s | exp ] > > We are able to deduce the bias for the exponent based on the input gcc gives > us > for floatunssisf: 0x9e == 2**31 when the mantissa is normalized. > For me, 0x9e == 2**30: - For int32_t: 31-bits for values, highest bit for sign. Mantissa can 'move' 30 bits to express 31-bits values, and 31st bit will never 'move'. - 0x9e - 0x1e(30) = 0x80, which is the smallest 8-bits number, in our case, 0x80 means: "do not 'move' mantissa bits". - According to IEEE standard, the exp of float type is 8-bits, so 0x80 is really the smallest value of exp. /* * Single exp analyzing: 0x9e - 0x1e(30) = 0x80 * * 7 6 5 4 3 2 1 0 * * 1 0 0 1 1 1 1 0 * * 0 0 0 1 1 1 1 0=> 0x1e(30) * * 1 0 0 0 0 0 0 0=> 0x80 */ [...] > > For double-precision things are more complicated. Precisely because there is > no dedicated fdouble_mul[1-4] instructions, but instead gcc is to use a normal > 128-bit integer multiplication on the mantissa. > > For double-precision it appears that the format is > > 63 57 40 > unpack [ overflow bits? | mantissa with implicit bit | guard bits ] > > 63 31 24 20 1980 > flags [ ?? | cmp flags | ?? | s | exp | ?? ] > > Similarly we can compute the bias for exp as 0x21b == 2**53. > Or is it 20 bits of exponent and 0x21b00 == 2**53? > For me, exp is (0x21b << 1): - According to IEEE standard for double, it's exp is 11 bits, the smallest exp is 0x400. - So for 0x21b00, we can only consider about bits between 0 and 17 (the highest bit must be 1). - And it is 11 bits, so the real value of 0x21b00 is (0x21b << 1). Then we know it 'move' 0x36(54) bits to get integer, then we know fdouble mantissa have 55-bits internally. /* * Double exp analyzing: (0x21b << 1) - 0x36(54) = 0x400 * * 17 16 15 14 13 12 11 10 9 8 76 5 4 3 2 1 0 * *1 0 0 0 0 1 1 0 1 1 00 0 0 0 0 0 0 * *0 0 0 0 0 1 1 0 1 1 0=> 0x36(54) * *1 0 0 0 0 0 0 0 0 0 0=> 0x400 * */ So for me, the format in C header file is: #pragma pack(push, 1) /* * Single format, it is 64-bit. */ typedef struct TileGXFPSFmt { /* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md */ uint64_t exp : 8; /* exp, 0x9e: 30 + TILEGX_F_EXP_FZERO */ uint64_t unknown0 : 1;/* unknown */ uint64_t sign : 1;/* Sign bit for the total value */ uint64_t unknown1 : 15; /* unknown */ /* Come from TILE-Gx ISA document, Table 7-2 for floating point */ uint64_t unordered : 1; /* The two are unordered */ uint64_t lt : 1; /* 1st is less than 2nd */ uint64_t le : 1; /* 1st is less than or equal to 2nd */ uint64_t gt : 1; /* 1st is greater than 2nd */ uint64_t ge : 1; /* 1st is greater than or equal to 2nd */ uint64_t eq : 1; /* The two operands are equal */ uint64_t neq : 1; /* The two operands are not equal */ /* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md */ uint64_t mantissa : 31; /* mantissa */ uint64_t unknown2 : 1;/* unknown */ } TileGXFPSFmt; /* * Dobule format, flag, 64-bit. */ typedef struct TileGXFPDFmtF { uint64_t unknown0 : 7;/* unknown */ uint64_t exp : 11;/* exp, 0x21b << 1: 54 + TILEGX_F_EXP_DZERO */ uint64_t unknown1 : 2;/* unknown */ uint64_t sign : 1;/* Sign bit for the total value */ uint64_t unknown2: 4; /* unknown */ /* Come from TILE-Gx ISA document, Table 7-2 for floating point */ uint64_t unordered : 1; /* The two are unordered */ uint64_t lt : 1; /* 1st is less than 2nd */ uint64_t le : 1; /* 1st is less than or equal to 2nd */ uint64_t gt : 1; /* 1st is greater than 2nd */ uint64_t ge : 1; /* 1st is greater than or equal to 2nd */ uint64_t eq : 1; /* The two operands are equal */ uint64_t neq : 1; /* The two operands are not equal */ uint64_t unknown3 : 32; /* unknown */ } TileGXFPDFmtF; /* * Dobule format, value, 64-bit. */ typedef struct TileGXFPDFmtV { uint64_t unknown0 : 4;/* unknown */ uint64_t mantissa : 55; /* mantissa */ uint64_t unknown1 : 5;/* unknown */ } TileGXFPDFmtV; #p