| Message ID | 20200722062902.24509-6-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | candidate fixes for 5.1-rc1 (testing, semihosting, OOM tcg, x86 fpu) | expand |
On 7/22/20 8:28 AM, Alex Bennée wrote: > It seems GetPhysicallyInstalledSystemMemory isn't available in the > MinGW headers so we have to declare it ourselves. Compile tested only. > > Cc: Stefan Weil <sw@weilnetz.de> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > util/oslib-win32.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > index 31030463cc9..f0f94833197 100644 > --- a/util/oslib-win32.c > +++ b/util/oslib-win32.c > @@ -43,6 +43,8 @@ > /* this must come after including "trace.h" */ > #include <shlobj.h> > > +WINBASEAPI BOOL WINAPI GetPhysicallyInstalledSystemMemory (PULONGLONG); > + > void *qemu_oom_check(void *ptr) > { > if (ptr == NULL) { > @@ -831,6 +833,15 @@ char *qemu_get_host_name(Error **errp) > > size_t qemu_get_host_physmem(void) > { > - /* currently unimplemented */ > - return 0; > + ULONGLONG mem; > + > + if (GetPhysicallyInstalledSystemMemory(&mem)) { > + if (mem > SIZE_MAX) { > + return SIZE_MAX; > + } else { > + return mem; > + } > + } else { > + return 0; > + } > } >
Am 22.07.20 um 08:28 schrieb Alex Bennée: > It seems GetPhysicallyInstalledSystemMemory isn't available in the > MinGW headers so we have to declare it ourselves. Compile tested only. It is available, at least for Mingw-w64 which I also use for cross builds on Debian, but is only included with _WIN32_WINNT >= 0x0601. Currently we set _WIN32_WINNT to 0x0600. Regards, Stefan
On Wed, Jul 22, 2020 at 12:03:27PM +0200, Stefan Weil wrote: > Am 22.07.20 um 08:28 schrieb Alex Bennée: > > > It seems GetPhysicallyInstalledSystemMemory isn't available in the > > MinGW headers so we have to declare it ourselves. Compile tested only. > > > It is available, at least for Mingw-w64 which I also use for cross > builds on Debian, but is only included with _WIN32_WINNT >= 0x0601. This would be equiv to requiring Windows 7 or newer > Currently we set _WIN32_WINNT to 0x0600. This is equiv to Windows 6 / Vista / Server 2008 So if we blindly declare GetPhysicallyInstalledSystemMemory ourselves, then we're likely going to fail at runtime when QEMU is used on Windows prior to Windows 7. Regards, Daniel
I would suggest use loadLibrary to retrieve the function, if not available, then return 0 (For Win Xp and Vista); On Wed, Jul 22, 2020 at 2:34 PM Alex Bennée <alex.bennee@linaro.org> wrote: > It seems GetPhysicallyInstalledSystemMemory isn't available in the > MinGW headers so we have to declare it ourselves. Compile tested only. > > Cc: Stefan Weil <sw@weilnetz.de> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > util/oslib-win32.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > index 31030463cc9..f0f94833197 100644 > --- a/util/oslib-win32.c > +++ b/util/oslib-win32.c > @@ -43,6 +43,8 @@ > /* this must come after including "trace.h" */ > #include <shlobj.h> > > +WINBASEAPI BOOL WINAPI GetPhysicallyInstalledSystemMemory (PULONGLONG); > + > void *qemu_oom_check(void *ptr) > { > if (ptr == NULL) { > @@ -831,6 +833,15 @@ char *qemu_get_host_name(Error **errp) > > size_t qemu_get_host_physmem(void) > { > - /* currently unimplemented */ > - return 0; > + ULONGLONG mem; > + > + if (GetPhysicallyInstalledSystemMemory(&mem)) { > + if (mem > SIZE_MAX) { > + return SIZE_MAX; > + } else { > + return mem; > + } > + } else { > + return 0; > + } > } > -- > 2.20.1 > > >
Am 22.07.20 um 12:32 schrieb 罗勇刚(Yonggang Luo): > I would suggest use loadLibrary to retrieve the function, if not > available, then return 0 (For Win Xp and Vista); Maybe using GlobalMemoryStatusEx is a better alternative. It is available since XP. Regards, Stefan ||
Stefan Weil <sw@weilnetz.de> writes: > Am 22.07.20 um 12:32 schrieb 罗勇刚(Yonggang Luo): > >> I would suggest use loadLibrary to retrieve the function, if not >> available, then return 0 (For Win Xp and Vista); > > > Maybe using GlobalMemoryStatusEx is a better alternative. It is > available since XP. I would welcome an alternative patch. I have no way to test if it works anyway.
Stefan Weil <sw@weilnetz.de> writes: > Am 22.07.20 um 08:28 schrieb Alex Bennée: > >> It seems GetPhysicallyInstalledSystemMemory isn't available in the >> MinGW headers so we have to declare it ourselves. Compile tested only. > > > It is available, at least for Mingw-w64 which I also use for cross > builds on Debian, but is only included with _WIN32_WINNT >= 0x0601. > > Currently we set _WIN32_WINNT to 0x0600. That would explain why some people see things working if they build with visual studio (which I presume has a higher setting). We could just wrap the body of the function in: #if (_WIN32_WINNT >= 0x0601) much like in commands-win32.c? Of course it wouldn't even be compile tested (I used the fedora docker image). We should probably clean up the test-mingw code to work with both the fedora and debian-w[32|64] images. > > Regards, > > Stefan
On Wed, Jul 22, 2020 at 12:33:47PM +0100, Alex Bennée wrote: > > Stefan Weil <sw@weilnetz.de> writes: > > > Am 22.07.20 um 08:28 schrieb Alex Bennée: > > > >> It seems GetPhysicallyInstalledSystemMemory isn't available in the > >> MinGW headers so we have to declare it ourselves. Compile tested only. > > > > > > It is available, at least for Mingw-w64 which I also use for cross > > builds on Debian, but is only included with _WIN32_WINNT >= 0x0601. > > > > Currently we set _WIN32_WINNT to 0x0600. > > That would explain why some people see things working if they build with > visual studio (which I presume has a higher setting). We could just wrap > the body of the function in: No, that's not how it works. We define _WIN32_WINNT in qemu/osdep.h, and this causes the Windows headers to hide any functions that post-date that version. This is similar to how you might set _POSIX_C_SOURCE / _XOPEN_SOURCE to control UNIX header visibility. IOW, the use of visual studio shouldn't affect it. > #if (_WIN32_WINNT >= 0x0601) > > much like in commands-win32.c? > > Of course it wouldn't even be compile tested (I used the fedora docker > image). We should probably clean up the test-mingw code to work with > both the fedora and debian-w[32|64] images. I'd just go for GlobalMemoryStatusEx like Stefan suggests. We use this in libvirt and GNULIB already and it does the job. Regards, Daniel
On Wed, Jul 22, 2020 at 12:31:06PM +0100, Alex Bennée wrote: > > Stefan Weil <sw@weilnetz.de> writes: > > > Am 22.07.20 um 12:32 schrieb 罗勇刚(Yonggang Luo): > > > >> I would suggest use loadLibrary to retrieve the function, if not > >> available, then return 0 (For Win Xp and Vista); > > > > > > Maybe using GlobalMemoryStatusEx is a better alternative. It is > > available since XP. > > I would welcome an alternative patch. I have no way to test if it works > anyway. Something like this should work: LPMEMORYSTATUSEX info = { .dwLength = sizeof(LPMEMORYSTATUSEX), }; if (!GlobalMemoryStatusEx(&info)) { ...error report... } return info.ullTotalPhys; (or ullAvailPhys for current free memory) Regards, Daniel
diff --git a/util/oslib-win32.c b/util/oslib-win32.c index 31030463cc9..f0f94833197 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -43,6 +43,8 @@ /* this must come after including "trace.h" */ #include <shlobj.h> +WINBASEAPI BOOL WINAPI GetPhysicallyInstalledSystemMemory (PULONGLONG); + void *qemu_oom_check(void *ptr) { if (ptr == NULL) { @@ -831,6 +833,15 @@ char *qemu_get_host_name(Error **errp) size_t qemu_get_host_physmem(void) { - /* currently unimplemented */ - return 0; + ULONGLONG mem; + + if (GetPhysicallyInstalledSystemMemory(&mem)) { + if (mem > SIZE_MAX) { + return SIZE_MAX; + } else { + return mem; + } + } else { + return 0; + } }
It seems GetPhysicallyInstalledSystemMemory isn't available in the MinGW headers so we have to declare it ourselves. Compile tested only. Cc: Stefan Weil <sw@weilnetz.de> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- util/oslib-win32.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)