[v1] Accelerators patches for 2025-07-04

[PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Philippe Mathieu-Daudé 7 months, 1 week ago

No need for accel-specific @dirty field when we have
a generic one in CPUState.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Message-Id: <20250703173248.44995-20-philmd@linaro.org>
---
 target/i386/nvmm/nvmm-all.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/target/i386/nvmm/nvmm-all.c b/target/i386/nvmm/nvmm-all.c
index f1c6120ccf1..aea61a6fd2a 100644
--- a/target/i386/nvmm/nvmm-all.c
+++ b/target/i386/nvmm/nvmm-all.c
@@ -30,7 +30,6 @@ struct AccelCPUState {
     struct nvmm_vcpu vcpu;
     uint8_t tpr;
     bool stop;
-    bool dirty;
 
     /* Window-exiting for INTs/NMIs. */
     bool int_window_exit;
@@ -508,7 +507,7 @@ nvmm_io_callback(struct nvmm_io *io)
     }
 
     /* Needed, otherwise infinite loop. */
-    current_cpu->accel->dirty = false;
+    current_cpu->vcpu_dirty = false;
 }
 
 static void
@@ -517,7 +516,7 @@ nvmm_mem_callback(struct nvmm_mem *mem)
     cpu_physical_memory_rw(mem->gpa, mem->data, mem->size, mem->write);
 
     /* Needed, otherwise infinite loop. */
-    current_cpu->accel->dirty = false;
+    current_cpu->vcpu_dirty = false;
 }
 
 static struct nvmm_assist_callbacks nvmm_callbacks = {
@@ -727,9 +726,9 @@ nvmm_vcpu_loop(CPUState *cpu)
      * Inner VCPU loop.
      */
     do {
-        if (cpu->accel->dirty) {
+        if (cpu->vcpu_dirty) {
             nvmm_set_registers(cpu);
-            cpu->accel->dirty = false;
+            cpu->vcpu_dirty = false;
         }
 
         if (qcpu->stop) {
@@ -827,32 +826,32 @@ static void
 do_nvmm_cpu_synchronize_state(CPUState *cpu, run_on_cpu_data arg)
 {
     nvmm_get_registers(cpu);
-    cpu->accel->dirty = true;
+    cpu->vcpu_dirty = true;
 }
 
 static void
 do_nvmm_cpu_synchronize_post_reset(CPUState *cpu, run_on_cpu_data arg)
 {
     nvmm_set_registers(cpu);
-    cpu->accel->dirty = false;
+    cpu->vcpu_dirty = false;
 }
 
 static void
 do_nvmm_cpu_synchronize_post_init(CPUState *cpu, run_on_cpu_data arg)
 {
     nvmm_set_registers(cpu);
-    cpu->accel->dirty = false;
+    cpu->vcpu_dirty = false;
 }
 
 static void
 do_nvmm_cpu_synchronize_pre_loadvm(CPUState *cpu, run_on_cpu_data arg)
 {
-    cpu->accel->dirty = true;
+    cpu->vcpu_dirty = true;
 }
 
 void nvmm_cpu_synchronize_state(CPUState *cpu)
 {
-    if (!cpu->accel->dirty) {
+    if (!cpu->vcpu_dirty) {
         run_on_cpu(cpu, do_nvmm_cpu_synchronize_state, RUN_ON_CPU_NULL);
     }
 }
@@ -982,7 +981,7 @@ nvmm_init_vcpu(CPUState *cpu)
         }
     }
 
-    qcpu->dirty = true;
+    qcpu->vcpu_dirty = true;
     cpu->accel = qcpu;
 
     return 0;
-- 
2.49.0

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Thomas Huth 3 weeks, 6 days ago

On 04/07/2025 12.14, Philippe Mathieu-Daudé wrote:
> No need for accel-specific @dirty field when we have
> a generic one in CPUState.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> Message-Id: <20250703173248.44995-20-philmd@linaro.org>
> ---
>   target/i386/nvmm/nvmm-all.c | 21 ++++++++++-----------
>   1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/target/i386/nvmm/nvmm-all.c b/target/i386/nvmm/nvmm-all.c
> index f1c6120ccf1..aea61a6fd2a 100644
> --- a/target/i386/nvmm/nvmm-all.c
> +++ b/target/i386/nvmm/nvmm-all.c
...
> @@ -982,7 +981,7 @@ nvmm_init_vcpu(CPUState *cpu)
>           }
>       }
>   
> -    qcpu->dirty = true;
> +    qcpu->vcpu_dirty = true;
>       cpu->accel = qcpu;
>   
>       return 0;

FYI, this does not seem to compile:

../src/target/i386/nvmm/nvmm-all.c: In function 'nvmm_init_vcpu':
../src/target/i386/nvmm/nvmm-all.c:988:9: error: 'AccelCPUState' has no 
member named 'vcpu_dirty'
   988 |     qcpu->vcpu_dirty = true;
       |         ^~

Is anybody checking the netbsd builds at all?
(I'm currently trying to update test/vm/netbsd to version 10.1, that's how I 
noticed it)

  Thomas

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Philippe Mathieu-Daudé 3 weeks, 6 days ago

On 13/1/26 20:32, Thomas Huth wrote:
> On 04/07/2025 12.14, Philippe Mathieu-Daudé wrote:
>> No need for accel-specific @dirty field when we have
>> a generic one in CPUState.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>> Message-Id: <20250703173248.44995-20-philmd@linaro.org>
>> ---
>>   target/i386/nvmm/nvmm-all.c | 21 ++++++++++-----------
>>   1 file changed, 10 insertions(+), 11 deletions(-)
>>
>> diff --git a/target/i386/nvmm/nvmm-all.c b/target/i386/nvmm/nvmm-all.c
>> index f1c6120ccf1..aea61a6fd2a 100644
>> --- a/target/i386/nvmm/nvmm-all.c
>> +++ b/target/i386/nvmm/nvmm-all.c
> ...
>> @@ -982,7 +981,7 @@ nvmm_init_vcpu(CPUState *cpu)
>>           }
>>       }
>> -    qcpu->dirty = true;
>> +    qcpu->vcpu_dirty = true;
>>       cpu->accel = qcpu;
>>       return 0;
> 
> FYI, this does not seem to compile:
> 
> ../src/target/i386/nvmm/nvmm-all.c: In function 'nvmm_init_vcpu':
> ../src/target/i386/nvmm/nvmm-all.c:988:9: error: 'AccelCPUState' has no 
> member named 'vcpu_dirty'
>    988 |     qcpu->vcpu_dirty = true;
>        |         ^~

s/qcpu/cpu/

> 
> Is anybody checking the netbsd builds at all?

3 reviewers and 6 months later.

Should we add a policy for bitrotting untested code?

> (I'm currently trying to update test/vm/netbsd to version 10.1, that's 
> how I noticed it)
> 
>   Thomas
>

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Daniel P. Berrangé 3 weeks, 5 days ago

On Tue, Jan 13, 2026 at 09:30:32PM +0100, Philippe Mathieu-Daudé wrote:
> On 13/1/26 20:32, Thomas Huth wrote:
> > On 04/07/2025 12.14, Philippe Mathieu-Daudé wrote:
> > > No need for accel-specific @dirty field when we have
> > > a generic one in CPUState.
> > > 
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> > > Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
> > > Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> > > Message-Id: <20250703173248.44995-20-philmd@linaro.org>
> > > ---
> > >   target/i386/nvmm/nvmm-all.c | 21 ++++++++++-----------
> > >   1 file changed, 10 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/target/i386/nvmm/nvmm-all.c b/target/i386/nvmm/nvmm-all.c
> > > index f1c6120ccf1..aea61a6fd2a 100644
> > > --- a/target/i386/nvmm/nvmm-all.c
> > > +++ b/target/i386/nvmm/nvmm-all.c
> > ...
> > > @@ -982,7 +981,7 @@ nvmm_init_vcpu(CPUState *cpu)
> > >           }
> > >       }
> > > -    qcpu->dirty = true;
> > > +    qcpu->vcpu_dirty = true;
> > >       cpu->accel = qcpu;
> > >       return 0;
> > 
> > FYI, this does not seem to compile:
> > 
> > ../src/target/i386/nvmm/nvmm-all.c: In function 'nvmm_init_vcpu':
> > ../src/target/i386/nvmm/nvmm-all.c:988:9: error: 'AccelCPUState' has no
> > member named 'vcpu_dirty'
> >    988 |     qcpu->vcpu_dirty = true;
> >        |         ^~
> 
> s/qcpu/cpu/
> 
> > 
> > Is anybody checking the netbsd builds at all?
> 
> 3 reviewers and 6 months later.
> 
> Should we add a policy for bitrotting untested code?

Since we don't cover netbsd in GitLab CI, all responsibility for keeping
it working lies with whomever cares about the distro. No contributors
or maintainers in general should feel obligated to fix bugs in untested
platforms unless they specifically care about the platform.

> > (I'm currently trying to update test/vm/netbsd to version 10.1, that's
> > how I noticed it)

MAINTAINERS lists no one against test/vm/netbsd.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Markus Armbruster 3 weeks, 5 days ago

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 13/1/26 20:32, Thomas Huth wrote:
>> On 04/07/2025 12.14, Philippe Mathieu-Daudé wrote:
>>> No need for accel-specific @dirty field when we have
>>> a generic one in CPUState.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
>>> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>>> Message-Id: <20250703173248.44995-20-philmd@linaro.org>
>>> ---
>>>   target/i386/nvmm/nvmm-all.c | 21 ++++++++++-----------
>>>   1 file changed, 10 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/target/i386/nvmm/nvmm-all.c b/target/i386/nvmm/nvmm-all.c
>>> index f1c6120ccf1..aea61a6fd2a 100644
>>> --- a/target/i386/nvmm/nvmm-all.c
>>> +++ b/target/i386/nvmm/nvmm-all.c
>> ...
>>> @@ -982,7 +981,7 @@ nvmm_init_vcpu(CPUState *cpu)
>>>           }
>>>       }
>>> -    qcpu->dirty = true;
>>> +    qcpu->vcpu_dirty = true;
>>>       cpu->accel = qcpu;
>>>       return 0;
>> FYI, this does not seem to compile:
>> ../src/target/i386/nvmm/nvmm-all.c: In function 'nvmm_init_vcpu':
>> ../src/target/i386/nvmm/nvmm-all.c:988:9: error: 'AccelCPUState' has no member named 'vcpu_dirty'
>>    988 |     qcpu->vcpu_dirty = true;
>>        |         ^~
>
> s/qcpu/cpu/
>
>> Is anybody checking the netbsd builds at all?
>
> 3 reviewers and 6 months later.

I have good news!  Because this does not compile since 10.1, and we're
already in the 11.0 cycle, we can rip it out immediately without doing
the deprecation dance.

> Should we add a policy for bitrotting untested code?

What policy do you have in mind?

>> (I'm currently trying to update test/vm/netbsd to version 10.1, that's how I noticed it)
>>   Thomas
>>

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Daniel P. Berrangé 3 weeks, 5 days ago

On Wed, Jan 14, 2026 at 08:38:26AM +0100, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
> > On 13/1/26 20:32, Thomas Huth wrote:
> >> On 04/07/2025 12.14, Philippe Mathieu-Daudé wrote:
> >>> No need for accel-specific @dirty field when we have
> >>> a generic one in CPUState.
> >>>
> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> >>> Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
> >>> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> >>> Message-Id: <20250703173248.44995-20-philmd@linaro.org>
> >>> ---
> >>>   target/i386/nvmm/nvmm-all.c | 21 ++++++++++-----------
> >>>   1 file changed, 10 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/target/i386/nvmm/nvmm-all.c b/target/i386/nvmm/nvmm-all.c
> >>> index f1c6120ccf1..aea61a6fd2a 100644
> >>> --- a/target/i386/nvmm/nvmm-all.c
> >>> +++ b/target/i386/nvmm/nvmm-all.c
> >> ...
> >>> @@ -982,7 +981,7 @@ nvmm_init_vcpu(CPUState *cpu)
> >>>           }
> >>>       }
> >>> -    qcpu->dirty = true;
> >>> +    qcpu->vcpu_dirty = true;
> >>>       cpu->accel = qcpu;
> >>>       return 0;
> >> FYI, this does not seem to compile:
> >> ../src/target/i386/nvmm/nvmm-all.c: In function 'nvmm_init_vcpu':
> >> ../src/target/i386/nvmm/nvmm-all.c:988:9: error: 'AccelCPUState' has no member named 'vcpu_dirty'
> >>    988 |     qcpu->vcpu_dirty = true;
> >>        |         ^~
> >
> > s/qcpu/cpu/
> >
> >> Is anybody checking the netbsd builds at all?
> >
> > 3 reviewers and 6 months later.
> 
> I have good news!  Because this does not compile since 10.1, and we're
> already in the 11.0 cycle, we can rip it out immediately without doing
> the deprecation dance.
> 
> > Should we add a policy for bitrotting untested code?
> 
> What policy do you have in mind?

Implicitly I consider the policy to effectively be

 (1) Platforms untested by GitLab CI are not guaranteed to work at
     any time, including in formal releases.
 (2) No contributors/maintainers are obligated to investigate or
     fix breakage in untested platforms
 (3) Those whom care about the platform are expected to perform
     manual testing and contribute fixes as required.

This doesn't imply we should automatically rip it out, but if we see
no evidence of (3) for a prolonged period of time, and no sign of it
being used downstream in any way, it is worth considering the cost /
benefit.

In the case of NetBSD something must be working to some extent since
it appears that 10.1.0 QEMU is present in the pkg repos:

   https://pkgsrc.se/emulators/qemu

so that argues against ripping stuff out even if we notice breakage.

> >> (I'm currently trying to update test/vm/netbsd to version 10.1, that's how I noticed it)
> >>   Thomas
> >> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Paolo Bonzini 3 weeks, 5 days ago

On 1/14/26 10:19, Daniel P. Berrangé wrote:
> This doesn't imply we should automatically rip it out, but if we see
> no evidence of (3) for a prolonged period of time, and no sign of it
> being used downstream in any way, it is worth considering the cost /
> benefit.
> 
> In the case of NetBSD something must be working to some extent since
> it appears that 10.1.0 QEMU is present in the pkg repos:
> 
>     https://pkgsrc.se/emulators/qemu
> 
> so that argues against ripping stuff out even if we notice breakage.

And indeed their pkgsrc has the same patch that Philippe has now
submitted for inclusion in qemu.git:

https://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/emulators/qemu/patches/patch-target_i386_nvmm_nvmm-all.c.diff?r1=1.10;r2=1.11

---- target/i386/nvmm/nvmm-all.c.orig	2024-11-20 22:48:05.000000000 +0000
+--- target/i386/nvmm/nvmm-all.c.orig	2025-08-26 18:32:38.000000000 +0000
  +++ target/i386/nvmm/nvmm-all.c
-@@ -1057,7 +1057,11 @@ nvmm_process_section(MemoryRegionSection
+@@ -984,7 +984,7 @@ nvmm_init_vcpu(CPUState *cpu)
+         }
+     }
+
+-    qcpu->vcpu_dirty = true;
++    cpu->vcpu_dirty = true;
+     cpu->accel = qcpu;
+
+     return 0;
+@@ -1059,7 +1059,11 @@ nvmm_process_section(MemoryRegionSection
       unsigned int delta;
       uintptr_t hva;

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Markus Armbruster 3 weeks, 5 days ago

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 1/14/26 10:19, Daniel P. Berrangé wrote:
>> This doesn't imply we should automatically rip it out, but if we see
>> no evidence of (3) for a prolonged period of time, and no sign of it
>> being used downstream in any way, it is worth considering the cost /
>> benefit.
>>
>> In the case of NetBSD something must be working to some extent since
>> it appears that 10.1.0 QEMU is present in the pkg repos:
>>
>>     https://pkgsrc.se/emulators/qemu
>>
>> so that argues against ripping stuff out even if we notice breakage.
>
> And indeed their pkgsrc has the same patch that Philippe has now
> submitted for inclusion in qemu.git:
>
> https://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/emulators/qemu/patches/patch-target_i386_nvmm_nvmm-all.c.diff?r1=1.10;r2=1.11
>
> ---- target/i386/nvmm/nvmm-all.c.orig	2024-11-20 22:48:05.000000000 +0000
> +--- target/i386/nvmm/nvmm-all.c.orig	2025-08-26 18:32:38.000000000 +0000
>  +++ target/i386/nvmm/nvmm-all.c
> -@@ -1057,7 +1057,11 @@ nvmm_process_section(MemoryRegionSection
> +@@ -984,7 +984,7 @@ nvmm_init_vcpu(CPUState *cpu)
> +         }
> +     }
> +
> +-    qcpu->vcpu_dirty = true;
> ++    cpu->vcpu_dirty = true;
> +     cpu->accel = qcpu;
> +
> +     return 0;
> +@@ -1059,7 +1059,11 @@ nvmm_process_section(MemoryRegionSection
>       unsigned int delta;
>       uintptr_t hva;

That they didn't immediately post the fix upstream is a bit of a
disappointment.  Deep in the weeds, I guess.

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Reinoud Zandijk 3 weeks, 5 days ago

On Wed, Jan 14, 2026 at 11:45:35AM +0100, Markus Armbruster wrote:
> That they didn't immediately post the fix upstream is a bit of a
> disappointment.  Deep in the weeds, I guess.

Sorry to break this but even submitting simple patches and getting them into
the qemu source tree hasn't been an easy nor enjoyable experience to me to
understate it so once in a while, a new version is pulled into pkgsrc and
patches are made on top and if they start to grow or diverge too much a patch
round can be made requested on the qemu repo. This is just my experience of
course.

With regards,
Reinoud

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Thomas Huth 3 weeks, 5 days ago

On 14/01/2026 12.28, Reinoud Zandijk wrote:
> On Wed, Jan 14, 2026 at 11:45:35AM +0100, Markus Armbruster wrote:
>> That they didn't immediately post the fix upstream is a bit of a
>> disappointment.  Deep in the weeds, I guess.
> 
> Sorry to break this but even submitting simple patches and getting them into
> the qemu source tree hasn't been an easy nor enjoyable experience to me

Sorry to hear that (and yes, I agree, it can be difficult indeed some times 
for bigger patch sets), but at least for simple patches like this one-line 
fix, you can always CC: qemu-trivial@nongnu.org which should help to get 
those in rather easily, I hope.

  HTH,
   Thomas

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Paolo Bonzini 3 weeks, 5 days ago

On 1/14/26 12:28, Reinoud Zandijk wrote:
> On Wed, Jan 14, 2026 at 11:45:35AM +0100, Markus Armbruster wrote:
>> That they didn't immediately post the fix upstream is a bit of a
>> disappointment.  Deep in the weeds, I guess.
> 
> Sorry to break this but even submitting simple patches and getting them into
> the qemu source tree hasn't been an easy nor enjoyable experience to me to
> understate it

Have you actually tried since 2021 when your last commit was? 
https://patchew.org/search?q=project%3AQEMU+from%3Areinoud says you 
didn't but I may be missing something.

In fact your only contribution was a really big one, and I say this not 
to understate you work but because *almost anything* would be easier 
than submitting a new accelerator.

> so once in a while, a new version is pulled into pkgsrc and
> patches are made on top and if they start to grow or diverge too much a patch
> round can be made requested on the qemu repo. This is just my experience of
> course.

We're not asking you to keep CI up-to-date (which Thomas is doing now, 
despite having no specific need that I know of to support NetBSD), just 
to *report* failure to build from source and tell us "hey, that's how we 
fixed it".  Otherwise we have the false impression that no one even 
cares about new QEMU on NetBSD.

Thanks,

Paolo

ps: sorry about the previous copy of the message, I somehow fat-fingered 
the "encrypt message" command...

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Reinoud Zandijk 3 weeks, 4 days ago

On Wed, Jan 14, 2026 at 01:17:20PM +0100, Paolo Bonzini wrote:
> On 1/14/26 12:28, Reinoud Zandijk wrote:
> > Sorry to break this but even submitting simple patches and getting them into
> > the qemu source tree hasn't been an easy nor enjoyable experience to me to
> > understate it
> 
> Have you actually tried since 2021 when your last commit was?
> https://patchew.org/search?q=project%3AQEMU+from%3Areinoud says you didn't
> but I may be missing something.
> 
> In fact your only contribution was a really big one, and I say this not to
> understate you work but because *almost anything* would be easier than
> submitting a new accelerator.

I can't claim all the credits :) Most of the low level work was done by maxv,
the author of NVMM. IIRC I updated the local patch and modified it to work
with the newer build framework and committed it into the tree. I have to admit
I haven't contributed more since but I still use Qemu daily, with the NVMM
accelerator :) It makes such a big difference.

> > so once in a while, a new version is pulled into pkgsrc and
> > patches are made on top and if they start to grow or diverge too much a patch
> > round can be made requested on the qemu repo. This is just my experience of
> > course.
> 
> We're not asking you to keep CI up-to-date (which Thomas is doing now,
> despite having no specific need that I know of to support NetBSD), just to
> *report* failure to build from source and tell us "hey, that's how we fixed
> it".  Otherwise we have the false impression that no one even cares about
> new QEMU on NetBSD.

As pkgsrc normally tracks releases and has a directory with local patches on
top of that, build errors only show up when we bump the version. Version 10.2
is now in pkgsrc though I am running 10.1.3 so there is surely interest in
keeping it running. I could try to commit some more trivial patches in the
repo here to reduce the diffs some more.

Thanks for the feedback,
Reinoud

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Markus Armbruster 3 weeks, 4 days ago

Reinoud Zandijk <reinoud@NetBSD.org> writes:

> On Wed, Jan 14, 2026 at 01:17:20PM +0100, Paolo Bonzini wrote:
>> On 1/14/26 12:28, Reinoud Zandijk wrote:

[...]

>> We're not asking you to keep CI up-to-date (which Thomas is doing now,
>> despite having no specific need that I know of to support NetBSD), just to
>> *report* failure to build from source and tell us "hey, that's how we fixed
>> it".  Otherwise we have the false impression that no one even cares about
>> new QEMU on NetBSD.
>
> As pkgsrc normally tracks releases and has a directory with local patches on
> top of that, build errors only show up when we bump the version. Version 10.2
> is now in pkgsrc though I am running 10.1.3 so there is surely interest in
> keeping it running. I could try to commit some more trivial patches in the
> repo here to reduce the diffs some more.

Fixing the build long after it broke is less than ideal.  What would it
take to get NetBSD covered in CI?

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Alex Bennée 3 weeks, 4 days ago

Markus Armbruster <armbru@redhat.com> writes:

> Reinoud Zandijk <reinoud@NetBSD.org> writes:
>
>> On Wed, Jan 14, 2026 at 01:17:20PM +0100, Paolo Bonzini wrote:
>>> On 1/14/26 12:28, Reinoud Zandijk wrote:
>
> [...]
>
>>> We're not asking you to keep CI up-to-date (which Thomas is doing now,
>>> despite having no specific need that I know of to support NetBSD), just to
>>> *report* failure to build from source and tell us "hey, that's how we fixed
>>> it".  Otherwise we have the false impression that no one even cares about
>>> new QEMU on NetBSD.
>>
>> As pkgsrc normally tracks releases and has a directory with local patches on
>> top of that, build errors only show up when we bump the version. Version 10.2
>> is now in pkgsrc though I am running 10.1.3 so there is surely interest in
>> keeping it running. I could try to commit some more trivial patches in the
>> repo here to reduce the diffs some more.
>
> Fixing the build long after it broke is less than ideal.  What would it
> take to get NetBSD covered in CI?

We have the ability to run tests via the vm-build:

  make vm-build-netbsd

however to take full advantage that needs a runner with a reasonable
amount of memory and access to KVM so it can run reasonably fast. Does
anyone want to host a dedicated x86_64 custom runner?


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Philippe Mathieu-Daudé 3 weeks, 4 days ago

On 15/1/26 17:43, Alex Bennée wrote:
> Markus Armbruster <armbru@redhat.com> writes:
> 
>> Reinoud Zandijk <reinoud@NetBSD.org> writes:
>>
>>> On Wed, Jan 14, 2026 at 01:17:20PM +0100, Paolo Bonzini wrote:
>>>> On 1/14/26 12:28, Reinoud Zandijk wrote:
>>
>> [...]
>>
>>>> We're not asking you to keep CI up-to-date (which Thomas is doing now,
>>>> despite having no specific need that I know of to support NetBSD), just to
>>>> *report* failure to build from source and tell us "hey, that's how we fixed
>>>> it".  Otherwise we have the false impression that no one even cares about
>>>> new QEMU on NetBSD.
>>>
>>> As pkgsrc normally tracks releases and has a directory with local patches on
>>> top of that, build errors only show up when we bump the version. Version 10.2
>>> is now in pkgsrc though I am running 10.1.3 so there is surely interest in
>>> keeping it running. I could try to commit some more trivial patches in the
>>> repo here to reduce the diffs some more.
>>
>> Fixing the build long after it broke is less than ideal.  What would it
>> take to get NetBSD covered in CI?
> 
> We have the ability to run tests via the vm-build:
> 
>    make vm-build-netbsd
> 
> however to take full advantage that needs a runner with a reasonable
> amount of memory and access to KVM so it can run reasonably fast. Does
> anyone want to host a dedicated x86_64 custom runner?

Pierrick had a clever & simpler way to get that:
https://lore.kernel.org/qemu-devel/ae956364-78e6-4067-b0eb-94213891a112@linaro.org/

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Paolo Bonzini 3 weeks, 5 days ago

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Paolo Bonzini 3 weeks, 5 days ago

On 1/14/26 11:45, Markus Armbruster wrote:
>> ---- target/i386/nvmm/nvmm-all.c.orig	2024-11-20 22:48:05.000000000 +0000
>> +--- target/i386/nvmm/nvmm-all.c.orig	2025-08-26 18:32:38.000000000 +0000
>>   +++ target/i386/nvmm/nvmm-all.c
>> -@@ -1057,7 +1057,11 @@ nvmm_process_section(MemoryRegionSection
>> +@@ -984,7 +984,7 @@ nvmm_init_vcpu(CPUState *cpu)
>> +         }
>> +     }
>> +
>> +-    qcpu->vcpu_dirty = true;
>> ++    cpu->vcpu_dirty = true;
>> +     cpu->accel = qcpu;
>> +
>> +     return 0;
>> +@@ -1059,7 +1059,11 @@ nvmm_process_section(MemoryRegionSection
>>        unsigned int delta;
>>        uintptr_t hva;
> 
> That they didn't immediately post the fix upstream is a bit of a
> disappointment.  Deep in the weeds, I guess.

Eh, it happens. :/  For the future, since Ryo and Reinoud are in Cc, it 
would have been enough to contact anyone on IRC or reply to the release 
announcement.

Paolo

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Markus Armbruster 3 weeks, 5 days ago

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 1/14/26 11:45, Markus Armbruster wrote:
>>> ---- target/i386/nvmm/nvmm-all.c.orig	2024-11-20 22:48:05.000000000 +0000
>>> +--- target/i386/nvmm/nvmm-all.c.orig	2025-08-26 18:32:38.000000000 +0000
>>>   +++ target/i386/nvmm/nvmm-all.c
>>> -@@ -1057,7 +1057,11 @@ nvmm_process_section(MemoryRegionSection
>>> +@@ -984,7 +984,7 @@ nvmm_init_vcpu(CPUState *cpu)
>>> +         }
>>> +     }
>>> +
>>> +-    qcpu->vcpu_dirty = true;
>>> ++    cpu->vcpu_dirty = true;
>>> +     cpu->accel = qcpu;
>>> +
>>> +     return 0;
>>> +@@ -1059,7 +1059,11 @@ nvmm_process_section(MemoryRegionSection
>>>        unsigned int delta;
>>>        uintptr_t hva;
>> That they didn't immediately post the fix upstream is a bit of a
>> disappointment.  Deep in the weeds, I guess.
>
> Eh, it happens. :/  For the future, since Ryo and Reinoud are in Cc, it would have been enough to contact anyone on IRC or reply to the release announcement.

Yes, it happens, especially when you're deep in the weeds.

Policy regarding platforms / features with out CI coverage (was: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field)

Posted by Markus Armbruster 3 weeks, 5 days ago

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Wed, Jan 14, 2026 at 08:38:26AM +0100, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:

[...]

>> > Should we add a policy for bitrotting untested code?
>> 
>> What policy do you have in mind?
>
> Implicitly I consider the policy to effectively be
>
>  (1) Platforms untested by GitLab CI are not guaranteed to work at
>      any time, including in formal releases.
>  (2) No contributors/maintainers are obligated to investigate or
>      fix breakage in untested platforms
>  (3) Those whom care about the platform are expected to perform
>      manual testing and contribute fixes as required.
>
> This doesn't imply we should automatically rip it out, but if we see
> no evidence of (3) for a prolonged period of time, and no sign of it
> being used downstream in any way, it is worth considering the cost /
> benefit.

Sounds good to me.  Put it in docs/ somewhere?

[...]

Re: [PULL 13/31] accel/nvmm: Replace @dirty field by generic CPUState::vcpu_dirty field

Posted by Philippe Mathieu-Daudé 3 weeks, 6 days ago

On 13/1/26 21:30, Philippe Mathieu-Daudé wrote:
> On 13/1/26 20:32, Thomas Huth wrote:
>> On 04/07/2025 12.14, Philippe Mathieu-Daudé wrote:
>>> No need for accel-specific @dirty field when we have
>>> a generic one in CPUState.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
>>> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>>> Message-Id: <20250703173248.44995-20-philmd@linaro.org>
>>> ---
>>>   target/i386/nvmm/nvmm-all.c | 21 ++++++++++-----------
>>>   1 file changed, 10 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/target/i386/nvmm/nvmm-all.c b/target/i386/nvmm/nvmm-all.c
>>> index f1c6120ccf1..aea61a6fd2a 100644
>>> --- a/target/i386/nvmm/nvmm-all.c
>>> +++ b/target/i386/nvmm/nvmm-all.c
>> ...
>>> @@ -982,7 +981,7 @@ nvmm_init_vcpu(CPUState *cpu)
>>>           }
>>>       }
>>> -    qcpu->dirty = true;
>>> +    qcpu->vcpu_dirty = true;
>>>       cpu->accel = qcpu;
>>>       return 0;
>>
>> FYI, this does not seem to compile:
>>
>> ../src/target/i386/nvmm/nvmm-all.c: In function 'nvmm_init_vcpu':
>> ../src/target/i386/nvmm/nvmm-all.c:988:9: error: 'AccelCPUState' has 
>> no member named 'vcpu_dirty'
>>    988 |     qcpu->vcpu_dirty = true;
>>        |         ^~
> 
> s/qcpu/cpu/
> 
>>
>> Is anybody checking the netbsd builds at all?
> 
> 3 reviewers and 6 months later.

and 7 commits on top of this PR :)

$ git log --oneline 2098164a6be..master -- target/i386/nvmm/nvmm-all.c
d5e1d2dea11 target/i386: clear CPU_INTERRUPT_SIPI for all accelerators
5699f6a4409 target/i386/nvmm: Inline cpu_physical_memory_rw() in 
nvmm_mem_callback
9a191d3782d cpus: clear exit_request in qemu_process_cpu_events
f084ff128b6 accel: use atomic accesses for exit_request
ac6c8a390b4 accel: use store_release/load_acquire for cross-thread 
exit_request
602d5ebba26 treewide: clear bits of cs->interrupt_request with 
cpu_reset_interrupt()
87511341c30 add cpu_test_interrupt()/cpu_set_interrupt() helpers and use 
them tree wide
f96b157ebb9 Merge tag 'accel-20250715' of https://github.com/philmd/qemu 
into staging
f7a7e7dd217 accel: Extract AccelClass definition to 'accel/accel-ops.h'
d93972d88b0 target/i386: nvmm, whpx: add accel/CPU class that sets host 
vendor
51e18961999 accel: Propagate AccelState to AccelClass::init_machine()
b6637bd5561 accel/nvmm: Expose nvmm_enabled() to common code

> 
> Should we add a policy for bitrotting untested code?
> 
>> (I'm currently trying to update test/vm/netbsd to version 10.1, that's 
>> how I noticed it)
>>
>>   Thomas
>>
>