Re: [Qemu-devel] [PATCH v2 1/3] compiler: define QEMU_CACHELINE_SIZE

[Geert Martin Ijewski]

Am 06.06.2017 um 19:39 schrieb Richard Henderson:
> On 06/06/2017 09:11 AM, Emilio G. Cota wrote:
> > On Tue, Jun 06, 2017 at 01:39:45 -0400, Pranith Kumar wrote:
> > > On Mon, Jun 5, 2017 at 6:49 PM, Emilio G. Cota <address@hidden> wrote:
> > > > This is a constant used as a hint for padding structs to hopefully 
> > > > avoid
> > > > false cache line sharing.
> > > >
> > > > The constant can be set at configure time by defining 
> > > > QEMU_CACHELINE_SIZE
> > > > via --extra-cflags. If not set there, we try to obtain the value from
> > > > the machine running the configure script. If we fail, we default to
> > > > reasonable values, i.e. 128 bytes for ppc64 and 64 bytes for all 
> > > > others.
> > (snip)
> > > Is there any reason not to use sysconf(_SC_LEVEL1_DCACHE_LINESIZE)?
> >
> > I tried using sysconf, but it doesn't work on the PowerPC machine I have
> > access to (it returns 0). It might be a machine-specific thing though-I
> > don't know. Here's the machine's `uname -a':
> >    Linux gcc2-power8.osuosl.org 3.10.0-514.10.2.el7.ppc64le #1 SMP Fri 
> > Mar \
> >      3 16:16:38 GMT 2017 ppc64le ppc64le ppc64le GNU/Linux
>
> Well that's unfortunate.
>
> Doing some digging, the kernel has provided the info to userland via elf 
> auxv data since the beginning of time (aka initial git repository 
> build), but glibc still does not export that information properly for ppc.
>
> For ppc, you can get what we want from qemu_getauxval(AT_ICACHEBSIZE).  
> Indeed, we already have 4 different system dependent methods for 
> determining the icache size in tcg/ppc/tcg-target.inc.c.
>
> So what I think we ought to do is create a new util/cachesize.c like so:
>
> unsigned qemu_icache_linesize = 64;
> unsigned qemu_dcache_linesize = 64;
>
> static void init_icache_data(void)
> {
> #ifdef _SC_LEVEL1_ICACHE_LINESIZE
>      {
>          long x = sysconf(_SC_LEVEL1_ICACHE_LINESIZE);
>          if (x > 0) {
>              qemu_icache_linesize = x;
>              return;
>          }
>      }
> #endif
> #ifdef AT_ICACHEBSIZE
>      {
>          unsigned long x = qemu_getauxval(AT_ICACHEBSIZE);
>          if (x > 0) {
>              qemu_icache_linesize = x;
>              return;
>          }
>      }
> #endif
>      // Other system specific methods.

On a fully patched Windows 10 with an i5-4690 this code works for me (TM):

#ifdef _WIN32
    {
        DWORD bufferSize = 0;
        if (!GetLogicalProcessorInformation(0, &bufferSize) &&
                GetLastError() == ERROR_INSUFFICIENT_BUFFER)
        {
            PSYSTEM_LOGICAL_PROCESSOR_INFORMATION buffer =

(PSYSTEM_LOGICAL_PROCESSOR_INFORMATION)g_malloc0(bufferSize);
            if (GetLogicalProcessorInformation(buffer, &bufferSize)) {
                size_t i = 0,
                    numOfProcessors =
                        bufferSize /
                        sizeof(SYSTEM_LOGICAL_PROCESSOR_INFORMATION);
                for (; i < numOfProcessors; i++) {
                    if (buffer[i].Relationship == RelationCache &&
                        buffer[i].Cache.Level == 1 &&
                        (  buffer[i].Cache.Type == CacheUnified ||
                           buffer[i].Cache.Type == CacheInstruction)
                        )
                    {
                        qemu_icache_linesize = buffer[i].Cache.LineSize;
                        break;
                    }
                }
            }
            g_free(buffer);
        }
    }
#endif

I don't particularly like that stair of ifs style, so I guess if I were 
to do a proper patch this should become a function.
> }
>
> static void init_dcache_data(void)
> {
>      // Similarly.

The code from above, just s/CacheInstruction/CacheData/ and 
s/qemu_icache/qemu_dcache/
> }
>
> static void __attribute__((constructor)) init_cache_data(void)
> {
>      init_icache_data();
>      init_dcache_data();
> }
>
> In particular, I think you want to be padding to the icache linesize 
> rather than the dcache linesize since what we're attempting is to avoid 
> writable data in the icache.
>
>
> r~

To quote from the documentation:
"RelationCache: [... snip ...]
Windows Server 2003:  This value is not supported until Windows Server 
2003 with SP1 and Windows XP Professional x64 Edition." --
https://msdn.microsoft.com/en-us/library/windows/desktop/ms686694(v=vs.85).aspx

I'm not sure if that is considered a problem, as both systems aren't 
supported anymore for almost 2 years now.

Geert