On Thu, Jan 16, 2020 at 1:00 PM Arnd Bergmann <a...@arndb.de> wrote: > > On Thu, Jan 16, 2020 at 12:27 PM Aleksandar Markovic > <aleksandar.m.m...@gmail.com> wrote: > > On Thursday, January 16, 2020, Aleksandar Markovic > > <aleksandar.m.m...@gmail.com> wrote: > >> On Wednesday, January 15, 2020, Laurent Vivier <laur...@vivier.eu> wrote: > >>> Le 15/01/2020 à 20:17, Filip Bozuta a écrit : > >>> > On 15.1.20. 17:37, Arnd Bergmann wrote: > >>> >> On Wed, Jan 15, 2020 at 5:32 PM Laurent Vivier <laur...@vivier.eu> > >>> >> wrote: > >>> >>> Le 15/01/2020 à 17:18, Arnd Bergmann a écrit : > >>> >>>> On Wed, Jan 15, 2020 at 4:53 PM Filip Bozuta > >>> >>>> <filip.boz...@rt-rk.com> wrote: > >>> >>>>> This patch implements functionality of following ioctl: > >>> >>>>> > >>> >>>>> SNDRV_TIMER_IOCTL_TREAD - Setting enhanced time read > >>> >>>>> > >>> >>>>> Sets enhanced time read which is used for reading time with > >>> >>>>> timestamps > >>> >>>>> and events. The third ioctl's argument is a pointer to an > >>> >>>>> 'int'. Enhanced > >>> >>>>> reading is set if the third argument is different than 0, > >>> >>>>> otherwise normal > >>> >>>>> time reading is set. > >>> >>>>> > >>> >>>>> Implementation notes: > >>> >>>>> > >>> >>>>> Because the implemented ioctl has 'int' as its third argument, > >>> >>>>> the > >>> >>>>> implementation was straightforward. > >>> >>>>> > >>> >>>>> Signed-off-by: Filip Bozuta <filip.boz...@rt-rk.com> > >>> >>>> I think this one is wrong when you go between 32-bit and 64-bit > >>> >>> kernel uses an "int" and "int" is always 32bit. > >>> >>> The problem is most likely with timespec I think. > >>> >>> > >>> >>>> targets, and it gets worse with the kernel patches that just got > >>> >>>> merged for linux-5.5, which extends the behavior to deal with > >>> >>>> 64-bit time_t on 32-bit architectures. > >>> >>>> > >>> >>>> Please have a look at > >>> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/?h=80fe7430c70859 > >>> >>>> > >>> >>> Yes, we already had the same kind of problem with SIOCGSTAMP and > >>> >>> SIOCGSTAMPNS. > >>> >>> > >>> >>> Do the kernel patches add new ioctl numbers to differentiate 32bit and > >>> >>> 64bit time_t? > >>> >> Yes, though SNDRV_TIMER_IOCTL_TREAD is worse: the ioctl argument > >>> >> is a pure 'int' that decides what format you get when you 'read' from > >>> >> the > >>> >> same file descriptor. > >>> >> > >>> >> For emulating 64-bit on 32-bit kernels, you have to use the new > >>> >> SNDRV_TIMER_IOCTL_TREAD64, and for emulating 32-bit on > >>> >> 64-bit kernels, you probably have to return -ENOTTY to > >>> >> SNDRV_TIMER_IOCTL_TREAD_OLD unless you also want to > >>> >> emulate the read() behavior. > >>> >> When a 32-bit process calls SNDRV_TIMER_IOCTL_TREAD64, > >>> >> you can translate that into the 64-bit > >>> >> SNDRV_TIMER_IOCTL_TREAD_OLD. > >>> > > >>> > Thank you for bringing this up to my attention. Unfortunately i have > >>> > some duties of academic nature in next month so i won't have much time > >>> > fix this bug. I will try to fix this as soon as possible. > >>> > >>> Could you at least to try to have a mergeable series before you have to > >>> stop to work on this? > >>> > >>> You can only manage the case before the change reported by Arnd (I will > >>> manage the new case myself later). > > >> > >> Sorry for interjecting myself into this discussion, but I just want to let > >> you know about some related practicalities. > >> > >> Filip is a student that is from time to time (usually between two exam > >> seasons) an intern in our company, working 4 hours each work day. He spent > >> his internship in different teams in prevous years, and, from around > >> mid-October 2019, was appointed to QEMU team. After some introductory > >> tasks, he was assigned his main task: linux-user support for RTCs and ALSA > >> timers. This series is the result of his work, and, to my great pleasure, > >> is virtually entirely his independant work. I am positive he can complete > >> the series by himself, even in the light of additional complexities > >> mentioned in this thread. > >> > >> However, his exam season just started (Jan. 15th), and lasts till Feb. > >> 15th. Our policy, in general, is not to burden the students during exam > >> seasons, and that is why we can't expect prompt updates from him for the > >> time being. > >> > >> In view of this, Laurent, please take Filip's status into consideration. > >> As far as mergeability is concerned, my impression is that patches 1-6 and > >> 13 are ready for merging, while patches 7-12 would require some additional > >> (netlink-support-like) work, that would unfortunately be possible only > >> after Feb. 15th. > >> > > > Laurent, hi again. > > > > I am not completely familiar with all details of Filip's work, since, as I > > said, he had > > large degree of independance (which was intentional, and is a desired and > > good > > thing IMHO), but taking a closer look, a question starting lingering: Do we > > need > > special handling od read() even for RTC devices - not only ALSA timer > > devices? > > Adding Alexandre Belloni and the RTC list to Cc for more expertise. Alexandre, > this question is about how qemu-user should emulate the rtc ioctl interface > when > running a user binary for a foreign architecture. The ioctl emulation seems > fine > to me, but read() from /dev/rtc is probably not. > > As I understand it, reading from /dev/rtc is one of the more obscure features. > This would return either 32 bits or 64 bits of structured data from the > kernel, > depending on how much data was requested and whether the kernel runs > as 64 bit. A 32-bit process running on a 64-bit kernel will get the correct > result when it asks for 4 bytes, but probably not when it asks for 8 bytes. > (we could fix this with an explict check for in_compat_syscall() in the kernel > function). > > A process running on qemu with the opposite endianess will always get the > wrong result (unless the kernel returns 0), and emulating 64-bit task on > a 32-bit kernel will result in only four bytes to be read, which also likely > results in incorrect behavior. >
Alexandre (and Arnd too, or any other person knowledgeable in the area), I just need to clarify a couple of details with you, please. Firstly, here is what man page rtc(4) says: "The /dev/rtc (or /dev/rtc0, /dev/rtc1, etc.) device can be opened only once (until it is closed) and it is read-only. On read(2) and select(2) the calling process is blocked until the next interrupt from that RTC is received. Following the interrupt, the process can read a long integer, of which the least significant byte contains a bit mask encoding the types of interrupt that occurred, while the remaining 3 bytes contain the number of interrupts since the last read(2)." So, it looks read() will always return only 4 bytes of useful info (regardless of host being 32-bit/64-bit). My questions are: - Is the description in man page genuinely accurate? - To me (but I am really an outsider to using RTC in applications), this feature (blocking read()/select()) even looks very nice and convenient, in all fairness. But I would like to ask you: Is this feature used rarely or frequently by other libraries/tools/etc.? In other words, is the feature "obscure" or "crucial" part of RTC kernel support? Or, something in between? - Does MC146818 support this feature? Thanks a lot in advance for any response! Aleksandar P.S. for Arnd: I sent this message only to you, by mistake, around 15 min ago. Now I include everybody. > Arnd