Re: [PATCH v10 3/9] KVM: Extend the memslot to support fd-based private memory

[Chao Peng]

On Mon, Dec 19, 2022 at 03:36:28PM +0100, Borislav Petkov wrote:
> On Fri, Dec 02, 2022 at 02:13:41PM +0800, Chao Peng wrote:
> > In memory encryption usage, guest memory may be encrypted with special
> > key and can be accessed only by the guest itself. We call such memory
> > private memory. It's valueless and sometimes can cause problem to allow
> 
> valueless?
> 
> I can't parse that.

It's unnecessary and ...

> 
> > userspace to access guest private memory. This new KVM memslot extension
> > allows guest private memory being provided through a restrictedmem
> > backed file descriptor(fd) and userspace is restricted to access the
> > bookmarked memory in the fd.
> 
> bookmarked?

userspace is restricted to access the memory content in the fd.

> 
> > This new extension, indicated by the new flag KVM_MEM_PRIVATE, adds two
> > additional KVM memslot fields restricted_fd/restricted_offset to allow
> > userspace to instruct KVM to provide guest memory through restricted_fd.
> > 'guest_phys_addr' is mapped at the restricted_offset of restricted_fd
> > and the size is 'memory_size'.
> > 
> > The extended memslot can still have the userspace_addr(hva). When use, a
> 
> "When un use, ..."

When both userspace_addr and restricted_fd/offset were used, ...

> 
> ...
> 
> > diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> > index a8e379a3afee..690cb21010e7 100644
> > --- a/arch/x86/kvm/Kconfig
> > +++ b/arch/x86/kvm/Kconfig
> > @@ -50,6 +50,8 @@ config KVM
> > select INTERVAL_TREE
> > select HAVE_KVM_PM_NOTIFIER if PM
> > select HAVE_KVM_MEMORY_ATTRIBUTES
> > +   select HAVE_KVM_RESTRICTED_MEM if X86_64
> > +   select RESTRICTEDMEM if HAVE_KVM_RESTRICTED_MEM
> 
> Those deps here look weird.
> 
> RESTRICTEDMEM should be selected by TDX_GUEST as it can't live without
> it.

RESTRICTEDMEM is needed by TDX_HOST, not TDX_GUEST.

> 
> Then you don't have to select HAVE_KVM_RESTRICTED_MEM simply because of
> X86_64 - you need that functionality when the respective guest support
> is enabled in KVM.

Letting the actual feature(e.g. TDX or pKVM) select it or add dependency
sounds a viable and clearer solution. Sean, let me know your opinion.

> 
> Then, looking forward into your patchset, I'm not sure you even
> need HAVE_KVM_RESTRICTED_MEM - you could make it all depend on
> CONFIG_RESTRICTEDMEM. But that's KVM folks call - I'd always aim for
> less Kconfig items because we have waay too many.

The only reason to add another HAVE_KVM_RESTRICTED_MEM is some code only
works for 64bit[*] and CONFIG_RESTRICTEDMEM is not sufficient to enforce
that.

[*] https://lore.kernel.org/all/ykjlfu98hzovt...@google.com/

Thanks,
Chao
> 
> Thx.
> 
> -- 
> Regards/Gruss,
> Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] hvf: arm: Add support for GICv3

[Alexander Graf]

Hi Zenghui,

On 20.12.22 08:14, Zenghui Yu wrote:

On 2022/12/20 6:08, Alexander Graf wrote:

We currently only support GICv2 emulation.


Before looking into it, I think it's worth finalizing the GIC version in
the hvf case - only v2 is allowed and fail early if user selects the
unsupported versions. Currently finalize_gic_version() does not deal
with hvf at all.



Currently finalize_gic_version() treats HVF the same as TCG, which is 
incorrect. However, with this patch applied, they happen to match.


I don't think it's worth changing the finalize_gic_version() 
implementation to reflect the gicv2 only state for HVF. Instead, let's 
rather get GICv3 support in and then add explicit handling for HVF on top.


Alex

Re: [PATCH 1/5] target/arm: only build psci for TCG

[Alexander Graf]

Hey Fabiano,

On 19.12.22 12:42, Fabiano Rosas wrote:

Claudio Fontana  writes:


Ciao Alex,

On 12/19/22 11:47, Alexander Graf wrote:

Hey Claudio,

On 19.12.22 09:37, Claudio Fontana wrote:

On 12/16/22 22:59, Alexander Graf wrote:

Hi Claudio,

If the PSCI implementation becomes TCG only, can we also move to a tcg accel 
directory? It slowly gets super confusing to keep track of which files are 
supposed to be generic target code and which ones TCG specific>
Alex

Hi Alex, Fabiano, Peter and all,

that was the plan but at the time of:

https://lore.kernel.org/all/20210416162824.25131-1-cfont...@suse.de/

Peter mentioned that HVF AArch64 might use that code too:

https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg00509.html

so from v2 to v3 the series changed to not move the code under tcg/ , expecting HVF to be 
reusing that code "soon".

I see that your hvf code in hvf/ implements psci, is there some plan to reuse 
pieces from the tcg implementation now?

I originally reused the PSCI code in earlier versions of my hvf patch
set, but then we realized that some functions like remote CPU reset are
wired in a TCG specific view of the world with full target CPU register
ownership. So if we want to actually share it, we'll need to abstract it
up a level.

Hence I'd suggest to move it to a TCG directory for now and then later
move it back into a generic helper if we want / need to. The code just
simply isn't generic yet.

Or alternatively, you create a patch (set) to actually merge the 2
implementations into a generic one again which then can live at a
generic place :)


Alex

Thanks for the clarification, I'll leave the choice up to Fabiano now, since he 
is working on the series currently :-)

Ciao,

Claudio

Hello, thank you all for the comments.

I like the idea of merging the two implementations. However, I won't get
to it anytime soon. There's still ~70 patches in the original series
that I need to understand, rebase and test, including the introduction
of the tcg directory.



Sure, I am definitely fine with leaving them separate for now as well :).



I'd say we merge this as is now, since this patch has no
dependencies. Later when I introduce the tcg directory I can move the
code there along with the other tcg-only files. I'll take note to come
back to the PSCI code as well.


I'm confused about the patch ordering :). Why is it easier to move the 
psci.c compilation target from generic to an if(CONFIG_TCG) only to 
later move it into a tcg/ directory? Wouldn't it be easier to create a 
tcg/ directory from the start and just put it there?


The current approach just looks like duplicate effort to me.


Alex

Re: [PULL 00/21] Hexagon update: bug fixes, performance, idef-parser

[Philippe Mathieu-Daudé]

On 20/12/22 00:19, Philippe Mathieu-Daudé wrote:

Hi,

On 16/12/22 21:48, Taylor Simpson wrote:






12-21)
Instruction definition parser (idef-parser) from rev.ng
Parses the instruction semantics and generates TCG


Building QEMU with Clang I'm now getting:

target/hexagon/idef-parser.p/idef-parser.tab.c:2197:9: error: variable 
'yynerrs' set but not used [-Werror,-Wunused-but-set-variable]

     int yynerrs = 0;
     ^


idef-parser.tab.c is built using:

193 idef_parser = executable(
194 'idef-parser',
195 [flex.process(idef_parser_dir / 'idef-parser.lex'),
196  bison.process(idef_parser_dir / 'idef-parser.y'),
197  idef_parser_dir / 'parser-helpers.c'],
198 include_directories: ['idef-parser', '../../include/'],
199 dependencies: [glib_dep],
200 c_args: ['-Wextra'],
  
201 native: true
202 )

(see commit c0a41ee631 "target/hexagon: import parser for idef-parser")

Do we really need this level? IIUC the problem with -Wextra is using a
newer compiler toolchain it can include warnings we haven't fixed. Maybe
worthwhile but it can break from times to times.

Using '-Wextra -Wno-unused-but-set-variable' seems a hack. I guess I'd
simply remove -Wextra for simplicity, since no much value is added here.

Regards,

Phil.

Re: [PATCH v10 2/9] KVM: Introduce per-page memory attributes

[Chao Peng]

On Mon, Dec 19, 2022 at 11:17:22AM +0100, Borislav Petkov wrote:
> On Mon, Dec 19, 2022 at 04:15:32PM +0800, Chao Peng wrote:
> > Tamping down with error number a bit:
> > 
> > if (attrs->flags)
> > return -ENXIO;
> > if (attrs->attributes & ~supported_attrs)
> > return -EOPNOTSUPP;
> > if (!PAGE_ALIGNED(attrs->address) || !PAGE_ALIGNED(attrs->size) ||
> > attrs->size == 0)
> > return -EINVAL;
> > if (attrs->address + attrs->size < attrs->address)
> > return -E2BIG;
> 
> Yap, better.
> 
> I guess you should add those to the documentation of the ioctl too
> so that people can find out why it fails. Or, well, they can look
> at the code directly too but still... imagine some blurb about
> user-friendliness here...

Thanks for reminding. Yes KVM api doc is the right place to put these
documentation in.

Thanks,
Chao
> 
> :-)
> 
> -- 
> Regards/Gruss,
> Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v10 1/9] mm: Introduce memfd_restricted system call to create restricted user memory

[Chao Peng]

On Mon, Dec 19, 2022 at 08:48:10AM +, Huang, Kai wrote:
> On Mon, 2022-12-19 at 15:53 +0800, Chao Peng wrote:
> > > 
> > > [...]
> > > 
> > > > +
> > > > +   /*
> > > > +* These pages are currently unmovable so don't place them into
> > > > movable
> > > > +* pageblocks (e.g. CMA and ZONE_MOVABLE).
> > > > +*/
> > > > +   mapping = memfd->f_mapping;
> > > > +   mapping_set_unevictable(mapping);
> > > > +   mapping_set_gfp_mask(mapping,
> > > > +    mapping_gfp_mask(mapping) & 
> > > > ~__GFP_MOVABLE);
> > > 
> > > But, IIUC removing __GFP_MOVABLE flag here only makes page allocation from
> > > non-
> > > movable zones, but doesn't necessarily prevent page from being migrated.  
> > > My
> > > first glance is you need to implement either a_ops->migrate_folio() or 
> > > just
> > > get_page() after faulting in the page to prevent.
> > 
> > The current api restrictedmem_get_page() already does this, after the
> > caller calling it, it holds a reference to the page. The caller then
> > decides when to call put_page() appropriately.
> 
> I tried to dig some history. Perhaps I am missing something, but it seems 
> Kirill
> said in v9 that this code doesn't prevent page migration, and we need to
> increase page refcount in restrictedmem_get_page():
> 
> https://lore.kernel.org/linux-mm/20221129112139.usp6dqhbih47q...@box.shutemov.name/
> 
> But looking at this series it seems restrictedmem_get_page() in this v10 is
> identical to the one in v9 (except v10 uses 'folio' instead of 'page')?

restrictedmem_get_page() increases page refcount several versions ago so
no change in v10 is needed. You probably missed my reply:

https://lore.kernel.org/linux-mm/20221129135844.ga902...@chaop.bj.intel.com/

The current solution is clear: unless we have better approach, we will
let restrictedmem user (KVM in this case) to hold the refcount to
prevent page migration.

Thanks,
Chao
> 
> Anyway if this is not fixed, then it should be fixed.  Otherwise, a comment at
> the place where page refcount is increased will be helpful to help people
> understand page migration is actually prevented.
> 

Re: [PATCH v2 2/3] hvf: implement guest debugging on Apple Silicon hosts

[Francesco Cagnin]

Hi,
indeed, the patch doesn't keep separate copies of debug registers and
just directly sets cp15.* values, as I was not aware of the issue—thanks
for the detailed explanation.

I remain available to implement the required fixes, but I'd need
some guidance on how to proceed. Thanks,
-- 
Francesco

Re: [PATCH 01/11] contrib/gitdm: add C-Sky to the domain map

[Guo Ren]

On Tue, Dec 20, 2022 at 9:13 AM LIU Zhiwei  wrote:
>
> The c-sky domain mail is no longer being used for Guo Ren and me.
>
> C-SKY company and C-SKY arch is still existing, but its website domain
> has been moved to https://www.t-head.cn/.
Acked-by: Guo Ren 

>
> Otherwise,
>
> Reviewed-by: LIU Zhiwei 
>
>
> Thanks,
> Zhiwei
>
> On 2022/12/19 20:19, Alex Bennée wrote:
> > Although the majority of the recent work has been done by LIU Zhiwei
> > it does appear to be a corporate effort.
> >
> > Signed-off-by: Alex Bennée 
> > Cc: Guo Ren 
> > Cc: Liu Zhiwei 
> > ---
> >   contrib/gitdm/domain-map | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map
> > index 3727918641..6f3a24b399 100644
> > --- a/contrib/gitdm/domain-map
> > +++ b/contrib/gitdm/domain-map
> > @@ -7,6 +7,7 @@
> >   amd.com AMD
> >   baidu.com   Baidu
> >   bytedance.com   ByteDance
> > +c-sky.com   C-Sky
> >   cmss.chinamobile.com China Mobile
> >   citrix.com  Citrix
> >   crudebyte.com   Crudebyte
>


-- 
Best Regards
 Guo Ren

Re: [PATCH] hvf: arm: Add support for GICv3

[Zenghui Yu via]

On 2022/12/20 6:08, Alexander Graf wrote:

We currently only support GICv2 emulation.


Before looking into it, I think it's worth finalizing the GIC version in
the hvf case - only v2 is allowed and fail early if user selects the
unsupported versions. Currently finalize_gic_version() does not deal
with hvf at all.

Re: [PATCH 5/4] target/i386: Restrict 'qapi-commands-machine.h' to system emulation

[Philippe Mathieu-Daudé]

On 20/12/22 01:01, Richard Henderson wrote:

On 12/19/22 13:22, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daude

Since commit a0e61807a3 ("qapi: Remove QMP events and commands from
user-mode builds") we don't generate the "qapi-commands-machine.h"
header in a user-emulation-only build.

Guard qmp_query_cpu_definitions() within CONFIG_USER_ONLY; move
x86_cpu_class_check_missing_features() closer since it is only used
by this QMP command handler.

Signed-off-by: Philippe Mathieu-Daudé
---
  target/i386/cpu.c | 74 +--
  1 file changed, 39 insertions(+), 35 deletions(-)


Not moving these out to a separate file?  Anyway,


Few static functions are used, making it hard to split it out.
I'll have another look.


Reviewed-by: Richard Henderson 


Thanks!

Re: [PATCH v11 5/5] docs: Add generic vhost-vdpa device documentation

[longpeng2--- via]

在 2022/12/20 14:15, Michael S. Tsirkin 写道:

On Tue, Dec 20, 2022 at 08:02:51AM +0800, Longpeng (Mike, Cloud Infrastructure 
Service Product Dept.) wrote:



在 2022/12/20 5:37, Michael S. Tsirkin 写道:

On Fri, Dec 16, 2022 at 11:33:49AM +0800, Jason Wang wrote:

On Thu, Dec 15, 2022 at 9:50 PM Longpeng(Mike)  wrote:


From: Longpeng 

Signed-off-by: Longpeng 
---
   .../devices/vhost-vdpa-generic-device.rst | 68 +++
   1 file changed, 68 insertions(+)
   create mode 100644 docs/system/devices/vhost-vdpa-generic-device.rst

diff --git a/docs/system/devices/vhost-vdpa-generic-device.rst 
b/docs/system/devices/vhost-vdpa-generic-device.rst
new file mode 100644
index 00..24c825ef1a
--- /dev/null
+++ b/docs/system/devices/vhost-vdpa-generic-device.rst
@@ -0,0 +1,68 @@
+
+=
+vhost-vDPA generic device
+=
+
+This document explains the usage of the vhost-vDPA generic device.
+
+
+Description
+---
+
+vDPA(virtio data path acceleration) device is a device that uses a datapath
+which complies with the virtio specifications with vendor specific control
+path.
+
+QEMU provides two types of vhost-vDPA devices to enable the vDPA device, one
+is type sensitive which means QEMU needs to know the actual device type
+(e.g. net, blk, scsi) and another is called "vhost-vDPA generic device" which
+is type insensitive.
+
+The vhost-vDPA generic device builds on the vhost-vdpa subsystem and virtio
+subsystem. It is quite small, but it can support any type of virtio device.
+
+
+Requirements
+
+Linux 5.18+
+iproute2/vdpa 5.12.0+
+
+
+Examples
+
+
+1. Prepare the vhost-vDPA backends, here is an example using vdpa_sim_blk
+   device:
+
+::
+  host# modprobe vhost_vdpa
+  host# modprobe vdpa_sim_blk


Nit: it's probably better to add driver binding steps here.


+  host# vdpa dev add mgmtdev vdpasim_blk name blk0
+  (...you can see the vhost-vDPA device under /dev directory now...)


And then the vhost char dev name could be fetch via

ls /sys/bus/vdpa/device/blk0/vhost-vdpa*

With the above changes.

Acked-by: Jason Wang 

Thanks



Sounds minor enough, I'll queue, pls fix with a patch on top.


OK, thanks. I'll send when I'm free.


Is this going to be in the next couple of weeks? then ok.


I think I can send out during this weekend.


We do want this addressed by let's say rc3.


+  host# ls -l /dev/vhost-vdpa-*
+  crw--- 1 root root 236, 0 Nov  2 00:49 /dev/vhost-vdpa-0
+
+Note:
+It needs some vendor-specific steps to provision the vDPA device if you're
+using real HW devices, such as loading the vendor-specific vDPA driver and
+binding the device to the driver.
+
+
+2. Start the virtual machine:
+
+Start QEMU with virtio-mmio bus:
+
+::
+  host# qemu-system  \
+  -M microvm -m 512 -smp 2 -kernel ... -initrd ...   \
+  -device vhost-vdpa-device,vhostdev=/dev/vhost-vdpa-0   \
+  ...
+
+
+Start QEMU with virtio-pci bus:
+
+::
+  host# qemu-system  \
+  -M pc -m 512 -smp 2\
+  -device vhost-vdpa-device-pci,vhostdev=/dev/vhost-vdpa-0   \
+  ...
--
2.23.0



.


.

Re: [PATCH v11 5/5] docs: Add generic vhost-vdpa device documentation

[Michael S. Tsirkin]

On Tue, Dec 20, 2022 at 08:02:51AM +0800, Longpeng (Mike, Cloud Infrastructure 
Service Product Dept.) wrote:
> 
> 
> 在 2022/12/20 5:37, Michael S. Tsirkin 写道:
> > On Fri, Dec 16, 2022 at 11:33:49AM +0800, Jason Wang wrote:
> > > On Thu, Dec 15, 2022 at 9:50 PM Longpeng(Mike)  
> > > wrote:
> > > > 
> > > > From: Longpeng 
> > > > 
> > > > Signed-off-by: Longpeng 
> > > > ---
> > > >   .../devices/vhost-vdpa-generic-device.rst | 68 +++
> > > >   1 file changed, 68 insertions(+)
> > > >   create mode 100644 docs/system/devices/vhost-vdpa-generic-device.rst
> > > > 
> > > > diff --git a/docs/system/devices/vhost-vdpa-generic-device.rst 
> > > > b/docs/system/devices/vhost-vdpa-generic-device.rst
> > > > new file mode 100644
> > > > index 00..24c825ef1a
> > > > --- /dev/null
> > > > +++ b/docs/system/devices/vhost-vdpa-generic-device.rst
> > > > @@ -0,0 +1,68 @@
> > > > +
> > > > +=
> > > > +vhost-vDPA generic device
> > > > +=
> > > > +
> > > > +This document explains the usage of the vhost-vDPA generic device.
> > > > +
> > > > +
> > > > +Description
> > > > +---
> > > > +
> > > > +vDPA(virtio data path acceleration) device is a device that uses a 
> > > > datapath
> > > > +which complies with the virtio specifications with vendor specific 
> > > > control
> > > > +path.
> > > > +
> > > > +QEMU provides two types of vhost-vDPA devices to enable the vDPA 
> > > > device, one
> > > > +is type sensitive which means QEMU needs to know the actual device type
> > > > +(e.g. net, blk, scsi) and another is called "vhost-vDPA generic 
> > > > device" which
> > > > +is type insensitive.
> > > > +
> > > > +The vhost-vDPA generic device builds on the vhost-vdpa subsystem and 
> > > > virtio
> > > > +subsystem. It is quite small, but it can support any type of virtio 
> > > > device.
> > > > +
> > > > +
> > > > +Requirements
> > > > +
> > > > +Linux 5.18+
> > > > +iproute2/vdpa 5.12.0+
> > > > +
> > > > +
> > > > +Examples
> > > > +
> > > > +
> > > > +1. Prepare the vhost-vDPA backends, here is an example using 
> > > > vdpa_sim_blk
> > > > +   device:
> > > > +
> > > > +::
> > > > +  host# modprobe vhost_vdpa
> > > > +  host# modprobe vdpa_sim_blk
> > > 
> > > Nit: it's probably better to add driver binding steps here.
> > > 
> > > > +  host# vdpa dev add mgmtdev vdpasim_blk name blk0
> > > > +  (...you can see the vhost-vDPA device under /dev directory now...)
> > > 
> > > And then the vhost char dev name could be fetch via
> > > 
> > > ls /sys/bus/vdpa/device/blk0/vhost-vdpa*
> > > 
> > > With the above changes.
> > > 
> > > Acked-by: Jason Wang 
> > > 
> > > Thanks
> > 
> > 
> > Sounds minor enough, I'll queue, pls fix with a patch on top.
> > 
> OK, thanks. I'll send when I'm free.

Is this going to be in the next couple of weeks? then ok.
We do want this addressed by let's say rc3.

> > > > +  host# ls -l /dev/vhost-vdpa-*
> > > > +  crw--- 1 root root 236, 0 Nov  2 00:49 /dev/vhost-vdpa-0
> > > > +
> > > > +Note:
> > > > +It needs some vendor-specific steps to provision the vDPA device if 
> > > > you're
> > > > +using real HW devices, such as loading the vendor-specific vDPA driver 
> > > > and
> > > > +binding the device to the driver.
> > > > +
> > > > +
> > > > +2. Start the virtual machine:
> > > > +
> > > > +Start QEMU with virtio-mmio bus:
> > > > +
> > > > +::
> > > > +  host# qemu-system  \
> > > > +  -M microvm -m 512 -smp 2 -kernel ... -initrd ...   \
> > > > +  -device vhost-vdpa-device,vhostdev=/dev/vhost-vdpa-0   \
> > > > +  ...
> > > > +
> > > > +
> > > > +Start QEMU with virtio-pci bus:
> > > > +
> > > > +::
> > > > +  host# qemu-system  \
> > > > +  -M pc -m 512 -smp 2\
> > > > +  -device vhost-vdpa-device-pci,vhostdev=/dev/vhost-vdpa-0   \
> > > > +  ...
> > > > --
> > > > 2.23.0
> > > > 
> > 
> > .

[PATCH v3 2/2] target/ppc: Check DEXCR on hash{st, chk} instructions

[Nicholas Miehlbradt]

Adds checks to the hashst and hashchk instructions to only execute if
enabled by the relevant aspect in the DEXCR and HDEXCR.

This behaviour is guarded behind TARGET_PPC64 since Power10 is
currently the only implementation which has the DEXCR.

Reviewed-by: Daniel Henrique Barboza 
Signed-off-by: Nicholas Miehlbradt 
---
 target/ppc/excp_helper.c | 58 +---
 1 file changed, 43 insertions(+), 15 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 94adcb766b..add4d54ae7 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2902,29 +2902,57 @@ static uint64_t hash_digest(uint64_t ra, uint64_t rb, 
uint64_t key)
 return stage1_h ^ stage1_l;
 }
 
+static void do_hash(CPUPPCState *env, target_ulong ea, target_ulong ra,
+target_ulong rb, uint64_t key, bool store)
+{
+uint64_t calculated_hash = hash_digest(ra, rb, key), loaded_hash;
+
+if (store) {
+cpu_stq_data_ra(env, ea, calculated_hash, GETPC());
+} else {
+loaded_hash = cpu_ldq_data_ra(env, ea, GETPC());
+if (loaded_hash != calculated_hash) {
+raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
+POWERPC_EXCP_TRAP, GETPC());
+}
+}
+}
+
 #include "qemu/guest-random.h"
 
-#define HELPER_HASH(op, key, store)   \
+#ifdef TARGET_PPC64
+#define HELPER_HASH(op, key, store, dexcr_aspect) \
 void helper_##op(CPUPPCState *env, target_ulong ea, target_ulong ra,  \
  target_ulong rb) \
 { \
-uint64_t calculated_hash = hash_digest(ra, rb, key), loaded_hash; \
-  \
-if (store) {  \
-cpu_stq_data_ra(env, ea, calculated_hash, GETPC());   \
-} else {  \
-loaded_hash = cpu_ldq_data_ra(env, ea, GETPC());  \
-if (loaded_hash != calculated_hash) { \
-raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, \
-POWERPC_EXCP_TRAP, GETPC());  \
-} \
+if (env->msr & R_MSR_PR_MASK) {   \
+if (!(env->spr[SPR_DEXCR] & R_DEXCR_PRO_##dexcr_aspect##_MASK ||  \
+env->spr[SPR_HDEXCR] & R_HDEXCR_ENF_##dexcr_aspect##_MASK))   \
+return;   \
+} else if (!(env->msr & R_MSR_HV_MASK)) { \
+if (!(env->spr[SPR_DEXCR] & R_DEXCR_PNH_##dexcr_aspect##_MASK ||  \
+env->spr[SPR_HDEXCR] & R_HDEXCR_ENF_##dexcr_aspect##_MASK))   \
+return;   \
+} else if (!(env->msr & R_MSR_S_MASK)) {  \
+if (!(env->spr[SPR_HDEXCR] & R_HDEXCR_HNU_##dexcr_aspect##_MASK)) \
+return;   \
 } \
+  \
+do_hash(env, ea, ra, rb, key, store); \
+}
+#else
+#define HELPER_HASH(op, key, store, dexcr_aspect) \
+void helper_##op(CPUPPCState *env, target_ulong ea, target_ulong ra,  \
+ target_ulong rb) \
+{ \
+do_hash(env, ea, ra, rb, key, store); \
 }
+#endif /* TARGET_PPC64 */
 
-HELPER_HASH(HASHST, env->spr[SPR_HASHKEYR], true)
-HELPER_HASH(HASHCHK, env->spr[SPR_HASHKEYR], false)
-HELPER_HASH(HASHSTP, env->spr[SPR_HASHPKEYR], true)
-HELPER_HASH(HASHCHKP, env->spr[SPR_HASHPKEYR], false)
+HELPER_HASH(HASHST, env->spr[SPR_HASHKEYR], true, NPHIE)
+HELPER_HASH(HASHCHK, env->spr[SPR_HASHKEYR], false, NPHIE)
+HELPER_HASH(HASHSTP, env->spr[SPR_HASHPKEYR], true, PHIE)
+HELPER_HASH(HASHCHKP, env->spr[SPR_HASHPKEYR], false, PHIE)
 #endif /* CONFIG_TCG */
 
 #if !defined(CONFIG_USER_ONLY)
-- 
2.34.1

[PATCH v3 1/2] target/ppc: Implement the DEXCR and HDEXCR

[Nicholas Miehlbradt]

Define the DEXCR and HDEXCR as special purpose registers.

Each register occupies two SPR indicies, one which can be read in an
unprivileged state and one which can be modified in the appropriate
priviliged state, however both indicies refer to the same underlying
value.

Note that the ISA uses the abbreviation UDEXCR in two different
contexts: the userspace DEXCR, the SPR index which can be read from
userspace (implemented in this patch), and the ultravisor DEXCR, the
equivalent register for the ultravisor state (not implemented).

Signed-off-by: Nicholas Miehlbradt 
---
v2: Clearing of upper 32 bits of DEXCR is now performed on read from
problem state rather than on write in privileged state.
v3: Fix typos
---
 target/ppc/cpu.h| 19 +++
 target/ppc/cpu_init.c   | 25 +
 target/ppc/spr_common.h |  1 +
 target/ppc/translate.c  | 19 +++
 4 files changed, 64 insertions(+)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 81d4263a07..3923f174f8 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1068,6 +1068,21 @@ struct ppc_radix_page_info {
 uint32_t entries[PPC_PAGE_SIZES_MAX_SZ];
 };
 
+/*/
+/* Dynamic Execution Control Register */
+
+#define DEXCR_ASPECT(name, num)\
+FIELD(DEXCR, PNH_##name, PPC_BIT_NR(num), 1)   \
+FIELD(DEXCR, PRO_##name, PPC_BIT_NR(num + 32), 1)  \
+FIELD(HDEXCR, HNU_##name, PPC_BIT_NR(num), 1)  \
+FIELD(HDEXCR, ENF_##name, PPC_BIT_NR(num + 32), 1) \
+
+DEXCR_ASPECT(SBHE, 0)
+DEXCR_ASPECT(IBRTPD, 1)
+DEXCR_ASPECT(SRAPD, 4)
+DEXCR_ASPECT(NPHIE, 5)
+DEXCR_ASPECT(PHIE, 6)
+
 /*/
 /* The whole PowerPC CPU context */
 
@@ -1674,9 +1689,11 @@ void ppc_compat_add_property(Object *obj, const char 
*name,
 #define SPR_BOOKE_GIVOR13 (0x1BC)
 #define SPR_BOOKE_GIVOR14 (0x1BD)
 #define SPR_TIR   (0x1BE)
+#define SPR_UHDEXCR   (0x1C7)
 #define SPR_PTCR  (0x1D0)
 #define SPR_HASHKEYR  (0x1D4)
 #define SPR_HASHPKEYR (0x1D5)
+#define SPR_HDEXCR(0x1D7)
 #define SPR_BOOKE_SPEFSCR (0x200)
 #define SPR_Exxx_BBEAR(0x201)
 #define SPR_Exxx_BBTAR(0x202)
@@ -1865,8 +1882,10 @@ void ppc_compat_add_property(Object *obj, const char 
*name,
 #define SPR_RCPU_L2U_RA2  (0x32A)
 #define SPR_MPC_MD_DBRAM1 (0x32A)
 #define SPR_RCPU_L2U_RA3  (0x32B)
+#define SPR_UDEXCR(0x32C)
 #define SPR_TAR   (0x32F)
 #define SPR_ASDR  (0x330)
+#define SPR_DEXCR (0x33C)
 #define SPR_IC(0x350)
 #define SPR_VTB   (0x351)
 #define SPR_MMCRC (0x353)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index cbf0081374..6433f4fdfd 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5727,6 +5727,30 @@ static void register_power10_hash_sprs(CPUPPCState *env)
 hashpkeyr_initial_value);
 }
 
+static void register_power10_dexcr_sprs(CPUPPCState *env)
+{
+spr_register(env, SPR_DEXCR, "DEXCR",
+SPR_NOACCESS, SPR_NOACCESS,
+_read_generic, _write_generic,
+0);
+
+spr_register(env, SPR_UDEXCR, "DEXCR",
+_read_dexcr_ureg, SPR_NOACCESS,
+_read_dexcr_ureg, SPR_NOACCESS,
+0);
+
+spr_register_hv(env, SPR_HDEXCR, "HDEXCR",
+SPR_NOACCESS, SPR_NOACCESS,
+SPR_NOACCESS, SPR_NOACCESS,
+_read_generic, _write_generic,
+0);
+
+spr_register(env, SPR_UHDEXCR, "HDEXCR",
+_read_dexcr_ureg, SPR_NOACCESS,
+_read_dexcr_ureg, SPR_NOACCESS,
+0);
+}
+
 /*
  * Initialize PMU counter overflow timers for Power8 and
  * newer Power chips when using TCG.
@@ -6402,6 +6426,7 @@ static void init_proc_POWER10(CPUPPCState *env)
 register_power8_rpr_sprs(env);
 register_power9_mmu_sprs(env);
 register_power10_hash_sprs(env);
+register_power10_dexcr_sprs(env);
 
 /* FIXME: Filter fields properly based on privilege level */
 spr_register_kvm_hv(env, SPR_PSSCR, "PSSCR", NULL, NULL, NULL, NULL,
diff --git a/target/ppc/spr_common.h b/target/ppc/spr_common.h
index b5a5bc6895..8437eb0340 100644
--- a/target/ppc/spr_common.h
+++ b/target/ppc/spr_common.h
@@ -195,6 +195,7 @@ void spr_read_ebb_upper32(DisasContext *ctx, int gprn, int 
sprn);
 void spr_write_ebb_upper32(DisasContext *ctx, int sprn, int gprn);
 void spr_write_hmer(DisasContext *ctx, int sprn, int gprn);
 void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn);
+void spr_read_dexcr_ureg(DisasContext *ctx, int gprn, int sprn);
 #endif
 
 void register_low_BATs(CPUPPCState *env);
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 19c1d17cb0..0e4cabcf54 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1249,6 +1249,25 @@ void 

[PATCH v3 0/2] target/ppc: Implement Dynamic Execution Control Registers

[Nicholas Miehlbradt]

Implements the Dynamic Execution Control Register (DEXCR) and the
Hypervisor Dynamic Execution Control Register (HDEXCR) in TCG as
defined in Power ISA 3.1B. Only aspects 5 (Non-privileged hash instruction
enable) and 6 (Privileged hash instruction enable) have architectural
effects. Other aspects can be manipulated but have no effect on execution.

Adds checks to these registers in the hashst and hashchk instructions so
that they are executed as nops when not enabled.

There is currently an RFC for the kernel interface for the DEXCR on the 
Linux PPC mailing list:
https://lore.kernel.org/linuxppc-dev/20221128024458.46121-1-bg...@linux.ibm.com/

v3:
 - Fix typos

v2:
 - Clearing of upper 32 bits of userspace registers is now performed on 
   register read rather than register write. 

Nicholas Miehlbradt (2):
  target/ppc: Implement the DEXCR and HDEXCR
  target/ppc: Check DEXCR on hash{st, chk} instructions

 target/ppc/cpu.h | 19 +
 target/ppc/cpu_init.c| 25 +
 target/ppc/excp_helper.c | 58 +---
 target/ppc/spr_common.h  |  1 +
 target/ppc/translate.c   | 19 +
 5 files changed, 107 insertions(+), 15 deletions(-)

-- 
2.34.1

Re: [PATCH 01/11] contrib/gitdm: add C-Sky to the domain map

[LIU Zhiwei]

The c-sky domain mail is no longer being used for Guo Ren and me.

C-SKY company and C-SKY arch is still existing, but its website domain 
has been moved to https://www.t-head.cn/.


Otherwise,

Reviewed-by: LIU Zhiwei 


Thanks,
Zhiwei

On 2022/12/19 20:19, Alex Bennée wrote:

Although the majority of the recent work has been done by LIU Zhiwei
it does appear to be a corporate effort.

Signed-off-by: Alex Bennée 
Cc: Guo Ren 
Cc: Liu Zhiwei 
---
  contrib/gitdm/domain-map | 1 +
  1 file changed, 1 insertion(+)

diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map
index 3727918641..6f3a24b399 100644
--- a/contrib/gitdm/domain-map
+++ b/contrib/gitdm/domain-map
@@ -7,6 +7,7 @@
  amd.com AMD
  baidu.com   Baidu
  bytedance.com   ByteDance
+c-sky.com   C-Sky
  cmss.chinamobile.com China Mobile
  citrix.com  Citrix
  crudebyte.com   Crudebyte

Re: [PATCH-for-8.0 7/7] hw/mips/gt64xxx_pci: Move it to hw/pci-host/

[Richard Henderson]

On 12/9/22 07:15, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daudé

The GT-64120 is a north-bridge, and it is not MIPS specific.
Move it with the other north-bridge devices.

Signed-off-by: Philippe Mathieu-Daudé
---
  MAINTAINERS   | 2 +-
  hw/mips/Kconfig   | 6 --
  hw/mips/meson.build   | 1 -
  hw/mips/trace-events  | 6 --
  hw/pci-host/Kconfig   | 6 ++
  hw/{mips/gt64xxx_pci.c => pci-host/gt64120.c} | 0
  hw/pci-host/meson.build   | 1 +
  hw/pci-host/trace-events  | 7 +++
  meson.build   | 1 -
  9 files changed, 15 insertions(+), 15 deletions(-)
  delete mode 100644 hw/mips/trace-events
  rename hw/{mips/gt64xxx_pci.c => pci-host/gt64120.c} (100%)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH-for-8.0 6/7] hw/mips/meson: Make gt64xxx_pci.c endian-agnostic

[Richard Henderson]

On 12/9/22 07:15, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daudé

The single machine using this device explicitly sets its
endianness. We don't need to set a default. This allow us
to remove the target specificity from the build system.

Signed-off-by: Philippe Mathieu-Daudé
---
  hw/mips/gt64xxx_pci.c | 2 +-
  hw/mips/meson.build   | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH-for-8.0 5/7] hw/mips/malta: Explicit GT64120 endianness upon device creation

[Richard Henderson]

On 12/9/22 07:15, Philippe Mathieu-Daudé wrote:

Propagate the controller endianess from the machine, setting
the "cpu-little-endian" property.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/mips/malta.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)


Modulo using qdev_prop_set_bool,
Reviewed-by: Richard Henderson 


r~



diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index ba92022f87..1f4e0c7acc 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1390,7 +1390,9 @@ void mips_malta_init(MachineState *machine)
  stl_p(memory_region_get_ram_ptr(bios_copy) + 0x10, 0x0420);
  
  /* Northbridge */

-dev = sysbus_create_simple("gt64120", -1, NULL);
+dev = qdev_new("gt64120");
+qdev_prop_set_bit(dev, "cpu-little-endian", !be);
+sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
  pci_bus = PCI_BUS(qdev_get_child_bus(dev, "pci"));
  
  /* Southbridge */

Re: [PATCH-for-8.0 4/7] hw/mips/gt64xxx_pci: Add a 'cpu-little-endian' qdev property

[Richard Henderson]

On 12/9/22 07:15, Philippe Mathieu-Daudé wrote:

+static Property gt64120_properties[] = {
+DEFINE_PROP_BIT("cpu-little-endian", GT64120State,
+features, FEAT_CPU_LE, !TARGET_BIG_ENDIAN),


Unless you're really planning on more feature bits, DEFINE_PROP_BOOL would be 
better.


r~

Re: [PATCH-for-8.0 3/7] hw/mips/gt64xxx_pci: Manage endian bits with the RegisterField API

[Richard Henderson]

On 12/9/22 07:15, Philippe Mathieu-Daudé wrote:

  /* CPU Configuration */
-#if TARGET_BIG_ENDIAN
  s->regs[GT_CPU]   = 0x;
-#else
-s->regs[GT_CPU]   = 0x1000;
-#endif
+s->regs[GT_CPU] = FIELD_DP32(s->regs[GT_CPU], GT_CPU, Endianess, cpu_le);


Missing set to zero before deposit.
Though I wonder if

= cpu_le ? R_GT_CPU_Endianness_MASK : 0

might be easier?


-#if TARGET_BIG_ENDIAN
  s->regs[GT_PCI0_CMD]  = 0x;
-#else
-s->regs[GT_PCI0_CMD]  = 0x00010001;
-#endif
+s->regs[GT_PCI0_CMD] = FIELD_DP32(s->regs[GT_PCI0_CMD],
+  GT_PCI0_CMD, MByteSwap, cpu_le);
+s->regs[GT_PCI0_CMD] = FIELD_DP32(s->regs[GT_PCI0_CMD],
+  GT_PCI0_CMD, SByteSwap, cpu_le);


This one at least has the zero, but it might as well use the masks like above.


-#if TARGET_BIG_ENDIAN
  s->regs[GT_PCI1_CMD]  = 0x;
-#else
-s->regs[GT_PCI1_CMD]  = 0x00010001;
-#endif
+s->regs[GT_PCI1_CMD] = FIELD_DP32(s->regs[GT_PCI1_CMD],
+  GT_PCI1_CMD, MByteSwap, cpu_le);
+s->regs[GT_PCI1_CMD] = FIELD_DP32(s->regs[GT_PCI1_CMD],
+  GT_PCI1_CMD, SByteSwap, cpu_le);


Likewise.

r~

Re: [PATCH-for-8.0 2/7] hw/mips/gt64xxx_pci: Let the GT64120 manage the lower 512MiB hole

[Richard Henderson]

On 12/9/22 07:15, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daudé

Per the comment in the Malta board, the [0x.-0x2000.]
range is decoded by the GT64120, so move the "empty_slot" there.

Signed-off-by: Philippe Mathieu-Daudé
---
  configs/devices/mips-softmmu/common.mak | 1 -
  hw/mips/Kconfig | 1 +
  hw/mips/gt64xxx_pci.c   | 8 
  hw/mips/malta.c | 7 ---
  4 files changed, 9 insertions(+), 8 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH-for-8.0 1/7] hw/mips/Kconfig: Introduce CONFIG_GT64120 to select gt64xxx_pci.c

[Richard Henderson]

On 12/9/22 07:15, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daudé

Signed-off-by: Philippe Mathieu-Daudé
---
  hw/mips/Kconfig | 6 ++
  hw/mips/meson.build | 3 ++-
  2 files changed, 8 insertions(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v11 5/5] docs: Add generic vhost-vdpa device documentation

[longpeng2--- via]

在 2022/12/20 5:37, Michael S. Tsirkin 写道:

On Fri, Dec 16, 2022 at 11:33:49AM +0800, Jason Wang wrote:

On Thu, Dec 15, 2022 at 9:50 PM Longpeng(Mike)  wrote:


From: Longpeng 

Signed-off-by: Longpeng 
---
  .../devices/vhost-vdpa-generic-device.rst | 68 +++
  1 file changed, 68 insertions(+)
  create mode 100644 docs/system/devices/vhost-vdpa-generic-device.rst

diff --git a/docs/system/devices/vhost-vdpa-generic-device.rst 
b/docs/system/devices/vhost-vdpa-generic-device.rst
new file mode 100644
index 00..24c825ef1a
--- /dev/null
+++ b/docs/system/devices/vhost-vdpa-generic-device.rst
@@ -0,0 +1,68 @@
+
+=
+vhost-vDPA generic device
+=
+
+This document explains the usage of the vhost-vDPA generic device.
+
+
+Description
+---
+
+vDPA(virtio data path acceleration) device is a device that uses a datapath
+which complies with the virtio specifications with vendor specific control
+path.
+
+QEMU provides two types of vhost-vDPA devices to enable the vDPA device, one
+is type sensitive which means QEMU needs to know the actual device type
+(e.g. net, blk, scsi) and another is called "vhost-vDPA generic device" which
+is type insensitive.
+
+The vhost-vDPA generic device builds on the vhost-vdpa subsystem and virtio
+subsystem. It is quite small, but it can support any type of virtio device.
+
+
+Requirements
+
+Linux 5.18+
+iproute2/vdpa 5.12.0+
+
+
+Examples
+
+
+1. Prepare the vhost-vDPA backends, here is an example using vdpa_sim_blk
+   device:
+
+::
+  host# modprobe vhost_vdpa
+  host# modprobe vdpa_sim_blk


Nit: it's probably better to add driver binding steps here.


+  host# vdpa dev add mgmtdev vdpasim_blk name blk0
+  (...you can see the vhost-vDPA device under /dev directory now...)


And then the vhost char dev name could be fetch via

ls /sys/bus/vdpa/device/blk0/vhost-vdpa*

With the above changes.

Acked-by: Jason Wang 

Thanks



Sounds minor enough, I'll queue, pls fix with a patch on top.


OK, thanks. I'll send when I'm free.


+  host# ls -l /dev/vhost-vdpa-*
+  crw--- 1 root root 236, 0 Nov  2 00:49 /dev/vhost-vdpa-0
+
+Note:
+It needs some vendor-specific steps to provision the vDPA device if you're
+using real HW devices, such as loading the vendor-specific vDPA driver and
+binding the device to the driver.
+
+
+2. Start the virtual machine:
+
+Start QEMU with virtio-mmio bus:
+
+::
+  host# qemu-system  \
+  -M microvm -m 512 -smp 2 -kernel ... -initrd ...   \
+  -device vhost-vdpa-device,vhostdev=/dev/vhost-vdpa-0   \
+  ...
+
+
+Start QEMU with virtio-pci bus:
+
+::
+  host# qemu-system  \
+  -M pc -m 512 -smp 2\
+  -device vhost-vdpa-device-pci,vhostdev=/dev/vhost-vdpa-0   \
+  ...
--
2.23.0



.

Re: [PATCH 5/4] target/i386: Restrict 'qapi-commands-machine.h' to system emulation

[Richard Henderson]

On 12/19/22 13:22, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daude

Since commit a0e61807a3 ("qapi: Remove QMP events and commands from
user-mode builds") we don't generate the "qapi-commands-machine.h"
header in a user-emulation-only build.

Guard qmp_query_cpu_definitions() within CONFIG_USER_ONLY; move
x86_cpu_class_check_missing_features() closer since it is only used
by this QMP command handler.

Signed-off-by: Philippe Mathieu-Daudé
---
  target/i386/cpu.c | 74 +--
  1 file changed, 39 insertions(+), 35 deletions(-)


Not moving these out to a separate file?  Anyway,

Reviewed-by: Richard Henderson 

r~

Re: [PATCH 4/4] target/ppc: Restrict 'qapi-commands-machine.h' to system emulation

[Richard Henderson]

On 12/19/22 13:10, Philippe Mathieu-Daudé wrote:

Since commit a0e61807a3 ("qapi: Remove QMP events and commands from
user-mode builds") we don't generate the "qapi-commands-machine.h"
header in a user-emulation-only build.

Move the QMP functions from cpu_init.c (which is always compiled) to
monitor.c (which is only compiled when system-emulation is selected).
Note ppc_cpu_class_by_name() is used by both file units, so we expose
its prototype in "cpu-qom.h".

Signed-off-by: Philippe Mathieu-Daudé
---
  target/ppc/cpu-qom.h  |  2 ++
  target/ppc/cpu_init.c | 48 +
  target/ppc/monitor.c  | 50 ++-
  3 files changed, 52 insertions(+), 48 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 3/4] target/mips: Restrict 'qapi-commands-machine.h' to system emulation

[Richard Henderson]

On 12/19/22 13:10, Philippe Mathieu-Daudé wrote:

Since commit a0e61807a3 ("qapi: Remove QMP events and commands from
user-mode builds") we don't generate the "qapi-commands-machine.h"
header in a user-emulation-only build.

Extract the QMP functions from cpu.c (which is always compiled) to
the new 'sysemu/monitor.c' unit (which is only compiled when system
emulation is selected).

Signed-off-by: Philippe Mathieu-Daudé
---
  target/mips/cpu.c  | 29 -
  target/mips/sysemu/meson.build |  1 +
  target/mips/sysemu/monitor.c   | 39 ++
  3 files changed, 40 insertions(+), 29 deletions(-)
  create mode 100644 target/mips/sysemu/monitor.c


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 2/4] target/loongarch: Restrict 'qapi-commands-machine.h' to system emulation

[Richard Henderson]

On 12/19/22 13:10, Philippe Mathieu-Daudé wrote:

Since commit a0e61807a3 ("qapi: Remove QMP events and commands from
user-mode builds") we don't generate the "qapi-commands-machine.h"
header in a user-emulation-only build.

Extract the QMP functions from cpu.c (which is always compiled) to
the new 'cpu-monitor.c' unit (which is only compiled when system
emulation is selected).

Signed-off-by: Philippe Mathieu-Daudé
---
  target/loongarch/cpu-monitor.c | 37 ++
  target/loongarch/cpu.c | 27 -
  target/loongarch/meson.build   |  1 +
  3 files changed, 38 insertions(+), 27 deletions(-)
  create mode 100644 target/loongarch/cpu-monitor.c


Why cpu-monitor.c and not monitor.c like for target/arm/?

Otherwise,
Reviewed-by: Richard Henderson 


r~

Re: [PATCH 1/4] target/arm: Restrict 'qapi-commands-machine.h' to system emulation

[Richard Henderson]

On 12/19/22 13:10, Philippe Mathieu-Daudé wrote:

Since commit a0e61807a3 ("qapi: Remove QMP events and commands from
user-mode builds") we don't generate the "qapi-commands-machine.h"
header in a user-emulation-only build.

Move the QMP functions from helper.c (which is always compiled) to
monitor.c (which is only compiled when system-emulation is selected).

Signed-off-by: Philippe Mathieu-Daudé
---
  target/arm/helper.c   | 29 -
  target/arm/m_helper.c |  1 -
  target/arm/monitor.c  | 28 
  3 files changed, 28 insertions(+), 30 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PULL v3 00/50] Block layer patches

[Fabiano Rosas]

Kevin Wolf  writes:

> The following changes since commit 48804eebd4a327e4b11f902ba80a00876ee53a43:
>
>   Merge tag 'pull-misc-2022-12-14' of https://repo.or.cz/qemu/armbru into 
> staging (2022-12-15 10:13:46 +)
>
> are available in the Git repository at:
>
>   https://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 1b3ff9feb942c2ad0b01ac931e99ad451ab0ef39:
>
>   block: GRAPH_RDLOCK for functions only called by co_wrappers (2022-12-15 
> 16:08:23 +0100)
>
> v3:
> - Dropped "configure: Enable -Wthread-safety if present" because FreeBSD
>   has TSA annotations in its pthread locking functions, so we would have
>   to annotate the use of every lock in QEMU first before we can enable
>   it.
>
> v2:
> - Changed TSA capability name to "mutex" to work with older clang
>   versions. The tsan-build CI job succeeds now.
>
> 
> Block layer patches
>
> - Code cleanups around block graph modification
> - Simplify drain
> - coroutine_fn correctness fixes, including splitting generated
>   coroutine wrappers into co_wrapper (to be called only from
>   non-coroutine context) and co_wrapper_mixed (both coroutine and
>   non-coroutine context)
> - Introduce a block graph rwlock
>
> 
> Emanuele Giuseppe Esposito (21):
>   block-io: introduce coroutine_fn duplicates for 
> bdrv_common_block_status_above callers
>   block-copy: add coroutine_fn annotations
>   nbd/server.c: add coroutine_fn annotations
>   block-backend: replace bdrv_*_above with blk_*_above
>   block/vmdk: add coroutine_fn annotations
>   block: avoid duplicating filename string in bdrv_create
>   block: distinguish between bdrv_create running in coroutine and not
>   block: bdrv_create_file is a coroutine_fn
>   block: rename generated_co_wrapper in co_wrapper_mixed
>   block-coroutine-wrapper.py: introduce co_wrapper
>   block-coroutine-wrapper.py: support functions without bs arg
>   block-coroutine-wrapper.py: support also basic return types
>   block: convert bdrv_create to co_wrapper
>   block/dirty-bitmap: convert coroutine-only functions to co_wrapper
>   graph-lock: Implement guard macros
>   async: Register/unregister aiocontext in graph lock list
>   block: wrlock in bdrv_replace_child_noperm
>   block: remove unnecessary assert_bdrv_graph_writable()
>   block: assert that graph read and writes are performed correctly
>   block-coroutine-wrapper.py: introduce annotations that take the graph 
> rdlock
>   block: use co_wrapper_mixed_bdrv_rdlock in functions taking the rdlock
>
> Kevin Wolf (24):
>   qed: Don't yield in bdrv_qed_co_drain_begin()
>   test-bdrv-drain: Don't yield in .bdrv_co_drained_begin/end()
>   block: Revert .bdrv_drained_begin/end to non-coroutine_fn
>   block: Remove drained_end_counter
>   block: Inline bdrv_drain_invoke()
>   block: Fix locking for bdrv_reopen_queue_child()
>   block: Drain individual nodes during reopen
>   block: Don't use subtree drains in bdrv_drop_intermediate()
>   stream: Replace subtree drain with a single node drain
>   block: Remove subtree drains
>   block: Call drain callbacks only once
>   block: Remove ignore_bds_parents parameter from drain_begin/end.
>   block: Drop out of coroutine in bdrv_do_drained_begin_quiesce()
>   block: Don't poll in bdrv_replace_child_noperm()
>   block: Remove poll parameter from bdrv_parent_drained_begin_single()
>   block: Factor out bdrv_drain_all_begin_nopoll()

Hi, 

With today's master at c15dc499cc (Merge tag 'pull-misc-20221218'
of https://gitlab.com/rth7680/qemu into staging, 2022-12-19),

I get a test failure:

$ make check-avocado 
AVOCADO_TESTS=../tests/avocado/reverse_debugging.py:ReverseDebugging_X86_64.test_x86_64_pc
...
Output: qemu-system-x86_64: ../block/block-backend.c:2572:
blk_root_drained_poll: Assertion `blk->quiesce_counter' failed.

Bisect points to this patch
da0bd74434 (block: Factor out bdrv_drain_all_begin_nopoll(), 2022-12-07)

Command from avocado logs (paths cut to save space):

./qemu-system-x86_64 -display none -vga none \
-chardev socket,id=mon,path=monitor.sock \
-mon chardev=mon,mode=control -machine pc \
-chardev socket,id=console,path=console.sock,server=on,wait=off \
-serial chardev:console -icount 
shift=7,rr=record,rrfile=replay.bin,rrsnapshot=init \
-net none -drive file=disk.qcow2,if=none

Happens with arm as well:

./qemu-system-aarch64 -display none -vga none \
-chardev socket,id=mon,path=monitor.sock \
-mon chardev=mon,mode=control -machine virt \
-chardev socket,id=console,path=console.sock,server=on,wait=off \
-serial chardev:console -cpu cortex-a53 -icount \
 shift=7,rr=record,rrfile=replay.bin,rrsnapshot=init \
-net none -drive file=disk.qcow2,if=none \
-kernel vmlinuz

>   Import clang-tsa.h
>   

Re: [PULL 00/45] riscv-to-apply queue

[Alistair Francis]

On Tue, Dec 20, 2022 at 1:12 AM Peter Maydell  wrote:
>
> On Mon, 19 Dec 2022 at 02:29, Alistair Francis
>  wrote:
> >
> > From: Alistair Francis 
> >
> > The following changes since commit 562d4af32ec2213061f844b3838223fd7711b56a:
> >
> >   Merge tag 'pull-loongarch-20221215' of https://gitlab.com/gaosong/qemu 
> > into staging (2022-12-18 13:53:29 +)
> >
> > are available in the Git repository at:
> >
> >   https://github.com/alistair23/qemu.git tags/pull-riscv-to-apply-20221219-3
> >
> > for you to fetch changes up to e59b3c6ece6a1351aeca6b916cd9674e23d15e89:
> >
> >   hw/intc: sifive_plic: Fix the pending register range check (2022-12-19 
> > 10:42:14 +1000)
> >
> > 
> > First RISC-V PR for QEMU 8.0
> >
> > * Fix PMP propagation for tlb
> > * Collection of bug fixes
> > * Add the `FIELDx_1CLEAR()` macro
> > * Bump the OpenTitan supported version
> > * Add smstateen support
> > * Support native debug icount trigger
> > * Remove the redundant ipi-id property in the virt machine
> > * Support cache-related PMU events in virtual mode
> > * Add some missing PolarFire SoC io regions
> > * Fix mret exception cause when no pmp rule is configured
> > * Fix bug where disabling compressed instructions would crash QEMU
> > * Add Zawrs ISA extension support
> > * A range of code refactoring and cleanups
>
> Hi -- gpg says your key expired last week. What keyserver can I
> download the updated key from, please ?

Sorry about that.

You should be able to get a valid key from:

https://keys.openpgp.org/search?q=F6C4AC46D4934868D3B8CE8F21E10D29DF977054

Alistair

>
> thanks
> -- PMM

Re: [PULL 00/21] Hexagon update: bug fixes, performance, idef-parser

[Philippe Mathieu-Daudé]

Hi,

On 16/12/22 21:48, Taylor Simpson wrote:






12-21)
Instruction definition parser (idef-parser) from rev.ng
Parses the instruction semantics and generates TCG


Building QEMU with Clang I'm now getting:

target/hexagon/idef-parser.p/idef-parser.tab.c:2197:9: error: variable 
'yynerrs' set but not used [-Werror,-Wunused-but-set-variable]

int yynerrs = 0;
^

Regards,

Phil.

[PULL 0/2] Hppa fixes patches

[Helge Deller]

The following changes since commit c15dc499cc22f4a71639e42e36051e118761c25f:

  Merge tag 'pull-misc-20221218' of https://gitlab.com/rth7680/qemu into 
staging (2022-12-19 10:28:57 +)

are available in the Git repository at:

  https://github.com/hdeller/qemu-hppa.git tags/hppa-fixes-pull-request

for you to fetch changes up to 59f8c04b222ff4b9f3799fe92a7e5d427ae48197:

  target/hppa: Fix fid instruction emulation (2022-12-19 23:14:06 +0100)


target/hppa patches



Helge Deller (2):
  target/hppa: Generate illegal instruction exception for 64-bit
instructions
  target/hppa: Fix fid instruction emulation

 target/hppa/insns.decode |  5 +
 target/hppa/translate.c  | 23 +--
 2 files changed, 22 insertions(+), 6 deletions(-)

--
2.38.1

[PULL 2/2] target/hppa: Fix fid instruction emulation

[Helge Deller]

The fid instruction (Floating-Point Identify) puts the FPU model and
revision into the Status Register. Since those values shouldn't be 0,
store values there which a PCX-L2 (for 32-bit) or a PCX-W2 (for 64-bit)
would return. Noticed while trying to install MPE/iX.

Signed-off-by: Helge Deller 
Reviewed-by: Richard Henderson 
---
 target/hppa/insns.decode |  5 +
 target/hppa/translate.c  | 11 +++
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index c7a7e997f9..27341d27b2 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -388,10 +388,7 @@ fmpyfadd_d  101110 rm1:5 rm2:5 ... 0 1 ..0 0 0 neg:1 
t:5ra3=%rc32

 # Floating point class 0

-# FID.  With r = t = 0, which via fcpy puts 0 into fr0.
-# This is machine/revision = 0, which is reserved for simulator.
-fcpy_f  001100 0 0 0 00 0   \
- r=0 t=0
+fid_f   001100 0 0 000 00 00 0

 fcpy_f  001100 . . 010 00 .. .  @f0c_0
 fabs_f  001100 . . 011 00 .. .  @f0c_0
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index d15b9e27c7..981f8ee03d 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3622,6 +3622,17 @@ static void gen_fcpy_f(TCGv_i32 dst, TCGv_env unused, 
TCGv_i32 src)
 tcg_gen_mov_i32(dst, src);
 }

+static bool trans_fid_f(DisasContext *ctx, arg_fid_f *a)
+{
+nullify_over(ctx);
+#if TARGET_REGISTER_BITS == 64
+save_frd(0, tcg_const_i64(0x130800ULL)); /* PA8700 (PCX-W2) */
+#else
+save_frd(0, tcg_const_i64(0x0f0800ULL)); /* PA7300LC (PCX-L2) */
+#endif
+return nullify_end(ctx);
+}
+
 static bool trans_fcpy_f(DisasContext *ctx, arg_fclass01 *a)
 {
 return do_fop_wew(ctx, a->t, a->r, gen_fcpy_f);
--
2.38.1

[PULL 1/2] target/hppa: Generate illegal instruction exception for 64-bit instructions

[Helge Deller]

Qemu currently emulates a 32-bit CPU only, and crashes with this error
when it faces a 64-bit load (e.g.  "ldd 0(r26),r0") or a 64-bit store
(e.g. "std r26,0(r26)") instruction in the guest:

ERROR:../qemu/tcg/tcg-op.c:2822:tcg_canonicalize_memop: code should not be 
reached

Add checks for 64-bit sizes and generate an illegal instruction
exception if necessary.

Signed-off-by: Helge Deller 
Reviewed-by: Richard Henderson 
---
 target/hppa/translate.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 1af77473da..d15b9e27c7 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2899,14 +2899,22 @@ static bool trans_cmpiclr(DisasContext *ctx, arg_rri_cf 
*a)

 static bool trans_ld(DisasContext *ctx, arg_ldst *a)
 {
-return do_load(ctx, a->t, a->b, a->x, a->scale ? a->size : 0,
+if (unlikely(TARGET_REGISTER_BITS == 32 && a->size > MO_32)) {
+return gen_illegal(ctx);
+} else {
+return do_load(ctx, a->t, a->b, a->x, a->scale ? a->size : 0,
a->disp, a->sp, a->m, a->size | MO_TE);
+}
 }

 static bool trans_st(DisasContext *ctx, arg_ldst *a)
 {
 assert(a->x == 0 && a->scale == 0);
-return do_store(ctx, a->t, a->b, a->disp, a->sp, a->m, a->size | MO_TE);
+if (unlikely(TARGET_REGISTER_BITS == 32 && a->size > MO_32)) {
+return gen_illegal(ctx);
+} else {
+return do_store(ctx, a->t, a->b, a->disp, a->sp, a->m, a->size | 
MO_TE);
+}
 }

 static bool trans_ldc(DisasContext *ctx, arg_ldst *a)
--
2.38.1

Re: [RFC] gitlab: introduce s390x wasmtime job

[Alex Bennée]

Ilya Leoshkevich  writes:

> On Fri, 2022-12-16 at 15:10 +, Alex Bennée wrote:
>> 
>> Ilya Leoshkevich  writes:
>> 
>> > On Tue, 2022-07-05 at 15:40 +0100, Peter Maydell wrote:
>> > > On Tue, 5 Jul 2022 at 15:37, Ilya Leoshkevich 
>> > > wrote:
>> > > > 
>> > > > On Tue, 2022-07-05 at 14:57 +0100, Peter Maydell wrote:
>> > > > > On Tue, 5 Jul 2022 at 14:04, Daniel P. Berrangé
>> > > > > 
>> > > > > wrote:
>> > > > > > If we put this job in QEMU CI someone will have to be able
>> > > > > > to
>> > > > > > interpret the results when it fails.
>> > > > > 
>> > > > > In particular since this is qemu-user, the answer is probably
>> > > > > at least some of the time going to be "oh, well, qemu-user
>> > > > > isn't
>> > > > > reliable
>> > > > > if you do complicated things in the guest". I'd be pretty
>> > > > > wary of
>> > > > > our
>> > > > > having
>> > > > > a "pass a big complicated guest code test suite under linux-
>> > > > > user
>> > > > > mode"
>> > > > > in the CI path.
>> > > 
>> > > > Actually exercising qemu-user is one of the goals here: just as
>> > > > an
>> > > > example, one of the things that the test suite found was commit
>> > > > 9a12adc704f9 ("linux-user/s390x: Fix unwinding from signal
>> > > > handlers"),
>> > > > so it's not only about the ISA.
>> > > > 
>> > > > At least for s390x, we've noticed that various projects use
>> > > > qemu-user-based setups in their CI (either calling it
>> > > > explicitly,
>> > > > or
>> > > > via binfmt-misc), and we would like these workflows to be
>> > > > reliable,
>> > > > even if they try complicated (within reason) things there.
>> > > 
>> > > I also would like them to be reliable. But I don't think
>> > > *testing* these things is the difficulty: it is having
>> > > people who are willing to spend time on the often quite
>> > > difficult tasks of identifying why something intermittently
>> > > fails and doing the necessary design and implementation work
>> > > to correct the problem. Sometimes this is easy (as in the
>> > > s390 regression above) but quite often it is not (eg when
>> > > multiple threads are in use, or the guest wants to do
>> > > something complicated with clone(), etc).
>> > > 
>> > > thanks
>> > > -- PMM
>> > > 
>> > 
>> > For what it's worth, we can help analyzing and fixing failures
>> > detected
>> > by the s390x wasmtime job. If something breaks, we will have to
>> > look at
>> > it anyway, and it's better to do this sooner than later.
>> 
>> Sorry for necroing an old thread but I just wanted to add my 2p.
>
> Thanks for that though; I've been cherry-picking this patch into my
> private trees for some time now, and would be happy to see it go
> upstream in some form.
>
>> I think making 3rd party test suites easily available to developers
>> is a worthy
>> goal and there are a number that I would like to see including LTP
>> and
>> kvm-unit-tests. As others have pointed out I'm less sure about adding
>> it
>> to the gating CI.
>
> Another third-party test suite that I found useful was the valgrind's
> one. I'll post my thoughts about integrating wasmtime's and valgrind's
> test suites below, unfortunately I'm not too familiar with LTP and
> kvm-unit-tests.
>
> Not touching the gating CI is fine for me.
>
>> If we want to go forward with this we should probably think about how
>> we
>> would approach this generally:
>> 
>>   - tests/third-party-suites/FOO?
>
> Sounds good to me.
>
>>   - should we use avocado as a wrapper or something else?
>>     - make check-?
>
> avocado sounds good; we might have to add a second wrapper for
> producing tap output (see below).
>
> One should definitely be able to specify the testsuite and the
> architecture, e.g. `make check-third-party-wasmtime-s390x`.
>
> In addition, we need to either hardcode or let the user choose
> the way the testsuite it built and executed. I see 3 possibilities:
>
> - Fully on the host. Easiest to implement, the results are also easy
>   to debug. But this requires installing cross-toolchains manually,
>   which is simple on some distros and not-so-simple on the others.
>
> - Provide the toolchain as a Docker image. For wasmtime, the toolchain
>   would include the Rust compiler and Cargo. This solves the problem
>   with configuring the host, but introduces the next choice one has to
>   make:
>
>   - Build qemu on the host. Then qemu binary would have to be
> compatible with the container (e.g. no references to the latest
> greatest glibc functions).
>
> This is because wastime testsuite needs to run inside the
> container: it's driven by Cargo, which is not available on the 
> host. It is possible to only build tests with Cargo and then run
> the resulting binaries manually, but there is more than one and I'm
> not sure how to get a list of them (if we decide to do this, in the
> worst case the list can be hardcoded).
>
> For valgrind it's a bit easier, since the test runner is not as
> complex as Cargo, and 

[PATCH v4 04b/27] tcg: Massage tcg_reg_alloc_op()

[Philippe Mathieu-Daudé]

In preparation of introducing paired registers in the next
commit, massage tcg_reg_alloc_op() a bit by converting it to
a switch with a single case: the current non-paired register.

Signed-off-by: Richard Henderson 
[PMD: Split from bigger patch, 2/3]
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/tcg/tcg.h |  1 +
 tcg/tcg.c | 98 ---
 2 files changed, 59 insertions(+), 40 deletions(-)

diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index d84bae6e3f..2f7bbf4882 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -951,6 +951,7 @@ typedef struct TCGArgConstraint {
 unsigned ct : 16;
 unsigned alias_index : 4;
 unsigned sort_index : 4;
+unsigned pair : 1;  /* 0: not paired, 1: illegal */
 bool oalias : 1;
 bool ialias : 1;
 bool newreg : 1;
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 92141bd79a..67cf36ace8 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -3550,8 +3550,8 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp 
*op)
 
 /* satisfy input constraints */
 for (k = 0; k < nb_iargs; k++) {
-TCGRegSet i_preferred_regs;
-bool allocate_new_reg;
+TCGRegSet i_preferred_regs, i_required_regs;
+bool allocate_new_reg, copyto_new_reg;
 
 i = def->args_ct[nb_oargs + k].sort_index;
 arg = op->args[i];
@@ -3568,43 +3568,54 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp 
*op)
 
 reg = ts->reg;
 i_preferred_regs = 0;
+i_required_regs = arg_ct->regs;
 allocate_new_reg = false;
+copyto_new_reg = false;
 
-if (arg_ct->ialias) {
-i_preferred_regs = op->output_pref[arg_ct->alias_index];
+switch (arg_ct->pair) {
+case 0: /* not paired */
+if (arg_ct->ialias) {
+i_preferred_regs = op->output_pref[arg_ct->alias_index];
 
-/*
- * If the input is readonly, then it cannot also be an
- * output and aliased to itself.  If the input is not
- * dead after the instruction, we must allocate a new
- * register and move it.
- */
-if (temp_readonly(ts) || !IS_DEAD_ARG(i)) {
-allocate_new_reg = true;
-} else if (ts->val_type == TEMP_VAL_REG) {
 /*
- * Check if the current register has already been
- * allocated for another input.
+ * If the input is not dead after the instruction,
+ * we must allocate a new register and move it.
  */
-allocate_new_reg = tcg_regset_test_reg(i_allocated_regs, reg);
+if (!IS_DEAD_ARG(i)) {
+allocate_new_reg = true;
+} else if (ts->val_type == TEMP_VAL_REG) {
+/*
+ * Check if the current register has already been
+ * allocated for another input.
+ */
+allocate_new_reg =
+tcg_regset_test_reg(i_allocated_regs, reg);
+}
 }
+if (!allocate_new_reg) {
+temp_load(s, ts, i_required_regs, i_allocated_regs,
+  i_preferred_regs);
+reg = ts->reg;
+allocate_new_reg = !tcg_regset_test_reg(i_required_regs, reg);
+}
+if (allocate_new_reg) {
+/*
+ * Allocate a new register matching the constraint
+ * and move the temporary register into it.
+ */
+temp_load(s, ts, tcg_target_available_regs[ts->type],
+  i_allocated_regs, 0);
+reg = tcg_reg_alloc(s, i_required_regs, i_allocated_regs,
+i_preferred_regs, ts->indirect_base);
+copyto_new_reg = true;
+}
+break;
+
+default:
+g_assert_not_reached();
 }
 
-if (!allocate_new_reg) {
-temp_load(s, ts, arg_ct->regs, i_allocated_regs, i_preferred_regs);
-reg = ts->reg;
-allocate_new_reg = !tcg_regset_test_reg(arg_ct->regs, reg);
-}
-
-if (allocate_new_reg) {
-/*
- * Allocate a new register matching the constraint
- * and move the temporary register into it.
- */
-temp_load(s, ts, tcg_target_available_regs[ts->type],
-  i_allocated_regs, 0);
-reg = tcg_reg_alloc(s, arg_ct->regs, i_allocated_regs,
-i_preferred_regs, ts->indirect_base);
+if (copyto_new_reg) {
 if (!tcg_out_mov(s, ts->type, reg, ts->reg)) {
 /*
  * Cross register class move not supported.  Sync the
@@ -3656,15 +3667,22 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp 
*op)
 

Re: [PATCH v4 04/27] tcg: Introduce paired register allocation

[Philippe Mathieu-Daudé]

Splitting this patch in 3 to ease review.

Philippe Mathieu-Daudé (2):
  tcg: Massage process_op_defs()
  tcg: Massage tcg_reg_alloc_op()

Richard Henderson (1):
  tcg: Introduce paired register allocation

 include/tcg/tcg.h |   2 +
 tcg/tcg.c | 475 ++
 2 files changed, 403 insertions(+), 74 deletions(-)

-- 
2.38.1

[PATCH v4 04a/27] tcg: Massage process_op_defs()

[Philippe Mathieu-Daudé]

In preparation of introducing paired registers,
massage a bit process_op_defs()'s switch case.

Signed-off-by: Richard Henderson 
[PMD: Split from bigger patch, 1/3]
Signed-off-by: Philippe Mathieu-Daudé 
---
 tcg/tcg.c | 61 +++
 1 file changed, 34 insertions(+), 27 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index c330d114bc..92141bd79a 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -2012,7 +2012,7 @@ static void process_op_defs(TCGContext *s)
 for (op = 0; op < NB_OPS; op++) {
 TCGOpDef *def = _op_defs[op];
 const TCGTargetOpDef *tdefs;
-int i, nb_args;
+int i, o, nb_args;
 
 if (def->flags & TCG_OPF_NOT_PRESENT) {
 continue;
@@ -2034,53 +2034,60 @@ static void process_op_defs(TCGContext *s)
 
 for (i = 0; i < nb_args; i++) {
 const char *ct_str = tdefs->args_ct_str[i];
+bool input_p = i >= def->nb_oargs;
+
 /* Incomplete TCGTargetOpDef entry. */
 tcg_debug_assert(ct_str != NULL);
 
-while (*ct_str != '\0') {
-switch(*ct_str) {
-case '0' ... '9':
-{
-int oarg = *ct_str - '0';
-tcg_debug_assert(ct_str == tdefs->args_ct_str[i]);
-tcg_debug_assert(oarg < def->nb_oargs);
-tcg_debug_assert(def->args_ct[oarg].regs != 0);
-def->args_ct[i] = def->args_ct[oarg];
-/* The output sets oalias.  */
-def->args_ct[oarg].oalias = true;
-def->args_ct[oarg].alias_index = i;
-/* The input sets ialias. */
-def->args_ct[i].ialias = true;
-def->args_ct[i].alias_index = oarg;
-}
-ct_str++;
-break;
-case '&':
-def->args_ct[i].newreg = true;
-ct_str++;
-break;
+switch (*ct_str) {
+case '0' ... '9':
+o = *ct_str - '0';
+tcg_debug_assert(input_p);
+tcg_debug_assert(o < def->nb_oargs);
+tcg_debug_assert(def->args_ct[o].regs != 0);
+tcg_debug_assert(!def->args_ct[o].oalias);
+def->args_ct[i] = def->args_ct[o];
+/* The output sets oalias.  */
+def->args_ct[o].oalias = 1;
+def->args_ct[o].alias_index = i;
+/* The input sets ialias. */
+def->args_ct[i].ialias = 1;
+def->args_ct[i].alias_index = o;
+tcg_debug_assert(ct_str[1] == '\0');
+continue;
+
+case '&':
+tcg_debug_assert(!input_p);
+def->args_ct[i].newreg = true;
+ct_str++;
+break;
+}
+
+do {
+switch (*ct_str) {
 case 'i':
 def->args_ct[i].ct |= TCG_CT_CONST;
-ct_str++;
 break;
 
 /* Include all of the target-specific constraints. */
 
 #undef CONST
 #define CONST(CASE, MASK) \
-case CASE: def->args_ct[i].ct |= MASK; ct_str++; break;
+case CASE: def->args_ct[i].ct |= MASK; break;
 #define REGS(CASE, MASK) \
-case CASE: def->args_ct[i].regs |= MASK; ct_str++; break;
+case CASE: def->args_ct[i].regs |= MASK; break;
 
 #include "tcg-target-con-str.h"
 
 #undef REGS
 #undef CONST
 default:
+case '0' ... '9':
+case '&':
 /* Typo in TCGTargetOpDef constraint. */
 g_assert_not_reached();
 }
-}
+} while (*++ct_str != '\0');
 }
 
 /* TCGTargetOpDef entry with too much information? */
-- 
2.38.1

[PATCH v4 04c/27] tcg: Introduce paired register allocation

[Philippe Mathieu-Daudé]

From: Richard Henderson 

There are several instances where we need to be able to
allocate a pair of registers to related inputs/outputs.
Add 'p' and 'm' register constraints for this, in order to
be able to allocate the even/odd register first or second.

Signed-off-by: Richard Henderson 
[PMD: Split patch in 3]
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/tcg/tcg.h |   3 +-
 tcg/tcg.c | 318 --
 2 files changed, 312 insertions(+), 9 deletions(-)

diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 2f7bbf4882..5c2254ce9f 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -951,7 +951,8 @@ typedef struct TCGArgConstraint {
 unsigned ct : 16;
 unsigned alias_index : 4;
 unsigned sort_index : 4;
-unsigned pair : 1;  /* 0: not paired, 1: illegal */
+unsigned pair_index : 4;
+unsigned pair : 2;  /* 0: none, 1: first, 2: second, 3: second alias */
 bool oalias : 1;
 bool ialias : 1;
 bool newreg : 1;
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 67cf36ace8..98d51e538c 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1969,15 +1969,32 @@ static void tcg_dump_ops(TCGContext *s, FILE *f, bool 
have_prefs)
 static int get_constraint_priority(const TCGOpDef *def, int k)
 {
 const TCGArgConstraint *arg_ct = >args_ct[k];
-int n;
+int n = ctpop64(arg_ct->regs);
 
-if (arg_ct->oalias) {
-/* an alias is equivalent to a single register */
-n = 1;
-} else {
-n = ctpop64(arg_ct->regs);
+/*
+ * Sort constraints of a single register first, which includes output
+ * aliases (which must exactly match the input already allocated).
+ */
+if (n == 1 || arg_ct->oalias) {
+return INT_MAX;
 }
-return TCG_TARGET_NB_REGS - n + 1;
+
+/*
+ * Sort register pairs next, first then second immediately after.
+ * Arbitrarily sort multiple pairs by the index of the first reg;
+ * there shouldn't be many pairs.
+ */
+switch (arg_ct->pair) {
+case 1:
+case 3:
+return (k + 1) * 2;
+case 2:
+return (arg_ct->pair_index + 1) * 2 - 1;
+}
+
+/* Finally, sort by decreasing register count. */
+assert(n > 1);
+return -n;
 }
 
 /* sort from highest priority to lowest */
@@ -2012,7 +2029,8 @@ static void process_op_defs(TCGContext *s)
 for (op = 0; op < NB_OPS; op++) {
 TCGOpDef *def = _op_defs[op];
 const TCGTargetOpDef *tdefs;
-int i, o, nb_args;
+bool saw_alias_pair = false;
+int i, o, i2, o2, nb_args;
 
 if (def->flags & TCG_OPF_NOT_PRESENT) {
 continue;
@@ -2053,6 +2071,9 @@ static void process_op_defs(TCGContext *s)
 /* The input sets ialias. */
 def->args_ct[i].ialias = 1;
 def->args_ct[i].alias_index = o;
+if (def->args_ct[i].pair) {
+saw_alias_pair = true;
+}
 tcg_debug_assert(ct_str[1] == '\0');
 continue;
 
@@ -2061,6 +2082,38 @@ static void process_op_defs(TCGContext *s)
 def->args_ct[i].newreg = true;
 ct_str++;
 break;
+
+case 'p': /* plus */
+/* Allocate to the register after the previous. */
+tcg_debug_assert(i > (input_p ? def->nb_oargs : 0));
+o = i - 1;
+tcg_debug_assert(!def->args_ct[o].pair);
+tcg_debug_assert(!def->args_ct[o].ct);
+def->args_ct[i] = (TCGArgConstraint){
+.pair = 2,
+.pair_index = o,
+.regs = def->args_ct[o].regs << 1,
+};
+def->args_ct[o].pair = 1;
+def->args_ct[o].pair_index = i;
+tcg_debug_assert(ct_str[1] == '\0');
+continue;
+
+case 'm': /* minus */
+/* Allocate to the register before the previous. */
+tcg_debug_assert(i > (input_p ? def->nb_oargs : 0));
+o = i - 1;
+tcg_debug_assert(!def->args_ct[o].pair);
+tcg_debug_assert(!def->args_ct[o].ct);
+def->args_ct[i] = (TCGArgConstraint){
+.pair = 1,
+.pair_index = o,
+.regs = def->args_ct[o].regs >> 1,
+};
+def->args_ct[o].pair = 2;
+def->args_ct[o].pair_index = i;
+tcg_debug_assert(ct_str[1] == '\0');
+continue;
 }
 
 do {
@@ -2084,6 +2137,8 @@ static void process_op_defs(TCGContext *s)
 default:
 case '0' ... '9':
 case '&':
+case 'p':
+case 'm':
 /* Typo in TCGTargetOpDef constraint. */
 g_assert_not_reached();
 }
@@ -2093,6 +2148,79 @@ static 

[PATCH] hvf: arm: Add support for GICv3

[Alexander Graf]

We currently only support GICv2 emulation. To also support GICv3, we will
need to pass a few system registers into their respective handler functions.

This patch adds support for HVF to call into the TCG callbacks for GICv3
system register handlers. This is safe because the GICv3 TCG code is generic
as long as we limit ourselves to EL0 and EL1 - which are the only modes
supported by HVF.

To make sure nobody trips over that, we also annotate callbacks that don't
work in HVF mode, such as EL state change hooks.

With GICv3 support in place, we can run with more than 8 vCPUs.

Signed-off-by: Alexander Graf 
---
 hw/intc/arm_gicv3_cpuif.c   |   8 +-
 target/arm/hvf/hvf.c| 151 
 target/arm/hvf/trace-events |   2 +
 3 files changed, 160 insertions(+), 1 deletion(-)

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index b17b29288c..b4e387268c 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -21,6 +21,7 @@
 #include "hw/irq.h"
 #include "cpu.h"
 #include "target/arm/cpregs.h"
+#include "sysemu/tcg.h"
 
 /*
  * Special case return value from hppvi_index(); must be larger than
@@ -2810,6 +2811,8 @@ void gicv3_init_cpuif(GICv3State *s)
  * which case we'd get the wrong value.
  * So instead we define the regs with no ri->opaque info, and
  * get back to the GICv3CPUState from the CPUARMState.
+ *
+ * These CP regs callbacks can be called from either TCG or HVF code.
  */
 define_arm_cp_regs(cpu, gicv3_cpuif_reginfo);
 
@@ -2905,6 +2908,9 @@ void gicv3_init_cpuif(GICv3State *s)
 define_arm_cp_regs(cpu, gicv3_cpuif_ich_apxr23_reginfo);
 }
 }
-arm_register_el_change_hook(cpu, gicv3_cpuif_el_change_hook, cs);
+if (tcg_enabled()) {
+/* We can only trap EL changes with TCG for now */
+arm_register_el_change_hook(cpu, gicv3_cpuif_el_change_hook, cs);
+}
 }
 }
diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 060aa0ccf4..8ea4be5f30 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -80,6 +80,33 @@
 #define SYSREG_PMCCNTR_EL0SYSREG(3, 3, 9, 13, 0)
 #define SYSREG_PMCCFILTR_EL0  SYSREG(3, 3, 14, 15, 7)
 
+#define SYSREG_ICC_AP0R0_EL1 SYSREG(3, 0, 12, 8, 4)
+#define SYSREG_ICC_AP0R1_EL1 SYSREG(3, 0, 12, 8, 5)
+#define SYSREG_ICC_AP0R2_EL1 SYSREG(3, 0, 12, 8, 6)
+#define SYSREG_ICC_AP0R3_EL1 SYSREG(3, 0, 12, 8, 7)
+#define SYSREG_ICC_AP1R0_EL1 SYSREG(3, 0, 12, 9, 0)
+#define SYSREG_ICC_AP1R1_EL1 SYSREG(3, 0, 12, 9, 1)
+#define SYSREG_ICC_AP1R2_EL1 SYSREG(3, 0, 12, 9, 2)
+#define SYSREG_ICC_AP1R3_EL1 SYSREG(3, 0, 12, 9, 3)
+#define SYSREG_ICC_ASGI1R_EL1SYSREG(3, 0, 12, 11, 6)
+#define SYSREG_ICC_BPR0_EL1  SYSREG(3, 0, 12, 8, 3)
+#define SYSREG_ICC_BPR1_EL1  SYSREG(3, 0, 12, 12, 3)
+#define SYSREG_ICC_CTLR_EL1  SYSREG(3, 0, 12, 12, 4)
+#define SYSREG_ICC_DIR_EL1   SYSREG(3, 0, 12, 11, 1)
+#define SYSREG_ICC_EOIR0_EL1 SYSREG(3, 0, 12, 8, 1)
+#define SYSREG_ICC_EOIR1_EL1 SYSREG(3, 0, 12, 12, 1)
+#define SYSREG_ICC_HPPIR0_EL1SYSREG(3, 0, 12, 8, 2)
+#define SYSREG_ICC_HPPIR1_EL1SYSREG(3, 0, 12, 12, 2)
+#define SYSREG_ICC_IAR0_EL1  SYSREG(3, 0, 12, 8, 0)
+#define SYSREG_ICC_IAR1_EL1  SYSREG(3, 0, 12, 12, 0)
+#define SYSREG_ICC_IGRPEN0_EL1   SYSREG(3, 0, 12, 12, 6)
+#define SYSREG_ICC_IGRPEN1_EL1   SYSREG(3, 0, 12, 12, 7)
+#define SYSREG_ICC_PMR_EL1   SYSREG(3, 0, 4, 6, 0)
+#define SYSREG_ICC_RPR_EL1   SYSREG(3, 0, 12, 11, 3)
+#define SYSREG_ICC_SGI0R_EL1 SYSREG(3, 0, 12, 11, 7)
+#define SYSREG_ICC_SGI1R_EL1 SYSREG(3, 0, 12, 11, 5)
+#define SYSREG_ICC_SRE_EL1   SYSREG(3, 0, 12, 12, 5)
+
 #define WFX_IS_WFE (1 << 0)
 
 #define TMR_CTL_ENABLE  (1 << 0)
@@ -788,6 +815,43 @@ static bool is_id_sysreg(uint32_t reg)
SYSREG_CRM(reg) < 8;
 }
 
+static uint32_t hvf_reg2cp_reg(uint32_t reg)
+{
+return ENCODE_AA64_CP_REG(CP_REG_ARM64_SYSREG_CP,
+  (reg >> 10) & 0xf,
+  (reg >> 1) & 0xf,
+  (reg >> 20) & 0x3,
+  (reg >> 14) & 0x7,
+  (reg >> 17) & 0x7);
+}
+
+static bool hvf_sysreg_read_cp(CPUState *cpu, uint32_t reg, uint64_t *val)
+{
+ARMCPU *arm_cpu = ARM_CPU(cpu);
+CPUARMState *env = _cpu->env;
+const ARMCPRegInfo *ri;
+
+ri = get_arm_cp_reginfo(arm_cpu->cp_regs, hvf_reg2cp_reg(reg));
+if (ri) {
+if (ri->accessfn) {
+if (ri->accessfn(env, ri, true) != CP_ACCESS_OK) {
+return false;
+}
+}
+if (ri->type & ARM_CP_CONST) {
+*val = ri->resetvalue;
+} else if (ri->readfn) {
+*val = ri->readfn(env, ri);
+} else {
+*val = CPREG_FIELD64(env, ri);
+}
+trace_hvf_vgic_read(ri->name, *val);
+

Re: [PATCH v2 4/7] hw/misc: Allwinner AXP-209 Emulation

[Strahinja Jankovic]

On Mon, Dec 19, 2022 at 8:15 AM Philippe Mathieu-Daudé
 wrote:
>
> On 18/12/22 23:39, Strahinja Jankovic wrote:
> > Hi,
> >
> > On Sun, Dec 18, 2022 at 11:23 PM Philippe Mathieu-Daudé
> >  wrote:
> >>
> >> On 18/12/22 23:12, Strahinja Jankovic wrote:
> >>> Hi,
> >>>
> >>> On Sun, Dec 18, 2022 at 11:07 PM Philippe Mathieu-Daudé
> >>>  wrote:
> 
>  On 18/12/22 22:19, Strahinja Jankovic wrote:
> > This patch adds minimal support for AXP-209 PMU.
> > Most important is chip ID since U-Boot SPL expects version 0x1. Besides
> > the chip ID register, reset values for two more registers used by A10
> > U-Boot SPL are covered.
> >
> > Signed-off-by: Strahinja Jankovic 
> > ---
> > hw/arm/Kconfig  |   1 +
> > hw/misc/Kconfig |   4 +
> > hw/misc/allwinner-axp-209.c | 238 
> > 
> > hw/misc/meson.build |   1 +
> > hw/misc/trace-events|   5 +
> > 5 files changed, 249 insertions(+)
> > create mode 100644 hw/misc/allwinner-axp-209.c
>
>
> > +config ALLWINNER_AXP_209
> 
>  This controller is not specific to AllWinner. It can be plugged on any
>  i2c bus. Please rename it AXP209_PMU. Otherwise LGTM!
> >>>
> >>> Do you mean only in Kconfig, or everywhere (file name, function names,
> >>> etc.)? Thanks.
> >>
> >> Keeping the file / functions names with 'allwinner' would give the false
> >> idea this is AllWinner specific; rather confusing isn't it? Besides it
> >> is not part of the SoC, this is an external component sitting on the
> >> bus. So "everywhere".
> >
> > I was expecting that answer, but I wanted to check :)
> > When I started writing code for AXP209 it was indeed without allwinner
> > prefix, but then checkpatch.pl complained about missing lines in
> > MAINTAINERS. At that time it was easier to add the prefix (and get a
> > match in MAINTAINERS) than to investigate if I should update the
> > MAINTAINERS file or ignore the checkpatch.pl complaint.
> >
> > I will update everything related to AXP209 so it does not have the
> > Allwinner prefix.
>
> Add a new AXP209 MAINTAINERS's section with your name :) Or add the new
> file to the Allwinner-a10 section and add your name as designated
> reviewer or co-maintainer :)

Thanks, I will take one of these suggested approaches.

Best regards,
Strahinja

Re: [PATCH v2 0/7] Enable Cubieboard A10 boot SPL from SD card

[Strahinja Jankovic]

Hi,

On Mon, Dec 19, 2022 at 8:11 AM Philippe Mathieu-Daudé
 wrote:
>
> On 18/12/22 23:57, Strahinja Jankovic wrote:
> > Hi,
> >
> > I just looked around a bit more. Would OpenWrt image be acceptable?
>
> Sure.
>
> > It looks like there are releases for cubieboard that are kept for a
> > longer time, and I just checked that they work properly, so I could
> > add them to the acceptance test.
> >
> > Best regards,
> > Strahinja
> >
> > On Sun, Dec 18, 2022 at 11:34 PM Strahinja Jankovic
> >  wrote:
> >>
> >> Hi,
> >>
> >>
> >> On Sun, Dec 18, 2022 at 11:17 PM Philippe Mathieu-Daudé
> >>  wrote:
> >>>
> >>> On 18/12/22 22:19, Strahinja Jankovic wrote:
>  This patch series adds missing Allwinner A10 modules needed for
>  successful SPL boot:
>  - Clock controller module
>  - DRAM controller
>  - I2C0 controller (added also for Allwinner H3 since it is the same)
>  - AXP-209 connected to I2C0 bus
> 
>  It also updates Allwinner A10 emulation so SPL is copied from attached
>  SD card if `-kernel` parameter is not passed when starting QEMU
>  (approach adapted from Allwinner H3 implementation).
> 
>  Boot from SD card has been tested with Cubieboard Armbian SD card image 
>  and custom
>  Yocto image built for Cubieboard.
>  Example usage for Armbian image:
>  qemu-system-arm -M cubieboard -nographic -sd 
>  ~/Armbian_22.11.0-trunk_Cubieboard_kinetic_edge_6.0.7.img
> >>>
> >>> As a follow-up, could you add a test similar to
> >>> test_arm_orangepi_bionic_20_08() in tests/avocado/boot_linux_console.py?
> >>>
> >>> This test could be refactored as do_test_arm_allwinner_armbian(), called
> >>> for orangepi and cubieboard with the corresponding url / hash.
> >>
> >> I was planning to do it (I already have a patch for cubieboard), but
> >> there is a problem with finding a stable-enough image for cubieboard
> >> for automated testing.
> >> Unfortunately, Cubieboard does not have supported Armbian images as
> >> OrangePi-PC has (it only has weekly builds).
>
> Developers shouldn't worry about that but focus on the testing. The
> QEMU project doesn't have dedicated sysadmin / storage, but if it had
> it should be their problem, not yours. Meanwhile such a test is still
> useful. Artifacts come with their hash, so can be uploaded elsewhere
> later.
>
> >> From the images that can
> >> be found on archive.armbian.com, there is only one bionic image
> >> (21.02) and it won't boot because it hangs due to the musb issue (I
> >> tried both with my patches and without, by extracting
> >> kernel/dtb/initrd).
>
> Similar approach:
> https://lore.kernel.org/qemu-devel/20201018205551.1537927-4-f4...@amsat.org/
>
> >> Other images are focal, but for some reason, in
> >> those images it is impossible to interrupt U-Boot (tested with 21.08),
> >> so I could not append to bootcmd to make boot process more verbose and
> >> easier to monitor for automated testing.
> >>
> >> That is why, for now, I would suggest not updating the SPL/SD boot
> >> test for Cubieboard.
>
> Niek can tell how many times his tests catched regressions for his
> OrangePi machine; I remember at least 3 occasions :)

Thanks for the suggestions, I will add the SPL boot test and use
OpenWrt image for it.

Best regards,
Strahinja

>
> Regards,
>
> Phil.

Re: [PATCH 7/7] libvhost-user: Switch to unsigned int for inuse field in struct VuVirtq

[Marcel Holtmann]

Hi Philippe,

>> It seems there is no need to keep the inuse field signed and end up with
>> compiler warnings for sign-compare.
>>   CC   libvhost-user.o
>> libvhost-user.c: In function ‘vu_queue_pop’:
>> libvhost-user.c:2763:19: error: comparison of integer expressions of 
>> different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
>>  2763 | if (vq->inuse >= vq->vring.num) {
>>   |   ^~
>> libvhost-user.c: In function ‘vu_queue_rewind’:
>> libvhost-user.c:2808:13: error: comparison of integer expressions of 
>> different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
>>  2808 | if (num > vq->inuse) {
>>   | ^
>> Instead of casting the comparision to unsigned int, just make the inuse
>> field unsigned int in the fist place.
>> Signed-off-by: Marcel Holtmann 
>> ---
>>  subprojects/libvhost-user/libvhost-user.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/subprojects/libvhost-user/libvhost-user.h 
>> b/subprojects/libvhost-user/libvhost-user.h
>> index aea7ec5061d5..8cda9b8f577a 100644
>> --- a/subprojects/libvhost-user/libvhost-user.h
>> +++ b/subprojects/libvhost-user/libvhost-user.h
>> @@ -343,7 +343,7 @@ typedef struct VuVirtq {
>>  /* Notification enabled? */
>>  bool notification;
>>  -int inuse;
>> +unsigned int inuse;
> 
> Another use in subprojects/libvduse/libvduse.c.
> 
> Possibly both could be renamed refcount which is more meaningful.

I quickly tried libvduse.[ch] and it has clearly less warnings than
libvhost-user, but some things could be applied there as well.

I looks like only the _GNU_SOURCE and the unsigned int inuse are
applicable, but there is a strict-aliasing issue that I first have
to fully understand.

If patches are acceptable, I send them as well for libvduse.

Regards

Marcel

[PATCH 0/3] Compiler warning fixes for libvduse

[Marcel Holtmann]

When using libvduse library in an external project it would be good if
it can cope with various different compiler settings, this fixes two
obvious ones similar to the patches for libvhost-user.

It also contains a 3rd one, that is marked as RFC since I have no idea
what is correct here. From the structures point of view the assignment
that is done makes no sense to me. I might have to dig into
specification to figure it out or it better be commented accordingly to
tell the compiler that it got this part wrong.

Marcel Holtmann (3):
  libvduse: Provide _GNU_SOURCE when compiling outside of QEMU
  libvduse: Switch to unsigned int for inuse field in struct VduseVirtq
  libvduse: Fix assignment in vring_set_avail_event

 subprojects/libvduse/libvduse.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

-- 
2.38.1

[PATCH RFC 3/3] libvduse: Fix assignment in vring_set_avail_event

[Marcel Holtmann]

  CC   libvduse.o
libvduse.c: In function ‘vring_set_avail_event’:
libvduse.c:603:7: error: dereferencing type-punned pointer will break 
strict-aliasing rules [-Werror=strict-aliasin]
  603 | *((uint16_t *)>vring.used->ring[vq->vring.num]) = htole16(val);
  |  ~^

Signed-off-by: Marcel Holtmann 
---
 subprojects/libvduse/libvduse.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
index 338ad5e352e7..51a4ba1b6878 100644
--- a/subprojects/libvduse/libvduse.c
+++ b/subprojects/libvduse/libvduse.c
@@ -582,7 +582,10 @@ void vduse_queue_notify(VduseVirtq *vq)
 
 static inline void vring_set_avail_event(VduseVirtq *vq, uint16_t val)
 {
-*((uint16_t *)>vring.used->ring[vq->vring.num]) = htole16(val);
+vring_used_elem_t *ring = >vring.used->ring[vq->vring.num];
+
+/* FIXME: Is this actually correct since this is __virtio32 id; */
+ring->id = htole16(val);
 }
 
 static bool vduse_queue_map_single_desc(VduseVirtq *vq, unsigned int *p_num_sg,
-- 
2.38.1

[PATCH 2/3] libvduse: Switch to unsigned int for inuse field in struct VduseVirtq

[Marcel Holtmann]

It seems there is no need to keep the inuse field signed and end up with
compiler warnings for sign-compare.

  CC   libvduse.o
libvduse.c: In function ‘vduse_queue_pop’:
libvduse.c:789:19: error: comparison of integer expressions of different 
signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
  789 | if (vq->inuse >= vq->vring.num) {
  |   ^~

Instead of casting the comparison to unsigned int, just make the inuse
field unsigned int in the fist place.

Signed-off-by: Marcel Holtmann 
---
 subprojects/libvduse/libvduse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
index c871bd331a6b..338ad5e352e7 100644
--- a/subprojects/libvduse/libvduse.c
+++ b/subprojects/libvduse/libvduse.c
@@ -101,7 +101,7 @@ struct VduseVirtq {
 uint16_t signalled_used;
 bool signalled_used_valid;
 int index;
-int inuse;
+unsigned int inuse;
 bool ready;
 int fd;
 VduseDev *dev;
-- 
2.38.1

[PATCH 1/3] libvduse: Provide _GNU_SOURCE when compiling outside of QEMU

[Marcel Holtmann]

When the libvduse sources are used by another project, it can not be
guaranteed that _GNU_SOURCE is set by the build system. If it is for
example not set, errors like this show up.

  CC   libvduse.o
libvduse.c: In function ‘vduse_log_get’:
libvduse.c:172:9: error: implicit declaration of function ‘ftruncate’; did you 
mean ‘strncat’? [-Werror=implicit-function-declaration]
  172 | if (ftruncate(fd, size) == -1) {
  | ^
  | strncat

The simplest way to allow external complication of libvduse.[ch] by
setting _GNU_SOURCE if it is not already set by the build system.

Signed-off-by: Marcel Holtmann 
---
 subprojects/libvduse/libvduse.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
index e089d4d546cf..c871bd331a6b 100644
--- a/subprojects/libvduse/libvduse.c
+++ b/subprojects/libvduse/libvduse.c
@@ -16,6 +16,10 @@
  * later.  See the COPYING file in the top-level directory.
  */
 
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+
 #include 
 #include 
 #include 
-- 
2.38.1

Re: [RFC] gitlab: introduce s390x wasmtime job

[Ilya Leoshkevich]

On Fri, 2022-12-16 at 15:10 +, Alex Bennée wrote:
> 
> Ilya Leoshkevich  writes:
> 
> > On Tue, 2022-07-05 at 15:40 +0100, Peter Maydell wrote:
> > > On Tue, 5 Jul 2022 at 15:37, Ilya Leoshkevich 
> > > wrote:
> > > > 
> > > > On Tue, 2022-07-05 at 14:57 +0100, Peter Maydell wrote:
> > > > > On Tue, 5 Jul 2022 at 14:04, Daniel P. Berrangé
> > > > > 
> > > > > wrote:
> > > > > > If we put this job in QEMU CI someone will have to be able
> > > > > > to
> > > > > > interpret the results when it fails.
> > > > > 
> > > > > In particular since this is qemu-user, the answer is probably
> > > > > at least some of the time going to be "oh, well, qemu-user
> > > > > isn't
> > > > > reliable
> > > > > if you do complicated things in the guest". I'd be pretty
> > > > > wary of
> > > > > our
> > > > > having
> > > > > a "pass a big complicated guest code test suite under linux-
> > > > > user
> > > > > mode"
> > > > > in the CI path.
> > > 
> > > > Actually exercising qemu-user is one of the goals here: just as
> > > > an
> > > > example, one of the things that the test suite found was commit
> > > > 9a12adc704f9 ("linux-user/s390x: Fix unwinding from signal
> > > > handlers"),
> > > > so it's not only about the ISA.
> > > > 
> > > > At least for s390x, we've noticed that various projects use
> > > > qemu-user-based setups in their CI (either calling it
> > > > explicitly,
> > > > or
> > > > via binfmt-misc), and we would like these workflows to be
> > > > reliable,
> > > > even if they try complicated (within reason) things there.
> > > 
> > > I also would like them to be reliable. But I don't think
> > > *testing* these things is the difficulty: it is having
> > > people who are willing to spend time on the often quite
> > > difficult tasks of identifying why something intermittently
> > > fails and doing the necessary design and implementation work
> > > to correct the problem. Sometimes this is easy (as in the
> > > s390 regression above) but quite often it is not (eg when
> > > multiple threads are in use, or the guest wants to do
> > > something complicated with clone(), etc).
> > > 
> > > thanks
> > > -- PMM
> > > 
> > 
> > For what it's worth, we can help analyzing and fixing failures
> > detected
> > by the s390x wasmtime job. If something breaks, we will have to
> > look at
> > it anyway, and it's better to do this sooner than later.
> 
> Sorry for necroing an old thread but I just wanted to add my 2p.

Thanks for that though; I've been cherry-picking this patch into my
private trees for some time now, and would be happy to see it go
upstream in some form.

> I think making 3rd party test suites easily available to developers
> is a worthy
> goal and there are a number that I would like to see including LTP
> and
> kvm-unit-tests. As others have pointed out I'm less sure about adding
> it
> to the gating CI.

Another third-party test suite that I found useful was the valgrind's
one. I'll post my thoughts about integrating wasmtime's and valgrind's
test suites below, unfortunately I'm not too familiar with LTP and
kvm-unit-tests.

Not touching the gating CI is fine for me.

> If we want to go forward with this we should probably think about how
> we
> would approach this generally:
> 
>   - tests/third-party-suites/FOO?

Sounds good to me.

>   - should we use avocado as a wrapper or something else?
>     - make check-?

avocado sounds good; we might have to add a second wrapper for
producing tap output (see below).

One should definitely be able to specify the testsuite and the
architecture, e.g. `make check-third-party-wasmtime-s390x`.

In addition, we need to either hardcode or let the user choose
the way the testsuite it built and executed. I see 3 possibilities:

- Fully on the host. Easiest to implement, the results are also easy
  to debug. But this requires installing cross-toolchains manually,
  which is simple on some distros and not-so-simple on the others.

- Provide the toolchain as a Docker image. For wasmtime, the toolchain
  would include the Rust compiler and Cargo. This solves the problem
  with configuring the host, but introduces the next choice one has to
  make:

  - Build qemu on the host. Then qemu binary would have to be
compatible with the container (e.g. no references to the latest
greatest glibc functions).

This is because wastime testsuite needs to run inside the
container: it's driven by Cargo, which is not available on the 
host. It is possible to only build tests with Cargo and then run
the resulting binaries manually, but there is more than one and I'm
not sure how to get a list of them (if we decide to do this, in the
worst case the list can be hardcoded).

For valgrind it's a bit easier, since the test runner is not as
complex as Cargo, and can therefore just follow the check-tcg
approach.

  - Build qemu inside the container. 2x space and time required, one
might also have to install additional -dev packages for extra 

Re: [PATCH v11 5/5] docs: Add generic vhost-vdpa device documentation

[Michael S. Tsirkin]

On Fri, Dec 16, 2022 at 11:33:49AM +0800, Jason Wang wrote:
> On Thu, Dec 15, 2022 at 9:50 PM Longpeng(Mike)  wrote:
> >
> > From: Longpeng 
> >
> > Signed-off-by: Longpeng 
> > ---
> >  .../devices/vhost-vdpa-generic-device.rst | 68 +++
> >  1 file changed, 68 insertions(+)
> >  create mode 100644 docs/system/devices/vhost-vdpa-generic-device.rst
> >
> > diff --git a/docs/system/devices/vhost-vdpa-generic-device.rst 
> > b/docs/system/devices/vhost-vdpa-generic-device.rst
> > new file mode 100644
> > index 00..24c825ef1a
> > --- /dev/null
> > +++ b/docs/system/devices/vhost-vdpa-generic-device.rst
> > @@ -0,0 +1,68 @@
> > +
> > +=
> > +vhost-vDPA generic device
> > +=
> > +
> > +This document explains the usage of the vhost-vDPA generic device.
> > +
> > +
> > +Description
> > +---
> > +
> > +vDPA(virtio data path acceleration) device is a device that uses a datapath
> > +which complies with the virtio specifications with vendor specific control
> > +path.
> > +
> > +QEMU provides two types of vhost-vDPA devices to enable the vDPA device, 
> > one
> > +is type sensitive which means QEMU needs to know the actual device type
> > +(e.g. net, blk, scsi) and another is called "vhost-vDPA generic device" 
> > which
> > +is type insensitive.
> > +
> > +The vhost-vDPA generic device builds on the vhost-vdpa subsystem and virtio
> > +subsystem. It is quite small, but it can support any type of virtio device.
> > +
> > +
> > +Requirements
> > +
> > +Linux 5.18+
> > +iproute2/vdpa 5.12.0+
> > +
> > +
> > +Examples
> > +
> > +
> > +1. Prepare the vhost-vDPA backends, here is an example using vdpa_sim_blk
> > +   device:
> > +
> > +::
> > +  host# modprobe vhost_vdpa
> > +  host# modprobe vdpa_sim_blk
> 
> Nit: it's probably better to add driver binding steps here.
> 
> > +  host# vdpa dev add mgmtdev vdpasim_blk name blk0
> > +  (...you can see the vhost-vDPA device under /dev directory now...)
> 
> And then the vhost char dev name could be fetch via
> 
> ls /sys/bus/vdpa/device/blk0/vhost-vdpa*
> 
> With the above changes.
> 
> Acked-by: Jason Wang 
> 
> Thanks


Sounds minor enough, I'll queue, pls fix with a patch on top.

> > +  host# ls -l /dev/vhost-vdpa-*
> > +  crw--- 1 root root 236, 0 Nov  2 00:49 /dev/vhost-vdpa-0
> > +
> > +Note:
> > +It needs some vendor-specific steps to provision the vDPA device if you're
> > +using real HW devices, such as loading the vendor-specific vDPA driver and
> > +binding the device to the driver.
> > +
> > +
> > +2. Start the virtual machine:
> > +
> > +Start QEMU with virtio-mmio bus:
> > +
> > +::
> > +  host# qemu-system  \
> > +  -M microvm -m 512 -smp 2 -kernel ... -initrd ...   \
> > +  -device vhost-vdpa-device,vhostdev=/dev/vhost-vdpa-0   \
> > +  ...
> > +
> > +
> > +Start QEMU with virtio-pci bus:
> > +
> > +::
> > +  host# qemu-system  \
> > +  -M pc -m 512 -smp 2\
> > +  -device vhost-vdpa-device-pci,vhostdev=/dev/vhost-vdpa-0   \
> > +  ...
> > --
> > 2.23.0
> >

[PATCH 5/4] target/i386: Restrict 'qapi-commands-machine.h' to system emulation

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daude 

Since commit a0e61807a3 ("qapi: Remove QMP events and commands from
user-mode builds") we don't generate the "qapi-commands-machine.h"
header in a user-emulation-only build.

Guard qmp_query_cpu_definitions() within CONFIG_USER_ONLY; move
x86_cpu_class_check_missing_features() closer since it is only used
by this QMP command handler.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/i386/cpu.c | 74 +--
 1 file changed, 39 insertions(+), 35 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ae502f0bfe..435980dd3a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -31,11 +31,11 @@
 #include "qapi/error.h"
 #include "qapi/qapi-visit-machine.h"
 #include "qapi/qmp/qerror.h"
-#include "qapi/qapi-commands-machine-target.h"
 #include "standard-headers/asm-x86/kvm_para.h"
 #include "hw/qdev-properties.h"
 #include "hw/i386/topology.h"
 #ifndef CONFIG_USER_ONLY
+#include "qapi/qapi-commands-machine-target.h"
 #include "exec/address-spaces.h"
 #include "hw/boards.h"
 #include "hw/i386/sgx-epc.h"
@@ -4713,40 +4713,6 @@ static void x86_cpu_get_unavailable_features(Object 
*obj, Visitor *v,
 visit_type_strList(v, "unavailable-features", , errp);
 }
 
-/* Check for missing features that may prevent the CPU class from
- * running using the current machine and accelerator.
- */
-static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
- strList **list)
-{
-strList **tail = list;
-X86CPU *xc;
-Error *err = NULL;
-
-if (xcc->host_cpuid_required && !accel_uses_host_cpuid()) {
-QAPI_LIST_APPEND(tail, g_strdup("kvm"));
-return;
-}
-
-xc = X86_CPU(object_new_with_class(OBJECT_CLASS(xcc)));
-
-x86_cpu_expand_features(xc, );
-if (err) {
-/* Errors at x86_cpu_expand_features should never happen,
- * but in case it does, just report the model as not
- * runnable at all using the "type" property.
- */
-QAPI_LIST_APPEND(tail, g_strdup("type"));
-error_free(err);
-}
-
-x86_cpu_filter_features(xc, false);
-
-x86_cpu_list_feature_names(xc->filtered_features, tail);
-
-object_unref(OBJECT(xc));
-}
-
 /* Print all cpuid feature names in featureset
  */
 static void listflags(GList *features)
@@ -4875,6 +4841,42 @@ void x86_cpu_list(void)
 g_list_free(names);
 }
 
+#ifndef CONFIG_USER_ONLY
+
+/* Check for missing features that may prevent the CPU class from
+ * running using the current machine and accelerator.
+ */
+static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
+ strList **list)
+{
+strList **tail = list;
+X86CPU *xc;
+Error *err = NULL;
+
+if (xcc->host_cpuid_required && !accel_uses_host_cpuid()) {
+QAPI_LIST_APPEND(tail, g_strdup("kvm"));
+return;
+}
+
+xc = X86_CPU(object_new_with_class(OBJECT_CLASS(xcc)));
+
+x86_cpu_expand_features(xc, );
+if (err) {
+/* Errors at x86_cpu_expand_features should never happen,
+ * but in case it does, just report the model as not
+ * runnable at all using the "type" property.
+ */
+QAPI_LIST_APPEND(tail, g_strdup("type"));
+error_free(err);
+}
+
+x86_cpu_filter_features(xc, false);
+
+x86_cpu_list_feature_names(xc->filtered_features, tail);
+
+object_unref(OBJECT(xc));
+}
+
 static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
 {
 ObjectClass *oc = data;
@@ -4915,6 +4917,8 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error 
**errp)
 return cpu_list;
 }
 
+#endif /* !CONFIG_USER_ONLY */
+
 uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
 bool migratable_only)
 {
-- 
2.38.1

Re: [PATCH v5 0/3] Fix the virtio features negotiation flaw

[Michael S. Tsirkin]

On Mon, Dec 19, 2022 at 06:36:39AM -0500, huang...@chinatelecom.cn wrote:
> From: Hyman Huang(黄勇) 
> 
> v5:
> -fix the assert statement in [PATCH v4 3/3], reported by
>  xiangdong. 


Could you pls rebase on top of master retest and repost?
If I try I see some conflicts with vq reset work upstream.

Also, threading is weird. E.g. patch 1 has:
In-Reply-To: 
In-Reply-To: 

1st one is cover letter but second one seems just wrong.




> v4:
> -rebase on master
> -add stub function to fix build errors
> -code clean on [PATCH v2 1/2]: drop 'cleanup' parameter in
>  vhost_user_save_acked_features.
> -code clean on [PATCH v2 2/2]: make refactor of chr_closed_bh
>  a standalone patch.
> 
> Above changes are suggested by Michael and thanks very much.
> 
> Please review,
> 
> Yong
> 
> v3:
> -rebase on master
> -code clean on [PATCH v2 1/2]: keep the commit self-consistent and
>  do not modify the logic of saving acked_features. Just abstract the
>  util function.
> -modify the [PATCH v2 2/2] logic: change the behavior of saving
>  acked_features in chr_closed_bh: saving acked_features only if
>  features aren't 0. For the case of 0, we implement it in
>  virtio_net_set_features function, which will save the acked_features
>  in advance, including assign 0 to acked_features.
> 
> v2:
> Fix the typo in subject of [PATCH v2 2/2] 
> 
> v1:
> This is the version 1 of the series and it is exactly the same as
> RFC version, but fixing a typo in subject, which is reported by Michael. 
> 
> As for test for the behavior suggested by Michael, IMHO, it could be
> post in another series, since i found that testing the negotiation
> behavior using QGraph Test Framework requires more work than i thought.
> 
> The test patch may implement the following logic...
> 1. Introduce a fresh new qmp command to query netdev info, which show
>the NetClient status including guest features and acked_features.
> 2. Using vhost-user QGraph Test to check the behavior of the vhost user
>protocol cmd VHOST_USER_SET_FEATURES. 
> 3. Adding acked_features into TestServer, which receive the features
>set by QEMU.
> 4. Compare the acked_feature in TestServer with the acked_features 
>in the output of qmp query command.
> 
> Patch for RFC can be found in the following:
> https://patchew.org/QEMU/20220926063641.25038-1-huang...@chinatelecom.cn/
> 
> This patchset aim to fix the unexpected negotiation features for
> vhost-user netdev interface. 
> 
> Steps to reproduce the issue:
> Prepare a vm (CentOS 8 in my work scenario) with vhost-user
> backend interface and configure qemu as server mode. So dpdk
> would connect qemu's unix socket periodically.
> 
> 1. start vm in background and restart openvswitch service 
>concurrently and repeatedly in the process of vm start. 
> 
> 2. check if negotiated virtio features of port is "0x4000" at
>dpdk side by executing:
>ovs-vsctl list interface | grep features | grep {port_socket_path}
>
> 3. if features equals "0x4000", go to the vm and check if sending 
>arp package works, executing:
>arping {IP_ADDR}
>if vm interface is configured to boot with dhcp protocol, it
>would get no ip. 
> 
> After doing above steps, we'll find the arping not work, the ovs on
> host side has forwarded unexpected arp packages, which be added 0x
> in the head of ethenet frame.  Though qemu report some error when
> read/write cmd of vhost protocol during the process of vm start,
> like the following:
> 
> "Failed to set msg fds"
> "vhost VQ 0 ring restore failed: -22: Invalid argument (22)"
> 
> The vm does not stop or report more suggestive error message, it
> seems that everthing is ok. 
> 
> The root cause is that dpdk port negotiated nothing but only one
> VHOST_USER_F_PROTOCOL_FEATURES feature with vhost-user interface at
> qemu side, which is an unexpected behavior. qemu only load the
> VHOST_USER_F_PROTOCOL_FEATURES when VHOST_USER_SET_FEATURES and loss
> the guest features configured by front-end virtio driver using the
> VIRTIO_PCI_COMMON_GF addr, which is stored in acked_features field
> of struct vhost_dev.
> 
> To explain how the acked_features disappear, we may need to know the
> lifecyle of acked_features in vhost_dev during feature negotiation. 
> 
> 1. qemu init acked_features field of struct vhost_dev in vhost_net_init()
>by calling vhost_net_ack_features(), the init value fetched from
>acked_features field of struct NetVhostUserState, which is the backup
>role after vhost stopping or unix socket closed.
>In the first time, the acked_features of struct NetVhostUserState is 0
>so the init value of vhost_dev's acked_features also 0. 
> 
> 2. when guest virtio driver set features, qemu accept the features and
>call virtio_set_features to store the features as acked_features in
>vhost_dev.
> 
> 3. when unix socket closed or vhost_dev device doesn't work and be
>stopped unexpectedly, qemu will call chr_closed_bh or vhost_user_stop,

[PATCH 2/4] target/loongarch: Restrict 'qapi-commands-machine.h' to system emulation

[Philippe Mathieu-Daudé]

Since commit a0e61807a3 ("qapi: Remove QMP events and commands from
user-mode builds") we don't generate the "qapi-commands-machine.h"
header in a user-emulation-only build.

Extract the QMP functions from cpu.c (which is always compiled) to
the new 'cpu-monitor.c' unit (which is only compiled when system
emulation is selected).

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/loongarch/cpu-monitor.c | 37 ++
 target/loongarch/cpu.c | 27 -
 target/loongarch/meson.build   |  1 +
 3 files changed, 38 insertions(+), 27 deletions(-)
 create mode 100644 target/loongarch/cpu-monitor.c

diff --git a/target/loongarch/cpu-monitor.c b/target/loongarch/cpu-monitor.c
new file mode 100644
index 00..6c25957881
--- /dev/null
+++ b/target/loongarch/cpu-monitor.c
@@ -0,0 +1,37 @@
+/*
+ * QEMU LoongArch CPU (monitor definitions)
+ *
+ * SPDX-FileCopyrightText: 2021 Loongson Technology Corporation Limited
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/qapi-commands-machine-target.h"
+#include "cpu.h"
+
+static void loongarch_cpu_add_definition(gpointer data, gpointer user_data)
+{
+ObjectClass *oc = data;
+CpuDefinitionInfoList **cpu_list = user_data;
+CpuDefinitionInfo *info = g_new0(CpuDefinitionInfo, 1);
+const char *typename = object_class_get_name(oc);
+
+info->name = g_strndup(typename,
+   strlen(typename) - strlen("-" TYPE_LOONGARCH_CPU));
+info->q_typename = g_strdup(typename);
+
+QAPI_LIST_PREPEND(*cpu_list, info);
+}
+
+CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
+{
+CpuDefinitionInfoList *cpu_list = NULL;
+GSList *list;
+
+list = object_class_get_list(TYPE_LOONGARCH_CPU, false);
+g_slist_foreach(list, loongarch_cpu_add_definition, _list);
+g_slist_free(list);
+
+return cpu_list;
+}
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index e7b0e12be6..0fb853d915 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -12,7 +12,6 @@
 #include "qemu/module.h"
 #include "sysemu/qtest.h"
 #include "exec/exec-all.h"
-#include "qapi/qapi-commands-machine-target.h"
 #include "cpu.h"
 #include "internals.h"
 #include "fpu/softfloat-helpers.h"
@@ -744,29 +743,3 @@ static const TypeInfo loongarch_cpu_type_infos[] = {
 };
 
 DEFINE_TYPES(loongarch_cpu_type_infos)
-
-static void loongarch_cpu_add_definition(gpointer data, gpointer user_data)
-{
-ObjectClass *oc = data;
-CpuDefinitionInfoList **cpu_list = user_data;
-CpuDefinitionInfo *info = g_new0(CpuDefinitionInfo, 1);
-const char *typename = object_class_get_name(oc);
-
-info->name = g_strndup(typename,
-   strlen(typename) - strlen("-" TYPE_LOONGARCH_CPU));
-info->q_typename = g_strdup(typename);
-
-QAPI_LIST_PREPEND(*cpu_list, info);
-}
-
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-CpuDefinitionInfoList *cpu_list = NULL;
-GSList *list;
-
-list = object_class_get_list(TYPE_LOONGARCH_CPU, false);
-g_slist_foreach(list, loongarch_cpu_add_definition, _list);
-g_slist_free(list);
-
-return cpu_list;
-}
diff --git a/target/loongarch/meson.build b/target/loongarch/meson.build
index 6376f9e84b..dfd65ae742 100644
--- a/target/loongarch/meson.build
+++ b/target/loongarch/meson.build
@@ -17,6 +17,7 @@ loongarch_tcg_ss.add(zlib)
 
 loongarch_softmmu_ss = ss.source_set()
 loongarch_softmmu_ss.add(files(
+  'cpu-monitor.c',
   'machine.c',
   'tlb_helper.c',
   'constant_timer.c',
-- 
2.38.1

[PATCH 4/4] target/ppc: Restrict 'qapi-commands-machine.h' to system emulation

[Philippe Mathieu-Daudé]

Since commit a0e61807a3 ("qapi: Remove QMP events and commands from
user-mode builds") we don't generate the "qapi-commands-machine.h"
header in a user-emulation-only build.

Move the QMP functions from cpu_init.c (which is always compiled) to
monitor.c (which is only compiled when system-emulation is selected).
Note ppc_cpu_class_by_name() is used by both file units, so we expose
its prototype in "cpu-qom.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/ppc/cpu-qom.h  |  2 ++
 target/ppc/cpu_init.c | 48 +
 target/ppc/monitor.c  | 50 ++-
 3 files changed, 52 insertions(+), 48 deletions(-)

diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
index 89ff88f28c..6431c520c8 100644
--- a/target/ppc/cpu-qom.h
+++ b/target/ppc/cpu-qom.h
@@ -31,6 +31,8 @@
 
 OBJECT_DECLARE_CPU_TYPE(PowerPCCPU, PowerPCCPUClass, POWERPC_CPU)
 
+ObjectClass *ppc_cpu_class_by_name(const char *name);
+
 typedef struct CPUArchState CPUPPCState;
 typedef struct ppc_tb_t ppc_tb_t;
 typedef struct ppc_dcr_t ppc_dcr_t;
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index cbf0081374..7858cc4c6c 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -40,7 +40,6 @@
 #include "qemu/cutils.h"
 #include "disas/capstone.h"
 #include "fpu/softfloat.h"
-#include "qapi/qapi-commands-machine-target.h"
 
 #include "helper_regs.h"
 #include "internal.h"
@@ -6816,7 +6815,7 @@ static const char *ppc_cpu_lookup_alias(const char *alias)
 return NULL;
 }
 
-static ObjectClass *ppc_cpu_class_by_name(const char *name)
+ObjectClass *ppc_cpu_class_by_name(const char *name)
 {
 char *cpu_model, *typename;
 ObjectClass *oc;
@@ -6956,51 +6955,6 @@ void ppc_cpu_list(void)
 #endif
 }
 
-static void ppc_cpu_defs_entry(gpointer data, gpointer user_data)
-{
-ObjectClass *oc = data;
-CpuDefinitionInfoList **first = user_data;
-const char *typename;
-CpuDefinitionInfo *info;
-
-typename = object_class_get_name(oc);
-info = g_malloc0(sizeof(*info));
-info->name = g_strndup(typename,
-   strlen(typename) - strlen(POWERPC_CPU_TYPE_SUFFIX));
-
-QAPI_LIST_PREPEND(*first, info);
-}
-
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-CpuDefinitionInfoList *cpu_list = NULL;
-GSList *list;
-int i;
-
-list = object_class_get_list(TYPE_POWERPC_CPU, false);
-g_slist_foreach(list, ppc_cpu_defs_entry, _list);
-g_slist_free(list);
-
-for (i = 0; ppc_cpu_aliases[i].alias != NULL; i++) {
-PowerPCCPUAlias *alias = _cpu_aliases[i];
-ObjectClass *oc;
-CpuDefinitionInfo *info;
-
-oc = ppc_cpu_class_by_name(alias->model);
-if (oc == NULL) {
-continue;
-}
-
-info = g_malloc0(sizeof(*info));
-info->name = g_strdup(alias->alias);
-info->q_typename = g_strdup(object_class_get_name(oc));
-
-QAPI_LIST_PREPEND(cpu_list, info);
-}
-
-return cpu_list;
-}
-
 static void ppc_cpu_set_pc(CPUState *cs, vaddr value)
 {
 PowerPCCPU *cpu = POWERPC_CPU(cs);
diff --git a/target/ppc/monitor.c b/target/ppc/monitor.c
index 8250b1304e..36e5b5eff8 100644
--- a/target/ppc/monitor.c
+++ b/target/ppc/monitor.c
@@ -1,5 +1,5 @@
 /*
- * QEMU monitor
+ * QEMU PPC (monitor definitions)
  *
  * Copyright (c) 2003-2004 Fabrice Bellard
  *
@@ -28,6 +28,9 @@
 #include "qemu/ctype.h"
 #include "monitor/hmp-target.h"
 #include "monitor/hmp.h"
+#include "qapi/qapi-commands-machine-target.h"
+#include "cpu-models.h"
+#include "cpu-qom.h"
 
 static target_long monitor_get_ccr(Monitor *mon, const struct MonitorDef *md,
int val)
@@ -172,3 +175,48 @@ int target_get_monitor_def(CPUState *cs, const char *name, 
uint64_t *pval)
 
 return -EINVAL;
 }
+
+static void ppc_cpu_defs_entry(gpointer data, gpointer user_data)
+{
+ObjectClass *oc = data;
+CpuDefinitionInfoList **first = user_data;
+const char *typename;
+CpuDefinitionInfo *info;
+
+typename = object_class_get_name(oc);
+info = g_malloc0(sizeof(*info));
+info->name = g_strndup(typename,
+   strlen(typename) - strlen(POWERPC_CPU_TYPE_SUFFIX));
+
+QAPI_LIST_PREPEND(*first, info);
+}
+
+CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
+{
+CpuDefinitionInfoList *cpu_list = NULL;
+GSList *list;
+int i;
+
+list = object_class_get_list(TYPE_POWERPC_CPU, false);
+g_slist_foreach(list, ppc_cpu_defs_entry, _list);
+g_slist_free(list);
+
+for (i = 0; ppc_cpu_aliases[i].alias != NULL; i++) {
+PowerPCCPUAlias *alias = _cpu_aliases[i];
+ObjectClass *oc;
+CpuDefinitionInfo *info;
+
+oc = ppc_cpu_class_by_name(alias->model);
+if (oc == NULL) {
+continue;
+}
+
+info = g_malloc0(sizeof(*info));
+info->name = g_strdup(alias->alias);
+info->q_typename = 

[PATCH 1/4] target/arm: Restrict 'qapi-commands-machine.h' to system emulation

[Philippe Mathieu-Daudé]

Since commit a0e61807a3 ("qapi: Remove QMP events and commands from
user-mode builds") we don't generate the "qapi-commands-machine.h"
header in a user-emulation-only build.

Move the QMP functions from helper.c (which is always compiled) to
monitor.c (which is only compiled when system-emulation is selected).

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/arm/helper.c   | 29 -
 target/arm/m_helper.c |  1 -
 target/arm/monitor.c  | 28 
 3 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index bac2ea62c4..399603b680 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -27,7 +27,6 @@
 #include "sysemu/cpu-timers.h"
 #include "sysemu/kvm.h"
 #include "qemu/range.h"
-#include "qapi/qapi-commands-machine-target.h"
 #include "qapi/error.h"
 #include "qemu/guest-random.h"
 #ifdef CONFIG_TCG
@@ -8514,34 +8513,6 @@ void arm_cpu_list(void)
 g_slist_free(list);
 }
 
-static void arm_cpu_add_definition(gpointer data, gpointer user_data)
-{
-ObjectClass *oc = data;
-CpuDefinitionInfoList **cpu_list = user_data;
-CpuDefinitionInfo *info;
-const char *typename;
-
-typename = object_class_get_name(oc);
-info = g_malloc0(sizeof(*info));
-info->name = g_strndup(typename,
-   strlen(typename) - strlen("-" TYPE_ARM_CPU));
-info->q_typename = g_strdup(typename);
-
-QAPI_LIST_PREPEND(*cpu_list, info);
-}
-
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-CpuDefinitionInfoList *cpu_list = NULL;
-GSList *list;
-
-list = object_class_get_list(TYPE_ARM_CPU, false);
-g_slist_foreach(list, arm_cpu_add_definition, _list);
-g_slist_free(list);
-
-return cpu_list;
-}
-
 /*
  * Private utility function for define_one_arm_cp_reg_with_opaque():
  * add a single reginfo struct to the hash table.
diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c
index 355cd4d60a..11008528b5 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -26,7 +26,6 @@
 #include "sysemu/cpus.h"
 #include "sysemu/kvm.h"
 #include "qemu/range.h"
-#include "qapi/qapi-commands-machine-target.h"
 #include "qapi/error.h"
 #include "qemu/guest-random.h"
 #ifdef CONFIG_TCG
diff --git a/target/arm/monitor.c b/target/arm/monitor.c
index ecdd5ee817..c8fa524002 100644
--- a/target/arm/monitor.c
+++ b/target/arm/monitor.c
@@ -227,3 +227,31 @@ CpuModelExpansionInfo 
*qmp_query_cpu_model_expansion(CpuModelExpansionType type,
 
 return expansion_info;
 }
+
+static void arm_cpu_add_definition(gpointer data, gpointer user_data)
+{
+ObjectClass *oc = data;
+CpuDefinitionInfoList **cpu_list = user_data;
+CpuDefinitionInfo *info;
+const char *typename;
+
+typename = object_class_get_name(oc);
+info = g_malloc0(sizeof(*info));
+info->name = g_strndup(typename,
+   strlen(typename) - strlen("-" TYPE_ARM_CPU));
+info->q_typename = g_strdup(typename);
+
+QAPI_LIST_PREPEND(*cpu_list, info);
+}
+
+CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
+{
+CpuDefinitionInfoList *cpu_list = NULL;
+GSList *list;
+
+list = object_class_get_list(TYPE_ARM_CPU, false);
+g_slist_foreach(list, arm_cpu_add_definition, _list);
+g_slist_free(list);
+
+return cpu_list;
+}
-- 
2.38.1

[PATCH 0/4] target: Restrict 'qapi-commands-machine.h' to system emulation

[Philippe Mathieu-Daudé]

The "qapi-commands-machine.h" header is not generated in user-only
emulation. This series removes its use in user-emu code by moving
the QMP code depending on this header into a separate sysemu unit.

Philippe Mathieu-Daudé (4):
  target/arm: Restrict 'qapi-commands-machine.h' to system emulation
  target/loongarch: Restrict 'qapi-commands-machine.h' to system
emulation
  target/mips: Restrict 'qapi-commands-machine.h' to system emulation
  target/ppc: Restrict 'qapi-commands-machine.h' to system emulation

 target/arm/helper.c| 29 
 target/arm/m_helper.c  |  1 -
 target/arm/monitor.c   | 28 +++
 target/loongarch/cpu-monitor.c | 37 +
 target/loongarch/cpu.c | 27 --
 target/loongarch/meson.build   |  1 +
 target/mips/cpu.c  | 29 
 target/mips/sysemu/meson.build |  1 +
 target/mips/sysemu/monitor.c   | 39 ++
 target/ppc/cpu-qom.h   |  2 ++
 target/ppc/cpu_init.c  | 48 +---
 target/ppc/monitor.c   | 50 +-
 12 files changed, 158 insertions(+), 134 deletions(-)
 create mode 100644 target/loongarch/cpu-monitor.c
 create mode 100644 target/mips/sysemu/monitor.c

-- 
2.38.1

[PATCH 3/4] target/mips: Restrict 'qapi-commands-machine.h' to system emulation

[Philippe Mathieu-Daudé]

Since commit a0e61807a3 ("qapi: Remove QMP events and commands from
user-mode builds") we don't generate the "qapi-commands-machine.h"
header in a user-emulation-only build.

Extract the QMP functions from cpu.c (which is always compiled) to
the new 'sysemu/monitor.c' unit (which is only compiled when system
emulation is selected).

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/cpu.c  | 29 -
 target/mips/sysemu/meson.build |  1 +
 target/mips/sysemu/monitor.c   | 39 ++
 3 files changed, 40 insertions(+), 29 deletions(-)
 create mode 100644 target/mips/sysemu/monitor.c

diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 7a565466cb..7a37123419 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -32,7 +32,6 @@
 #include "hw/qdev-properties.h"
 #include "hw/qdev-clock.h"
 #include "semihosting/semihost.h"
-#include "qapi/qapi-commands-machine-target.h"
 #include "fpu_helper.h"
 
 const char regnames[32][3] = {
@@ -627,34 +626,6 @@ static void mips_cpu_register_types(void)
 
 type_init(mips_cpu_register_types)
 
-static void mips_cpu_add_definition(gpointer data, gpointer user_data)
-{
-ObjectClass *oc = data;
-CpuDefinitionInfoList **cpu_list = user_data;
-CpuDefinitionInfo *info;
-const char *typename;
-
-typename = object_class_get_name(oc);
-info = g_malloc0(sizeof(*info));
-info->name = g_strndup(typename,
-   strlen(typename) - strlen("-" TYPE_MIPS_CPU));
-info->q_typename = g_strdup(typename);
-
-QAPI_LIST_PREPEND(*cpu_list, info);
-}
-
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-CpuDefinitionInfoList *cpu_list = NULL;
-GSList *list;
-
-list = object_class_get_list(TYPE_MIPS_CPU, false);
-g_slist_foreach(list, mips_cpu_add_definition, _list);
-g_slist_free(list);
-
-return cpu_list;
-}
-
 /* Could be used by generic CPU object */
 MIPSCPU *mips_cpu_create_with_clock(const char *cpu_type, Clock *cpu_refclk)
 {
diff --git a/target/mips/sysemu/meson.build b/target/mips/sysemu/meson.build
index cefc227582..0e424c9cc6 100644
--- a/target/mips/sysemu/meson.build
+++ b/target/mips/sysemu/meson.build
@@ -3,5 +3,6 @@ mips_softmmu_ss.add(files(
   'cp0.c',
   'cp0_timer.c',
   'machine.c',
+  'monitor.c',
   'physaddr.c',
 ))
diff --git a/target/mips/sysemu/monitor.c b/target/mips/sysemu/monitor.c
new file mode 100644
index 00..6db4626412
--- /dev/null
+++ b/target/mips/sysemu/monitor.c
@@ -0,0 +1,39 @@
+/*
+ * QEMU MIPS CPU (monitor definitions)
+ *
+ * SPDX-FileCopyrightText: 2012 SUSE LINUX Products GmbH
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/qapi-commands-machine-target.h"
+#include "cpu.h"
+
+static void mips_cpu_add_definition(gpointer data, gpointer user_data)
+{
+ObjectClass *oc = data;
+CpuDefinitionInfoList **cpu_list = user_data;
+CpuDefinitionInfo *info;
+const char *typename;
+
+typename = object_class_get_name(oc);
+info = g_malloc0(sizeof(*info));
+info->name = g_strndup(typename,
+   strlen(typename) - strlen("-" TYPE_MIPS_CPU));
+info->q_typename = g_strdup(typename);
+
+QAPI_LIST_PREPEND(*cpu_list, info);
+}
+
+CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
+{
+CpuDefinitionInfoList *cpu_list = NULL;
+GSList *list;
+
+list = object_class_get_list(TYPE_MIPS_CPU, false);
+g_slist_foreach(list, mips_cpu_add_definition, _list);
+g_slist_free(list);
+
+return cpu_list;
+}
-- 
2.38.1

Re: [PATCH] thread-posix: add support for setting threads name on OpenBSD

[Philippe Mathieu-Daudé]

On 18/12/22 09:22, Brad Smith wrote:

Make use of pthread_set_name_np() to be able to set the threads name
on OpenBSD.

Signed-off-by: Brad Smith 
---
  meson.build  | 12 
  util/qemu-thread-posix.c |  9 -
  2 files changed, 20 insertions(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 1/6] disas: add G_GNUC_PRINTF to gstring_printf

[Stefan Weil via]

Am 19.12.22 um 14:02 schrieb Daniel P. Berrangé:

Signed-off-by: Daniel P. Berrangé 
---
  disas.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/disas.c b/disas.c
index 94d3b45042..31df8f5b89 100644
--- a/disas.c
+++ b/disas.c
@@ -239,6 +239,7 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong 
code,
  }
  }
  
+G_GNUC_PRINTF(2, 3)

  static int gstring_printf(FILE *stream, const char *fmt, ...)
  {
  /* We abuse the FILE parameter to pass a GString. */


The current code uses a different pattern:

static RETURN_TYPE G_GNUC_PRINTF(2, 3) function(argument list)

So I would have expected

static int G_GNUC_PRINTF(2, 3)
gstring_printf(FILE *stream, const char *fmt, ...)

Does that matter? Do we care for different patterns?

Stefan


OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: Plugin Memory Callback Debugging

[Aaron Lindsay]

Emilio,

On Dec 18 00:24, Emilio Cota wrote:
> On Tue, Nov 29, 2022 at 15:37:51 -0500, Aaron Lindsay wrote:
> (snip)
> > > Does this hint that there are cases where reset cpu->plugin_mem_cbs to 
> > > NULL is
> > > getting optimized away, but not the code to set it in the first place?
> > 
> > Is there anyone who could help take a look at this from the code gen
> > perspective?
> 
> Thanks for the report. Just adding assertions was enough to uncover
> several bugs. I did not reproduce the use-after-free, but by calling
> reset from a callback it's easy to see how it can occur.
> 
> I have fixes in https://github.com/cota/qemu/tree/plugins
> 
> Can you please give those a try?
> 
> BTW I created an issue on gitlab to track this
>   https://gitlab.com/qemu-project/qemu/-/issues/1381

Thanks so much for digging into this!

I rebased your plugins branch on top of v7.2.0 and tested with several
scenarios which reliably triggered the bug for me. None of them
reproduced the original problem (or hit other bugs!) with your fixes.

-Aaron

Re: [PATCH] .gitlab/issue_templates: Move suggestions into comments

[Alex Bennée]

Thomas Huth  writes:

> Many users forget to remove the suggestions from the bug template
> when creating a new issue. So when searching for strings like "s390x"
> or "Windows", you get a lot of unrelated issues in the results.
> Thus let's move the suggestions into HTML comments - so they will
> still show up in the markdown when editing the bug, while being
> hidden/ignored in the final text or in the search queries.
>
> Signed-off-by: Thomas Huth 

Queued to testing/next, thanks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH] gitlab-ci: Check building ppc64 without TCG

[Alex Bennée]

Thomas Huth  writes:

> Building QEMU for ppc64 hosts with --disable-tcg used to break a couple
> of times in the past, see e.g. commit a01b64cee7 ("target/ppc: Put do_rfi
> under a TCG-only block") or commit 049b4ad669 ("target/ppc: Fix build
> warnings when building with 'disable-tcg'"), so we should test this in
> our CI to avoid such regressions.
>
> Signed-off-by: Thomas Huth 

Queued to testing/next, thanks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH] gitlab-ci: Disable docs and GUIs for the build-tci and build-tcg-disabled jobs

[Alex Bennée]

Thomas Huth  writes:

> These jobs use their own "script:" section and thus do not profit from
> the global "--disable-docs" from the template. While we're at it, disable
> also some GUI front ends here since we do not gain any additional test
> coverage by compiling those here again.
>
> Signed-off-by: Thomas Huth 

Queued to testing/next, thanks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH] tests/docker: use prebuilt toolchain for debian-hexagon-cross

[Alex Bennée]

Mukilan Thiyagarajan  writes:

> The current docker image for cross compiling hexagon guests
> is manually built since it takes >2 hours to build from source.
>
> This patch:
>  1. Solves the above issue by using the prebuilt clang
> toolchain hosted on CodeLinaro [1] and maintained by QUIC [2].
>  2. The dockerfile is also switched from multi-stage to single stage
> build to allow the CI docker engine to reuse the layer cache.
>  3. Re-enables the hexagon-cross-container job to be always run in
> CI and makes it a non-optional dependency for the
> build-user-hexagon job.
>
> The changes for 1 & 2 together bring down the build time to
> ~3 minutes in GitLab CI when cache is reused and ~9 minutes
> when cache cannot be reused.
>
> [1]: https://github.com/CodeLinaro/hexagon-builder
> [2]: https://github.com/quic/toolchain_for_hexagon/releases/
>
> Based-on: <20221215095820.13374-1-quic._5fmthiy...@quicinc.com>
>  tests/tcg/multiarch: remove unused variable in linux-test
> Signed-off-by: Mukilan Thiyagarajan 

Queued to testing/next, thanks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

RE: [PULL 00/21] Hexagon update: bug fixes, performance, idef-parser

[Taylor Simpson]

> -Original Message-
> From: Peter Maydell 
> Sent: Monday, December 19, 2022 4:28 AM
> To: Taylor Simpson 
> Cc: qemu-devel@nongnu.org; richard.hender...@linaro.org;
> phi...@linaro.org; Brian Cain ; Matheus Bernardino
> (QUIC) ; stefa...@redhat.com
> Subject: Re: [PULL 00/21] Hexagon update: bug fixes, performance, idef-
> parser
> 
> On Fri, 16 Dec 2022 at 20:49, Taylor Simpson  wrote:
> >
> > The following changes since commit
> 4208e6ae114ac8266dcacc9696a443ce5c37b04e:
> >
> >   Merge tag 'pull-request-2022-12-15' of https://gitlab.com/thuth/qemu
> > into staging (2022-12-15 21:39:56 +)
> >
> > are available in the Git repository at:
> >
> >   https://github.com/quic/qemu tags/pull-hex-20221216-1
> >
> > for you to fetch changes up to
> 585a86b1041a45c3b4074440c7f1b54944570867:
> >
> >   target/hexagon: import additional tests (2022-12-16 12:30:28 -0800)
> 
> 
> Applied, thanks.

Thanks!!

> 
> Please update the changelog at https://wiki.qemu.org/ChangeLog/8.0
> for any user-visible changes. (In particular there is a section of the 
> changelog
> for 'Build Dependencies' -- please add something there indicating under what
> circumstances flex/bison are now required.)

I've forgotten my password.  How do I reset it?

Taylor

Re: [RFC PATCH for 8.0] backends/vhost-user: relax the ioeventfd check

[Stefan Hajnoczi]

On Fri, 2 Dec 2022 at 08:23, Alex Bennée  wrote:
>
> While you certainly need ioeventfds to work for KVM guests it
> shouldn't be limited to that. We can run vhost-user backends for TCG
> guests and either use ioeventfds or in band signalling.

I suggest tweaking this sentence so it's clear that you mean QEMU's
simulated eventfds rather than kvm.ko's real ioeventfds:
s/use eventfds or in band signalling/simulate ioeventfds or use in
band signalling/

>
> Maybe we should apply the same fix as b0aa77d36d (vhost-user: fix
> ioeventfd_enabled)?
>
> With this change I can run:
>
>   $QEMU $OPTS \
> -display gtk,gl=on \
> -device vhost-user-gpu-pci,chardev=vhgpu \
> -chardev socket,id=vhgpu,path=vhgpu.sock
>
> with:
>
>   ./contrib/vhost-user-gpu/vhost-user-gpu \
> -s vhgpu.sock \
> -v
>
> and at least see things start-up (although the display gets rotated by
> 180 degrees).
>
> Signed-off-by: Alex Bennée 
> ---
>  backends/vhost-user.c | 10 ++
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/backends/vhost-user.c b/backends/vhost-user.c
> index 5dedb2d987..87d43fb03a 100644
> --- a/backends/vhost-user.c
> +++ b/backends/vhost-user.c
> @@ -21,12 +21,6 @@
>  #include "io/channel-command.h"
>  #include "hw/virtio/virtio-bus.h"
>
> -static bool
> -ioeventfd_enabled(void)
> -{
> -return kvm_enabled() && kvm_eventfds_enabled();
> -}
> -
>  int
>  vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev,
>  unsigned nvqs, Error **errp)
> @@ -35,8 +29,8 @@ vhost_user_backend_dev_init(VhostUserBackend *b, 
> VirtIODevice *vdev,
>
>  assert(!b->vdev && vdev);
>
> -if (!ioeventfd_enabled()) {
> -error_setg(errp, "vhost initialization failed: requires kvm");
> +if (kvm_enabled() && !kvm_eventfds_enabled()) {
> +error_setg(errp, "vhost initialization failed: kvm required 
> ioeventfds");
>  return -1;
>  }

The check can be dropped completely. If kvm.ko doesn't support
ioeventfds QEMU can simulate them just like it does with TCG. In
practice I don't think this happens with a reasonably modern kernel
version.

Stefan

Re: [PATCH 5/5] tests/unit: Restrict machine-smp.c test to system emulation

[Richard Henderson]

On 12/19/22 09:08, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daude

Signed-off-by: Philippe Mathieu-Daudé
---
  tests/unit/meson.build | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 3/5] accel/tcg: Restrict 'qapi-commands-machine.h' to system emulation

[Richard Henderson]

On 12/19/22 09:08, Philippe Mathieu-Daudé wrote:

Since commit a0e61807a3 ("qapi: Remove QMP events and commands from
user-mode builds") we don't generate the "qapi-commands-machine.h"
header in a user-emulation-only build.

Rename 'hmp.c' as 'monitor.c' and move the QMP functions from
cpu-exec.c (which is always compiled) to monitor.c (which is only
compiled when system-emulation is selected).

Signed-off-by: Philippe Mathieu-Daudé
---
  accel/tcg/cpu-exec.c  |  88 +--
  accel/tcg/hmp.c   |  14 --
  accel/tcg/internal.h  |   3 ++
  accel/tcg/meson.build |   2 +-
  accel/tcg/monitor.c   | 105 ++
  5 files changed, 111 insertions(+), 101 deletions(-)
  delete mode 100644 accel/tcg/hmp.c
  create mode 100644 accel/tcg/monitor.c


Reviewed-by: Richard Henderson 


r~

Re: [PATCH 2/5] tcg/tcg-op-gvec: Remove unused "qemu/main-loop.h" header

[Richard Henderson]

On 12/19/22 09:08, Philippe Mathieu-Daudé wrote:

Signed-off-by: Philippe Mathieu-Daudé 
---
Added in db72581598 ("Include qemu/main-loop.h less") but now
I can't see what is requiring it.

I checked 'git diff db725815985..origin/master -- tcg/tcg-op-gvec.c'
to see if some code was removed without removing the header;
I haven't checked individual commits.
---
  tcg/tcg-op-gvec.c | 1 -
  1 file changed, 1 deletion(-)


I can only imagine that it was cut-and-paste.

Reviewed-by: Richard Henderson 

r~



diff --git a/tcg/tcg-op-gvec.c b/tcg/tcg-op-gvec.c
index 079a761b04..aacedd3e15 100644
--- a/tcg/tcg-op-gvec.c
+++ b/tcg/tcg-op-gvec.c
@@ -21,7 +21,6 @@
  #include "tcg/tcg.h"
  #include "tcg/tcg-op.h"
  #include "tcg/tcg-op-gvec.h"
-#include "qemu/main-loop.h"
  #include "tcg/tcg-gvec-desc.h"
  
  #define MAX_UNROLL  4

Re: [PATCH 1/5] exec: Remove unused 'qemu/timer.h' timer

[Richard Henderson]

On 12/19/22 09:08, Philippe Mathieu-Daudé wrote:

Signed-off-by: Philippe Mathieu-Daudé
---
  accel/tcg/cpu-exec.c  | 1 -
  accel/tcg/translate-all.c | 1 -
  include/exec/gen-icount.h | 1 -
  include/sysemu/cpus.h | 1 -
  tcg/tcg.c | 1 -
  5 files changed, 5 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 7/7] libvhost-user: Switch to unsigned int for inuse field in struct VuVirtq

[Philippe Mathieu-Daudé]

On 19/12/22 18:53, Marcel Holtmann wrote:

It seems there is no need to keep the inuse field signed and end up with
compiler warnings for sign-compare.

   CC   libvhost-user.o
libvhost-user.c: In function ‘vu_queue_pop’:
libvhost-user.c:2763:19: error: comparison of integer expressions of different 
signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
  2763 | if (vq->inuse >= vq->vring.num) {
   |   ^~
libvhost-user.c: In function ‘vu_queue_rewind’:
libvhost-user.c:2808:13: error: comparison of integer expressions of different 
signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
  2808 | if (num > vq->inuse) {
   | ^

Instead of casting the comparision to unsigned int, just make the inuse
field unsigned int in the fist place.

Signed-off-by: Marcel Holtmann 
---
  subprojects/libvhost-user/libvhost-user.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.h 
b/subprojects/libvhost-user/libvhost-user.h
index aea7ec5061d5..8cda9b8f577a 100644
--- a/subprojects/libvhost-user/libvhost-user.h
+++ b/subprojects/libvhost-user/libvhost-user.h
@@ -343,7 +343,7 @@ typedef struct VuVirtq {
  /* Notification enabled? */
  bool notification;
  
-int inuse;

+unsigned int inuse;


Another use in subprojects/libvduse/libvduse.c.

Possibly both could be renamed refcount which is more meaningful.

Anyhow,
Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 3/6] spec: Add NBD_OPT_EXTENDED_HEADERS

[Vladimir Sementsov-Ogievskiy]

On 11/15/22 01:46, Eric Blake wrote:

Add a new negotiation feature where the client and server agree to use
larger packet headers on every packet sent during transmission phase.
This has two purposes: first, it makes it possible to perform
operations like trim, write zeroes, and block status on more than 2^32
bytes in a single command.  For NBD_CMD_READ, replies are still
implicitly capped by the maximum block payload limits (generally 32M);
if you want to know if a hole larger than 32 bits can represent,
you'll use BLOCK_STATUS instead of hoping that a large READ will
either return a hole or report overflow.  But for
NBD_CMD_BLOCK_STATUS, it is very useful to be able to report a status
extent with a size larger than 32-bits, in some cases even if the
client's request was for smaller than 32-bits (such as when it is
known that the entire image is not sparse).  Thus, the wording chosen
here is careful to permit a server to use either flavor status chunk
type in its reply, and clients requesting extended headers must be
prepared for both reply types.

Second, when structured replies are active, clients have to deal with
the difference between 16- and 20-byte headers of simple
vs. structured replies, which impacts performance if the client must
perform multiple syscalls to first read the magic before knowing if
there are even additional bytes to read to learn a payload length.  In
extended header mode, all headers are the same width and there are no
simple replies permitted.  The layout of the reply header is more like
the request header; and including the client's offset in the reply
makes it easier to convert between absolute and relative offsets for
replies to NBD_CMD_READ.


Hmm.. Can't imagine, probably will see it in code patches.


 Similarly, by having extended mode use a
power-of-2 sizing, it becomes easier to manipulate arrays of headers
without worrying about an individual header crossing a cache line.


Do we really manipulate with such arrays in Qemu or Libnbd or Nbdkit?


However, note that this change only affects the headers; data payloads
can still be unaligned (for example, a client performing 1-byte reads
or writes).  We would need to negotiate yet another extension if we
wanted to ensure that all NBD transmission packets started on an
8-byte boundary after option haggling has completed.

This spec addition was done in parallel with proof of concept
implementations in qemu (server and client), libnbd (client), and
nbdkit (server).

Signed-off-by: Eric Blake 


In general looks good to me.

I'm not sure in extra bytes to get 8bytes-alignment.. Maybe that's a right way.


---
  doc/proto.md | 481 ++-
  1 file changed, 358 insertions(+), 123 deletions(-)

diff --git a/doc/proto.md b/doc/proto.md
index 53c334a..fde1e70 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -280,34 +280,53 @@ a soft disconnect.

  ### Transmission

-There are three message types in the transmission phase: the request,
-the simple reply, and the structured reply chunk.  The
+There are two general message types in the transmission phase: the
+request (simple or extended), and the reply (simple, structured, or
+extended).  The determination of which message headers to use is
+determined during handshaking phase, based on whether


"determination is determined" sounds strange

should we also define "compact" in this paragraph? [*]


+`NBD_OPT_STRUCTURED_REPLY` or `NBD_OPT_EXTENDED_HEADERS` was requested
+by the client and given a successful response by the server.  The
  transmission phase consists of a series of transactions, where the
  client submits requests and the server sends corresponding replies
  with either a single simple reply or a series of one or more
-structured reply chunks per request.  The phase continues until
-either side terminates transmission; this can be performed cleanly
-only by the client.
+structured or extended reply chunks per request.  The phase continues
+until either side terminates transmission; this can be performed
+cleanly only by the client.

  Note that without client negotiation, the server MUST use only simple
  replies, and that it is impossible to tell by reading the server
  traffic in isolation whether a data field will be present; the simple
  reply is also problematic for error handling of the `NBD_CMD_READ`
-request.  Therefore, structured replies can be used to create a
-a context-free server stream; see below.
+request.  Therefore, structured or extended replies can be used to
+create a a context-free server stream; see below.


s/a a/a/


+
+The results of client negotiation also determine whether the client
+and server will utilize only compact requests and replies, or whether


first place, where "compact" term appears and it is not defined. [*]


+both sides will use only extended packets.  Compact messages are the
+default, but inherently limit single transactions to a 32-bit window
+starting at a 64-bit offset.  

RE: [PATCH v2] Hexagon (target/hexagon) implement mutability mask for GPRs

[Taylor Simpson]

> -Original Message-
> From: Marco Liebel (QUIC) 
> Sent: Friday, December 16, 2022 2:04 PM
> To: qemu-devel@nongnu.org
> Cc: Brian Cain ; Taylor Simpson
> ; Marco Liebel (QUIC) 
> Subject: [PATCH v2] Hexagon (target/hexagon) implement mutability mask
> for GPRs
> 
> Some registers are defined to have immutable bits, this commit will
> implement that behavior.
> 
> Signed-off-by: Marco Liebel 
> ---
> This incorporates the feedback given on Brian's patch
> 
>  target/hexagon/genptr.c   |  43 -
>  tests/tcg/hexagon/Makefile.target |   3 +
>  tests/tcg/hexagon/reg_mut.c   | 307
> ++
>  3 files changed, 351 insertions(+), 2 deletions(-)  create mode 100644
> tests/tcg/hexagon/reg_mut.c
> 
> diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c index
> 806d0974ff..a91db8c76d 100644
> --- a/target/hexagon/genptr.c
> +++ b/target/hexagon/genptr.c
> @@ -30,6 +30,32 @@
>  #include "gen_tcg.h"
>  #include "gen_tcg_hvx.h"
> 
> +static const target_ulong reg_immut_masks[TOTAL_PER_THREAD_REGS] =
> {
> +[HEX_REG_USR] = 0xc13000c0,
> +[HEX_REG_PC] = UINT32_MAX,
> +[HEX_REG_GP] = 0x3f,
> +[HEX_REG_UPCYCLELO] = UINT32_MAX,
> +[HEX_REG_UPCYCLEHI] = UINT32_MAX,
> +[HEX_REG_UTIMERLO] = UINT32_MAX,
> +[HEX_REG_UTIMERHI] = UINT32_MAX,
> +};

UINT_MAX is confusing.  Use ~0 instead.

#define IMMUTABLE (~0)
...
[HEX_REG_PC] = IMMUTABLE
...

> +
> +static inline void gen_masked_reg_write(TCGv result, TCGv new_val, TCGv
> cur_val,
> +target_ulong reg_mask) {
> +if (reg_mask) {
> +TCGv tmp = tcg_temp_new();
> +
> +/* out_val = (in_val & reg_mask) | (cur_val & ~reg_mask) */

Comment doesn't match the TCG code below.  Should be
/* result = (new_val & ~reg_mask) | (cur_val & reg_mask) */

> +/* result is used to avoid creating a second temporary variable */
> +tcg_gen_andi_tl(result, new_val, ~reg_mask);
> +tcg_gen_andi_tl(tmp, cur_val, reg_mask);
> +tcg_gen_or_tl(result, result, tmp);
> +
> +tcg_temp_free(tmp);
> +}
> +}

All of the callers of this function pass the same TCGv for result and new_val.  
So, we are getting lucky that this function doesn't set result when reg_mask is 
zero.  In order to future-proof this function, either eliminate the result 
parameter or assign to it from new_val when reg_mask is zero.

> +
>  static inline void gen_log_predicated_reg_write(int rnum, TCGv val, int slot)
> {
>  TCGv zero = tcg_constant_tl(0);
> diff --git a/tests/tcg/hexagon/Makefile.target
> b/tests/tcg/hexagon/Makefile.target
> index 96a4d7a614..4e9a20960b 100644
> --- a/tests/tcg/hexagon/Makefile.target
> +++ b/tests/tcg/hexagon/Makefile.target
> @@ -43,9 +43,12 @@ HEX_TESTS += load_align  HEX_TESTS += atomics
> HEX_TESTS += fpstuff  HEX_TESTS += overflow
> +HEX_TESTS += reg_mut
> 
>  TESTS += $(HEX_TESTS)
> 
> +reg_mut: CFLAGS += -I$(SRC_PATH)/target/hexagon/
> +

You won't need this when you remove hex_regs.h from the test (see below).

>  # This test has to be compiled for the -mv67t target
>  usr: usr.c
>   $(CC) $(CFLAGS) -mv67t -O2 -Wno-inline-asm -Wno-expansion-to-
> defined $< -o $@ $(LDFLAGS) diff --git a/tests/tcg/hexagon/reg_mut.c
> b/tests/tcg/hexagon/reg_mut.c new file mode 100644 index
> 00..8bbc1559dd
> --- /dev/null
> +++ b/tests/tcg/hexagon/reg_mut.c
> @@ -0,0 +1,307 @@
> +
> +/*
> + *  Copyright(c) 2022 Qualcomm Innovation Center, Inc. All Rights Reserved.
> + *
> + *  This program is free software; you can redistribute it and/or
> +modify
> + *  it under the terms of the GNU General Public License as published
> +by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, see .
> + */
> +
> +#include 
> +#include 
> +
> +#include "hex_regs.h"

Don't include headers from QEMU  itself in the test cases.

You don't need this because you are just converting from the enum to a string.  
Just use the string itself and skip the switch statement.

> +
> +static int err;
> +
> +enum {
> +HEX_REG_PAIR_C9_8,
> +HEX_REG_PAIR_C11_10,
> +HEX_REG_PAIR_C15_14,
> +HEX_REG_PAIR_C31_30,
> +};

Shouldn't need this if you switch to using strings for the register names.

> +#define WRITE_REG(reg_name, output, input) \
> +asm volatile(reg_name " = %1\n\t" \
> + "%0 = " reg_name "\n\t" \
> + : "=r"(output) \
> + : "r"(input) \
> + : reg_name);
> +
> +#define 

Re: [PATCH v4 01/27] tcg: Fix tcg_reg_alloc_dup*

[Richard Henderson]

On 12/19/22 07:49, Alex Bennée wrote:


Richard Henderson  writes:


The assignment to mem_coherent should be done with any
modification, not simply with a newly allocated register.


What exactly does mem_coherent mean in this case? Is it that our
register store is potentially out of sync with live values in temp regs
or that we have memory loads and stores in flight?

I think it would be useful to add a doc patch for TCGTemp do specify
what the various fields mean. It would certainly help reviewers that
don't have it committed to memory ;-)



mem_coherent means that the register value and the memory value are the same, so that if 
we must release the register we do not need to save the value back to memory.



r~

[PATCH 5/7] libvhost-user: Declare uffdio_register early to make it C90 compliant

[Marcel Holtmann]

When using libvhost-user source in an external project that wants to
comply with the C90 standard, it is best to declare variables before
code.

  CC   libvhost-user.o
libvhost-user.c: In function ‘generate_faults’:
libvhost-user.c:683:9: error: ISO C90 forbids mixed declarations and code 
[-Werror=declaration-after-statement]
  683 | struct uffdio_register reg_struct;
  | ^~

In this case, it is also simple enough and doesn't cause any extra
ifdef additions.

Signed-off-by: Marcel Holtmann 
---
 subprojects/libvhost-user/libvhost-user.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index 211d31a4cc88..bf92cc85c086 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -626,6 +626,8 @@ generate_faults(VuDev *dev) {
 VuDevRegion *dev_region = >regions[i];
 int ret;
 #ifdef UFFDIO_REGISTER
+struct uffdio_register reg_struct;
+
 /*
  * We should already have an open ufd. Mark each memory
  * range as ufd.
@@ -659,7 +661,7 @@ generate_faults(VuDev *dev) {
 "%s: Failed to madvise(NOHUGEPAGE) region %d: %s\n",
 __func__, i, strerror(errno));
 }
-struct uffdio_register reg_struct;
+
 reg_struct.range.start = (uintptr_t)dev_region->mmap_addr;
 reg_struct.range.len = dev_region->size + dev_region->mmap_offset;
 reg_struct.mode = UFFDIO_REGISTER_MODE_MISSING;
-- 
2.38.1

[PATCH 3/7] libvhost-user: Cast rc variable to avoid compiler warning

[Marcel Holtmann]

The assert from recvmsg() return value against an uint32_t size field
from a protocol struct throws a compiler warning.

  CC   libvhost-user.o
In file included from libvhost-user.c:27:
libvhost-user.c: In function ‘vu_message_read_default’:
libvhost-user.c:363:19: error: comparison of integer expressions of different 
signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Werror=sign-compare]
  363 | assert(rc == vmsg->size);
  |   ^~

This is not critical, but annoying when the libvhost-user source are
used in an external project that has this compiler warning switched on.

Signed-off-by: Marcel Holtmann 
---
 subprojects/libvhost-user/libvhost-user.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index 67d75ece53b7..bcdf32a24f60 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -339,7 +339,7 @@ vu_message_read_default(VuDev *dev, int conn_fd, 
VhostUserMsg *vmsg)
 goto fail;
 }
 
-assert(rc == vmsg->size);
+assert((uint32_t)rc == vmsg->size);
 }
 
 return true;
-- 
2.38.1

[PATCH 0/7] Various compiler warning fixes for libvhost-user

[Marcel Holtmann]

The libvhost-user library is also useful for external usage outside of
QEMU and thus it would be nice if the two files libvhost-user.[ch] could
be just copied and used. However due to different compiler settings, a
lot of manual fixups are needed. This is the first attempt at some
obvious fixes that can be done without any harm to the code and its
readabilty.

Marcel Holtmann (7):
  libvhost-user: Provide _GNU_SOURCE when compiling outside of QEMU
  libvhost-user: Replace typeof with __typeof__
  libvhost-user: Cast rc variable to avoid compiler warning
  libvhost-user: Use unsigned int i for some for-loop iterations
  libvhost-user: Declare uffdio_register early to make it C90 compliant
  libvhost-user: Change dev->postcopy_ufd assignment to make it C90 compliant
  libvhost-user: Switch to unsigned int for inuse field in struct VuVirtq

 subprojects/libvhost-user/libvhost-user.c | 31 ++-
 subprojects/libvhost-user/libvhost-user.h |  2 +-
 2 files changed, 20 insertions(+), 13 deletions(-)

-- 
2.38.1

[PATCH 1/7] libvhost-user: Provide _GNU_SOURCE when compiling outside of QEMU

[Marcel Holtmann]

Then the libvhost-user sources are used by another project, it can not
be guaranteed that _GNU_SOURCE is set by the build system. If it is for
example not set, errors like this show up.

  CC   libvhost-user.o
libvhost-user.c: In function ‘vu_panic’:
libvhost-user.c:195:9: error: implicit declaration of function ‘vasprintf’; did 
you mean ‘vsprintf’? [-Werror=implicit-function-declaration]
  195 | if (vasprintf(, msg, ap) < 0) {
  | ^
  | vsprintf

The simplest way to allow external complication of libvhost-user.[ch] is
by setting _GNU_SOURCE if it is not already set by the build system.

Signed-off-by: Marcel Holtmann 
---
 subprojects/libvhost-user/libvhost-user.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index d6ee6e7d9168..b55b9e244d9a 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -13,6 +13,10 @@
  * later.  See the COPYING file in the top-level directory.
  */
 
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+
 /* this code avoids GLib dependency */
 #include 
 #include 
-- 
2.38.1

[PATCH 2/7] libvhost-user: Replace typeof with __typeof__

[Marcel Holtmann]

Strictly speaking only -std=gnu99 support the usage of typeof and for
easier inclusion in external projects, it is better to use __typeof__.

  CC   libvhost-user.o
libvhost-user.c: In function ‘vu_log_queue_fill’:
libvhost-user.c:86:13: error: implicit declaration of function ‘typeof’ 
[-Werror=implicit-function-declaration]
   86 | typeof(x) _min1 = (x);  \
  | ^~

Changing these two users of typeof makes the compiler happy and no extra
flags or pragmas need to be provided.

Signed-off-by: Marcel Holtmann 
---
 subprojects/libvhost-user/libvhost-user.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index b55b9e244d9a..67d75ece53b7 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -62,8 +62,8 @@
 #endif  /* !__GNUC__ */
 #ifndef MIN
 #define MIN(x, y) ({\
-typeof(x) _min1 = (x);  \
-typeof(y) _min2 = (y);  \
+__typeof__(x) _min1 = (x);  \
+__typeof__(y) _min2 = (y);  \
 (void) (&_min1 == &_min2);  \
 _min1 < _min2 ? _min1 : _min2; })
 #endif
-- 
2.38.1

Re: [PATCH 08/11] contrib/gitdm: Add Facebook the domain map

[Peter Delevoryas]

> On Dec 19, 2022, at 7:49 AM, Philippe Mathieu-Daudé  wrote:
> 
> !---|
> This Message Is From an External Sender
> 
> |---!
> 
> On 19/12/22 16:22, Peter Delevoryas wrote:
>>> On Dec 19, 2022, at 4:19 AM, Alex Bennée  wrote:
>>> 
>>> !---|
>>>  This Message Is From an External Sender
>>> 
>>> |---!
>>> 
>>> A number of Facebook developers contribute to the project. Peter can
>>> you confirm your want pjd.dev contributions counted here or as
>>> an individual contributor?
>> Oh, hey: yes I can confirm that, I want pjd.dev contributions counted here 
>> as fb stuff.
>> By the way: recently, every Facebook developers email has migrated to 
>> “usern...@meta.com”. So now all my fb.com email goes to my meta.com inbox. 
>> We may or may not want to include both emails. I think the fb.com emails 
>> will stay around for quite a while, but yeah.
> 
> gitdm tool is only use to display statistics from your past
> contributions. If you want the get_maintainers.pl script to
> use your updated email address, you probably want to send a
> patch to update the .mailmap file.

Ok: at this point, I just use pe...@pjd.dev  because the 
email filters at Facebook are so onerous and drop things mysteriously all the 
time, so I’ll probably just keep using that I guess. But maybe I’ll try 
submitting a single patch as p...@meta.com  sometime.

Thanks!
Peter

> 
>>> Signed-off-by: Alex Bennée 
>>> Cc: Iris Chen 
>>> Cc: Peter Delevoryas 
>>> Cc: Peter Delevoryas 
>>> Cc: Daniel Müller 
>>> ---
>>> contrib/gitdm/domain-map | 1 +
>>> contrib/gitdm/group-map-facebook | 5 +
>>> gitdm.config | 1 +
>>> 3 files changed, 7 insertions(+)
>>> create mode 100644 contrib/gitdm/group-map-facebook

Re: [PATCH 08/11] contrib/gitdm: Add Facebook the domain map

[Peter Delevoryas]

> On Dec 19, 2022, at 7:56 AM, Peter Delevoryas  wrote:
> 
> 
> 
>> On Dec 19, 2022, at 7:49 AM, Philippe Mathieu-Daudé  
>> wrote:
>> 
>> !---|
>> This Message Is From an External Sender
>> 
>> |---!
>> 
>> On 19/12/22 16:22, Peter Delevoryas wrote:
 On Dec 19, 2022, at 4:19 AM, Alex Bennée  wrote:
 
 !---|
 This Message Is From an External Sender
 
 |---!
 
 A number of Facebook developers contribute to the project. Peter can
 you confirm your want pjd.dev contributions counted here or as
 an individual contributor?
>>> Oh, hey: yes I can confirm that, I want pjd.dev contributions counted here 
>>> as fb stuff.
>>> By the way: recently, every Facebook developers email has migrated to 
>>> “usern...@meta.com”. So now all my fb.com email goes to my meta.com inbox. 
>>> We may or may not want to include both emails. I think the fb.com emails 
>>> will stay around for quite a while, but yeah.
>> 
>> gitdm tool is only use to display statistics from your past
>> contributions. If you want the get_maintainers.pl script to
>> use your updated email address, you probably want to send a
>> patch to update the .mailmap file.
> 
> Ok: at this point, I just use pe...@pjd.dev  because 
> the email filters at Facebook are so onerous and drop things mysteriously all 
> the time, so I’ll probably just keep using that I guess. But maybe I’ll try 
> submitting a single patch as p...@meta.com  sometime.

Oh, almost forgot:

Reviewed-by: Peter Delevoryas mailto:pe...@pjd.dev>>

> 
> Thanks!
> Peter
> 
>> 
 Signed-off-by: Alex Bennée 
 Cc: Iris Chen 
 Cc: Peter Delevoryas 
 Cc: Peter Delevoryas 
 Cc: Daniel Müller 
 ---
 contrib/gitdm/domain-map | 1 +
 contrib/gitdm/group-map-facebook | 5 +
 gitdm.config | 1 +
 3 files changed, 7 insertions(+)
 create mode 100644 contrib/gitdm/group-map-facebook

[PATCH 7/7] libvhost-user: Switch to unsigned int for inuse field in struct VuVirtq

[Marcel Holtmann]

It seems there is no need to keep the inuse field signed and end up with
compiler warnings for sign-compare.

  CC   libvhost-user.o
libvhost-user.c: In function ‘vu_queue_pop’:
libvhost-user.c:2763:19: error: comparison of integer expressions of different 
signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
 2763 | if (vq->inuse >= vq->vring.num) {
  |   ^~
libvhost-user.c: In function ‘vu_queue_rewind’:
libvhost-user.c:2808:13: error: comparison of integer expressions of different 
signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
 2808 | if (num > vq->inuse) {
  | ^

Instead of casting the comparision to unsigned int, just make the inuse
field unsigned int in the fist place.

Signed-off-by: Marcel Holtmann 
---
 subprojects/libvhost-user/libvhost-user.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.h 
b/subprojects/libvhost-user/libvhost-user.h
index aea7ec5061d5..8cda9b8f577a 100644
--- a/subprojects/libvhost-user/libvhost-user.h
+++ b/subprojects/libvhost-user/libvhost-user.h
@@ -343,7 +343,7 @@ typedef struct VuVirtq {
 /* Notification enabled? */
 bool notification;
 
-int inuse;
+unsigned int inuse;
 
 vu_queue_handler_cb handler;
 
-- 
2.38.1

[PATCH 4/7] libvhost-user: Use unsigned int i for some for-loop iterations

[Marcel Holtmann]

The sign-compare warning also hits some of the for-loops, but it easy
fixed by just making the iterator variable unsigned int.

  CC   libvhost-user.o
libvhost-user.c: In function ‘vu_gpa_to_va’:
libvhost-user.c:223:19: error: comparison of integer expressions of different 
signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Werror=sign-compare]
  223 | for (i = 0; i < dev->nregions; i++) {
  |   ^

Signed-off-by: Marcel Holtmann 
---
 subprojects/libvhost-user/libvhost-user.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index bcdf32a24f60..211d31a4cc88 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -192,7 +192,7 @@ vu_panic(VuDev *dev, const char *msg, ...)
 void *
 vu_gpa_to_va(VuDev *dev, uint64_t *plen, uint64_t guest_addr)
 {
-int i;
+unsigned int i;
 
 if (*plen == 0) {
 return NULL;
@@ -218,7 +218,7 @@ vu_gpa_to_va(VuDev *dev, uint64_t *plen, uint64_t 
guest_addr)
 static void *
 qva_to_va(VuDev *dev, uint64_t qemu_addr)
 {
-int i;
+unsigned int i;
 
 /* Find matching memory region.  */
 for (i = 0; i < dev->nregions; i++) {
@@ -621,7 +621,7 @@ map_ring(VuDev *dev, VuVirtq *vq)
 
 static bool
 generate_faults(VuDev *dev) {
-int i;
+unsigned int i;
 for (i = 0; i < dev->nregions; i++) {
 VuDevRegion *dev_region = >regions[i];
 int ret;
@@ -829,7 +829,7 @@ static inline bool reg_equal(VuDevRegion *vudev_reg,
 static bool
 vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
 VhostUserMemoryRegion m = vmsg->payload.memreg.region, *msg_region = 
-int i;
+unsigned int i;
 bool found = false;
 
 if (vmsg->fd_num > 1) {
@@ -895,7 +895,7 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
 static bool
 vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg *vmsg)
 {
-int i;
+unsigned int i;
 VhostUserMemory m = vmsg->payload.memory, *memory = 
 dev->nregions = memory->nregions;
 
@@ -972,7 +972,7 @@ vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg 
*vmsg)
 static bool
 vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)
 {
-int i;
+unsigned int i;
 VhostUserMemory m = vmsg->payload.memory, *memory = 
 
 for (i = 0; i < dev->nregions; i++) {
@@ -1980,7 +1980,7 @@ end:
 void
 vu_deinit(VuDev *dev)
 {
-int i;
+unsigned int i;
 
 for (i = 0; i < dev->nregions; i++) {
 VuDevRegion *r = >regions[i];
-- 
2.38.1

[PATCH 6/7] libvhost-user: Change dev->postcopy_ufd assignment to make it C90 compliant

[Marcel Holtmann]

The assignment of dev->postcopy_ufd can be moved into an else clause and
then the code becomes C90 compliant.

  CC   libvhost-user.o
libvhost-user.c: In function ‘vu_set_postcopy_advise’:
libvhost-user.c:1625:5: error: ISO C90 forbids mixed declarations and code 
[-Werror=declaration-after-statement]
 1625 | struct uffdio_api api_struct;
  | ^~

Understandable, it might be desired to avoid else clauses, but in this
case it seems clear enough and frankly the dev->postcopy_ufd is only
assigned once.

Signed-off-by: Marcel Holtmann 
---
 subprojects/libvhost-user/libvhost-user.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index bf92cc85c086..b28b66cdb159 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -1599,12 +1599,13 @@ vu_set_config(VuDev *dev, VhostUserMsg *vmsg)
 static bool
 vu_set_postcopy_advise(VuDev *dev, VhostUserMsg *vmsg)
 {
-dev->postcopy_ufd = -1;
 #ifdef UFFDIO_API
 struct uffdio_api api_struct;
 
 dev->postcopy_ufd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
 vmsg->size = 0;
+#else
+dev->postcopy_ufd = -1;
 #endif
 
 if (dev->postcopy_ufd == -1) {
-- 
2.38.1

Re: [PATCH] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.

[Richard Henderson]

On 12/18/22 20:04, TaiseiIto wrote:

Before this commit, when GDB attached an OS working on QEMU, order of FPU
stack registers printed by GDB command 'info float' was wrong. There was a
bug causing the problem in 'g' packets sent by QEMU to GDB. The packets have
values of registers of machine emulated by QEMU containing FPU stack
registers. There are 2 ways to specify a x87 FPU stack register. The first
is specifying by absolute indexed register names (R0, ..., R7). The second
is specifying by stack top relative indexed register names (ST0, ..., ST7).
Values of the FPU stack registers should be located in 'g' packet and be
ordered by the relative index. But QEMU had located these registers ordered
by the absolute index. After this commit, when QEMU reads registers to make
a 'g' packet, QEMU specifies FPU stack registers by the relative index.
Then, the registers are ordered correctly in the packet. As a result, GDB,
the packet receiver, can print FPU stack registers in the correct order.

Signed-off-by: TaiseiIto 
---
  target/i386/gdbstub.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)


Reviewed-by: Richard Henderson 


r~



diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
index c3a2cf6f28..6109ad166d 100644
--- a/target/i386/gdbstub.c
+++ b/target/i386/gdbstub.c
@@ -121,7 +121,9 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray 
*mem_buf, int n)
  return gdb_get_reg32(mem_buf, env->regs[gpr_map32[n]]);
  }
  } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
-floatx80 *fp = (floatx80 *) >fpregs[n - IDX_FP_REGS];
+int st_index = n - IDX_FP_REGS;
+int r_index = (st_index + env->fpstt) % 8;
+floatx80 *fp = >fpregs[r_index].d;
  int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low));
  len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high));
  return len;

Re: [RFC v4 3/3] hw/cxl: Multi-Region CXL Type-3 Devices (Volatile and Persistent)

[Gregory Price]

> I think an address space needs a memory region, not a memdev.
> Initialize a container region with memory_region_init()
> We could then add the two memdev associated regions (with different 
> attributes) as subregions using memory_region_add_subregion()
> 
> Similar is done for the system memory
> https://elixir.bootlin.com/qemu/latest/source/softmmu/physmem.c#L2675
> creates it.  Then it's mostly accessed by get_system_memory()
> 
> Memory is then added to that at particular base addresses via for example
> https://elixir.bootlin.com/qemu/latest/source/hw/arm/virt.c#L2210
> and similar.
> I think we can do the same here.
>

Ah, I'm just confused then, this seems reasonable

> Curious question on this lot. How are you testing it?  Some exciting scripts
> to bring the hdm decoders up etc for the volatile region? Or some prototype
> driver code?

Unfortunately I got pulled off this work for a bit to handle something
more pressing.  I have only tested that the existing functionality
(persistent mode) works as intended, and that at least the ndctl/cxl
tools report capacity as expected.

As of right now there is no code in the driver to actually touch these
regions in a way that works to be able to online these volatile regions
- at least so far as I know.

I don't remember where I left off, but some pmem-to-ram tutorials online
suggest you could online pmem regions like this

cxl create-region -d decoder0.0 -m mem0
echo offline > /sys/devices/system/memory/auto_online_blocks
ndctl create-namespace -f --mode devdax --continue
daxctl enable-device [device name]
daxctl reconfigure-device --mode=system-ram all

However I don't think this is successful in creating the dax devices,
and therefore the reconfiguring into ram.

Re: [RFC PATCH for 8.0] backends/vhost-user: relax the ioeventfd check

[Michael S. Tsirkin]

On Mon, Dec 19, 2022 at 05:48:50PM +, Alex Bennée wrote:
> 
> Alex Bennée  writes:
> 
> > While you certainly need ioeventfds to work for KVM guests it
> > shouldn't be limited to that. We can run vhost-user backends for TCG
> > guests and either use ioeventfds or in band signalling.
> >
> > Maybe we should apply the same fix as b0aa77d36d (vhost-user: fix
> > ioeventfd_enabled)?
> >
> > With this change I can run:
> >
> >   $QEMU $OPTS \
> > -display gtk,gl=on \
> > -device vhost-user-gpu-pci,chardev=vhgpu \
> > -chardev socket,id=vhgpu,path=vhgpu.sock
> >
> > with:
> >
> >   ./contrib/vhost-user-gpu/vhost-user-gpu \
> > -s vhgpu.sock \
> > -v
> >
> > and at least see things start-up (although the display gets rotated by
> > 180 degrees).
> >
> > Signed-off-by: Alex Bennée 
> > ---
> >  backends/vhost-user.c | 10 ++
> >  1 file changed, 2 insertions(+), 8 deletions(-)
> >
> > diff --git a/backends/vhost-user.c b/backends/vhost-user.c
> > index 5dedb2d987..87d43fb03a 100644
> > --- a/backends/vhost-user.c
> > +++ b/backends/vhost-user.c
> > @@ -21,12 +21,6 @@
> >  #include "io/channel-command.h"
> >  #include "hw/virtio/virtio-bus.h"
> >  
> > -static bool
> > -ioeventfd_enabled(void)
> > -{
> > -return kvm_enabled() && kvm_eventfds_enabled();
> > -}
> > -
> >  int
> >  vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev,
> >  unsigned nvqs, Error **errp)
> > @@ -35,8 +29,8 @@ vhost_user_backend_dev_init(VhostUserBackend *b, 
> > VirtIODevice *vdev,
> >  
> >  assert(!b->vdev && vdev);
> >  
> > -if (!ioeventfd_enabled()) {
> > -error_setg(errp, "vhost initialization failed: requires kvm");
> > +if (kvm_enabled() && !kvm_eventfds_enabled()) {
> > +error_setg(errp, "vhost initialization failed: kvm required 
> > ioeventfds");
> >  return -1;
> >  }
> 
> Gentle ping?
> 

Yea why not. Pls post a non RFC patch.

-- 
MST

Re: [RFC PATCH for 8.0] backends/vhost-user: relax the ioeventfd check

[Alex Bennée]

Alex Bennée  writes:

> While you certainly need ioeventfds to work for KVM guests it
> shouldn't be limited to that. We can run vhost-user backends for TCG
> guests and either use ioeventfds or in band signalling.
>
> Maybe we should apply the same fix as b0aa77d36d (vhost-user: fix
> ioeventfd_enabled)?
>
> With this change I can run:
>
>   $QEMU $OPTS \
> -display gtk,gl=on \
> -device vhost-user-gpu-pci,chardev=vhgpu \
> -chardev socket,id=vhgpu,path=vhgpu.sock
>
> with:
>
>   ./contrib/vhost-user-gpu/vhost-user-gpu \
> -s vhgpu.sock \
> -v
>
> and at least see things start-up (although the display gets rotated by
> 180 degrees).
>
> Signed-off-by: Alex Bennée 
> ---
>  backends/vhost-user.c | 10 ++
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/backends/vhost-user.c b/backends/vhost-user.c
> index 5dedb2d987..87d43fb03a 100644
> --- a/backends/vhost-user.c
> +++ b/backends/vhost-user.c
> @@ -21,12 +21,6 @@
>  #include "io/channel-command.h"
>  #include "hw/virtio/virtio-bus.h"
>  
> -static bool
> -ioeventfd_enabled(void)
> -{
> -return kvm_enabled() && kvm_eventfds_enabled();
> -}
> -
>  int
>  vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev,
>  unsigned nvqs, Error **errp)
> @@ -35,8 +29,8 @@ vhost_user_backend_dev_init(VhostUserBackend *b, 
> VirtIODevice *vdev,
>  
>  assert(!b->vdev && vdev);
>  
> -if (!ioeventfd_enabled()) {
> -error_setg(errp, "vhost initialization failed: requires kvm");
> +if (kvm_enabled() && !kvm_eventfds_enabled()) {
> +error_setg(errp, "vhost initialization failed: kvm required 
> ioeventfds");
>  return -1;
>  }

Gentle ping?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v4 20/27] tcg: Vary the allocation size for TCGOp

[Alex Bennée]

Richard Henderson  writes:

> We have been allocating a worst case number of arguments
> to support calls.  Instead, allow the size to vary.
> By default leave space for 4 args, to maximize reuse,
> but allow calls to increase the number of args to 32.
>
> Signed-off-by: Richard Henderson 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [RFC v4 3/3] hw/cxl: Multi-Region CXL Type-3 Devices (Volatile and Persistent)

[Jonathan Cameron via]

On Mon, 19 Dec 2022 11:12:34 -0500
Gregory Price  wrote:

> On Mon, Dec 19, 2022 at 12:42:11PM +, Jonathan Cameron wrote:
> > As a process thing, when reworking a patch I picked up for the
> > CXL qemu gitlab tree, drop the SOB that I added as it's not relevant
> > to the new patch.
> >   
> 
> ack
> 
> > Still no need to post a new version unless you particularly
> > want to or there are other changes to make.  
> 
> ack
> 
> > > +Deprecations
> > > +
> > > +
> > > +The Type 3 device [memdev] attribute has been deprecated in favor
> > > +of the [persistent-memdev] and [volatile-memdev] attributes. [memdev]  
> > 
> > That's not quite correct as it sort of implies we could previously use
> > memdev for the volatile case.  
> 
> An early version of the patch explicitly errored out / warned the user
> if they attempted to do this, but that got rebuffed.  This could be
> added back in.

Ah. I was unclear. I just mean that the deprecation text here is a little
misleading.  Not commenting on the functionality in this patch.

> 
> > > -return address_space_read(>hostmem_as, dpa_offset, attrs, 
> > > data, size);
> > > +if (vmr) {
> > > +if (dpa_offset <= int128_get64(vmr->size)) {
> > > +as = >hostvmem_as;  
> > 
> > As a follow up it might be worth exploring if we can combine the two 
> > address spaces.
> > I'm not keen to do it yet, because of no simple way to test it and it's 
> > less obviously
> > correct than just having separate address spaces.
> > 
> > Would involve mapping a the two hostmem regions into a container memory 
> > region which
> > would be the one we use to build the address space.  Advantage would be 
> > that we wouldn't
> > need special handling for which region it was in here or the write path as 
> > QEMUs
> > normal heirarchical memory regions would handle that for us.
> > I'm not 100% sure it would work though!
> >   
> 
> Originally I had tried putting them both into one, with the thought that
> since it's technically one device address space there should only be one
> way to access the address space instead of two.
> 
> After investigating, the address space has to be initialized with a
> memdev, and an address space only supports a single memdev, so i settled
> on two address spaces in order to keep the memdevs separate (considering
> each region may have different attributes).

I think an address space needs a memory region, not a memdev.
Initialize a container region with memory_region_init()
We could then add the two memdev associated regions (with different 
attributes) as subregions using memory_region_add_subregion()

Similar is done for the system memory
https://elixir.bootlin.com/qemu/latest/source/softmmu/physmem.c#L2675
creates it.  Then it's mostly accessed by get_system_memory()

Memory is then added to that at particular base addresses via for example
https://elixir.bootlin.com/qemu/latest/source/hw/arm/virt.c#L2210
and similar.
I think we can do the same here.

> 
> This opens the question as to how to actually represent a persistent
> memory device that can be partitioned as volatile.
> 
> Consider the following setup:
> 
> device[pmem 512mb, volatile 512 mb]
> produces:
> memdev(512mb, pmem) && memdev(512mb, volatile)
> 
> But if I partition the device to 256p/768v, when i access data in range
> [256mb,512mb), then i have volatile data going into the persistent memory
> store by nature of the fact that the capacity is located in that memdev.
> 
> An alternative would be to require a vmem region the same size as the
> pmem region (+ whatever additional volatile capacity), but this
> means using 2x the resources to represent the real capacity of the
> device. That's not good.

Do we care enough about the overhead, given this is emulation only?
Not that I think this is the way to go

> 
> Another alternative would be to create a wrapper memdev that encompasses
> persistent and volatile operations, and then create the partitioning
> logic on top of it, such that it can use a single memdev while handling
> whatever sematics only apply to each individual region.
> 
> The tl;dr here:  Going down to a single address space would probably
> require writing a wrapper memdev that abstracts away all the
> partitioning logic.  Maybe that's worth it?

For current setup, I think we can just create memory region that handles both 
of them.

For the partitioning case, lots of options.  I'm not sure what will work best.
Maybe we just decide we don't care if a persistent region (memdev-pmem) is 
presented
as volatile. I don't think it will do any real harm in the emulation, but maybe
I'm wrong on that?

> 
> > > @@ -744,30 +891,35 @@ static void validate_lsa_access(MemoryRegion *mr, 
> > > uint64_t size,
> > >  static uint64_t get_lsa(CXLType3Dev *ct3d, void *buf, uint64_t size,
> > >  uint64_t offset)
> > >  {
> > > -return size;
> > > +return 0;  
> > 
> > Hmm. Maybe this should return an error, but then 

Re: [PATCH v4 19/27] accel/tcg/plugin: Use copy_op in append_{udata,mem}_cb

[Alex Bennée]

Richard Henderson  writes:

> Better to re-use the existing function for copying ops.
>
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 

Acked-by: Alex Bennée 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v4 18/27] accel/tcg/plugin: Avoid duplicate copy in copy_call

[Alex Bennée]

Richard Henderson  writes:

> We copied all of the arguments in copy_op_nocheck.
> We only need to replace the one argument that we change.
>
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 

Acked-by: Alex Bennée 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v4 13/27] tcg: Introduce TCGCallReturnKind and TCGCallArgumentKind

[Alex Bennée]

Richard Henderson  writes:

> Prepare to replace a bunch of separate ifdefs with a
> consistent way to describe the ABI of a function call.
>
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v4 05/27] accel/tcg: Set cflags_next_tb in cpu_common_initfn

[Alex Bennée]

Richard Henderson  writes:

> While we initialize this value in cpu_common_reset, that
> isn't called during startup, so set it as well in init.
> This fixes -singlestep versus the very first TB.
>
> Fixes: 04f5b647ed07 ("accel/tcg: Handle -singlestep in curr_cflags")
> Signed-off-by: Richard Henderson 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH 3/5] accel/tcg: Restrict 'qapi-commands-machine.h' to system emulation

[Philippe Mathieu-Daudé]

Since commit a0e61807a3 ("qapi: Remove QMP events and commands from
user-mode builds") we don't generate the "qapi-commands-machine.h"
header in a user-emulation-only build.

Rename 'hmp.c' as 'monitor.c' and move the QMP functions from
cpu-exec.c (which is always compiled) to monitor.c (which is only
compiled when system-emulation is selected).

Signed-off-by: Philippe Mathieu-Daudé 
---
 accel/tcg/cpu-exec.c  |  88 +--
 accel/tcg/hmp.c   |  14 --
 accel/tcg/internal.h  |   3 ++
 accel/tcg/meson.build |   2 +-
 accel/tcg/monitor.c   | 105 ++
 5 files changed, 111 insertions(+), 101 deletions(-)
 delete mode 100644 accel/tcg/hmp.c
 create mode 100644 accel/tcg/monitor.c

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 298149ed07..5a7825dce1 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -20,7 +20,6 @@
 #include "qemu/osdep.h"
 #include "qemu/qemu-print.h"
 #include "qapi/error.h"
-#include "qapi/qapi-commands-machine.h"
 #include "qapi/type-helpers.h"
 #include "hw/core/tcg-cpu-ops.h"
 #include "trace.h"
@@ -64,8 +63,8 @@ typedef struct SyncClocks {
 #define MAX_DELAY_PRINT_RATE 20LL
 #define MAX_NB_PRINTS 100
 
-static int64_t max_delay;
-static int64_t max_advance;
+int64_t max_delay;
+int64_t max_advance;
 
 static void align_clocks(SyncClocks *sc, CPUState *cpu)
 {
@@ -1071,86 +1070,3 @@ void tcg_exec_unrealizefn(CPUState *cpu)
 tlb_destroy(cpu);
 g_free(cpu->tb_jmp_cache);
 }
-
-#ifndef CONFIG_USER_ONLY
-
-static void dump_drift_info(GString *buf)
-{
-if (!icount_enabled()) {
-return;
-}
-
-g_string_append_printf(buf, "Host - Guest clock  %"PRIi64" ms\n",
-   (cpu_get_clock() - icount_get()) / SCALE_MS);
-if (icount_align_option) {
-g_string_append_printf(buf, "Max guest delay %"PRIi64" ms\n",
-   -max_delay / SCALE_MS);
-g_string_append_printf(buf, "Max guest advance   %"PRIi64" ms\n",
-   max_advance / SCALE_MS);
-} else {
-g_string_append_printf(buf, "Max guest delay NA\n");
-g_string_append_printf(buf, "Max guest advance   NA\n");
-}
-}
-
-HumanReadableText *qmp_x_query_jit(Error **errp)
-{
-g_autoptr(GString) buf = g_string_new("");
-
-if (!tcg_enabled()) {
-error_setg(errp, "JIT information is only available with accel=tcg");
-return NULL;
-}
-
-dump_exec_info(buf);
-dump_drift_info(buf);
-
-return human_readable_text_from_str(buf);
-}
-
-HumanReadableText *qmp_x_query_opcount(Error **errp)
-{
-g_autoptr(GString) buf = g_string_new("");
-
-if (!tcg_enabled()) {
-error_setg(errp, "Opcode count information is only available with 
accel=tcg");
-return NULL;
-}
-
-tcg_dump_op_count(buf);
-
-return human_readable_text_from_str(buf);
-}
-
-#ifdef CONFIG_PROFILER
-
-int64_t dev_time;
-
-HumanReadableText *qmp_x_query_profile(Error **errp)
-{
-g_autoptr(GString) buf = g_string_new("");
-static int64_t last_cpu_exec_time;
-int64_t cpu_exec_time;
-int64_t delta;
-
-cpu_exec_time = tcg_cpu_exec_time();
-delta = cpu_exec_time - last_cpu_exec_time;
-
-g_string_append_printf(buf, "async time  %" PRId64 " (%0.3f)\n",
-   dev_time, dev_time / 
(double)NANOSECONDS_PER_SECOND);
-g_string_append_printf(buf, "qemu time   %" PRId64 " (%0.3f)\n",
-   delta, delta / (double)NANOSECONDS_PER_SECOND);
-last_cpu_exec_time = cpu_exec_time;
-dev_time = 0;
-
-return human_readable_text_from_str(buf);
-}
-#else
-HumanReadableText *qmp_x_query_profile(Error **errp)
-{
-error_setg(errp, "Internal profiler not compiled");
-return NULL;
-}
-#endif
-
-#endif /* !CONFIG_USER_ONLY */
diff --git a/accel/tcg/hmp.c b/accel/tcg/hmp.c
deleted file mode 100644
index bb67941420..00
--- a/accel/tcg/hmp.c
+++ /dev/null
@@ -1,14 +0,0 @@
-#include "qemu/osdep.h"
-#include "qemu/error-report.h"
-#include "qapi/error.h"
-#include "qapi/qapi-commands-machine.h"
-#include "exec/exec-all.h"
-#include "monitor/monitor.h"
-
-static void hmp_tcg_register(void)
-{
-monitor_register_hmp_info_hrt("jit", qmp_x_query_jit);
-monitor_register_hmp_info_hrt("opcount", qmp_x_query_opcount);
-}
-
-type_init(hmp_tcg_register);
diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h
index 6edff16fb0..130d7fd564 100644
--- a/accel/tcg/internal.h
+++ b/accel/tcg/internal.h
@@ -64,4 +64,7 @@ static inline target_ulong log_pc(CPUState *cpu, const 
TranslationBlock *tb)
 #endif
 }
 
+extern int64_t max_delay;
+extern int64_t max_advance;
+
 #endif /* ACCEL_TCG_INTERNAL_H */
diff --git a/accel/tcg/meson.build b/accel/tcg/meson.build
index 75e1dffb4d..d645c38112 100644
--- a/accel/tcg/meson.build
+++ b/accel/tcg/meson.build
@@ -16,7 +16,7 @@ specific_ss.add_all(when: 'CONFIG_TCG', if_true: tcg_ss)
 
 

[PATCH 5/5] tests/unit: Restrict machine-smp.c test to system emulation

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daude 

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/unit/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index ffa444f432..51f453e6c4 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -46,7 +46,6 @@ tests = {
   'test-uuid': [],
   'ptimer-test': ['ptimer-test-stubs.c', meson.project_source_root() / 
'hw/core/ptimer.c'],
   'test-qapi-util': [],
-  'test-smp-parse': [qom, meson.project_source_root() / 
'hw/core/machine-smp.c'],
   'test-interval-tree': [],
 }
 
@@ -134,6 +133,7 @@ if have_system
 'test-util-sockets': ['socket-helpers.c'],
 'test-base64': [],
 'test-bufferiszero': [],
+'test-smp-parse': [qom, meson.project_source_root() / 
'hw/core/machine-smp.c'],
 'test-vmstate': [migration, io],
 'test-yank': ['socket-helpers.c', qom, io, chardev]
   }
-- 
2.38.1

[PATCH 1/5] exec: Remove unused 'qemu/timer.h' timer

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 accel/tcg/cpu-exec.c  | 1 -
 accel/tcg/translate-all.c | 1 -
 include/exec/gen-icount.h | 1 -
 include/sysemu/cpus.h | 1 -
 tcg/tcg.c | 1 -
 5 files changed, 5 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 356fe348de..298149ed07 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -29,7 +29,6 @@
 #include "tcg/tcg.h"
 #include "qemu/atomic.h"
 #include "qemu/compiler.h"
-#include "qemu/timer.h"
 #include "qemu/rcu.h"
 #include "exec/log.h"
 #include "qemu/main-loop.h"
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index ed6656fb14..dea5ca81b5 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -49,7 +49,6 @@
 #include "exec/translator.h"
 #include "qemu/bitmap.h"
 #include "qemu/qemu-print.h"
-#include "qemu/timer.h"
 #include "qemu/main-loop.h"
 #include "qemu/cacheinfo.h"
 #include "exec/log.h"
diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h
index c57204ddad..166170b08e 100644
--- a/include/exec/gen-icount.h
+++ b/include/exec/gen-icount.h
@@ -2,7 +2,6 @@
 #define GEN_ICOUNT_H
 
 #include "exec/exec-all.h"
-#include "qemu/timer.h"
 
 /* Helpers for instruction counting code generation.  */
 
diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
index 1bace3379b..0535a4c68a 100644
--- a/include/sysemu/cpus.h
+++ b/include/sysemu/cpus.h
@@ -1,7 +1,6 @@
 #ifndef QEMU_CPUS_H
 #define QEMU_CPUS_H
 
-#include "qemu/timer.h"
 #include "sysemu/accel-ops.h"
 
 /* register accel-specific operations */
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 436fcf6ebd..1071fc00fe 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -34,7 +34,6 @@
 #include "qemu/cutils.h"
 #include "qemu/host-utils.h"
 #include "qemu/qemu-print.h"
-#include "qemu/timer.h"
 #include "qemu/cacheflush.h"
 #include "qemu/cacheinfo.h"
 
-- 
2.38.1

[PATCH 4/5] replay: Extract core API to 'exec/replay-core.h'

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daude 

replay API is used deeply within TCG common code (common to user
and system emulation). Unfortunately "sysemu/replay.h" requires
some QAPI headers for few system-specific declarations, example:

  void replay_input_event(QemuConsole *src, InputEvent *evt);

Since commit c2651c0eaa ("qapi/meson: Restrict UI module to system
emulation and tools") the QAPI header defining the InputEvent is
not generated anymore.
To keep it simple, extract the 'core' replay prototypes to a new
"exec/replay-core.h" header which we include in the TCG code that
doesn't need the rest of the replay API.

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS|  1 +
 accel/tcg/cpu-exec.c   |  2 +-
 accel/tcg/tcg-all.c|  2 +-
 accel/tcg/translator.c |  2 +-
 accel/tcg/user-exec-stub.c |  2 +-
 cpu.c  |  2 +-
 gdbstub/gdbstub.c  |  2 +-
 hw/core/ptimer.c   |  2 +-
 include/exec/replay-core.h | 76 ++
 include/sysemu/replay.h| 67 ++
 stubs/replay.c |  2 +-
 tests/unit/ptimer-test-stubs.c |  2 +-
 util/guest-random.c|  2 +-
 13 files changed, 91 insertions(+), 73 deletions(-)
 create mode 100644 include/exec/replay-core.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 3bd433b65a..04aa77fd37 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3206,6 +3206,7 @@ S: Supported
 F: replay/*
 F: block/blkreplay.c
 F: net/filter-replay.c
+F: include/exec/replay-core.h 
 F: include/sysemu/replay.h
 F: docs/devel/replay.rst
 F: docs/system/replay.rst
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 5a7825dce1..040de10440 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -37,7 +37,7 @@
 #include "sysemu/cpus.h"
 #include "exec/cpu-all.h"
 #include "sysemu/cpu-timers.h"
-#include "sysemu/replay.h"
+#include "exec/replay-core.h"
 #include "sysemu/tcg.h"
 #include "exec/helper-proto.h"
 #include "tb-jmp-cache.h"
diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 30b503fb22..5dab1ae9dd 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -25,7 +25,7 @@
 
 #include "qemu/osdep.h"
 #include "sysemu/tcg.h"
-#include "sysemu/replay.h"
+#include "exec/replay-core.h"
 #include "sysemu/cpu-timers.h"
 #include "tcg/tcg.h"
 #include "qapi/error.h"
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 061519691f..3a40f9eaca 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -16,7 +16,7 @@
 #include "exec/log.h"
 #include "exec/translator.h"
 #include "exec/plugin-gen.h"
-#include "sysemu/replay.h"
+#include "exec/replay-core.h"
 
 /* Pairs with tcg_clear_temp_count.
To be called by #TranslatorOps.{translate_insn,tb_stop} if
diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c
index 968cd3ca60..874e1f1a20 100644
--- a/accel/tcg/user-exec-stub.c
+++ b/accel/tcg/user-exec-stub.c
@@ -1,6 +1,6 @@
 #include "qemu/osdep.h"
 #include "hw/core/cpu.h"
-#include "sysemu/replay.h"
+#include "exec/replay-core.h"
 
 bool enable_cpu_pm = false;
 
diff --git a/cpu.c b/cpu.c
index 4a7d865427..5503e2ff12 100644
--- a/cpu.c
+++ b/cpu.c
@@ -33,7 +33,7 @@
 #endif
 #include "sysemu/tcg.h"
 #include "sysemu/kvm.h"
-#include "sysemu/replay.h"
+#include "exec/replay-core.h"
 #include "exec/cpu-common.h"
 #include "exec/exec-all.h"
 #include "exec/translate-all.h"
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index c3fbc31123..fb9c49e0fd 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -48,7 +48,7 @@
 #include "sysemu/runstate.h"
 #include "semihosting/semihost.h"
 #include "exec/exec-all.h"
-#include "sysemu/replay.h"
+#include "exec/replay-core.h"
 
 #include "internals.h"
 
diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index eb5ba1aff7..e03165febf 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -10,7 +10,7 @@
 #include "hw/ptimer.h"
 #include "migration/vmstate.h"
 #include "qemu/host-utils.h"
-#include "sysemu/replay.h"
+#include "exec/replay-core.h"
 #include "sysemu/cpu-timers.h"
 #include "sysemu/qtest.h"
 #include "block/aio.h"
diff --git a/include/exec/replay-core.h b/include/exec/replay-core.h
new file mode 100644
index 00..9ec20cb124
--- /dev/null
+++ b/include/exec/replay-core.h
@@ -0,0 +1,76 @@
+/*
+ * QEMU replay core API
+ *
+ * Copyright (c) 2010-2015 Institute for System Programming
+ * of the Russian Academy of Sciences.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef EXEC_REPLAY_H
+#define EXEC_REPLAY_H
+
+#include "qapi/qapi-types-replay.h"
+
+extern ReplayMode replay_mode;
+
+/* Replay process control functions */
+
+/*! Enables recording or saving event log with specified parameters */
+void replay_configure(struct QemuOpts *opts);
+/*! Initializes timers used for snapshotting and enables 

[PATCH 0/5] exec: QAPI housekeeping for user emulation

[Philippe Mathieu-Daudé]

Since there is no monitor in user emulation builds, we
don't generate the QAPI headers for the QMP commands.

I'm not sure when the --disable-system --disable-tools
configuration started to bitrot; it might not be super
important, but I'm using it to better sort SYSEMU vs
USER code, so I need to get it building again.

I'm trying to split a big series in smaller ones to reduce
maintainers spam.

Philippe Mathieu-Daude (2):
  replay: Extract core API to 'exec/replay-core.h'
  tests/unit: Restrict machine-smp.c test to system emulation

Philippe Mathieu-Daudé (3):
  exec: Remove unused 'qemu/timer.h' timer
  tcg/tcg-op-gvec: Remove unused "qemu/main-loop.h" header
  accel/tcg: Restrict 'qapi-commands-machine.h' to system emulation

 MAINTAINERS|   1 +
 accel/tcg/cpu-exec.c   |  91 +---
 accel/tcg/hmp.c|  14 -
 accel/tcg/internal.h   |   3 +
 accel/tcg/meson.build  |   2 +-
 accel/tcg/monitor.c| 105 +
 accel/tcg/tcg-all.c|   2 +-
 accel/tcg/translate-all.c  |   1 -
 accel/tcg/translator.c |   2 +-
 accel/tcg/user-exec-stub.c |   2 +-
 cpu.c  |   2 +-
 gdbstub/gdbstub.c  |   2 +-
 hw/core/ptimer.c   |   2 +-
 include/exec/gen-icount.h  |   1 -
 include/exec/replay-core.h |  76 
 include/sysemu/cpus.h  |   1 -
 include/sysemu/replay.h|  67 ++---
 stubs/replay.c |   2 +-
 tcg/tcg-op-gvec.c  |   1 -
 tcg/tcg.c  |   1 -
 tests/unit/meson.build |   2 +-
 tests/unit/ptimer-test-stubs.c |   2 +-
 util/guest-random.c|   2 +-
 23 files changed, 203 insertions(+), 181 deletions(-)
 delete mode 100644 accel/tcg/hmp.c
 create mode 100644 accel/tcg/monitor.c
 create mode 100644 include/exec/replay-core.h

-- 
2.38.1

[PATCH 2/5] tcg/tcg-op-gvec: Remove unused "qemu/main-loop.h" header

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
Added in db72581598 ("Include qemu/main-loop.h less") but now
I can't see what is requiring it.

I checked 'git diff db725815985..origin/master -- tcg/tcg-op-gvec.c'
to see if some code was removed without removing the header;
I haven't checked individual commits.
---
 tcg/tcg-op-gvec.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tcg/tcg-op-gvec.c b/tcg/tcg-op-gvec.c
index 079a761b04..aacedd3e15 100644
--- a/tcg/tcg-op-gvec.c
+++ b/tcg/tcg-op-gvec.c
@@ -21,7 +21,6 @@
 #include "tcg/tcg.h"
 #include "tcg/tcg-op.h"
 #include "tcg/tcg-op-gvec.h"
-#include "qemu/main-loop.h"
 #include "tcg/tcg-gvec-desc.h"
 
 #define MAX_UNROLL  4
-- 
2.38.1