Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-04-10 Thread Kornél Pál

On 4/10/2012 4:48 AM, Vitaliy Margolen wrote:

On 04/09/2012 12:14 PM, Kornél Pál wrote:

I think this would work. (But don't know whether it was accepted.)

No, it won't work. Wine itself (ntdll  kernel32) accesses it.


In my opinion it still would work, just would not be optimal.

Note that I also have poined out the following:

 Also note that wine is currently reading that page internally (the same
 page contains system32 path for example). To make this efficient wine
 should not use that page at all.

Kornel




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-04-09 Thread Francois Gouget
On Thu, 22 Mar 2012, Kornél Pál wrote:
[...]
 A bit better approach was to mark that page PAGE_GUARD. Then wine could get an
 indication that it needs to be updated. Frequent accesses were not impacted
 because PAGE_GUARD could be reset by an APC some time later.

Why reset PAGE_GUARD?
Would the following work ?
 * By default wineserver (or a separate process) does not update the
   shared page. This way there's no overhead.
 * The page is marked as PAGE_GUARD. When a process accesses it, the 
   user code makes an RPC to the wineserver (or the separate process) to 
   tell it to start updating the page and removes the PAGE_GARD flag.
 * The application can now access the page and gets smoothly updated 
   timestamps.
 * The wineserver (or the separate process) keeps track of which 
   processes need its services. When the last of them exits, it stops 
   updating the shared page so there's no overhead anymore.

-- 
Francois Gouget fgou...@free.fr  http://fgouget.free.fr/
  The last time religion ruled, it was called the dark ages.


Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-04-09 Thread Kornél Pál

On 4/9/2012 7:46 PM, Francois Gouget wrote:

On Thu, 22 Mar 2012, Kornél Pál wrote:
[...]

A bit better approach was to mark that page PAGE_GUARD. Then wine could get an
indication that it needs to be updated. Frequent accesses were not impacted
because PAGE_GUARD could be reset by an APC some time later.


Why reset PAGE_GUARD?


What I've descibed is using no shared memory (no wineserver involved) 
and the page is updated only on demand rather than periodically (no 
timer needed).


PAGE_GUARD has to be reset because the flag is cleared when it is 
accessed. (It is better suited for the use case you have described 
below.) See http://msdn.microsoft.com/en-us/library/aa366549.aspx



Would the following work ?
  * By default wineserver (or a separate process) does not update the
shared page. This way there's no overhead.
  * The page is marked as PAGE_GUARD. When a process accesses it, the
user code makes an RPC to the wineserver (or the separate process) to
tell it to start updating the page and removes the PAGE_GARD flag.
  * The application can now access the page and gets smoothly updated
timestamps.
  * The wineserver (or the separate process) keeps track of which
processes need its services. When the last of them exits, it stops
updating the shared page so there's no overhead anymore.



I think this would work. (But don't know whether it was accepted.)

You also should map the shared memory to that predefined address in 
addition to updating it from wineserver.


Also note that wine is currently reading that page internally (the same 
page contains system32 path for example). To make this efficient wine 
should not use that page at all.


Kornel




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-04-09 Thread Vitaliy Margolen

On 04/09/2012 12:14 PM, Kornél Pál wrote:

I think this would work. (But don't know whether it was accepted.)

No, it won't work. Wine itself (ntdll  kernel32) accesses it.

Vitaliy.




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-04-04 Thread Joey Yandle
 
 Writable shared memory is unacceptable, but as long as it's read-only
 from the client there's no problem in principle.
 

Excellent, thanks Alexandre.

I'm attaching my current shared memory diff.  It uses shm_open + mmap in
wineserver to map the shared memory area read/write, and shm_open +
NtCreateSection + NtMapViewOfSection in wine to map read only.  A thread
is run in wineserver which does a clock_nanosleep to set the times
without drift.

The wineserver code appears to work, and successfully writes to shared
memory.  The wine code fails in NtMapViewOfSection with error C018,
which is STATUS_CONFLICTING_ADDRESSES.  Can someone who understands
NtMapViewOfSection look at my code and tell me if I've done something
stupid?  I thought there might be code buried in NtMapViewOfSection
which tries to reserve that code block, but I didn't see it when I looked.

Some notes:

I didn't use CreateFileMapping/MapViewOfFile because that lives in
kernel32.dll, so I pulled the code in and called
NtCreateSection/NtMapViewOfSection directly.

I also had to pull out all of the ntdll code which writes to
USER_SHARED_DATA, and move that to wineserver.

I had to link ntdll and wineserver against -lrt; this should of course
be done more clealy via configure.

Finally, the mmap call in wineserver only succeeds if I pass in a
candidate address; if I pass in NULL, it fails with 'Operation not
permitted'.

cheers,

Joey
diff --git a/dlls/ntdll/Makefile.in b/dlls/ntdll/Makefile.in
index 0047731..80d7f5a 100644
--- a/dlls/ntdll/Makefile.in
+++ b/dlls/ntdll/Makefile.in
@@ -2,7 +2,7 @@ EXTRADEFS = -D_NTSYSTEM_
 MODULE= ntdll.dll
 IMPORTLIB = ntdll
 IMPORTS   = winecrt0
-EXTRALIBS = @IOKITLIB@ @LIBPTHREAD@
+EXTRALIBS = @IOKITLIB@ @LIBPTHREAD@ -lrt
 EXTRADLLFLAGS = -nodefaultlibs -Wl,--image-base,0x7bc0
 
 C_SRCS = \
diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c
index 381e2ac..e6112eb 100644
--- a/dlls/ntdll/loader.c
+++ b/dlls/ntdll/loader.c
@@ -2812,7 +2812,7 @@ void CDECL __wine_init_windows_dir( const WCHAR *windir, const WCHAR *sysdir )
 PLIST_ENTRY mark, entry;
 LPWSTR buffer, p;
 
-strcpyW( user_shared_data-NtSystemRoot, windir );
+/* strcpyW( user_shared_data-NtSystemRoot, windir ); */
 DIR_init_windows_dir( windir, sysdir );
 
 /* prepend the system dir to the name of the already created modules */
diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c
index 3ed8038..dea32ee 100644
--- a/dlls/ntdll/nt.c
+++ b/dlls/ntdll/nt.c
@@ -1038,6 +1038,7 @@ void fill_cpu_info(void)
 }
 if (!strcasecmp(line, flags) || !strcasecmp(line, features))
 {
+#if 0
 if (strstr(value, cx8))
 user_shared_data-ProcessorFeatures[PF_COMPARE_EXCHANGE_DOUBLE] = TRUE;
 if (strstr(value, cx16))
@@ -1062,6 +1063,7 @@ void fill_cpu_info(void)
 user_shared_data-ProcessorFeatures[PF_PAE_ENABLED] = TRUE;
 if (strstr(value, ht))
 cached_sci.FeatureSet |= CPU_FEATURE_HTT;
+#endif
 continue;
 }
 	}
diff --git a/dlls/ntdll/thread.c b/dlls/ntdll/thread.c
index e328c5f..eb926d2 100644
--- a/dlls/ntdll/thread.c
+++ b/dlls/ntdll/thread.c
@@ -23,6 +23,8 @@
 
 #include assert.h
 #include stdarg.h
+#include stdint.h
+#include errno.h
 #include sys/types.h
 #ifdef HAVE_SYS_MMAN_H
 #include sys/mman.h
@@ -183,6 +185,98 @@ done:
 return status;
 }
 
+static void map_user_shared_data(void) 
+{
+int shmfd;
+void *addr;
+
+FIXME(open KUSER_SHARED_DATA\n);
+shmfd = shm_open(/KUSER_SHARED_DATA, O_RDONLY | O_CREAT, 0600);
+
+FIXME(shmfd %d\n, shmfd);
+
+if(shmfd != -1) {
+HANDLE fdh = NULL, cfmh = NULL;
+NTSTATUS mstatus = STATUS_SUCCESS, cstatus = STATUS_SUCCESS;
+LARGE_INTEGER offset;
+ULONG protect = PAGE_READONLY;
+//ULONG protect = PAGE_READWRITE;
+SIZE_T count;
+
+if(0)
+{
+addr = (void *)0x7ffe;
+count = 0x1;
+addr = mmap(addr, count, PROT_READ, MAP_FILE|MAP_SHARED|MAP_FIXED, shmfd, 0);
+
+if(addr == MAP_FAILED) {
+FIXME(mmap(addr, SZ, PROT_READ, MAP_FILE|MAP_SHARED|MAP_FIXED, fd, 0) failed: %s\n, strerror(errno));
+return;
+}
+
+FIXME(mapped USER_SHARED_DATA to %p\n, addr);
+user_shared_data = addr;
+return;
+}
+
+FIXME(calling wine_server_fd_to_handle( shmfd, GENERIC_READ|SYNCHRONIZE, OBJ_INHERIT, fdh )\n);
+
+wine_server_fd_to_handle( shmfd, GENERIC_READ|SYNCHRONIZE, OBJ_INHERIT, fdh );
+
+FIXME(fdh %p\n, fdh);
+{
+static const int sec_flags = SEC_FILE | SEC_IMAGE | SEC_RESERVE | SEC_COMMIT | SEC_NOCACHE;
+
+DWORD access, sec_type;
+LARGE_INTEGER size;
+
+sec_type = protect  sec_flags;
+protect = ~sec_flags;
+  

Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-04-03 Thread Alexandre Julliard
Dmitry Timoshkov dmi...@baikal.ru writes:

 Joey Yandle dra...@dancingdragon.be wrote:

  Why do you need to update the data in wineserver and not in the client?
 
 The problem is that the timer updates need to be extremely precise, or
 they are worse than useless.  So we can either do that in every wine
 process, or do it once in wineserver and share the memory.
 
 On Windows, the kernel shares this memory with all applications for this
 exact reason.

 It's been continously said that any memory sharing scheme between a client
 and wineserver won't be accepeted, ever.

Writable shared memory is unacceptable, but as long as it's read-only
from the client there's no problem in principle.

-- 
Alexandre Julliard
julli...@winehq.org




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-04-02 Thread Joey Yandle
 
 I know they declined an in-process wineserver due to difficulties
 blaming the right component on a crash: is it the app or wine that's faulty?
 
 I found no other relevant discussion on wine-devel since January 2010,
 when searching for first 'share' and then 'server'.
 

If anyone does know how Alexandre et alia feel about sharing data
between wine and wineserver, please let me know.  It seems the closest
to what Windows does, in the context of wine.  And as long as the
mapping is read-only from wine, then it seems unlikely to cause too many
difficulties.

 Too bad if a fork is needed, couldn't the sharing be configurable?
 

It definitely could, and I would prefer to avoid a fork if at all
possible.  We could set a wine registry key and only start the
thread/memory map if it is present.

If this is an acceptable alternative, then I can submit patches which do
this.

cheers,

Joey




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-04-02 Thread Dmitry Timoshkov
Joey Yandle dra...@dancingdragon.be wrote:

 If anyone does know how Alexandre et alia feel about sharing data
 between wine and wineserver, please let me know.  It seems the closest
 to what Windows does, in the context of wine.  And as long as the
 mapping is read-only from wine, then it seems unlikely to cause too many
 difficulties.

Why do you need to update the data in wineserver and not in the client?
Consider each wineserver call just an RPC, and leave it alone, server
doesn't run anything in the background, and can't magically update timers
behind the client's back.

-- 
Dmitry.




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-04-02 Thread Joey Yandle
 Why do you need to update the data in wineserver and not in the client?

The problem is that the timer updates need to be extremely precise, or
they are worse than useless.  So we can either do that in every wine
process, or do it once in wineserver and share the memory.

On Windows, the kernel shares this memory with all applications for this
exact reason.

 Consider each wineserver call just an RPC, and leave it alone, server
 doesn't run anything in the background, and can't magically update timers
 behind the client's back.

I certainly agree that wineserver doesn't do this currently, but the
Windows kernel does, and there's no reason why wineserver can't.





Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-04-02 Thread Dmitry Timoshkov
Joey Yandle dra...@dancingdragon.be wrote:

  Why do you need to update the data in wineserver and not in the client?
 
 The problem is that the timer updates need to be extremely precise, or
 they are worse than useless.  So we can either do that in every wine
 process, or do it once in wineserver and share the memory.
 
 On Windows, the kernel shares this memory with all applications for this
 exact reason.

It's been continously said that any memory sharing scheme between a client
and wineserver won't be accepeted, ever.

  Consider each wineserver call just an RPC, and leave it alone, server
  doesn't run anything in the background, and can't magically update timers
  behind the client's back.
 
 I certainly agree that wineserver doesn't do this currently, but the
 Windows kernel does, and there's no reason why wineserver can't.

You need to make wineserver multithreaded then, and that idea has been
shot down as well.

-- 
Dmitry.




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-04-02 Thread Joey Yandle
 It's been continously said that any memory sharing scheme between a client
 and wineserver won't be accepeted, ever.

You're the second person to make that assertion without pointing out the
relevant discussion.  Such assertions are not helpful.

 You need to make wineserver multithreaded then, and that idea has been
 shot down as well.

Yes, either wine or wineserver will need a thread to set these values
with the required precision.

The current question is whether turning on such functionality via a
registry setting is acceptable, since it is clearly not okay for the
general case.




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-04-02 Thread Ken Thomases
On Apr 2, 2012, at 10:01 PM, Joey Yandle wrote:

 You need to make wineserver multithreaded then, and that idea has been
 shot down as well.
 
 Yes, either wine or wineserver will need a thread to set these values
 with the required precision.

Can you use a separate process -- a Win32 service -- to update this shared 
memory?

Regards,
Ken





Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-04-02 Thread Joey Yandle
 
 Can you use a separate process -- a Win32 service -- to update this shared 
 memory?
 

Something like plugplay.exe?  I don't see why not.  I'll look over the
plugplay.exe code and see if it looks promising.

cheers,

Joey




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-29 Thread Johan Gill
On Thu, Mar 22, 2012 at 5:19 AM, Joey Yandle dra...@dancingdragon.bewrote:

 
  It's been explained several times already that any approach to share data
  between wineserver and clients is going to be rejected.
 

 Can you point me to such an explanation?  I joined wine-devel just
 before posting my RFC.  If this has been discussed previously, I'd
 prefer to get the context before wasting my and others' time.


I know they declined an in-process wineserver due to difficulties blaming
the right component on a crash: is it the app or wine that's faulty?

I found no other relevant discussion on wine-devel since January 2010, when
searching for first 'share' and then 'server'.

Too bad if a fork is needed, couldn't the sharing be configurable?



Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-24 Thread Kornél Pál

On 3/22/2012 2:19 PM, Vitaliy Margolen wrote:

BTW one more thing, this change will most likely break number of copy
protection systems. Such as safedisk. They use user shared date times to
estimate time it took for some kernel operations. And some of those time
intervals are really tight.


I'm not sure that preventig measuring time is a good practice.

Since there are a plenty of ways to measure elapsed time, I don't think 
that this specific way should generally be prohibited.


Thank you.

Kornel




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-24 Thread Vitaliy Margolen

On 03/24/2012 08:56 AM, Kornél Pál wrote:

On 3/22/2012 2:19 PM, Vitaliy Margolen wrote:
Since there are a plenty of ways to measure elapsed time, I don't think that
this specific way should generally be prohibited.


I'm not saying it should be prohibited. I'm saying it fixes only one app and 
potentially breaks 100s more...


Vitaliy




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-24 Thread Joey Yandle
 
 BTW one more thing, this change will most likely break number of copy
 protection systems. Such as safedisk. They use user shared date times to
 estimate time it took for some kernel operations. And some of those time
 intervals are really tight.
 

I'm extremely confused by this statement.  Currently, some the
USER_SHARED_DATA values are written at process start, and some are not
written at all.  How can external code possibly rely on Wine's current
broken behavior?  Any code written for windows expects these values to
update every 15.6 ms.

If anything, I would expect updating these values to fix copy protection
schemes, not break them.




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-24 Thread Vitaliy Margolen

On 03/24/2012 01:09 PM, Joey Yandle wrote:

Any code written for windows expects these values to
update every 15.6 ms.
Exactly. Wine's wineserver  fake kernel in form of ntoskernl can not work 
with native speed by definition.


For example even earliest versions of safedisk (1.5) were really picky about 
how long some kernel calls take. This is to protect against debugger. When 
numbers don't increment (as happens now) protection is happy. When they 
start to increment, even on fast PCs round trip 
user-space-wineserver-ntoskrnl will take way longer then it should.


Of course this is all based on research I did 5 years ago. But surprisingly 
enough those old Safedisk versions still work, assuming you have supported 
gcc version, and few other requirements.


Vitaliy




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-24 Thread Joey Yandle
 When numbers don't increment (as happens now) protection is
 happy. When they start to increment, even on fast PCs round trip
 user-space-wineserver-ntoskrnl will take way longer then it should.
 

Indeed, that pathway will never be fast enough. I'm surprised it works
with no incrementing, but perhaps that's for backwards compatibility
with previous Windows versions.

So, to summarize:

1. Can't use a separate per-process thread because it will break
applications.

2. Can't used shared memory segment between wineserver and wine
processes because it violates design principles of wineserver.

3. Can't use TimerQueue/APC because too much jitter in callback times.

Is this a fair appraisal of the position of the Wine devs?  If so, I'll
fork a TOR-capable branch so others can play.  I'll start with my patch
for case #1, since it is small and works well for TOR.  If I can get #2
working properly, I'll switch to that, since it is more technically
correct, though as a larger patch it will be more difficult to maintain.

cheers,

Joey




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-22 Thread Kornél Pál

On 3/22/2012 6:00 AM, Dmitry Timoshkov wrote:

Joey Yandledra...@dancingdragon.be  wrote:
It's been explained several times already that any approach to share data
between wineserver and clients is going to be rejected.


I think this is the reason that timers in shared_user_data are not being 
updated. I also think that updating timers in every client process in 
every 15.6 ms is a lot of overhead.


Another approach could be no to allocate that address and provide up to 
date data in an signal/exception handler. This is a more complex 
approach and not sure whether possible in user mode. And of course would 
defeat the purpose using shared memory for high performance.


Thanks.

Kornel




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-22 Thread Kornél Pál

On 3/22/2012 7:43 AM, Kornél Pál wrote:

On 3/22/2012 6:00 AM, Dmitry Timoshkov wrote:

Joey Yandledra...@dancingdragon.be wrote:
It's been explained several times already that any approach to share data
between wineserver and clients is going to be rejected.


I think this is the reason that timers in shared_user_data are not being
updated. I also think that updating timers in every client process in
every 15.6 ms is a lot of overhead.

Another approach could be no to allocate that address and provide up to
date data in an signal/exception handler. This is a more complex
approach and not sure whether possible in user mode. And of course would
defeat the purpose using shared memory for high performance.


A bit better approach was to mark that page PAGE_GUARD. Then wine could 
get an indication that it needs to be updated. Frequent accesses were 
not impacted because PAGE_GUARD could be reset by an APC some time later.


wine in this case should not use this structure for performance reasons.

I'm not sure however that this would solve Joey's problem since he 
mentioned that APCs were not accurate enough.


Thanks.

Kornel




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-22 Thread Vitaliy Margolen

On 03/21/2012 10:19 PM, Joey Yandle wrote:


It's been explained several times already that any approach to share data
between wineserver and clients is going to be rejected.



Can you point me to such an explanation?  I joined wine-devel just
before posting my RFC.  If this has been discussed previously, I'd
prefer to get the context before wasting my and others' time.


BTW one more thing, this change will most likely break number of copy 
protection systems. Such as safedisk. They use user shared date times to 
estimate time it took for some kernel operations. And some of those time 
intervals are really tight.


-Vitaliy




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-21 Thread Marcus Meissner
On Tue, Mar 20, 2012 at 06:40:37PM -0700, Joey Yandle wrote:
 Hi everybody,
 
 As originally discovered by Carsten Juttner, Star Wars: The Old Republic
 uses the time values in KUSER_SHARED_DATA to trigger network send
 buffers.  wine does not currently update these values, so TOR does not
 make it past an initial server handshake.
 
 I'm attaching my current patch which fixes this.  It updates the values
 every 1560 nanoseconds as does Windows 7, and presumably all Windows
 versions.  The words of the timers are written in the correct order for
 applications to detect clock tick.
 
 It uses a separate thread which does nothing else; a previous attempt at
 an NtTimer based solution did not work, presumably due to irregularity
 in the exact callback times.  Attempts at setting thread priority were
 also abandoned after many people were unable to run the code.
 
 The patch does not check the return code of nanosleep; I could add code
 to nanosleep again for the remaining time in the case of an interrupt,
 but didn't think it was necessary.  I could also change it to use
 clock_nanosleep, which would allow for more absolute timing.
 
 Please let me know what you think.

- pthread_create() is pretty much a no-go.

  Use Win32 apis instead (CreateThread() and WaitFor*)

- The shared data structure should be modified with atomic
  accesses only. (Or use a criticalsection if possible.)


However, I do not know if we can have additional threads at all in a
Win32 process without confusing the win32 processes, or if this needs
to be solved differently (APC?).

Ciao, Marcus




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-21 Thread Joey Yandle
 
 - The shared data structure should be modified with atomic
   accesses only. (Or use a criticalsection if possible.)
 

According the this website, Windows updates the times too frequently to
use a critical section:


http://www.dcl.hpi.uni-potsdam.de/research/WRK/2007/08/getting-os-information-the-kuser_shared_data-structure/

The values are written in the order High2, Low, High1.  Applications
read them in the reverse order; as long as the two High values are the
same, the composite value is considered correct.  This is a fairly
common algorithm for detecting clock tick without atomic operations or
critical sections.

I could certainly use atomic operations for each of the 32-bit word
writes.  But since Windows apps expect the word write order, I don't
think it's necessary or advisable to do 64-bit atomic ops.

 
 However, I do not know if we can have additional threads at all in a
 Win32 process without confusing the win32 processes, or if this needs
 to be solved differently (APC?).
 

A previous version of my patch used NtTimer/APC:

  http://bugs.winehq.org/attachment.cgi?id=39308

The APC code is commented out but still present in that version of the
patch.

It did not work, by which I mean SW:TOR did not successfully login.  It
was clearly the best solution, so I'd be happy to return to it if we can
make the callback times more precise.

cheers,

Joey




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-21 Thread Joey Yandle
 However, I do not know if we can have additional threads at all in a
 Win32 process without confusing the win32 processes, or if this needs
 to be solved differently (APC?).

 
 A previous version of my patch used NtTimer/APC:
 

I'm attaching a cleaned up version of my APC code.  It does get called
regularly:

000f:fixme:thread:update_shared_data_time 129768277137647820
0041:fixme:win:GetWindowPlacement not supported on other process window
0x90076
0036:fixme:d3d:query_init Unhandled query type 0xc.
000f:fixme:thread:update_shared_data_time 129768277137814980

Though the interval is not exactly 15.600 ms, as it is with a separate
thread and nanosleep.

And most importantly, it does not allow SW:TOR to make it to the
character selection screen.

Could I have initialized the timer with different values to make it more
precise?

thanks,

Joey
diff --git a/dlls/ntdll/thread.c b/dlls/ntdll/thread.c
index e328c5f..a4cbdc3 100644
--- a/dlls/ntdll/thread.c
+++ b/dlls/ntdll/thread.c
@@ -23,6 +23,7 @@
 
 #include assert.h
 #include stdarg.h
+#include stdint.h
 #include sys/types.h
 #ifdef HAVE_SYS_MMAN_H
 #include sys/mman.h
@@ -67,6 +68,103 @@ static RTL_BITMAP fls_bitmap;
 static LIST_ENTRY tls_links;
 static int nb_threads = 1;
 
+static void CALLBACK shared_data_timer_apc(void* TimerContext, uint32_t TimerLowValue, int32_t TimerHighValue);
+static HANDLE create_shared_data_timer(void);
+static NTSTATUS set_shared_data_timer(HANDLE handle);
+
+static void update_shared_data_time(void)
+{
+LARGE_INTEGER now, start, irq;
+
+NtQuerySystemTime( now );
+
+FIXME(%lld\n, now.QuadPart);
+
+irq.QuadPart = (now.QuadPart - server_start_time);
+
+user_shared_data-InterruptTime.High2Time = irq.HighPart;
+user_shared_data-InterruptTime.LowPart = irq.LowPart;
+user_shared_data-InterruptTime.High1Time = irq.HighPart;
+
+user_shared_data-SystemTime.High2Time = now.HighPart;
+user_shared_data-SystemTime.LowPart = now.LowPart;
+user_shared_data-SystemTime.High1Time = now.HighPart;
+
+start.QuadPart = irq.QuadPart / 1;
+
+user_shared_data-u.TickCount.High2Time = start.HighPart;
+user_shared_data-u.TickCount.LowPart = start.LowPart;
+user_shared_data-u.TickCount.High1Time = start.HighPart;
+user_shared_data-TickCountLowDeprecated = start.LowPart;
+}
+
+static void CALLBACK shared_data_timer_apc(void* ctx, uint32_t low, int32_t high)
+{
+update_shared_data_time();
+set_shared_data_timer((HANDLE)ctx);
+}
+
+static HANDLE create_shared_data_timer(void)
+{
+HANDLE handle;
+NTSTATUS status;
+OBJECT_ATTRIBUTES attr;
+
+//FIXME(enter\n);
+
+attr.Length   = sizeof(attr);
+attr.RootDirectory= 0;
+attr.ObjectName   = NULL;
+attr.Attributes   = OBJ_OPENIF;
+attr.SecurityDescriptor   = NULL;
+attr.SecurityQualityOfService = NULL;
+
+status = NtCreateTimer( handle, TIMER_ALL_ACCESS, attr, NotificationTimer);
+
+if (status == STATUS_OBJECT_NAME_EXISTS)
+SetLastError( ERROR_ALREADY_EXISTS );
+else
+SetLastError( RtlNtStatusToDosError(status) );
+
+return handle;
+}
+
+static NTSTATUS set_shared_data_timer(HANDLE handle)
+{
+LARGE_INTEGER when;
+NTSTATUS status;
+
+//FIXME(enter\n);
+
+NtQuerySystemTime( when );
+when.QuadPart += 156000;
+
+status = NtSetTimer(handle, when, (PTIMER_APC_ROUTINE)shared_data_timer_apc, handle, 0, 0, NULL);
+if (status != STATUS_SUCCESS)
+{
+SetLastError( RtlNtStatusToDosError(status) );
+}
+
+return status;
+}
+
+
+static void* shared_data_thread(void *arg) 
+{
+struct timespec req, rem;
+
+req.tv_sec = 0;
+req.tv_nsec = 1560;
+
+while(1) {
+update_shared_data_time();
+nanosleep(req, rem);
+}
+
+return NULL;
+}
+
+
 /***
  *   get_unicode_string
  *
@@ -196,9 +294,10 @@ HANDLE thread_init(void)
 void *addr;
 SIZE_T size, info_size;
 HANDLE exe_file = 0;
-LARGE_INTEGER now;
 struct ntdll_thread_data *thread_data;
 static struct debug_info debug_info;  /* debug info for initial thread */
+pthread_t thread;
+int s;
 
 virtual_init();
 
@@ -288,16 +387,24 @@ HANDLE thread_init(void)
 }
 
 /* initialize time values in user_shared_data */
-NtQuerySystemTime( now );
-user_shared_data-SystemTime.LowPart = now.u.LowPart;
-user_shared_data-SystemTime.High1Time = user_shared_data-SystemTime.High2Time = now.u.HighPart;
-user_shared_data-u.TickCountQuad = (now.QuadPart - server_start_time) / 1;
-user_shared_data-u.TickCount.High2Time = user_shared_data-u.TickCount.High1Time;
-user_shared_data-TickCountLowDeprecated = user_shared_data-u.TickCount.LowPart;
 user_shared_data-TickCountMultiplier = 1  24;
 
+update_shared_data_time();
+
 fill_cpu_info();
 
+HANDLE timer = 

Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-21 Thread Kornél Pál

On 3/21/2012 2:20 PM, Marcus Meissner wrote:

However, I do not know if we can have additional threads at all in a
Win32 process without confusing the win32 processes, or if this needs
to be solved differently (APC?).


If user_shared_data was written by wineserver and mapped read-only to 
wine processes there was no need to create separate threads in wine 
processes. As I know Windows is sharing this structure and is updating 
it in kernel mode so wine behavior was similar if was updated by wineserver.


Thanks.

Kornel




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-21 Thread Joey Yandle
 
 If user_shared_data was written by wineserver and mapped read-only to
 wine processes there was no need to create separate threads in wine
 processes. As I know Windows is sharing this structure and is updating
 it in kernel mode so wine behavior was similar if was updated by
 wineserver.
 

Agreed, this is the ideal solution.  I'm just not sure how to do so
while maintaining the virtual address mapping required for user_shared_data.





Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-21 Thread Kornél Pál

On 3/21/2012 9:11 PM, Joey Yandle wrote:


If user_shared_data was written by wineserver and mapped read-only to
wine processes there was no need to create separate threads in wine
processes. As I know Windows is sharing this structure and is updating
it in kernel mode so wine behavior was similar if was updated by
wineserver.



Agreed, this is the ideal solution.  I'm just not sure how to do so
while maintaining the virtual address mapping required for user_shared_data.


MapViewOfFile (and the underlying NtMapViewOfSection) has support for 
specifying the address to map it.


Thanks.

Kornel




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-21 Thread Joey Yandle
 2. in server/main.c:
 
   int fd = shm_open(/KUSER_SHARED_DATA, O_RDWR | O_CREAT, 0600);
   // call MapViewOfFile to map fd to 0x7ffe
 

After looking over the code, I think I should just mmap() directly in
wineserver rather than using MapViewOfFile; I should however still use
MapViewOfFile in ntdll/thread.c.

Mr Google tells me that I can get a handle for MapViewOfFile by calling
CreateFileMapping.  The first arg of that is also a HANDLE, which I
should be able to get by calling wine_server_fd_to_handle (as is done
elsewhere in thread.c).

So I'm going to go ahead and try this now.  If anyone has a issue with
this approach, please let me know.

cheers,

Joey




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-21 Thread Joey Yandle
 
 So I'm going to go ahead and try this now.  If anyone has a issue with
 this approach, please let me know.
 

I implemented this approach, only to find that numerous places in ntdll/
write to user_shared_data.  I need to move all of that code to server/,
which will take a while, since much of it depends on other code
(#defines and the like) in the files.

I'm attaching my current diff, which throws an exception as soon as
someone tries to read one of the values which is no longer being written
from ntdll.  The diff is extremely ugly, but shows the approach.

cheers,

Joey




diff --git a/dlls/ntdll/Makefile.in b/dlls/ntdll/Makefile.in
index 0047731..80d7f5a 100644
--- a/dlls/ntdll/Makefile.in
+++ b/dlls/ntdll/Makefile.in
@@ -2,7 +2,7 @@ EXTRADEFS = -D_NTSYSTEM_
 MODULE= ntdll.dll
 IMPORTLIB = ntdll
 IMPORTS   = winecrt0
-EXTRALIBS = @IOKITLIB@ @LIBPTHREAD@
+EXTRALIBS = @IOKITLIB@ @LIBPTHREAD@ -lrt
 EXTRADLLFLAGS = -nodefaultlibs -Wl,--image-base,0x7bc0
 
 C_SRCS = \
diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c
index 381e2ac..84a4963 100644
--- a/dlls/ntdll/loader.c
+++ b/dlls/ntdll/loader.c
@@ -2812,7 +2812,7 @@ void CDECL __wine_init_windows_dir( const WCHAR *windir, const WCHAR *sysdir )
 PLIST_ENTRY mark, entry;
 LPWSTR buffer, p;
 
-strcpyW( user_shared_data-NtSystemRoot, windir );
+//strcpyW( user_shared_data-NtSystemRoot, windir );
 DIR_init_windows_dir( windir, sysdir );
 
 /* prepend the system dir to the name of the already created modules */
diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c
index 3ed8038..dea32ee 100644
--- a/dlls/ntdll/nt.c
+++ b/dlls/ntdll/nt.c
@@ -1038,6 +1038,7 @@ void fill_cpu_info(void)
 }
 if (!strcasecmp(line, flags) || !strcasecmp(line, features))
 {
+#if 0
 if (strstr(value, cx8))
 user_shared_data-ProcessorFeatures[PF_COMPARE_EXCHANGE_DOUBLE] = TRUE;
 if (strstr(value, cx16))
@@ -1062,6 +1063,7 @@ void fill_cpu_info(void)
 user_shared_data-ProcessorFeatures[PF_PAE_ENABLED] = TRUE;
 if (strstr(value, ht))
 cached_sci.FeatureSet |= CPU_FEATURE_HTT;
+#endif
 continue;
 }
 	}
diff --git a/dlls/ntdll/thread.c b/dlls/ntdll/thread.c
index e328c5f..8c2781b 100644
--- a/dlls/ntdll/thread.c
+++ b/dlls/ntdll/thread.c
@@ -23,6 +23,7 @@
 
 #include assert.h
 #include stdarg.h
+#include stdint.h
 #include sys/types.h
 #ifdef HAVE_SYS_MMAN_H
 #include sys/mman.h
@@ -67,6 +68,103 @@ static RTL_BITMAP fls_bitmap;
 static LIST_ENTRY tls_links;
 static int nb_threads = 1;
 
+static void CALLBACK shared_data_timer_apc(void* TimerContext, uint32_t TimerLowValue, int32_t TimerHighValue);
+static HANDLE create_shared_data_timer(void);
+static NTSTATUS set_shared_data_timer(HANDLE handle);
+
+static void update_shared_data_time(void)
+{
+LARGE_INTEGER now, start, irq;
+
+NtQuerySystemTime( now );
+
+//FIXME(%lld\n, now.QuadPart);
+
+irq.QuadPart = (now.QuadPart - server_start_time);
+
+user_shared_data-InterruptTime.High2Time = irq.HighPart;
+user_shared_data-InterruptTime.LowPart = irq.LowPart;
+user_shared_data-InterruptTime.High1Time = irq.HighPart;
+
+user_shared_data-SystemTime.High2Time = now.HighPart;
+user_shared_data-SystemTime.LowPart = now.LowPart;
+user_shared_data-SystemTime.High1Time = now.HighPart;
+
+start.QuadPart = irq.QuadPart / 1;
+
+user_shared_data-u.TickCount.High2Time = start.HighPart;
+user_shared_data-u.TickCount.LowPart = start.LowPart;
+user_shared_data-u.TickCount.High1Time = start.HighPart;
+user_shared_data-TickCountLowDeprecated = start.LowPart;
+}
+
+static void CALLBACK shared_data_timer_apc(void* ctx, uint32_t low, int32_t high)
+{
+update_shared_data_time();
+set_shared_data_timer((HANDLE)ctx);
+}
+
+static HANDLE create_shared_data_timer(void)
+{
+HANDLE handle;
+NTSTATUS status;
+OBJECT_ATTRIBUTES attr;
+
+//FIXME(enter\n);
+
+attr.Length   = sizeof(attr);
+attr.RootDirectory= 0;
+attr.ObjectName   = NULL;
+attr.Attributes   = OBJ_OPENIF;
+attr.SecurityDescriptor   = NULL;
+attr.SecurityQualityOfService = NULL;
+
+status = NtCreateTimer( handle, TIMER_ALL_ACCESS, attr, NotificationTimer);
+
+if (status == STATUS_OBJECT_NAME_EXISTS)
+SetLastError( ERROR_ALREADY_EXISTS );
+else
+SetLastError( RtlNtStatusToDosError(status) );
+
+return handle;
+}
+
+static NTSTATUS set_shared_data_timer(HANDLE handle)
+{
+LARGE_INTEGER when;
+NTSTATUS status;
+
+//FIXME(enter\n);
+
+NtQuerySystemTime( when );
+when.QuadPart += 156000;
+
+status = NtSetTimer(handle, when, (PTIMER_APC_ROUTINE)shared_data_timer_apc, handle, 0, 0, NULL);
+if (status != STATUS_SUCCESS)
+{
+SetLastError( RtlNtStatusToDosError(status) );
+}
+
+

Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-21 Thread Dmitry Timoshkov
Joey Yandle dra...@dancingdragon.be wrote:

 After looking over the code, I think I should just mmap() directly in
 wineserver rather than using MapViewOfFile; I should however still use
 MapViewOfFile in ntdll/thread.c.
 
 Mr Google tells me that I can get a handle for MapViewOfFile by calling
 CreateFileMapping.  The first arg of that is also a HANDLE, which I
 should be able to get by calling wine_server_fd_to_handle (as is done
 elsewhere in thread.c).
 
 So I'm going to go ahead and try this now.  If anyone has a issue with
 this approach, please let me know.

It's been explained several times already that any approach to share data
between wineserver and clients is going to be rejected.

-- 
Dmitry.




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-21 Thread Joey Yandle
 
 It's been explained several times already that any approach to share data
 between wineserver and clients is going to be rejected.
 

Can you point me to such an explanation?  I joined wine-devel just
before posting my RFC.  If this has been discussed previously, I'd
prefer to get the context before wasting my and others' time.

So if both a thread (pthread or windows) in the wine process and sharing
memory with wineserver are verboten, then it looks like the only option
is APC.  Can you think of any others?

If not, can you look at the APC based patch I posted and let me know how
to make the callbacks happen with more precise timing?  Windows sets
these values in the kernel via a hardware interrupt, so wine needs to
set them without reasonable precision.

thanks,

Joey




Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168

2012-03-21 Thread Vitaliy Margolen

On 03/21/2012 10:19 PM, Joey Yandle wrote:



If not, can you look at the APC based patch I posted and let me know how
to make the callbacks happen with more precise timing?
Yes, that is the only possible way to do this currently in Wine. Separate 
thread is a no-go. It will break some applications. And no, there is no way 
to make it more accurate. See other posts about this topic CreateTimerQueue.


- Vitaliy.