Re: [PATCH v4] Add timer_join to avoid racing in timer cleanup

Hi Paolo, could you please take a look? If you don't like my patch I will be happy if you suggest your solution for this problem. We can help with reviewing it. I grepped through the QEMU sources and found timer_del is called from a separate thread in accel/tcg/tcg-accel-ops-rr.c. It does not use

Re: [PATCH] iov: don't use void* in pointer arithmetic in headers

Hi Peter, thank you for looking into this. On Tue, Jul 9, 2024 at 2:39 AM Peter Maydell wrote: > We had the question on a previous "make this C++ > compatible" patch of "how much of this stuff is there?". Unfortunately, we don't know yet (we are still upgrading to QEMU8). If this is a burden fo

[PATCH] iov: don't use void* in pointer arithmetic in headers

void* pointer arithmetic is a GCC extentension which could not be available in other build tools (e.g. C++). This changes removes this assumption. Signed-off-by: Roman Kiryanov --- include/qemu/iov.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/qemu/iov.h b

Re: [PATCH v4] Add timer_join to avoid racing in timer cleanup

Hi Paolo, could you please take a look? Regards, Roman.

[PATCH v4] Add timer_join to avoid racing in timer cleanup

is done and it is safe to destroy the object pointed by opaque. Once this function is available, the example above will look like this: timer_del(&timer); timer_join(&timer); free(opaque); Signed-off-by: Roman Kiryanov --- v2: rebased to the right branch and removed Googl

Re: [PATCH v3] timer: Fix a race condition between timer's callback and destroying code

Paolo, thank you for your comments. On Thu, Jun 27, 2024 at 10:16 AM Paolo Bonzini wrote: > I think the easiest options would be: > > 1) if possible, allocate the timer and the callbackState statically in > the device. I think this assumption is not good for both QEMU and us. > 2) use "aio_wait

Re: [PATCH v3] timer: Fix a race condition between timer's callback and destroying code

On Thu, Jun 27, 2024 at 6:27 AM Paolo Bonzini wrote: > > On Thu, Jun 27, 2024 at 2:32 AM Roman Kiryanov wrote: > > +if (qatomic_read(&ts->cb_running)) { > > +qemu_event_wait(&timer_list->timers_done_ev); > > +} > > qemu_eve

[PATCH v3] timer: Fix a race condition between timer's callback and destroying code

and it will be safe to destroy timer's data once `timer_del` returns. Signed-off-by: Roman Kiryanov --- v2: rebased to the right branch and removed Google specific tags from the commit message. v3: if a timer's callback happens to be running (cb_running) wait until all timers a

[PATCH v2] timer: Fix a race condition between timer's callback and destroying code

and it will be safe to destroy timer's data once `timer_del` returns. Signed-off-by: Roman Kiryanov --- v2: rebased to the right branch and removed Google specific tags from the commit message. include/qemu/timer.h | 4 util/qemu-timer.c| 21 + 2 files ch

Re: [PATCH v2] exec: don't use void* in pointer arithmetic in headers

On Thu, Jun 20, 2024 at 11:42 PM Philippe Mathieu-Daudé wrote: > > Hi Roman, > > On 20/6/24 22:16, Roman Kiryanov wrote: > > Google-Bug-Id: 331190993 > > Again [*] I'm trying to figure what this tag is, no hit on > https://issuetracker.google.com/issues?q=id:33119

Re: [PATCH] include: move typeof_strip_qual to compiler.h, use it in QAPI_LIST_LENGTH()

Hi Manos, thank you for looking. On Tue, Jun 25, 2024 at 12:17 PM Manos Pitsidianakis wrote: > Should we add an #else to provide a fallback for cplusplus until the > alternative is merged? > > -#endif > +#else /* __cpluplus */ > +#define typeof_strip_qual typeof > +#endif /* __cplusplus */ You p

[PATCH] timer: Fix a race condition between timer's callback and destroying code

and it will be safe to destroy timer's data once `timer_del` returns. Google-Bug-Id: 348598513 Change-Id: I3cd6350587fb67c73bbee7b934d2c0c1acb27229 Signed-off-by: Roman Kiryanov --- include/qemu/timer.h | 4 util/qemu-timer.c| 22 ++ 2 files changed, 26 inserti

Re: [PATCH 1/1] include/qemu: Provide a C++ compatible version of typeof_strip_qual

Hi Philippe, thank you for looking. On Mon, Jun 24, 2024 at 7:27 PM Philippe Mathieu-Daudé wrote: > In particular this patch seems contained well enough > to be carried in forks were C++ _is_ used. Will you agree to take #ifdef __cplusplus and #error to the QEMU side in atomic.h and we will kee

Deleting the timer data without racing with its callback

Hi QEMU, we got a report that our code (a wrapper around the QEMU timer API) has a race condition. I tried to fix it, but unfortunately can't figure out how to do this properly. Please see the `timerlist_run_timers` code below: bool timerlist_run_timers(QEMUTimerList *timer_list) { ... q

Re: [PATCH 3/3] exec: use char* for pointer arithmetic

Hi Peter, thank you for looking. On Thu, Jun 20, 2024 at 12:09 PM Peter Maydell wrote: > I think this is the point where I would say "you're making the > code worse for upstream and the only benefit is to your out-of-tree > downstream code". If you want to build QEMU, use one of the compilers > t

[PATCH v2] exec: don't use void* in pointer arithmetic in headers

void* pointer arithmetic is a GCC extentension which could not be available in other build tools (e.g. C++). This changes removes this assumption. Google-Bug-Id: 331190993 Change-Id: I5a064853429f627c17a9213910811dea4ced6174 Signed-off-by: Roman Kiryanov --- v2: renamed from "use char

Re: [PATCH 3/3] exec: use char* for pointer arithmetic

On Thu, Jun 20, 2024 at 11:16 AM Paolo Bonzini wrote: > > > Would it work instead to declare MemoryRegionCache's ptr field as char*? > > > > I prefer to use char* only where there are strings. For unstructured data > > such as > > MemoryRegionCache, void* is more appropriate. > > Or uint8_t*...

Re: [PATCH 3/3] exec: use char* for pointer arithmetic

Hi Daniel and Alex, On Thu, Jun 20, 2024 at 8:10 AM Alex Bennée wrote: > > Daniel P. Berrangé writes: > > NB, QEMU is explicitly *NOT* targetting the C standard, we are > > targetting the C dialect supported by GCC and CLang only. IOW, > > if they have well defined behaviour for arithmetic on vo

Re: [PATCH 3/3] exec: use char* for pointer arithmetic

Hi Richard, On Tue, Jun 18, 2024 at 4:05 PM Richard Henderson wrote: > We require "char *" with a space. thank you for looking into this. I sent v2 for this one.

[PATCH v2] exec: use char* for pointer arithmetic

void* pointer arithmetic is not in the C standard. This change allows using the QEMU headers with a C++ compiler. Google-Bug-Id: 331190993 Change-Id: I5a064853429f627c17a9213910811dea4ced6174 Signed-off-by: Roman Kiryanov --- v2: change `char*` into `char *` (add the missing space). include

[PATCH 3/3] exec: use char* for pointer arithmetic

void* pointer arithmetic is not in the C standard. This change allows using the QEMU headers with a C++ compiler. Google-Bug-Id: 331190993 Change-Id: I5a064853429f627c17a9213910811dea4ced6174 Signed-off-by: Roman Kiryanov --- include/exec/memory.h | 8 include/exec

[PATCH 2/3] exec: avoid using C++ keywords in function parameters

to use the QEMU headers with a C++ compiler. Google-Bug-Id: 331190993 Change-Id: Ic4e49b9c791616bb22c973922772b0494706092c Signed-off-by: Roman Kiryanov --- include/exec/memory.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/exec/memory.h b/include/exec

[PATCH 1/3] exec: Make the MemOp enum cast explicit

to use the QEMU headers with a C++ compiler. Google-Bug-Id: 331190993 Change-Id: I785f2e65d192287f1f964d2840131b653755648c Signed-off-by: Roman Kiryanov --- include/exec/memop.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/exec/memop.h b/include/exec/memop.h index

Re: [PATCH] Make TARGET_PAGE_MASK typed as target_ulong

Hi Richard, thank you for looking into this. > No, this will cause failures, because we need this value to sign-extend to > when the > context includes {u}int64_t, and target_ulong is uint32_t. I did not expect this, good catch. I see QEMU uses size_t as the return type in qemu_target_page_size

[PATCH] Make TARGET_PAGE_MASK typed as target_ulong

PAGE_MASK' ~~~ ^ also this fixes the inconsitency in the return type of qemu_target_page_mask (int could be shorter than target_long). Signed-off-by: Roman Kiryanov --- include/exec/cpu-all.h | 4 ++-- include/exec/target_page.h | 2 +-

Re: Hermetic virtio-vsock in QEMU

Hi Stefano, On Tue, May 7, 2024 at 1:10 AM Stefano Garzarella wrote: > I have no experience with Windows, but what we need for vhost-user is: > > - AF_UNIX and be able to send file descriptors using ancillary data >(i.e. SCM_RIGHTS) As far as I understand, Windows does NOT support SCM_RIGHTS

Re: QEMU headers in C++

Hi Paolo, thank you for looking. On Thu, May 2, 2024 at 8:19 AM Paolo Bonzini wrote: > > Anyway, just out of curiosity I tried to see what it would take to > compile edu.c as C++ code, which I think lets us give a more informed > answer. > > There were a bunch of conflicts with C++ keyword, espec

QEMU headers in C++

Hi QEMU, I work in Android Studio Emulator and we would like to develop devices in C++. Unfortunately, QEMU headers cannot be used with C++ as is (e.g. they use C++ keywords as variable names or implicitly cast void* to T*). Will QEMU be open to accept patches from us to make QEMU headers C++ com

Re: Hermetic virtio-vsock in QEMU

n libvhost-user.h and I think it is not used in libvhost-user. Regards, Roman. On Mon, Apr 15, 2024 at 4:20 AM Daniel P. Berrangé wrote: > On Wed, Apr 03, 2024 at 02:30:33PM -0700, Roman Kiryanov wrote: > > Hi Peter, Alex and QEMU, > > > > I work in Android Studio Emul

Hermetic virtio-vsock in QEMU

Hi Peter, Alex and QEMU, I work in Android Studio Emulator and we use virtio-vsock to emulate devices (e.g. sensors) which live in the Emulator binary. We need to run on Windows and in environments without CONFIG_VHOST_VSOCK, that is why we cannot use vhost-vsock and invented our implementation. I

Re: [Qemu-devel] [PATCH] vmstate: Add VMSTATE_OPAQUE to save/load complex data structures

Hi Peter, > In any case, migration state on the wire needs to be > architecture/endianness/ Could you please point how the proposed change is architecture/endianness/ dependent? > implementation-independent, Could you please elaborate, what "implementation" you mean here? > so you can't just s

Re: [Qemu-devel] [PATCH] vmstate: Add VMSTATE_OPAQUE to save/load complex data structures

Hi Dave, thank you for looking. > Can you give me an example of where you would use it? We use it in our host memory sharing device. I used the existing macros for all fields I could, but unfortunately some state does not fit into them. We use this new macro to save/load memory allocators (for no