date:20220908

On Fri, Sep 9, 2022 at 2:38 PM Jason Wang  wrote:
>
> On Wed, Sep 7, 2022 at 12:36 AM Eugenio Pérez  wrote:
> >
> > To have enabled vlans at device startup may happen in the destination of
> > a live migration, so this configuration must be restored.
> >
> > At this moment the code is not accessible, since SVQ refuses to start if
> > vlan feature is exposed by the device.
> >
> > Signed-off-by: Eugenio Pérez 
> > ---
> >  net/vhost-vdpa.c | 46 --
> >  1 file changed, 44 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > index 4bc3fd01a8..ecbfd08eb9 100644
> > --- a/net/vhost-vdpa.c
> > +++ b/net/vhost-vdpa.c
> > @@ -100,6 +100,8 @@ static const uint64_t vdpa_svq_device_features =
> >  BIT_ULL(VIRTIO_NET_F_RSC_EXT) |
> >  BIT_ULL(VIRTIO_NET_F_STANDBY);
> >
> > +#define MAX_VLAN(1 << 12)   /* Per 802.1Q definition */
> > +
> >  VHostNetState *vhost_vdpa_get_vhost_net(NetClientState *nc)
> >  {
> >  VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> > @@ -423,6 +425,47 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
> >  return *s->status != VIRTIO_NET_OK;
> >  }
> >
> > +static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s,
> > +   const VirtIONet *n,
> > +   uint16_t vid)
> > +{
> > +ssize_t dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_VLAN,
> > +  VIRTIO_NET_CTRL_VLAN_ADD,
> > +  &vid, sizeof(vid));
> > +if (unlikely(dev_written < 0)) {
> > +return dev_written;
> > +}
> > +
> > +if (unlikely(*s->status != VIRTIO_NET_OK)) {
> > +return -EINVAL;
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +static int vhost_vdpa_net_load_vlan(VhostVDPAState *s,
> > +const VirtIONet *n)
> > +{
> > +uint64_t features = n->parent_obj.guest_features;
> > +
> > +if (!(features & BIT_ULL(VIRTIO_NET_F_CTRL_VLAN))) {
> > +return 0;
> > +}
> > +
> > +for (int i = 0; i < MAX_VLAN >> 5; i++) {
> > +for (int j = 0; n->vlans[i] && j <= 0x1f; j++) {
> > +if (n->vlans[i] & (1U << j)) {
> > +int r = vhost_vdpa_net_load_single_vlan(s, n, (i << 5) + 
> > j);
>
> This seems to cause a lot of latency if the driver has a lot of vlans.
>
> I wonder if it's simply to let all vlan traffic go by disabling
> CTRL_VLAN feature at vDPA layer.

Another idea is to extend the spec to allow us to accept a bitmap of
the vlan ids via a single command, then we will be fine.

Thanks

>
> Thanks
>
> > +if (unlikely(r != 0)) {
> > +return r;
> > +}
> > +}
> > +}
> > +}
> > +
> > +return 0;
> > +}
> > +
> >  static int vhost_vdpa_net_load(NetClientState *nc)
> >  {
> >  VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> > @@ -445,8 +488,7 @@ static int vhost_vdpa_net_load(NetClientState *nc)
> >  if (unlikely(r)) {
> >  return r;
> >  }
> > -
> > -return 0;
> > +return vhost_vdpa_net_load_vlan(s, n);
> >  }
> >
> >  static NetClientInfo net_vhost_vdpa_cvq_info = {
> > --
> > 2.31.1
> >

Re: [PATCH 3/3] vdpa: Support VLAN on nic control shadow virtqueue

On Wed, Sep 7, 2022 at 12:36 AM Eugenio Pérez  wrote:
>
> Update the virtio-net device model with each guest's update of vlan
> through control virtqueue, and accept creating a SVQ with a device
> exposing vlan feature bit.
>
> Done in the same commit since a malicious guest could send vlan
> commands otherwise.
>
> Signed-off-by: Eugenio Pérez 
> ---
>  net/vhost-vdpa.c | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index ecbfd08eb9..40f7c60399 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -94,6 +94,7 @@ static const uint64_t vdpa_svq_device_features =
>  BIT_ULL(VIRTIO_NET_F_MRG_RXBUF) |
>  BIT_ULL(VIRTIO_NET_F_STATUS) |
>  BIT_ULL(VIRTIO_NET_F_CTRL_VQ) |
> +BIT_ULL(VIRTIO_NET_F_CTRL_VLAN) |
>  BIT_ULL(VIRTIO_NET_F_MQ) |
>  BIT_ULL(VIRTIO_F_ANY_LAYOUT) |
>  BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR) |
> @@ -538,6 +539,16 @@ static bool vhost_vdpa_net_cvq_validate_cmd(const void 
> *out_buf, size_t len)
>__func__, ctrl.cmd);
>  };
>  break;
> +case VIRTIO_NET_CTRL_VLAN:
> +switch (ctrl->cmd) {
> +case VIRTIO_NET_CTRL_VLAN_ADD:
> +case VIRTIO_NET_CTRL_VLAN_DEL:
> +return true;
> +default:
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid vlan cmd %u\n",
> +  __func__, ctrl->cmd);
> +};

Considering we may add more features here, is it still worthwhile to
keep a whitelist like this?

Thanks

> +break;
>  default:
>  qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid control class %u\n",
>__func__, ctrl.class);
> --
> 2.31.1
>

Re: [PATCH 2/3] vdpa: load vlan configuration at NIC startup

On Wed, Sep 7, 2022 at 12:36 AM Eugenio Pérez  wrote:
>
> To have enabled vlans at device startup may happen in the destination of
> a live migration, so this configuration must be restored.
>
> At this moment the code is not accessible, since SVQ refuses to start if
> vlan feature is exposed by the device.
>
> Signed-off-by: Eugenio Pérez 
> ---
>  net/vhost-vdpa.c | 46 --
>  1 file changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 4bc3fd01a8..ecbfd08eb9 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -100,6 +100,8 @@ static const uint64_t vdpa_svq_device_features =
>  BIT_ULL(VIRTIO_NET_F_RSC_EXT) |
>  BIT_ULL(VIRTIO_NET_F_STANDBY);
>
> +#define MAX_VLAN(1 << 12)   /* Per 802.1Q definition */
> +
>  VHostNetState *vhost_vdpa_get_vhost_net(NetClientState *nc)
>  {
>  VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> @@ -423,6 +425,47 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
>  return *s->status != VIRTIO_NET_OK;
>  }
>
> +static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s,
> +   const VirtIONet *n,
> +   uint16_t vid)
> +{
> +ssize_t dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_VLAN,
> +  VIRTIO_NET_CTRL_VLAN_ADD,
> +  &vid, sizeof(vid));
> +if (unlikely(dev_written < 0)) {
> +return dev_written;
> +}
> +
> +if (unlikely(*s->status != VIRTIO_NET_OK)) {
> +return -EINVAL;
> +}
> +
> +return 0;
> +}
> +
> +static int vhost_vdpa_net_load_vlan(VhostVDPAState *s,
> +const VirtIONet *n)
> +{
> +uint64_t features = n->parent_obj.guest_features;
> +
> +if (!(features & BIT_ULL(VIRTIO_NET_F_CTRL_VLAN))) {
> +return 0;
> +}
> +
> +for (int i = 0; i < MAX_VLAN >> 5; i++) {
> +for (int j = 0; n->vlans[i] && j <= 0x1f; j++) {
> +if (n->vlans[i] & (1U << j)) {
> +int r = vhost_vdpa_net_load_single_vlan(s, n, (i << 5) + j);

This seems to cause a lot of latency if the driver has a lot of vlans.

I wonder if it's simply to let all vlan traffic go by disabling
CTRL_VLAN feature at vDPA layer.

Thanks

> +if (unlikely(r != 0)) {
> +return r;
> +}
> +}
> +}
> +}
> +
> +return 0;
> +}
> +
>  static int vhost_vdpa_net_load(NetClientState *nc)
>  {
>  VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> @@ -445,8 +488,7 @@ static int vhost_vdpa_net_load(NetClientState *nc)
>  if (unlikely(r)) {
>  return r;
>  }
> -
> -return 0;
> +return vhost_vdpa_net_load_vlan(s, n);
>  }
>
>  static NetClientInfo net_vhost_vdpa_cvq_info = {
> --
> 2.31.1
>

Re: [PATCH v2 2/2] [RfC] expose host-phys-bits to guest

2022-09-08 Thread Michael S. Tsirkin

On Fri, Sep 09, 2022 at 08:06:53AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > > I think we still want to key this one off host_phys_bits
> > > > so it works for e.g. hyperv emulation too.
> > > 
> > > I think that should be the case.  The chunks above change the
> > > host-phys-bits option from setting cpu->host_phys_bits to setting
> > > the FEAT_KVM_HINTS bit.  That should also happen with hyperv emulation
> > > enabled, and the bit should also be visible to the guest then, just at
> > > another location (base 0x4100 instead of 0x4000).
> > > 
> > > take care,
> > >   Gerd
> > 
> > 
> > You are right, I forgot. Hmm, ok. What about !cpu->expose_kvm ?
> > 
> > We have
> > 
> > if (!kvm_enabled() || !cpu->expose_kvm) {
> > env->features[FEAT_KVM] = 0;
> > }   
> > 
> > This is quick grep, I didn't check whether this is called
> > after the point where you currently use it, but
> > it frankly seems fragile to pass a generic user specified flag
> > inside a cpuid where everyone pokes at it.
> 
> I tried to avoid keeping the state of the host_phys_bits option at
> multiple places.  Maybe that wasn't a good idea after all.  How about
> doing this instead:
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 1db1278a599b..279fde095d7c 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6219,6 +6219,11 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
>  env->features[FEAT_KVM] = 0;
>  }
>  
> +if (kvm_enabled() && cpu->host_phys_bits) {
> +env->features[FEAT_KVM_HINTS] |=
> +(1U << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID);
> +}
> +
>  x86_cpu_enable_xsave_components(cpu);
>  
>  /* CPUID[EAX=7,ECX=0].EBX always increased level automatically: */
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index a1fd1f53791d..3335c57b21b2 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -459,6 +459,7 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, 
> uint32_t function,
>  }
>  } else if (function == KVM_CPUID_FEATURES && reg == R_EDX) {
>  ret |= 1U << KVM_HINTS_REALTIME;
> +ret |= 1U << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID;
>  }
>  
>  return ret;


/me nods.
That seems much more straight-forward.

-- 
MST

Re: [PATCH v3] audio: Add sndio backend

2022-09-08 Thread Volker Rümelin


Am 07.09.22 um 15:23 schrieb Alexandre Ratchov:

sndio is the native API used by OpenBSD, although it has been ported to
other *BSD's and Linux (packages for Ubuntu, Debian, Void, Arch, etc.).

Signed-off-by: Brad Smith
Signed-off-by: Alexandre Ratchov
---

References to the previous patch versions and related discussions are
here:

https://marc.info/?l=qemu-devel&m=163973393011543   (v2)
https://marc.info/?l=qemu-devel&m=163626248712444   (initial patch)

Here are the changes between v2 and v3 of this patch:

- fixed of typos in file-names in MAINTAINERS
- added Gerd Hoffmann to the M: entry in MAINTAINERS
- added missin S: entry in MAINTAINERS
- removed unused #include "qemu-common.h"
- bumped "Since:" version to 7.2 in qapi/audio.json
- regenerated scripts/meson-buildoptions.sh
- implement buffer_get_free() method, introduced by
   commit 9833438ef624155de879d4ed57ecfcd3464a0bbe

   audio: restore mixing-engine playback buffer size

Running "make update-buildoptions" triggered unrelated changes of
scripts/meson-buildoptions.sh, that I removed from the commit as they
are not related to sndio.

Tested on OpenBSD, still works as expected :-)

Regards,
Alexandre

  MAINTAINERS   |   7 +
  audio/audio.c |   1 +
  audio/audio_template.h|   2 +
  audio/meson.build |   1 +
  audio/sndioaudio.c| 565 ++
  meson.build   |   9 +-
  meson_options.txt |   4 +-
  qapi/audio.json   |  25 +-
  qemu-options.hx   |  16 +
  scripts/meson-buildoptions.sh |   7 +-
  10 files changed, 632 insertions(+), 5 deletions(-)
  create mode 100644 audio/sndioaudio.c



Tested again on Linux.

Reviewed-by: Volker Rümelin 
Tested-by: Volker Rümelin

Re: [PATCH v2 2/2] [RfC] expose host-phys-bits to guest

2022-09-08 Thread Gerd Hoffmann

  Hi,

> > > I think we still want to key this one off host_phys_bits
> > > so it works for e.g. hyperv emulation too.
> > 
> > I think that should be the case.  The chunks above change the
> > host-phys-bits option from setting cpu->host_phys_bits to setting
> > the FEAT_KVM_HINTS bit.  That should also happen with hyperv emulation
> > enabled, and the bit should also be visible to the guest then, just at
> > another location (base 0x4100 instead of 0x4000).
> > 
> > take care,
> >   Gerd
> 
> 
> You are right, I forgot. Hmm, ok. What about !cpu->expose_kvm ?
> 
> We have
> 
> if (!kvm_enabled() || !cpu->expose_kvm) {
> env->features[FEAT_KVM] = 0;
> }   
> 
> This is quick grep, I didn't check whether this is called
> after the point where you currently use it, but
> it frankly seems fragile to pass a generic user specified flag
> inside a cpuid where everyone pokes at it.

I tried to avoid keeping the state of the host_phys_bits option at
multiple places.  Maybe that wasn't a good idea after all.  How about
doing this instead:

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1db1278a599b..279fde095d7c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6219,6 +6219,11 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
 env->features[FEAT_KVM] = 0;
 }
 
+if (kvm_enabled() && cpu->host_phys_bits) {
+env->features[FEAT_KVM_HINTS] |=
+(1U << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID);
+}
+
 x86_cpu_enable_xsave_components(cpu);
 
 /* CPUID[EAX=7,ECX=0].EBX always increased level automatically: */
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index a1fd1f53791d..3335c57b21b2 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -459,6 +459,7 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t 
function,
 }
 } else if (function == KVM_CPUID_FEATURES && reg == R_EDX) {
 ret |= 1U << KVM_HINTS_REALTIME;
+ret |= 1U << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID;
 }
 
 return ret;

Re: [PATCH v2 2/2] [RfC] expose host-phys-bits to guest

2022-09-08 Thread Michael S. Tsirkin

On Fri, Sep 09, 2022 at 07:18:17AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > @@ -424,7 +426,10 @@ static void 
> > > microvm_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> > >  {
> > >  X86CPU *cpu = X86_CPU(dev);
> > >  
> > > -cpu->host_phys_bits = true; /* need reliable phys-bits */
> > > +/* need reliable phys-bits */
> > > +cpu->env.features[FEAT_KVM_HINTS] |=
> > > +(1 << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID);
> > > +
> > 
> > Do we need compat machinery for this?
> 
> Don't think so, microvm has no versioned machine types anyway.
> 
> > > --- a/target/i386/cpu.c
> > > +++ b/target/i386/cpu.c
> 
> > >  [FEAT_KVM_HINTS] = {
> > >  .type = CPUID_FEATURE_WORD,
> > >  .feat_names = {
> > > -"kvm-hint-dedicated", NULL, NULL, NULL,
> > > +"kvm-hint-dedicated", "host-phys-bits", NULL, NULL,
> 
> > > -DEFINE_PROP_BOOL("host-phys-bits", X86CPU, host_phys_bits, false),
> 
> > > -if (cpu->host_phys_bits) {
> > > +if (cpu->env.features[FEAT_KVM_HINTS] &
> > > +(1 << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID)) {
> > >  /* The user asked for us to use the host physical bits */
> > >  phys_bits = host_phys_bits;
> > >  if (cpu->host_phys_bits_limit &&
> > 
> > I think we still want to key this one off host_phys_bits
> > so it works for e.g. hyperv emulation too.
> 
> I think that should be the case.  The chunks above change the
> host-phys-bits option from setting cpu->host_phys_bits to setting
> the FEAT_KVM_HINTS bit.  That should also happen with hyperv emulation
> enabled, and the bit should also be visible to the guest then, just at
> another location (base 0x4100 instead of 0x4000).
> 
> take care,
>   Gerd


You are right, I forgot. Hmm, ok. What about !cpu->expose_kvm ?

We have

if (!kvm_enabled() || !cpu->expose_kvm) {
env->features[FEAT_KVM] = 0;
}   

This is quick grep, I didn't check whether this is called
after the point where you currently use it, but
it frankly seems fragile to pass a generic user specified flag
inside a cpuid where everyone pokes at it.


-- 
MST

Re: [PATCH v2 2/2] [RfC] expose host-phys-bits to guest

2022-09-08 Thread Gerd Hoffmann

  Hi,

> > @@ -424,7 +426,10 @@ static void microvm_device_pre_plug_cb(HotplugHandler 
> > *hotplug_dev,
> >  {
> >  X86CPU *cpu = X86_CPU(dev);
> >  
> > -cpu->host_phys_bits = true; /* need reliable phys-bits */
> > +/* need reliable phys-bits */
> > +cpu->env.features[FEAT_KVM_HINTS] |=
> > +(1 << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID);
> > +
> 
> Do we need compat machinery for this?

Don't think so, microvm has no versioned machine types anyway.

> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c

> >  [FEAT_KVM_HINTS] = {
> >  .type = CPUID_FEATURE_WORD,
> >  .feat_names = {
> > -"kvm-hint-dedicated", NULL, NULL, NULL,
> > +"kvm-hint-dedicated", "host-phys-bits", NULL, NULL,

> > -DEFINE_PROP_BOOL("host-phys-bits", X86CPU, host_phys_bits, false),

> > -if (cpu->host_phys_bits) {
> > +if (cpu->env.features[FEAT_KVM_HINTS] &
> > +(1 << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID)) {
> >  /* The user asked for us to use the host physical bits */
> >  phys_bits = host_phys_bits;
> >  if (cpu->host_phys_bits_limit &&
> 
> I think we still want to key this one off host_phys_bits
> so it works for e.g. hyperv emulation too.

I think that should be the case.  The chunks above change the
host-phys-bits option from setting cpu->host_phys_bits to setting
the FEAT_KVM_HINTS bit.  That should also happen with hyperv emulation
enabled, and the bit should also be visible to the guest then, just at
another location (base 0x4100 instead of 0x4000).

take care,
  Gerd

Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory

2022-09-08 Thread Andy Lutomirski


On 8/24/22 02:41, Chao Peng wrote:

On Tue, Aug 23, 2022 at 04:05:27PM +, Sean Christopherson wrote:

On Tue, Aug 23, 2022, David Hildenbrand wrote:

On 19.08.22 05:38, Hugh Dickins wrote:

On Fri, 19 Aug 2022, Sean Christopherson wrote:

On Thu, Aug 18, 2022, Kirill A . Shutemov wrote:

On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote:

On Wed, 6 Jul 2022, Chao Peng wrote:
But since then, TDX in particular has forced an effort into preventing
(by flags, seals, notifiers) almost everything that makes it shmem/tmpfs.

Are any of the shmem.c mods useful to existing users of shmem.c? No.
Is MFD_INACCESSIBLE useful or comprehensible to memfd_create() users? No.


But QEMU and other VMMs are users of shmem and memfd.  The new features 
certainly
aren't useful for _all_ existing users, but I don't think it's fair to say that
they're not useful for _any_ existing users.


Okay, I stand corrected: there exist some users of memfd_create()
who will also have use for "INACCESSIBLE" memory.


As raised in reply to the relevant patch, I'm not sure if we really have
to/want to expose MFD_INACCESSIBLE to user space. I feel like this is a
requirement of specific memfd_notifer (memfile_notifier) implementations
-- such as TDX that will convert the memory and MCE-kill the machine on
ordinary write access. We might be able to set/enforce this when
registering a notifier internally instead, and fail notifier
registration if a condition isn't met (e.g., existing mmap).

So I'd be curious, which other users of shmem/memfd would benefit from
(MMU)-"INACCESSIBLE" memory obtained via memfd_create()?


I agree that there's no need to expose the inaccessible behavior via uAPI.  
Making
it a kernel-internal thing that's negotiated/resolved when KVM binds to the fd
would align INACCESSIBLE with the UNMOVABLE and UNRECLAIMABLE flags (and any 
other
flags that get added in the future).

AFAICT, the user-visible flag is a holdover from the early RFCs and doesn't 
provide
any unique functionality.


That's also what I'm thinking. And I don't see problem immediately if
user has populated the fd at the binding time. Actually that looks an
advantage for previously discussed guest payload pre-loading.


I think this gets awkward. Trying to define sensible semantics for what 
happens if a shmem or similar fd gets used as secret guest memory and 
that fd isn't utterly and completely empty can get quite nasty.  For 
example:


If there are already mmaps, then TDX (much more so than SEV) really 
doesn't want to also use it as guest memory.


If there is already data in the fd, then maybe some technologies can use 
this for pre-population, but TDX needs explicit instructions in order to 
get the guest's hash right.



In general, it seems like it will be much more likely to actually work 
well if the user (uAPI) is required to declare to the kernel exactly 
what the fd is for (e.g. TDX secret memory, software-only secret memory, 
etc) before doing anything at all with it other than binding it to KVM.


INACCESSIBLE is a way to achieve this.  Maybe it's not the prettiest in 
the world -- I personally would rather see an explicit request for, say, 
TDX or SEV memory or maybe the memory that works for a particular KVM 
instance instead of something generic like INACCESSIBLE, but this is a 
pretty weak preference.  But I think that just starting with a plain 
memfd is a can of worms.

Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory

2022-09-08 Thread Andy Lutomirski

On 8/19/22 17:27, Kirill A. Shutemov wrote:

On Thu, Aug 18, 2022 at 08:00:41PM -0700, Hugh Dickins wrote:

On Thu, 18 Aug 2022, Kirill A . Shutemov wrote:

On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote:

If your memory could be swapped, that would be enough of a good reason
to make use of shmem.c: but it cannot be swapped; and although there
are some references in the mailthreads to it perhaps being swappable
in future, I get the impression that will not happen soon if ever.

If your memory could be migrated, that would be some reason to use
filesystem page cache (because page migration happens to understand
that type of memory): but it cannot be migrated.

Migration support is in pipeline. It is part of TDX 1.5 [1]. And swapping
theoretically possible, but I'm not aware of any plans as of now.

[1]
https://www.intel.com/content/www/us/en/developer/articles/technical/intel-trust-domain-extensions.html

I always forget, migration means different things to different audiences.
As an mm person, I was meaning page migration, whereas a virtualization
person thinks VM live migration (which that reference appears to be about),
a scheduler person task migration, an ornithologist bird migration, etc.

But you're an mm person too: you may have cited that reference in the
knowledge that TDX 1.5 Live Migration will entail page migration of the
kind I'm thinking of. (Anyway, it's not important to clarify that here.)

TDX 1.5 brings both.

In TDX speak, mm migration called relocation. See TDH.MEM.PAGE.RELOCATE.

This seems to be a pretty bad fit for the way that the core mm migrates
pages. The core mm unmaps the page, then moves (in software) the
contents to a new address, then faults it in. TDH.MEM.PAGE.RELOCATE
doesn't fit into that workflow very well. I'm not saying it can't be
done, but it won't just work.

Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory

2022-09-08 Thread Andy Lutomirski

On 8/18/22 06:24, Kirill A . Shutemov wrote:

On Wed, Aug 17, 2022 at 10:40:12PM -0700, Hugh Dickins wrote:

On Wed, 6 Jul 2022, Chao Peng wrote:

This is the v7 of this series which tries to implement the fd-based KVM
guest private memory.

Here at last are my reluctant thoughts on this patchset.

fd-based approach for supporting KVM guest private memory: fine.

Use or abuse of memfd and shmem.c: mistaken.

memfd_create() was an excellent way to put together the initial prototype.

But since then, TDX in particular has forced an effort into preventing
(by flags, seals, notifiers) almost everything that makes it shmem/tmpfs.

Are any of the shmem.c mods useful to existing users of shmem.c? No.
Is MFD_INACCESSIBLE useful or comprehensible to memfd_create() users? No.

What use do you have for a filesystem here? Almost none.
IIUC, what you want is an fd through which QEMU can allocate kernel
memory, selectively free that memory, and communicate fd+offset+length
to KVM. And perhaps an interface to initialize a little of that memory
from a template (presumably copied from a real file on disk somewhere).

You don't need shmem.c or a filesystem for that!

If your memory could be migrated, that would be some reason to use
filesystem page cache (because page migration happens to understand
that type of memory): but it cannot be migrated.

Migration support is in pipeline. It is part of TDX 1.5 [1]. And swapping
theoretically possible, but I'm not aware of any plans as of now.

[1]
https://www.intel.com/content/www/us/en/developer/articles/technical/intel-trust-domain-extensions.html

This thing?

https://cdrdv2.intel.com/v1/dl/getContent/733578

That looks like migration between computers, not between NUMA nodes. Or
am I missing something?

Re: Seeing qtest assertion failure with 7.1

2022-09-08 Thread Markus Armbruster

Peter Maydell  writes:

> On Thu, 8 Sept 2022 at 16:54, Patrick Venture  wrote:
>> On Wed, Sep 7, 2022 at 10:40 AM Peter Maydell  
>> wrote:
>>> Have a look in the source at what exactly the assertion
>>> failure in libqtest.c is checking for -- IIRC it's a pretty
>>> basic "did we open a socket fd" one. I think sometimes I
>>> used to see something like this if there's an old stale socket
>>> lying around in the test directory and the randomly generated
>>> socket filename happens to clash with it.
>
>> Thanks for the debugging tip! I can't reproduce it at this point. I
>> saw it 2-3 times, and now not at all.  So more than likely it's
>> exactly what you're describing.
>
> Mmm. We do clean up the socket after ourselves in the test
> harness, but I think what can happen is that if a test case
> crashes then the cleanup doesn't happen. Then there's a stale
> file left in the build tree, and then you only hit it if you
> get unlucky with PID allocation on a future run...

Yes, and that's bad behavior.

I think we should run each test in its own directory, which we delete
afterwards.  That way anything the test creates there will be cleaned up
whether it succeeds or fails.

Re: [PATCH v3] audio: add help option for -audio and -audiodev

2022-09-08 Thread Markus Armbruster

Paolo Bonzini  writes:

> No, there's nothing yet.

What about QOM introspection?  Oh, we're not using QOM there.  Would
QOMification be desirable?

[PATCH v2 2/2] Update linux headers to v6.0-rc4

commit 7e18e42e4b280c85b76967a9106a13ca61c16179

Signed-off-by: Chenyi Qiang 
---
 include/standard-headers/asm-x86/bootparam.h  |   7 +-
 include/standard-headers/drm/drm_fourcc.h |  73 +++-
 include/standard-headers/linux/ethtool.h  |  29 +--
 include/standard-headers/linux/input.h|  12 +-
 include/standard-headers/linux/pci_regs.h |  30 ++-
 include/standard-headers/linux/vhost_types.h  |  17 +-
 include/standard-headers/linux/virtio_9p.h|   2 +-
 .../standard-headers/linux/virtio_config.h|   7 +-
 include/standard-headers/linux/virtio_ids.h   |  14 +-
 include/standard-headers/linux/virtio_net.h   |  34 +++-
 include/standard-headers/linux/virtio_pci.h   |   2 +
 include/standard-headers/linux/virtio_ring.h  |  16 +-
 linux-headers/asm-arm64/kvm.h |  33 +++-
 linux-headers/asm-generic/unistd.h|   4 +-
 linux-headers/asm-riscv/kvm.h |  22 +++
 linux-headers/asm-riscv/unistd.h  |   3 +-
 linux-headers/asm-s390/kvm.h  |   1 +
 linux-headers/asm-x86/kvm.h   |  33 ++--
 linux-headers/asm-x86/mman.h  |  14 --
 linux-headers/linux/kvm.h | 172 +-
 linux-headers/linux/userfaultfd.h |  10 +-
 linux-headers/linux/vduse.h   |  47 +
 linux-headers/linux/vfio.h|   4 +-
 linux-headers/linux/vfio_zdev.h   |   7 +
 linux-headers/linux/vhost.h   |  35 +++-
 25 files changed, 538 insertions(+), 90 deletions(-)

diff --git a/include/standard-headers/asm-x86/bootparam.h 
b/include/standard-headers/asm-x86/bootparam.h
index b2aaad10e5..0b06d2bff1 100644
--- a/include/standard-headers/asm-x86/bootparam.h
+++ b/include/standard-headers/asm-x86/bootparam.h
@@ -10,12 +10,13 @@
 #define SETUP_EFI  4
 #define SETUP_APPLE_PROPERTIES 5
 #define SETUP_JAILHOUSE6
+#define SETUP_CC_BLOB  7
+#define SETUP_IMA  8
 #define SETUP_RNG_SEED 9
+#define SETUP_ENUM_MAX SETUP_RNG_SEED
 
 #define SETUP_INDIRECT (1<<31)
-
-/* SETUP_INDIRECT | max(SETUP_*) */
-#define SETUP_TYPE_MAX (SETUP_INDIRECT | SETUP_JAILHOUSE)
+#define SETUP_TYPE_MAX (SETUP_ENUM_MAX | SETUP_INDIRECT)
 
 /* ram_size flags */
 #define RAMDISK_IMAGE_START_MASK   0x07FF
diff --git a/include/standard-headers/drm/drm_fourcc.h 
b/include/standard-headers/drm/drm_fourcc.h
index 4888f85f69..48b620cbef 100644
--- a/include/standard-headers/drm/drm_fourcc.h
+++ b/include/standard-headers/drm/drm_fourcc.h
@@ -558,7 +558,7 @@ extern "C" {
  *
  * The main surface is Y-tiled and is at plane index 0 whereas CCS is linear
  * and at index 1. The clear color is stored at index 2, and the pitch should
- * be ignored. The clear color structure is 256 bits. The first 128 bits
+ * be 64 bytes aligned. The clear color structure is 256 bits. The first 128 
bits
  * represents Raw Clear Color Red, Green, Blue and Alpha color each represented
  * by 32 bits. The raw clear color is consumed by the 3d engine and generates
  * the converted clear color of size 64 bits. The first 32 bits store the Lower
@@ -571,6 +571,53 @@ extern "C" {
  */
 #define I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC fourcc_mod_code(INTEL, 8)
 
+/*
+ * Intel Tile 4 layout
+ *
+ * This is a tiled layout using 4KB tiles in a row-major layout. It has the 
same
+ * shape as Tile Y at two granularities: 4KB (128B x 32) and 64B (16B x 4). It
+ * only differs from Tile Y at the 256B granularity in between. At this
+ * granularity, Tile Y has a shape of 16B x 32 rows, but this tiling has a 
shape
+ * of 64B x 8 rows.
+ */
+#define I915_FORMAT_MOD_4_TILED fourcc_mod_code(INTEL, 9)
+
+/*
+ * Intel color control surfaces (CCS) for DG2 render compression.
+ *
+ * The main surface is Tile 4 and at plane index 0. The CCS data is stored
+ * outside of the GEM object in a reserved memory area dedicated for the
+ * storage of the CCS data for all RC/RC_CC/MC compressible GEM objects. The
+ * main surface pitch is required to be a multiple of four Tile 4 widths.
+ */
+#define I915_FORMAT_MOD_4_TILED_DG2_RC_CCS fourcc_mod_code(INTEL, 10)
+
+/*
+ * Intel color control surfaces (CCS) for DG2 media compression.
+ *
+ * The main surface is Tile 4 and at plane index 0. For semi-planar formats
+ * like NV12, the Y and UV planes are Tile 4 and are located at plane indices
+ * 0 and 1, respectively. The CCS for all planes are stored outside of the
+ * GEM object in a reserved memory area dedicated for the storage of the
+ * CCS data for all RC/RC_CC/MC compressible GEM objects. The main surface
+ * pitch is required to be a multiple of four Tile 4 widths.
+ */
+#define I915_FORMAT_MOD_4_TILED_DG2_MC_CCS fourcc_mod_code(INTEL, 11)
+
+/*
+ * Intel Color Control Surface with Clear Color (CCS) for DG2 render 
compression.
+ *
+ * The ma

[PATCH v2 0/2] Update linux headers to v6.0-rc4 and fix the clang build error

After updating linux headers to v6.0-rc, clang build on x86 target would
generate warnings related to -Wgnu-variable-sized-type-not-at-end.

Simply turn off this warning in this patch set.

---

Change logs
v1 -> v2:
- Change the patch order. (Peter Maydell)
- Expand the commit message in patch 1. (Peter Maydell)
- v1: 
https://lore.kernel.org/qemu-devel/20220908080749.32211-1-chenyi.qi...@intel.com/

---

Chenyi Qiang (2):
  configure: Add -Wno-gnu-variable-sized-type-not-at-end
  Update linux headers to v6.0-rc4

 configure |   1 +
 include/standard-headers/asm-x86/bootparam.h  |   7 +-
 include/standard-headers/drm/drm_fourcc.h |  73 +++-
 include/standard-headers/linux/ethtool.h  |  29 +--
 include/standard-headers/linux/input.h|  12 +-
 include/standard-headers/linux/pci_regs.h |  30 ++-
 include/standard-headers/linux/vhost_types.h  |  17 +-
 include/standard-headers/linux/virtio_9p.h|   2 +-
 .../standard-headers/linux/virtio_config.h|   7 +-
 include/standard-headers/linux/virtio_ids.h   |  14 +-
 include/standard-headers/linux/virtio_net.h   |  34 +++-
 include/standard-headers/linux/virtio_pci.h   |   2 +
 include/standard-headers/linux/virtio_ring.h  |  16 +-
 linux-headers/asm-arm64/kvm.h |  33 +++-
 linux-headers/asm-generic/unistd.h|   4 +-
 linux-headers/asm-riscv/kvm.h |  22 +++
 linux-headers/asm-riscv/unistd.h  |   3 +-
 linux-headers/asm-s390/kvm.h  |   1 +
 linux-headers/asm-x86/kvm.h   |  33 ++--
 linux-headers/asm-x86/mman.h  |  14 --
 linux-headers/linux/kvm.h | 172 +-
 linux-headers/linux/userfaultfd.h |  10 +-
 linux-headers/linux/vduse.h   |  47 +
 linux-headers/linux/vfio.h|   4 +-
 linux-headers/linux/vfio_zdev.h   |   7 +
 linux-headers/linux/vhost.h   |  35 +++-
 26 files changed, 539 insertions(+), 90 deletions(-)

-- 
2.17.1

[PATCH v2 1/2] configure: Add -Wno-gnu-variable-sized-type-not-at-end

In recent linux headers update to v6.0-rc, it switched GNU
'zero-length-array' extension to the C-standard-defined flexible array
member. e.g.

struct kvm_msrs {
__u32 nmsrs; /* number of msrs in entries */
__u32 pad;

-   struct kvm_msr_entry entries[0];
+   struct kvm_msr_entry entries[];
};

Those (unlike the GNU zero-length-array) have some extra restrictions like
'this must be put at the end of a struct', which clang build would complain
about. e.g. the current code

struct {
struct kvm_msrs info;
struct kvm_msr_entry entries[1];
} msr_data = { }

generates the warning like:

target/i386/kvm/kvm.c:2868:25: error: field 'info' with variable sized
type 'struct kvm_msrs' not at the end of a struct or class is a GNU
extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
struct kvm_msrs info;
^
In fact, the variable length 'entries[]' field in 'info' is zero-sized in
GNU defined semantics, which can give predictable offset for 'entries[1]'
in local msr_data. The local defined struct is just there to force a stack
allocation large enough for 1 kvm_msr_entry, a clever trick but requires to
turn off this clang warning.

Suggested-by: Daniel P. Berrangé 
Reviewed-by: Richard Henderson 
Signed-off-by: Chenyi Qiang 
---
 configure | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index 575dde1c1f..7e0a1a4187 100755
--- a/configure
+++ b/configure
@@ -1258,6 +1258,7 @@ add_to nowarn_flags -Wno-string-plus-int
 add_to nowarn_flags -Wno-typedef-redefinition
 add_to nowarn_flags -Wno-tautological-type-limit-compare
 add_to nowarn_flags -Wno-psabi
+add_to nowarn_flags -Wno-gnu-variable-sized-type-not-at-end
 
 gcc_flags="$warn_flags $nowarn_flags"
 
-- 
2.17.1

Re: [PATCH v4 0/6] Vhost-vdpa Shadow Virtqueue multiqueue support.

On Tue, Sep 6, 2022 at 11:07 PM Eugenio Pérez  wrote:
>
> This series enables shadowed CVQ to intercept multiqueue commands through
> shadowed CVQ, update the virtio NIC device model so qemu send it in a
> migration, and the restore of that MQ state in the destination.
>
> v3:
> * Accept ctrl class and cmd in vhost_vdpa_net_load_cmd, so it's in charge of
>   building the whole buffer
> * Rename cvq_cmd_in_buffer to status.
>
> v2:
> * Add vhost_vdpa_net_load_cmd helper to avoid out buffers castings.
> * Make cvq_cmd_in_buffer virtio_net_ctrl_ack type.
>
> Eugenio Pérez (6):
>   vdpa: Make VhostVDPAState cvq_cmd_in_buffer control ack type
>   vdpa: extract vhost_vdpa_net_load_mac from vhost_vdpa_net_load
>   vdpa: Add vhost_vdpa_net_load_mq
>   vdpa: validate MQ CVQ commands
>   virtio-net: Update virtio-net curr_queue_pairs in vdpa backends
>   vdpa: Allow MQ feature in SVQ
>
>  hw/net/virtio-net.c |  17 +++
>  net/vhost-vdpa.c| 119 
>  2 files changed, 93 insertions(+), 43 deletions(-)

Applied.

Thanks

>
> --
> 2.31.1
>
>

Re: [PATCH] hw/net/tulip: Fix DMA reentrancy issue with stack overflow (CVE-2022-2962)

On Sat, Aug 27, 2022 at 3:03 PM Thomas Huth  wrote:
>
> The Tulip NIC can be used to trigger an endless recursion when its
> descriptors are set up to its own MMIO address space. Fix it by
> limiting the DMA accesses to normal memory.
>
> Fixes: CVE-2022-2962
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1171
> Signed-off-by: Thomas Huth 

Zheyu has posted a similar path which has been merged:

commit 36a894aeb64a2e02871016da1c37d4a4ca109182
Author: Zheyu Ma 
Date:   Sun Aug 21 20:43:43 2022 +0800

net: tulip: Restrict DMA engine to memories

Thanks

> ---
>  hw/net/tulip.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
> index 097e905bec..b9e42c322a 100644
> --- a/hw/net/tulip.c
> +++ b/hw/net/tulip.c
> @@ -70,7 +70,7 @@ static const VMStateDescription vmstate_pci_tulip = {
>  static void tulip_desc_read(TULIPState *s, hwaddr p,
>  struct tulip_descriptor *desc)
>  {
> -const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
> +const MemTxAttrs attrs = { .memory = true };
>
>  if (s->csr[0] & CSR0_DBO) {
>  ldl_be_pci_dma(&s->dev, p, &desc->status, attrs);
> @@ -88,7 +88,7 @@ static void tulip_desc_read(TULIPState *s, hwaddr p,
>  static void tulip_desc_write(TULIPState *s, hwaddr p,
>  struct tulip_descriptor *desc)
>  {
> -const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
> +const MemTxAttrs attrs = { .memory = true };
>
>  if (s->csr[0] & CSR0_DBO) {
>  stl_be_pci_dma(&s->dev, p, desc->status, attrs);
> --
> 2.31.1
>

Re: [PATCH] e1000e: set RX desc status with DD flag in a separate operation

On Sat, Aug 27, 2022 at 12:06 AM Ding Hui  wrote:
>
> Like commit 034d00d48581 ("e1000: set RX descriptor status in
> a separate operation"), there is also same issue in e1000e, which
> would cause lost packets or stop sending packets to VM with DPDK.
>
> Do similar fix in e1000e.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/402
> Signed-off-by: Ding Hui 
> ---
>  hw/net/e1000e_core.c | 54 +++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
> index 208e3e0d79..b8038e4014 100644
> --- a/hw/net/e1000e_core.c
> +++ b/hw/net/e1000e_core.c
> @@ -1364,6 +1364,58 @@ struct NetRxPkt *pkt, const E1000E_RSSInfo *rss_info,
>  }
>  }
>
> +static inline void
> +e1000e_pci_dma_write_rx_desc(E1000ECore *core, dma_addr_t addr,
> + uint8_t *desc, dma_addr_t len)
> +{
> +PCIDevice *dev = core->owner;
> +
> +if (e1000e_rx_use_legacy_descriptor(core)) {
> +struct e1000_rx_desc *d = (struct e1000_rx_desc *) desc;
> +size_t offset = offsetof(struct e1000_rx_desc, status);
> +typeof(d->status) status = d->status;
> +
> +d->status &= ~E1000_RXD_STAT_DD;
> +pci_dma_write(dev, addr, desc, len);
> +
> +if (status & E1000_RXD_STAT_DD) {
> +d->status = status;
> +pci_dma_write(dev, addr + offset, &status, sizeof(status));
> +}
> +} else {
> +if (core->mac[RCTL] & E1000_RCTL_DTYP_PS) {
> +union e1000_rx_desc_packet_split *d =
> +(union e1000_rx_desc_packet_split *) desc;
> +size_t offset = offsetof(union e1000_rx_desc_packet_split,
> +wb.middle.status_error);
> +typeof(d->wb.middle.status_error) status =
> +d->wb.middle.status_error;

Any reason to use typeof here? Its type is known to be uint32_t?

> +
> +d->wb.middle.status_error &= ~E1000_RXD_STAT_DD;
> +pci_dma_write(dev, addr, desc, len);
> +
> +if (status & E1000_RXD_STAT_DD) {
> +d->wb.middle.status_error = status;
> +pci_dma_write(dev, addr + offset, &status, sizeof(status));
> +}
> +} else {
> +union e1000_rx_desc_extended *d =
> +(union e1000_rx_desc_extended *) desc;
> +size_t offset = offsetof(union e1000_rx_desc_extended,
> +wb.upper.status_error);
> +typeof(d->wb.upper.status_error) status = 
> d->wb.upper.status_error;

So did here.

Thanks

> +
> +d->wb.upper.status_error &= ~E1000_RXD_STAT_DD;
> +pci_dma_write(dev, addr, desc, len);
> +
> +if (status & E1000_RXD_STAT_DD) {
> +d->wb.upper.status_error = status;
> +pci_dma_write(dev, addr + offset, &status, sizeof(status));
> +}
> +}
> +}
> +}
> +
>  typedef struct e1000e_ba_state_st {
>  uint16_t written[MAX_PS_BUFFERS];
>  uint8_t cur_idx;
> @@ -1600,7 +1652,7 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct 
> NetRxPkt *pkt,
>
>  e1000e_write_rx_descr(core, desc, is_last ? core->rx_pkt : NULL,
> rss_info, do_ps ? ps_hdr_len : 0, 
> &bastate.written);
> -pci_dma_write(d, base, &desc, core->rx_desc_len);
> +e1000e_pci_dma_write_rx_desc(core, base, desc, core->rx_desc_len);
>
>  e1000e_ring_advance(core, rxi,
>  core->rx_desc_len / E1000_MIN_RX_DESC_LEN);
> --
> 2.17.1
>

Re: [PATCH 2/2] configure: Add -Wno-gnu-variable-sized-type-not-at-end





On 9/8/2022 6:54 PM, Peter Maydell wrote:

On Thu, 8 Sept 2022 at 10:09, Daniel P. Berrangé  wrote:


On Thu, Sep 08, 2022 at 09:53:44AM +0100, Peter Maydell wrote:

On Thu, 8 Sept 2022 at 09:08, Chenyi Qiang  wrote:


After updating linux headers to v6.0-rc, clang build on x86 target would
generate warnings like:

target/i386/kvm/kvm.c:470:25: error: field 'info' with variable sized
type 'struct kvm_msrs' not at the end of a struct or class is a GNU
extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
 struct kvm_msrs info;
 ^
target/i386/kvm/kvm.c:1701:27: error: field 'cpuid' with variable sized
type 'struct kvm_cpuid2' not at the end of a struct or class is a GNU
extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
 struct kvm_cpuid2 cpuid;
   ^
target/i386/kvm/kvm.c:2868:25: error: field 'info' with variable sized
type 'struct kvm_msrs' not at the end of a struct or class is a GNU
extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
 struct kvm_msrs info;
 ^

Considering that it is OK to use GNU extension in QEMU (e.g. g_auto stuff),
it is acceptable to turn off this warning, which is only relevant to people
striving for fully portable C code.


Can we get the kernel folks to fix their headers not to
use GCC extensions like this ? It's not a big deal for us
I guess, but in general it doesn't seem great that the
kernel headers rely on userspace to silence warnings...


The kernel headers are fine actually - it is C99 compliant to have
a unsized array field in structs. eg

The problem is in the QEMU code which is taking the kernel declared
struct, and then embedding it inside another struct. e.g. the
kernel exposes:

   struct kvm_msrs {
 __u32 nmsrs; /* number of msrs in entries */
 __u32 pad;

 struct kvm_msr_entry entries[];
   };

which we then use as:

   uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index)
   {
 struct {
 struct kvm_msrs info;
 struct kvm_msr_entry entries[1];
 } msr_data = {};


Hmm, and the kernel used to define the 'entries' field as 'entries[0]',
which is the GNU 'zero-length-array' extension. So the kernel has
switched to the C-standard-defined flexible array member. Those
(unlike the GNU zero-length-array) have some extra restrictions
like this "can't put the struct into another struct" one. So
effectively we've moved from a GCC extension that clang doesn't
complain about to one that it does complain about.


IOW, our locally defined struct is just there to force a stack
allocation large enough for 1 kvm_msr_entry. A clever trick, but
requires that we turn off the CLang portability warning


I guess that's the most reasonable thing. Might be worth
expanding on some of this in the commit message, though.

Also, this patch disabling the warning needs to come before
the patch where the headers are updated; otherwise this will
break bisection with clang.



Indeed. If necessary, I would expand the commit message and send the 
next version.



thanks
-- PMM

[PATCH] virtio-gpu: update scanout if there is any area covered by the rect

2022-09-08 Thread Dongwon Kim

The scanout is currently updated only if the whole rect is inside the
scanout space. This is not a correct condition because the scanout should
be updated even a small area in the scanout space is covered by the rect.

Cc: Gerd Hoffmann 
Signed-off-by: Dongwon Kim 
---
 hw/display/virtio-gpu.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..5e15c79b94 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -515,9 +515,10 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
 for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
 scanout = &g->parent_obj.scanout[i];
 if (scanout->resource_id == res->resource_id &&
-rf.r.x >= scanout->x && rf.r.y >= scanout->y &&
-rf.r.x + rf.r.width <= scanout->x + scanout->width &&
-rf.r.y + rf.r.height <= scanout->y + scanout->height &&
+rf.r.x < scanout->x + scanout->width &&
+rf.r.x + rf.r.width >= scanout->x &&
+rf.r.y < scanout->y + scanout->height &&
+rf.r.y + rf.r.height >= scanout->y &&
 console_has_gl(scanout->con)) {
 dpy_gl_update(scanout->con, 0, 0, scanout->width,
   scanout->height);
-- 
2.20.1

Re: [PATCH 2/2] target/riscv: remove fixed numbering from GDB xml feature files

2022-09-08 Thread Palmer Dabbelt


On Wed, 31 Aug 2022 01:41:23 PDT (-0700), aburg...@redhat.com wrote:

The fixed register numbering in the various GDB feature files for
RISC-V only exists because these files were originally copied from the
GDB source tree.

However, the fixed numbering only exists in the GDB source tree so
that GDB, when it connects to a target that doesn't provide a target
description, will use a specific numbering scheme.

That numbering scheme is designed to be compatible with the first
versions of QEMU (for RISC-V), that didn't send a target description,
and relied on a fixed numbering scheme.

Because of the way that QEMU manages its target descriptions,
recording the number of registers in each feature, and just relying on
GDB's numbering starting from 0, then I propose that we remove all the
fixed numbering from the RISC-V feature xml files, and just rely on
the standard numbering scheme.  Plenty of other targets manage their
xml files this way, e.g. ARM, AArch64, Loongarch, m68k, rx, and s390.

Signed-off-by: Andrew Burgess 
---
 gdb-xml/riscv-32bit-cpu.xml | 6 +-
 gdb-xml/riscv-32bit-fpu.xml | 6 +-
 gdb-xml/riscv-64bit-cpu.xml | 6 +-
 gdb-xml/riscv-64bit-fpu.xml | 6 +-
 4 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/gdb-xml/riscv-32bit-cpu.xml b/gdb-xml/riscv-32bit-cpu.xml
index 0d07aaec85..466f2c0648 100644
--- a/gdb-xml/riscv-32bit-cpu.xml
+++ b/gdb-xml/riscv-32bit-cpu.xml
@@ -5,13 +5,9 @@
  are permitted in any medium without royalty provided the copyright
  notice and this notice are preserved.  -->

-
-
 
 
-  
+  
   
   
   
diff --git a/gdb-xml/riscv-32bit-fpu.xml b/gdb-xml/riscv-32bit-fpu.xml
index 84a44ba8df..24aa087031 100644
--- a/gdb-xml/riscv-32bit-fpu.xml
+++ b/gdb-xml/riscv-32bit-fpu.xml
@@ -5,13 +5,9 @@
  are permitted in any medium without royalty provided the copyright
  notice and this notice are preserved.  -->

-
-
 
 
-  
+  
   
   
   
diff --git a/gdb-xml/riscv-64bit-cpu.xml b/gdb-xml/riscv-64bit-cpu.xml
index b8aa424ae4..c4d83de09b 100644
--- a/gdb-xml/riscv-64bit-cpu.xml
+++ b/gdb-xml/riscv-64bit-cpu.xml
@@ -5,13 +5,9 @@
  are permitted in any medium without royalty provided the copyright
  notice and this notice are preserved.  -->

-
-
 
 
-  
+  
   
   
   
diff --git a/gdb-xml/riscv-64bit-fpu.xml b/gdb-xml/riscv-64bit-fpu.xml
index 9856a9d1d3..d0f17f9984 100644
--- a/gdb-xml/riscv-64bit-fpu.xml
+++ b/gdb-xml/riscv-64bit-fpu.xml
@@ -5,10 +5,6 @@
  are permitted in any medium without royalty provided the copyright
  notice and this notice are preserved.  -->

-
-
 
 

@@ -17,7 +13,7 @@
 
   

-  
+  
   
   
   


Reviewed-by: Palmer Dabbelt 

Thanks.

Re: [PATCH v3] audio: add help option for -audio and -audiodev

2022-09-08 Thread Paolo Bonzini

No, there's nothing yet.

Paolo

Il gio 8 set 2022, 15:47 Claudio Fontana  ha scritto:

> On 9/8/22 11:39, Paolo Bonzini wrote:
> > Queued, thanks.
> >
> > Paolo
> >
> >
>
> Thanks. When it comes to programmatic checks about what QEMU supports in
> terms of audio,
>
> is there something that can be done with QMP?
>
> I checked the QMP manual at:
>
>
> https://qemu.readthedocs.io/en/latest/interop/qemu-qmp-ref.html#qapidoc-2948
>
> but in the "Audio" section there is a bunch of Objects and enums defined,
> but no command to query them...
>
> Thanks,
>
> Claudio
>
>

Re: [PATCH 3/4] hw/timer: ibex_timer.c: Update register addresses

2022-09-08 Thread Tyler Ng

On Thu, Sep 8, 2022 at 4:56 AM Alistair Francis 
wrote:

> On Fri, Sep 2, 2022 at 3:24 AM Tyler Ng  wrote:
> >
> > Updates the register addresses to match the OpenTitan spec.
> >
> > These changes were made in this commit:
> >
> https://github.com/lowRISC/opentitan/commit/a25e162b8f91bd0ca32258c83d1d480f93327204
>
> Thanks for the patch
>
> We try to keep all OpenTitan devices in sync with each other. QEMU
> currently supports OT commit 217a0168ba118503c166a9587819e3811eeb0c0c
>
> We don't want to update a single device without updating all of them.
> If you want you are welcome to update all devices to a newer commit
>
> Also, the commits QEMU supports are generally driven by Tock, as
> that's the software running on QEMU OT. Have a look here for the board
> https://github.com/tock/tock/tree/master/boards/opentitan or here for
> the latest update (which QEMU already supports)
> https://github.com/tock/tock/pull/3056
>
> Alistair
>
> Alright, thanks for the info. I'll look into it; the AON spec could have
changed since that commit as I was using the latest version.

-Tyler


> >
> > Signed-off-by: Tyler Ng 
> > ---
> >  hw/timer/ibex_timer.c | 20 ++--
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/timer/ibex_timer.c b/hw/timer/ibex_timer.c
> > index 8c2ca364da..9ffd4821e8 100644
> > --- a/hw/timer/ibex_timer.c
> > +++ b/hw/timer/ibex_timer.c
> > @@ -38,19 +38,19 @@ REG32(ALERT_TEST, 0x00)
> >  FIELD(ALERT_TEST, FATAL_FAULT, 0, 1)
> >  REG32(CTRL, 0x04)
> >  FIELD(CTRL, ACTIVE, 0, 1)
> > -REG32(CFG0, 0x100)
> > -FIELD(CFG0, PRESCALE, 0, 12)
> > -FIELD(CFG0, STEP, 16, 8)
> > -REG32(LOWER0, 0x104)
> > -REG32(UPPER0, 0x108)
> > -REG32(COMPARE_LOWER0, 0x10C)
> > -REG32(COMPARE_UPPER0, 0x110)
> > -REG32(INTR_ENABLE, 0x114)
> > +REG32(INTR_ENABLE, 0x100)
> >  FIELD(INTR_ENABLE, IE_0, 0, 1)
> > -REG32(INTR_STATE, 0x118)
> > +REG32(INTR_STATE, 0x104)
> >  FIELD(INTR_STATE, IS_0, 0, 1)
> > -REG32(INTR_TEST, 0x11C)
> > +REG32(INTR_TEST, 0x108)
> >  FIELD(INTR_TEST, T_0, 0, 1)
> > +REG32(CFG0, 0x10C)
> > +FIELD(CFG0, PRESCALE, 0, 12)
> > +FIELD(CFG0, STEP, 16, 8)
> > +REG32(LOWER0, 0x110)
> > +REG32(UPPER0, 0x114)
> > +REG32(COMPARE_LOWER0, 0x118)
> > +REG32(COMPARE_UPPER0, 0x11C)
> >
> >  static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq)
> >  {
> > --
> > 2.30.2
> >
>

Re: [PATCH 1/4] hw/watchdog: wdt_ibex_aon.c: Implement the watchdog for the OpenTitan

2022-09-08 Thread Tyler Ng

On Thu, Sep 8, 2022 at 4:52 AM Alistair Francis 
wrote:

> On Fri, Sep 2, 2022 at 3:29 AM Tyler Ng  wrote:
> >
> > This commit adds most of an implementation of the OpenTitan Always-On
> > Timer. The documentation for this timer is found here:
> >
> > https://docs.opentitan.org/hw/ip/aon_timer/doc/
> >
> > The implementation includes most of the watchdog features; it does not
> > implement the wakeup timer.
> >
> > An important note: the OpenTitan board uses the sifive_plic. The plic
> > will not be able to claim the bark interrupt (158) because the sifive
> > plic sets priority[158], but checks priority[157] for the priority, so
> > it thinks that the interrupt's priority is 0 (effectively disabled).
> >
> > Changed Files:
> > hw/riscv/Kconfig: Add configuration for the watchdog.
> > hw/riscv/opentitan.c: Connect AON Timer to the OpenTitan board.
> >
> > hw/watchdog/Kconfig: Configuration for the watchdog.
> > hw/watchdog/meson.build: Compile the watchdog.
> > hw/watchdog/wdt_ibex_aon.c: The watchdog itself.
> >
> > include/hw/riscv/opentitan.h: Add watchdog bark/wakeup IRQs and timer.
> > include/hw/watchdog/wdt_ibex_aon.h: Add watchdog.
> >
> > tests/qtest/ibex-aon-timer-test.c: Ibex Timer test.
> > tests/qtest/meson.build: Build the timer test.
> >
> > Signed-off-by: Tyler Ng 
> > ---
> >  hw/riscv/Kconfig   |   4 +
> >  hw/riscv/opentitan.c   |  22 +-
> >  hw/watchdog/Kconfig|   3 +
> >  hw/watchdog/meson.build|   1 +
> >  hw/watchdog/wdt_ibex_aon.c | 432 +
> >  include/hw/riscv/opentitan.h   |   9 +-
> >  include/hw/watchdog/wdt_ibex_aon.h |  67 +
> >  tests/qtest/ibex-aon-timer-test.c  | 189 +
> >  tests/qtest/meson.build|   3 +
> >  9 files changed, 725 insertions(+), 5 deletions(-)
> >  create mode 100644 hw/watchdog/wdt_ibex_aon.c
> >  create mode 100644 include/hw/watchdog/wdt_ibex_aon.h
> >  create mode 100644 tests/qtest/ibex-aon-timer-test.c
> >
> > diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> > index 79ff61c464..72094010be 100644
> > --- a/hw/riscv/Kconfig
> > +++ b/hw/riscv/Kconfig
> > @@ -4,6 +4,9 @@ config RISCV_NUMA
> >  config IBEX
> >  bool
> >
> > +config IBEX_AON
> > +bool
> > +
> >  config MICROCHIP_PFSOC
> >  bool
> >  select CADENCE_SDHCI
> > @@ -20,6 +23,7 @@ config MICROCHIP_PFSOC
> >  config OPENTITAN
> >  bool
> >  select IBEX
> > +select IBEX_AON
> >  select UNIMP
> >
> >  config SHAKTI_C
> > diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> > index 4495a2c039..10834b831f 100644
> > --- a/hw/riscv/opentitan.c
> > +++ b/hw/riscv/opentitan.c
> > @@ -1,4 +1,5 @@
> >  /*
> > +ptimer_
> >   * QEMU RISC-V Board Compatible with OpenTitan FPGA platform
> >   *
> >   * Copyright (c) 2020 Western Digital
> > @@ -47,7 +48,7 @@ static const MemMapEntry ibex_memmap[] = {
> >  [IBEX_DEV_RSTMGR] = {  0x4041,  0x1000  },
> >  [IBEX_DEV_CLKMGR] = {  0x4042,  0x1000  },
> >  [IBEX_DEV_PINMUX] = {  0x4046,  0x1000  },
> > -[IBEX_DEV_PADCTRL] ={  0x4047,  0x1000  },
>
> What about the pad controller?
>

Can you point me to what memory map is being used in QEMU right now? I was
using the top_earlgrey memory map, which has the aon timer base address at
0x4047_.


>
> > +[IBEX_DEV_AON_TIMER] =  {  0x4047,  0x1000  },
> >  [IBEX_DEV_FLASH_CTRL] = {  0x4100,  0x1000  },
> >  [IBEX_DEV_AES] ={  0x4110,  0x1000  },
> >  [IBEX_DEV_HMAC] =   {  0x4111,  0x1000  },
> > @@ -121,6 +122,8 @@ static void lowrisc_ibex_soc_init(Object *obj)
> >
> >  object_initialize_child(obj, "timer", &s->timer, TYPE_IBEX_TIMER);
> >
> > +object_initialize_child(obj, "aon_timer", &s->aon_timer,
> > TYPE_IBEX_AON_TIMER);
> > +
> >  for (int i = 0; i < OPENTITAN_NUM_SPI_HOSTS; i++) {
> >  object_initialize_child(obj, "spi_host[*]", &s->spi_host[i],
> >  TYPE_IBEX_SPI_HOST);
> > @@ -205,6 +208,7 @@ static void lowrisc_ibex_soc_realize(DeviceState
> > *dev_soc, Error **errp)
> > 3, qdev_get_gpio_in(DEVICE(&s->plic),
> > IBEX_UART0_RX_OVERFLOW_IRQ));
> >
> > +/* RV Timer */
> >  if (!sysbus_realize(SYS_BUS_DEVICE(&s->timer), errp)) {
> >  return;
> >  }
> > @@ -215,6 +219,20 @@ static void lowrisc_ibex_soc_realize(DeviceState
> > *dev_soc, Error **errp)
> >  qdev_connect_gpio_out(DEVICE(&s->timer), 0,
> >qdev_get_gpio_in(DEVICE(qemu_get_cpu(0)),
> > IRQ_M_TIMER));
> > +
> > +/* AON Timer */
> > +if (!sysbus_realize(SYS_BUS_DEVICE(&s->aon_timer), errp)) {
> > +return;
> > +}
> > +sysbus_mmio_map(SYS_BUS_DEVICE(&s->aon_timer), 0,
> > memmap[IBEX_DEV_AON_TIMER].base);
> > +sysbus_connect_irq(SYS_BUS_DEVICE(&s->aon_ti

Re: [PATCH v2 12/20] disas/nanomips: Replace std::string type

2022-09-08 Thread Milica Lazarevic

Thanks, it's more clear to me now! I'll modify it in the next by the 
suggestions.

From: Richard Henderson 
Sent: Thursday, September 8, 2022 11:14 PM
To: Milica Lazarevic ; th...@redhat.com 

Cc: qemu-devel@nongnu.org ; cfont...@suse.de 
; berra...@redhat.com ; 
pbonz...@redhat.com ; vince.delvecc...@mediatek.com 
; peter.mayd...@linaro.org 
; Djordje Todorovic ; 
mips3...@gmail.com ; Dragan Mladjenovic 

Subject: Re: [PATCH v2 12/20] disas/nanomips: Replace std::string type

On 9/8/22 20:16, Milica Lazarevic wrote:
> This would be better written as
>
>   char *reg_list[33];
>
>   assert(count <= 32);
>   for (c = 0; c < count; c++) {
>   bool use_gp = ...
>   uint64 this_rt = ...
>   /* glib usage below requires casting away const */
>   reg_list[c] = (char *)GPR(this_rt);
>   }
>   reg_list[count] = NULL;
>
>   return g_strjoinv(",", reg_list);
>
>
> In the implementation you suggested, there's one comma missing in the output.
> For example, instead of having:
>> 0x802021ce: 1e12 SAVE 0x10,ra,s0
> We're having this:
>< 0x802021ce: 1e12 SAVE 0x10ra,s0

Oh, right, because SAVE of zero registers is legal, and even useful as an 
adjustment to
the stack pointer.

> So, I'm assuming that there needs to exist one more concatenation between the 
> comma and
> the result of the g_strjoinv function?
> Maybe something like
>  return g_strconcat((char *)",", (char *)g_strjoinv(",", reg_list), NULL);

Well, written like that you'd leak the result of g_strjoinv.

A better solution is to first element of reg_list be "", so that it's still 
just a single
memory allocation.

> I think this interface should be
>
>   char **dis,
>
> so that...
>
> > @@ -746,25 +647,26 @@ static int Disassemble(const uint16 *data, 
> std::string & dis,
> >* an ASE attribute and the requested 
> version
> >* not having that attribute
> >*/
> > -dis = "ASE attribute mismatch";
> > +strcpy(dis, "ASE attribute mismatch");
>
> these become
>
>   *dis = g_strdup("string");
>
> and the usage in nanomips_dis does not assume a fixed sized buffer.
>
>
> r~
>
>
> I'm not sure why the fixed size buffer would be a problem here since the 
> buffer size has
> already been limited by the caller.
> I.e. in the print_insn_nanomips function, the buf variable is defined as:
> char buf[200];

There would be no such declaration with the above change.


r~

Re: [PATCH] tcg/ppc: Optimize 26-bit jumps

2022-09-08 Thread Richard Henderson


On 9/8/22 22:18, Leandro Lupori wrote:

PowerPC64 processors handle direct branches better than indirect
ones, resulting in less stalled cycles and branch misses.

However, PPC's tb_target_set_jmp_target() was only using direct
branches for 16-bit jumps, while PowerPC64's unconditional branch
instructions are able to handle displacements of up to 26 bits.
To take advantage of this, now jumps whose displacements fit in
between 17 and 26 bits are also converted to direct branches.


This doesn't work because you have to be able to unset the jump as well, and your two step 
sequence doesn't handle that.  (You wind up with the two insn address load reset, but the 
jump continuing to the previous target -- boom.)


For v2.07+, you could use stq to update 4 insns atomically.

For v3.1+, you can eliminate TCG_REG_TB, using prefixed pc-relative addressing instead. 
Which brings you back to only needing to update 8 bytes atomically (select either paddi to 
compute address to feed to following mtctr+bcctr, or direct branch + nop leaving the 
mtctr+bcctr alone and unreachable).


(Actually, there are lots of updates one could make to tcg/ppc for v3.1...)


r~

[PATCH] tcg/ppc: Optimize 26-bit jumps

2022-09-08 Thread Leandro Lupori

PowerPC64 processors handle direct branches better than indirect
ones, resulting in less stalled cycles and branch misses.

However, PPC's tb_target_set_jmp_target() was only using direct
branches for 16-bit jumps, while PowerPC64's unconditional branch
instructions are able to handle displacements of up to 26 bits.
To take advantage of this, now jumps whose displacements fit in
between 17 and 26 bits are also converted to direct branches.

Signed-off-by: Leandro Lupori 
---
 tcg/ppc/tcg-target.c.inc | 86 ++--
 1 file changed, 73 insertions(+), 13 deletions(-)

diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
index 1cbd047ab3..a776685a3b 100644
--- a/tcg/ppc/tcg-target.c.inc
+++ b/tcg/ppc/tcg-target.c.inc
@@ -1847,14 +1847,69 @@ static void tcg_out_mb(TCGContext *s, TCGArg a0)
 tcg_out32(s, insn);
 }
 
+static inline void ppc_replace_insn(uintptr_t rx, uintptr_t rw,
+uint32_t offs, tcg_insn_unit i)
+{
+rx += offs;
+rw += offs;
+
+qatomic_set((uint32_t *)rw, i);
+smp_wmb();  /* Make sure this instruction is modified before others */
+flush_idcache_range(rx, rw, 4);
+}
+
+static inline void ppc64_replace_insn_pair(uintptr_t rx, uintptr_t rw,
+uint32_t offs, tcg_insn_unit i1, tcg_insn_unit i2)
+{
+uint64_t pair;
+
+rx += offs;
+rw += offs;
+
+#if HOST_BIG_ENDIAN
+pair = (uint64_t)i1 << 32 | i2;
+#else
+pair = (uint64_t)i2 << 32 | i1;
+#endif
+
+/*
+ * This function is only called on ppc64. Avoid the _Static_assert
+ * within qatomic_set that would fail to build a ppc32 host.
+ */
+qatomic_set__nocheck((uint64_t *)rw, pair);
+smp_wmb();  /* Make sure these instructions are modified before others */
+flush_idcache_range(rx, rw, 8);
+}
+
 void tb_target_set_jmp_target(uintptr_t tc_ptr, uintptr_t jmp_rx,
   uintptr_t jmp_rw, uintptr_t addr)
 {
 if (TCG_TARGET_REG_BITS == 64) {
-tcg_insn_unit i1, i2;
+tcg_insn_unit i1, i2, i3;
 intptr_t tb_diff = addr - tc_ptr;
 intptr_t br_diff = addr - (jmp_rx + 4);
-uint64_t pair;
+
+/*
+ * Here we need to change (up to) 3 instructions in an atomic way.
+ * As it's not possible to change all 3 at the same time, we perform
+ * the changes in multiple steps, in a way that results in valid code
+ * in each step.
+ *
+ * We handle 3 jump sizes: 16, 26 and 32 bits.
+ *
+ * The first step is to restore the last instruction, if needed,
+ * that is only changed by 26-bit jumps, that would become an
+ * equivalent 32-bit jump.
+ */
+i3 = MTSPR | RS(TCG_REG_TB) | CTR;
+if ((uint32_t)jmp_rw != i3) {
+ppc_replace_insn(jmp_rx, jmp_rw, 8, i3);
+}
+
+/*
+ * Next, for the 16-bit and 32-bit cases, we just need to replace the
+ * first 2 instructions and we're done.
+ */
 
 /* This does not exercise the range of the branch, but we do
still need to be able to load the new value of TCG_REG_TB.
@@ -1862,28 +1917,33 @@ void tb_target_set_jmp_target(uintptr_t tc_ptr, 
uintptr_t jmp_rx,
 if (tb_diff == (int16_t)tb_diff) {
 i1 = ADDI | TAI(TCG_REG_TB, TCG_REG_TB, tb_diff);
 i2 = B | (br_diff & 0x3fc);
+ppc64_replace_insn_pair(jmp_rx, jmp_rw, 0, i1, i2);
+
 } else {
 intptr_t lo = (int16_t)tb_diff;
 intptr_t hi = (int32_t)(tb_diff - lo);
 assert(tb_diff == hi + lo);
 i1 = ADDIS | TAI(TCG_REG_TB, TCG_REG_TB, hi >> 16);
 i2 = ADDI | TAI(TCG_REG_TB, TCG_REG_TB, lo);
+ppc64_replace_insn_pair(jmp_rx, jmp_rw, 0, i1, i2);
+
+/*
+ * For the 26-bit case, the final step is to replace the
+ * last instruction with a direct branch. Note that in this case
+ * the branch is performed 1 instruction after the 16-bit case,
+ * so br_diff needs to be adjusted properly.
+ */
+br_diff -= 4;
+if (br_diff >= -0x200 && br_diff <= 0x1fc) {
+i3 = B | (br_diff & 0x3fc);
+ppc_replace_insn(jmp_rx, jmp_rw, 8, i3);
+}
 }
-#if HOST_BIG_ENDIAN
-pair = (uint64_t)i1 << 32 | i2;
-#else
-pair = (uint64_t)i2 << 32 | i1;
-#endif
 
-/* As per the enclosing if, this is ppc64.  Avoid the _Static_assert
-   within qatomic_set that would fail to build a ppc32 host.  */
-qatomic_set__nocheck((uint64_t *)jmp_rw, pair);
-flush_idcache_range(jmp_rx, jmp_rw, 8);
 } else {
 intptr_t diff = addr - jmp_rx;
 tcg_debug_assert(in_range_b(diff));
-qatomic_set((uint32_t *)jmp_rw, B | (diff & 0x3fc));
-flush_idcache_range(jmp_rx, jmp_rw, 4);
+ppc_replace_insn(jmp_rx, jmp_rw, 0, B | (diff & 0x3f

Re: [PATCH v2 12/20] disas/nanomips: Replace std::string type

2022-09-08 Thread Richard Henderson

On 9/8/22 20:16, Milica Lazarevic wrote:

This would be better written as

  char *reg_list[33];

  assert(count <= 32);
  for (c = 0; c < count; c++) {
  bool use_gp = ...
  uint64 this_rt = ...
  /* glib usage below requires casting away const */
  reg_list[c] = (char *)GPR(this_rt);
  }
  reg_list[count] = NULL;

  return g_strjoinv(",", reg_list);

In the implementation you suggested, there's one comma missing in the output.
For example, instead of having:
   > 0x802021ce: 1e12 SAVE 0x10,ra,s0
We're having this:
   < 0x802021ce: 1e12 SAVE 0x10ra,s0

Oh, right, because SAVE of zero registers is legal, and even useful as an adjustment to 
the stack pointer.

So, I'm assuming that there needs to exist one more concatenation between the comma and 
the result of the g_strjoinv function?

Maybe something like
     return g_strconcat((char *)",", (char *)g_strjoinv(",", reg_list), NULL);

Well, written like that you'd leak the result of g_strjoinv.

A better solution is to first element of reg_list be "", so that it's still just a single 
memory allocation.

I think this interface should be

  char **dis,

so that...

> @@ -746,25 +647,26 @@ static int Disassemble(const uint16 *data, std::string 
& dis,
>    * an ASE attribute and the requested 
version
>    * not having that attribute
>    */
> -    dis = "ASE attribute mismatch";
> +    strcpy(dis, "ASE attribute mismatch");

these become

  *dis = g_strdup("string");

and the usage in nanomips_dis does not assume a fixed sized buffer.

r~

I'm not sure why the fixed size buffer would be a problem here since the buffer size has 
already been limited by the caller.

I.e. in the print_insn_nanomips function, the buf variable is defined as:
char buf[200];

There would be no such declaration with the above change.

r~

Re: [PATCH v2 1/5] msmouse: Handle mouse reset

2022-09-08 Thread Peter Maydell

On Thu, 8 Sept 2022 at 18:43, Arwed Meyer  wrote:
>
> Detect mouse reset via RTS or DTR line:
> Don't send or process anything while in reset.
> When coming out of reset, send ID sequence first thing.
> This allows msmouse to be detected by common mouse drivers.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/77
> Signed-off-by: Arwed Meyer 

How does this work across migration? There doesn't seem
to be anything that handles migration of the existing
state inside the MouseChardev either, though...

-- PMM

Re: [PATCH 42/42] hw/i386/acpi-build: Resolve PIIX ISA bridge rather than ACPI controller

2022-09-08 Thread Bernhard Beschow

Am 1. September 2022 21:05:03 UTC schrieb "Philippe Mathieu-Daudé" 
:
>On 1/9/22 18:26, Bernhard Beschow wrote:
>> Resolving the PIIX ISA bridge rather than the PIIX ACPI controller mirrors
>> the ICH9 code one line below.
>> 
>> Signed-off-by: Bernhard Beschow 
>> ---
>>   hw/i386/acpi-build.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 8af75b1e22..d7bb1ccb26 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -288,7 +288,7 @@ static void acpi_get_pm_info(MachineState *machine, 
>> AcpiPmInfo *pm)
>> static void acpi_get_misc_info(AcpiMiscInfo *info)
>>   {
>> -Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM);
>> +Object *piix = object_resolve_type_unambiguous(TYPE_PIIX_PCI_DEVICE);
>>   Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE);
>>   assert(!!piix != !!lpc);
>>   
>
>This looks correct to  me w.r.t the hardware, but my understanding is
>some x86 machines allow abusing the PIIX ACPI PCI function, by plugging
>it alone, without the rest of the south bridge... Then this patch would
>regress such Frankenstein use :/

TYPE_PIIX4_PM is not user-creatable and is only instantiated in pc_piix.c if 
the southbridge is as well, so those should be equivalent. Since 
acpi_get_misc_info() is more about the board and south bridge and not about PM, 
I think checking for TYPE_PIIX_PCI_DEVICE is therefore more appropriate here.

>
>Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH v7 08/14] hw/ppc: set machine->fdt in pegasos2_machine_reset()

2022-09-08 Thread BALATON Zoltan


On Thu, 8 Sep 2022, Daniel Henrique Barboza wrote:

We'll introduce a QMP/HMP command that requires machine->fdt to be set
properly.

Cc: BALATON Zoltan 
Cc: qemu-...@nongnu.org
Signed-off-by: Daniel Henrique Barboza 


Reviewed-by: BALATON Zoltan 


---
hw/ppc/pegasos2.c | 4 
1 file changed, 4 insertions(+)

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 61f4263953..ecf682b148 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -331,6 +331,10 @@ static void pegasos2_machine_reset(MachineState *machine)

vof_build_dt(fdt, pm->vof);
vof_client_open_store(fdt, pm->vof, "/chosen", "stdout", "/failsafe");
+
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = fdt;
+
pm->cpu->vhyp = PPC_VIRTUAL_HYPERVISOR(machine);
}

Re: [PATCH v7 05/14] hw/ppc: set machine->fdt in bamboo_load_device_tree()

2022-09-08 Thread BALATON Zoltan


On Thu, 8 Sep 2022, Daniel Henrique Barboza wrote:

This will enable support for 'dumpdtb' QMP/HMP command for the bamboo
machine.

Setting machine->fdt requires a MachineState pointer to be used inside
bamboo_load_device_tree(). Let's change the function to receive this
pointer from the caller. 'ramsize' and 'kernel_cmdline' can be retrieved
directly from the 'machine' pointer.

Cc: Cédric Le Goater 
Signed-off-by: Daniel Henrique Barboza 


Reviewed-by: BALATON Zoltan 


---
hw/ppc/ppc440_bamboo.c | 25 ++---
1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index ea945a1c99..9cc58fccf9 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -34,6 +34,8 @@
#include "hw/qdev-properties.h"
#include "qapi/error.h"

+#include 
+
#define BINARY_DEVICE_TREE_FILE "bamboo.dtb"

/* from u-boot */
@@ -56,14 +58,13 @@ static const ram_addr_t ppc440ep_sdram_bank_sizes[] = {

static hwaddr entry;

-static int bamboo_load_device_tree(hwaddr addr,
- uint32_t ramsize,
- hwaddr initrd_base,
- hwaddr initrd_size,
- const char *kernel_cmdline)
+static int bamboo_load_device_tree(MachineState *machine,
+   hwaddr addr,
+   hwaddr initrd_base,
+   hwaddr initrd_size)
{
int ret = -1;
-uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(ramsize) };
+uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(machine->ram_size) };
char *filename;
int fdt_size;
void *fdt;
@@ -98,7 +99,7 @@ static int bamboo_load_device_tree(hwaddr addr,
fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
}
ret = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
-  kernel_cmdline);
+  machine->kernel_cmdline);
if (ret < 0) {
fprintf(stderr, "couldn't set /chosen/bootargs\n");
}
@@ -119,7 +120,10 @@ static int bamboo_load_device_tree(hwaddr addr,
  tb_freq);

rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
-g_free(fdt);
+
+/* Set ms->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = fdt;
+
return 0;
}

@@ -163,7 +167,6 @@ static void main_cpu_reset(void *opaque)
static void bamboo_init(MachineState *machine)
{
const char *kernel_filename = machine->kernel_filename;
-const char *kernel_cmdline = machine->kernel_cmdline;
const char *initrd_filename = machine->initrd_filename;
unsigned int pci_irq_nrs[4] = { 28, 27, 26, 25 };
MemoryRegion *address_space_mem = get_system_memory();
@@ -289,8 +292,8 @@ static void bamboo_init(MachineState *machine)

/* If we're loading a kernel directly, we must load the device tree too. */
if (kernel_filename) {
-if (bamboo_load_device_tree(FDT_ADDR, machine->ram_size, RAMDISK_ADDR,
-initrd_size, kernel_cmdline) < 0) {
+if (bamboo_load_device_tree(machine, FDT_ADDR,
+RAMDISK_ADDR, initrd_size) < 0) {
error_report("couldn't load device tree");
exit(1);
}

Re: [PATCH v7 07/14] hw/ppc: set machine->fdt in xilinx_load_device_tree()

2022-09-08 Thread BALATON Zoltan


On Thu, 8 Sep 2022, Daniel Henrique Barboza wrote:

This will enable support for 'dumpdtb' QMP/HMP command for the
virtex_ml507 machine.

Setting machine->fdt requires a MachineState pointer to be used inside
xilinx_load_device_tree(). Let's change the function to receive this
pointer from the caller. kernel_cmdline' can be retrieved directly from
the 'machine' pointer. 'ramsize' wasn't being used so can be removed.

Cc: Edgar E. Iglesias 
Signed-off-by: Daniel Henrique Barboza 


Reviewed-by: BALATON Zoltan 


---
hw/ppc/virtex_ml507.c | 25 ++---
1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index 493ea0c19f..13cace229b 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -45,6 +45,8 @@
#include "hw/qdev-properties.h"
#include "ppc405.h"

+#include 
+
#define EPAPR_MAGIC(0x45504150)
#define FLASH_SIZE (16 * MiB)

@@ -144,11 +146,10 @@ static void main_cpu_reset(void *opaque)
}

#define BINARY_DEVICE_TREE_FILE "virtex-ml507.dtb"
-static int xilinx_load_device_tree(hwaddr addr,
-  uint32_t ramsize,
-  hwaddr initrd_base,
-  hwaddr initrd_size,
-  const char *kernel_cmdline)
+static int xilinx_load_device_tree(MachineState *machine,
+   hwaddr addr,
+   hwaddr initrd_base,
+   hwaddr initrd_size)
{
char *path;
int fdt_size;
@@ -190,18 +191,21 @@ static int xilinx_load_device_tree(hwaddr addr,
error_report("couldn't set /chosen/linux,initrd-end");
}

-r = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", kernel_cmdline);
+r = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
+machine->kernel_cmdline);
if (r < 0)
fprintf(stderr, "couldn't set /chosen/bootargs\n");
cpu_physical_memory_write(addr, fdt, fdt_size);
-g_free(fdt);
+
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = fdt;
+
return fdt_size;
}

static void virtex_init(MachineState *machine)
{
const char *kernel_filename = machine->kernel_filename;
-const char *kernel_cmdline = machine->kernel_cmdline;
hwaddr initrd_base = 0;
int initrd_size = 0;
MemoryRegion *address_space_mem = get_system_memory();
@@ -294,9 +298,8 @@ static void virtex_init(MachineState *machine)
boot_info.fdt = high + (8192 * 2);
boot_info.fdt &= ~8191;

-xilinx_load_device_tree(boot_info.fdt, machine->ram_size,
-initrd_base, initrd_size,
-kernel_cmdline);
+xilinx_load_device_tree(machine, boot_info.fdt,
+initrd_base, initrd_size);
}
env->load_info = &boot_info;
}

[PATCH v7 10/14] hw/ppc: set machine->fdt in spapr machine

The pSeries machine never bothered with the common machine->fdt
attribute. We do all the FDT related work using spapr->fdt_blob.

We're going to introduce a QMP/HMP command to dump the FDT, which will
rely on setting machine->fdt properly to work across all machine
archs/types.

Let's set machine->fdt in two places where we manipulate the FDT:
spapr_machine_reset() and CAS. There are other places where the FDT is
manipulated in the pSeries machines, most notably the hotplug/unplug
path. For now we'll acknowledge that we won't have the most accurate
representation of the FDT, depending on the current machine state, when
using this QMP/HMP fdt command. Making the internal FDT representation
always match the actual FDT representation that the guest is using is a
problem for another day.

spapr->fdt_blob is left untouched for now. To replace it with
machine->fdt, since we're migrating spapr->fdt_blob, we would need to
migrate machine->fdt as well. This is something that we would like to to
do keep our code simpler but it's also a work we'll leave for later.

Reviewed-by: David Gibson 
Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/spapr.c   | 3 +++
 hw/ppc/spapr_hcall.c | 8 
 2 files changed, 11 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index fb790b61e4..170bbfd199 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1713,6 +1713,9 @@ static void spapr_machine_reset(MachineState *machine)
 spapr->fdt_initial_size = spapr->fdt_size;
 spapr->fdt_blob = fdt;
 
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = fdt;
+
 /* Set up the entry state */
 first_ppc_cpu->env.gpr[5] = 0;
 
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index a8d4a6bcf0..891206e893 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1256,6 +1256,14 @@ target_ulong do_client_architecture_support(PowerPCCPU 
*cpu,
 spapr->fdt_initial_size = spapr->fdt_size;
 spapr->fdt_blob = fdt;
 
+/*
+ * Set the machine->fdt pointer again since we just freed
+ * it above (by freeing spapr->fdt_blob). We set this
+ * pointer to enable support for the 'dumpdtb' QMP/HMP
+ * command.
+ */
+MACHINE(spapr)->fdt = fdt;
+
 return H_SUCCESS;
 }
 
-- 
2.37.2

[PATCH v7 14/14] qmp/hmp, device_tree.c: introduce dumpdtb

To save the FDT blob we have the '-machine dumpdtb=' property.
With this property set, the machine saves the FDT in  and exit.
The created file can then be converted to plain text dts format using
'dtc'.

There's nothing particularly sophisticated into saving the FDT that
can't be done with the machine at any state, as long as the machine has
a valid FDT to be saved.

The 'dumpdtb' command receives a 'filename' paramenter and, if a valid
FDT is available, it'll save it in a file 'filename'. In short, this is
a '-machine dumpdtb' that can be fired on demand via QMP/HMP.

A valid FDT consists of a FDT that was created using libfdt being
retrieved via 'current_machine->fdt' in device_tree.c. This condition is
met by most FDT users in QEMU.

This command will always be executed in-band (i.e. holding BQL),
avoiding potential race conditions with machines that might change the
FDT during runtime (e.g. PowerPC 'pseries' machine).

Cc: Dr. David Alan Gilbert 
Cc: Markus Armbruster 
Cc: Alistair Francis 
Cc: David Gibson 
Acked-by: Dr. David Alan Gilbert 
Signed-off-by: Daniel Henrique Barboza 
---
 hmp-commands.hx  | 15 +++
 include/sysemu/device_tree.h |  1 +
 monitor/misc.c   |  1 +
 qapi/machine.json| 18 ++
 softmmu/device_tree.c| 31 +++
 5 files changed, 66 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 182e639d14..753669a2eb 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1800,3 +1800,18 @@ ERST
   "\n\t\t\t\t\t limit on a specified virtual cpu",
 .cmd= hmp_cancel_vcpu_dirty_limit,
 },
+
+#if defined(CONFIG_FDT)
+{
+.name   = "dumpdtb",
+.args_type  = "filename:F",
+.params = "filename",
+.help   = "save the FDT in the 'filename' file to be decoded using 
dtc",
+.cmd= hmp_dumpdtb,
+},
+
+SRST
+``dumpdtb`` *filename*
+  Save the FDT in the 'filename' file to be decoded using dtc.
+ERST
+#endif
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index ef060a9759..e7c5441f56 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -136,6 +136,7 @@ int qemu_fdt_add_path(void *fdt, const char *path);
 } while (0)
 
 void qemu_fdt_dumpdtb(void *fdt, int size);
+void hmp_dumpdtb(Monitor *mon, const QDict *qdict);
 
 /**
  * qemu_fdt_setprop_sized_cells_from_array:
diff --git a/monitor/misc.c b/monitor/misc.c
index 3d2312ba8d..e7dd63030b 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -49,6 +49,7 @@
 #include "sysemu/blockdev.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/tpm.h"
+#include "sysemu/device_tree.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qstring.h"
diff --git a/qapi/machine.json b/qapi/machine.json
index abb2f48808..9f0c8c8374 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1664,3 +1664,21 @@
  '*size': 'size',
  '*max-size': 'size',
  '*slots': 'uint64' } }
+
+##
+# @dumpdtb:
+#
+# Save the FDT in dtb format.
+#
+# @filename: name of the FDT file to be created
+#
+# Since: 7.2
+#
+# Example:
+#   {"execute": "dumpdtb"}
+#"arguments": { "filename": "fdt.dtb" } }
+#
+##
+{ 'command': 'dumpdtb',
+  'data': { 'filename': 'str' },
+  'if': 'CONFIG_FDT' }
diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index 6ca3fad285..7031dcf89d 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -26,6 +26,9 @@
 #include "hw/loader.h"
 #include "hw/boards.h"
 #include "qemu/config-file.h"
+#include "qapi/qapi-commands-machine.h"
+#include "qapi/qmp/qdict.h"
+#include "monitor/hmp.h"
 
 #include 
 
@@ -643,3 +646,31 @@ out:
 g_free(propcells);
 return ret;
 }
+
+void qmp_dumpdtb(const char *filename, Error **errp)
+{
+g_autoptr(GError) err = NULL;
+int size;
+
+if (!current_machine->fdt) {
+error_setg(errp, "This machine doesn't have a FDT");
+return;
+}
+
+size = fdt_totalsize(current_machine->fdt);
+
+if (!g_file_set_contents(filename, current_machine->fdt, size, &err)) {
+error_setg(errp, "Error saving FDT to file %s: %s",
+   filename, err->message);
+}
+}
+
+void hmp_dumpdtb(Monitor *mon, const QDict *qdict)
+{
+const char *filename = qdict_get_str(qdict, "filename");
+Error *local_err = NULL;
+
+qmp_dumpdtb(filename, &local_err);
+
+hmp_handle_error(mon, local_err);
+}
-- 
2.37.2

[PATCH v7 08/14] hw/ppc: set machine->fdt in pegasos2_machine_reset()

We'll introduce a QMP/HMP command that requires machine->fdt to be set
properly.

Cc: BALATON Zoltan 
Cc: qemu-...@nongnu.org
Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/pegasos2.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 61f4263953..ecf682b148 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -331,6 +331,10 @@ static void pegasos2_machine_reset(MachineState *machine)
 
 vof_build_dt(fdt, pm->vof);
 vof_client_open_store(fdt, pm->vof, "/chosen", "stdout", "/failsafe");
+
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = fdt;
+
 pm->cpu->vhyp = PPC_VIRTUAL_HYPERVISOR(machine);
 }
 
-- 
2.37.2

[PATCH v7 13/14] hw/xtensa: set machine->fdt in xtfpga_init()

This will enable support for the 'dumpdtb' QMP/HMP command for all
xtensa machines that uses a FDT.

Signed-off-by: Daniel Henrique Barboza 
---
 hw/xtensa/meson.build | 2 +-
 hw/xtensa/xtfpga.c| 6 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/xtensa/meson.build b/hw/xtensa/meson.build
index 1d5835df4b..ebba51cc74 100644
--- a/hw/xtensa/meson.build
+++ b/hw/xtensa/meson.build
@@ -6,6 +6,6 @@ xtensa_ss.add(files(
 ))
 xtensa_ss.add(when: 'CONFIG_XTENSA_SIM', if_true: files('sim.c'))
 xtensa_ss.add(when: 'CONFIG_XTENSA_VIRT', if_true: files('virt.c'))
-xtensa_ss.add(when: 'CONFIG_XTENSA_XTFPGA', if_true: files('xtfpga.c'))
+xtensa_ss.add(when: 'CONFIG_XTENSA_XTFPGA', if_true: [files('xtfpga.c'), fdt])
 
 hw_arch += {'xtensa': xtensa_ss}
diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index 2a5556a35f..867427c3d9 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -50,6 +50,8 @@
 #include "hw/xtensa/mx_pic.h"
 #include "migration/vmstate.h"
 
+#include 
+
 typedef struct XtfpgaFlashDesc {
 hwaddr base;
 size_t size;
@@ -377,7 +379,9 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, 
MachineState *machine)
 cur_tagptr = put_tag(cur_tagptr, BP_TAG_FDT,
  sizeof(dtb_addr), &dtb_addr);
 cur_lowmem = QEMU_ALIGN_UP(cur_lowmem + fdt_size, 4 * KiB);
-g_free(fdt);
+
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = fdt;
 }
 #else
 if (dtb_filename) {
-- 
2.37.2

[PATCH v7 06/14] hw/ppc: set machine->fdt in sam460ex_load_device_tree()

This will enable support for 'dumpdtb' QMP/HMP command for the sam460ex
machine.

Setting machine->fdt requires a MachineState pointer to be used inside
sam460ex_load_device_tree(). Let's change the function to receive this
pointer from the caller. 'ramsize' and 'kernel_cmdline' can be retrieved
directly from the 'machine' pointer.

Cc: BALATON Zoltan 
Reviewed-by: BALATON Zoltan 
Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/sam460ex.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 850bb3b817..5d09d3c6ab 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -131,13 +131,12 @@ static int sam460ex_load_uboot(void)
 return 0;
 }
 
-static int sam460ex_load_device_tree(hwaddr addr,
- uint32_t ramsize,
+static int sam460ex_load_device_tree(MachineState *machine,
+ hwaddr addr,
  hwaddr initrd_base,
- hwaddr initrd_size,
- const char *kernel_cmdline)
+ hwaddr initrd_size)
 {
-uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(ramsize) };
+uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(machine->ram_size) };
 char *filename;
 int fdt_size;
 void *fdt;
@@ -171,7 +170,8 @@ static int sam460ex_load_device_tree(hwaddr addr,
 qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
   (initrd_base + initrd_size));
 
-qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", kernel_cmdline);
+qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
+machine->kernel_cmdline);
 
 /* Copy data from the host device tree into the guest. Since the guest can
  * directly access the timebase without host involvement, we must expose
@@ -208,7 +208,9 @@ static int sam460ex_load_device_tree(hwaddr addr,
   EBC_FREQ);
 
 rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
-g_free(fdt);
+
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = fdt;
 
 return fdt_size;
 }
@@ -496,9 +498,8 @@ static void sam460ex_init(MachineState *machine)
 if (machine->kernel_filename) {
 int dt_size;
 
-dt_size = sam460ex_load_device_tree(FDT_ADDR, machine->ram_size,
-RAMDISK_ADDR, initrd_size,
-machine->kernel_cmdline);
+dt_size = sam460ex_load_device_tree(machine, FDT_ADDR,
+RAMDISK_ADDR, initrd_size);
 
 boot_info->dt_base = FDT_ADDR;
 boot_info->dt_size = dt_size;
-- 
2.37.2

[PATCH v7 12/14] hw/riscv: set machine->fdt in spike_board_init()

This will enable support for the 'dumpdtb' QMP/HMP command for the spike
machine.

Cc: Palmer Dabbelt 
Cc: Alistair Francis 
Cc: Bin Meng 
Reviewed-by: Alistair Francis 
Signed-off-by: Daniel Henrique Barboza 
---
 hw/riscv/spike.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 5ba34543c8..1e1d752c00 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -40,6 +40,8 @@
 #include "sysemu/device_tree.h"
 #include "sysemu/sysemu.h"
 
+#include 
+
 static const MemMapEntry spike_memmap[] = {
 [SPIKE_MROM] = { 0x1000, 0xf000 },
 [SPIKE_HTIF] = {  0x100, 0x1000 },
@@ -304,6 +306,10 @@ static void spike_board_init(MachineState *machine)
 /* Compute the fdt load address in dram */
 fdt_load_addr = riscv_load_fdt(memmap[SPIKE_DRAM].base,
machine->ram_size, s->fdt);
+
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = s->fdt;
+
 /* load the reset vector */
 riscv_setup_rom_reset_vec(machine, &s->soc[0], memmap[SPIKE_DRAM].base,
   memmap[SPIKE_MROM].base,
-- 
2.37.2

[PATCH v7 05/14] hw/ppc: set machine->fdt in bamboo_load_device_tree()

This will enable support for 'dumpdtb' QMP/HMP command for the bamboo
machine.

Setting machine->fdt requires a MachineState pointer to be used inside
bamboo_load_device_tree(). Let's change the function to receive this
pointer from the caller. 'ramsize' and 'kernel_cmdline' can be retrieved
directly from the 'machine' pointer.

Cc: Cédric Le Goater 
Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/ppc440_bamboo.c | 25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index ea945a1c99..9cc58fccf9 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -34,6 +34,8 @@
 #include "hw/qdev-properties.h"
 #include "qapi/error.h"
 
+#include 
+
 #define BINARY_DEVICE_TREE_FILE "bamboo.dtb"
 
 /* from u-boot */
@@ -56,14 +58,13 @@ static const ram_addr_t ppc440ep_sdram_bank_sizes[] = {
 
 static hwaddr entry;
 
-static int bamboo_load_device_tree(hwaddr addr,
- uint32_t ramsize,
- hwaddr initrd_base,
- hwaddr initrd_size,
- const char *kernel_cmdline)
+static int bamboo_load_device_tree(MachineState *machine,
+   hwaddr addr,
+   hwaddr initrd_base,
+   hwaddr initrd_size)
 {
 int ret = -1;
-uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(ramsize) };
+uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(machine->ram_size) };
 char *filename;
 int fdt_size;
 void *fdt;
@@ -98,7 +99,7 @@ static int bamboo_load_device_tree(hwaddr addr,
 fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
 }
 ret = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
-  kernel_cmdline);
+  machine->kernel_cmdline);
 if (ret < 0) {
 fprintf(stderr, "couldn't set /chosen/bootargs\n");
 }
@@ -119,7 +120,10 @@ static int bamboo_load_device_tree(hwaddr addr,
   tb_freq);
 
 rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
-g_free(fdt);
+
+/* Set ms->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = fdt;
+
 return 0;
 }
 
@@ -163,7 +167,6 @@ static void main_cpu_reset(void *opaque)
 static void bamboo_init(MachineState *machine)
 {
 const char *kernel_filename = machine->kernel_filename;
-const char *kernel_cmdline = machine->kernel_cmdline;
 const char *initrd_filename = machine->initrd_filename;
 unsigned int pci_irq_nrs[4] = { 28, 27, 26, 25 };
 MemoryRegion *address_space_mem = get_system_memory();
@@ -289,8 +292,8 @@ static void bamboo_init(MachineState *machine)
 
 /* If we're loading a kernel directly, we must load the device tree too. */
 if (kernel_filename) {
-if (bamboo_load_device_tree(FDT_ADDR, machine->ram_size, RAMDISK_ADDR,
-initrd_size, kernel_cmdline) < 0) {
+if (bamboo_load_device_tree(machine, FDT_ADDR,
+RAMDISK_ADDR, initrd_size) < 0) {
 error_report("couldn't load device tree");
 exit(1);
 }
-- 
2.37.2

[PATCH v7 11/14] hw/riscv: set machine->fdt in sifive_u_machine_init()

This will enable support for 'dumpdtb' QMP/HMP command for the sifive_u
machine.

Cc: Alistair Francis 
Cc: Bin Meng 
Cc: Palmer Dabbelt 
Reviewed-by: Alistair Francis 
Signed-off-by: Daniel Henrique Barboza 
---
 hw/riscv/sifive_u.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index e4c814a3ea..b139824aab 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -634,6 +634,9 @@ static void sifive_u_machine_init(MachineState *machine)
 start_addr_hi32 = (uint64_t)start_addr >> 32;
 }
 
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = s->fdt;
+
 /* reset vector */
 uint32_t reset_vec[12] = {
 s->msel,   /* MSEL pin state */
-- 
2.37.2

[PATCH v7 04/14] hw/ppc: set machine->fdt in ppce500_load_device_tree()

This will enable support for 'dumpdtb' QMP/HMP command for the e500
machine.

Cc: Cédric Le Goater 
Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/e500.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 32495d0123..ea5f947824 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -47,6 +47,8 @@
 #include "hw/i2c/i2c.h"
 #include "hw/irq.h"
 
+#include 
+
 #define EPAPR_MAGIC(0x45504150)
 #define DTC_LOAD_PAD   0x180
 #define DTC_PAD_MASK   0xF
@@ -600,7 +602,16 @@ done:
 cpu_physical_memory_write(addr, fdt, fdt_size);
 }
 ret = fdt_size;
-g_free(fdt);
+
+/*
+ * Update the machine->fdt pointer to enable support for the
+ * 'dumpdtb' QMP/HMP command.
+ *
+ * The FDT is re-created during reset, so free machine->fdt
+ * to avoid leaking the old FDT.
+ */
+g_free(machine->fdt);
+machine->fdt = fdt;
 
 out:
 g_free(pci_map);
-- 
2.37.2

[PATCH v7 07/14] hw/ppc: set machine->fdt in xilinx_load_device_tree()

This will enable support for 'dumpdtb' QMP/HMP command for the
virtex_ml507 machine.

Setting machine->fdt requires a MachineState pointer to be used inside
xilinx_load_device_tree(). Let's change the function to receive this
pointer from the caller. kernel_cmdline' can be retrieved directly from
the 'machine' pointer. 'ramsize' wasn't being used so can be removed.

Cc: Edgar E. Iglesias 
Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/virtex_ml507.c | 25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index 493ea0c19f..13cace229b 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -45,6 +45,8 @@
 #include "hw/qdev-properties.h"
 #include "ppc405.h"
 
+#include 
+
 #define EPAPR_MAGIC(0x45504150)
 #define FLASH_SIZE (16 * MiB)
 
@@ -144,11 +146,10 @@ static void main_cpu_reset(void *opaque)
 }
 
 #define BINARY_DEVICE_TREE_FILE "virtex-ml507.dtb"
-static int xilinx_load_device_tree(hwaddr addr,
-  uint32_t ramsize,
-  hwaddr initrd_base,
-  hwaddr initrd_size,
-  const char *kernel_cmdline)
+static int xilinx_load_device_tree(MachineState *machine,
+   hwaddr addr,
+   hwaddr initrd_base,
+   hwaddr initrd_size)
 {
 char *path;
 int fdt_size;
@@ -190,18 +191,21 @@ static int xilinx_load_device_tree(hwaddr addr,
 error_report("couldn't set /chosen/linux,initrd-end");
 }
 
-r = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", kernel_cmdline);
+r = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
+machine->kernel_cmdline);
 if (r < 0)
 fprintf(stderr, "couldn't set /chosen/bootargs\n");
 cpu_physical_memory_write(addr, fdt, fdt_size);
-g_free(fdt);
+
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = fdt;
+
 return fdt_size;
 }
 
 static void virtex_init(MachineState *machine)
 {
 const char *kernel_filename = machine->kernel_filename;
-const char *kernel_cmdline = machine->kernel_cmdline;
 hwaddr initrd_base = 0;
 int initrd_size = 0;
 MemoryRegion *address_space_mem = get_system_memory();
@@ -294,9 +298,8 @@ static void virtex_init(MachineState *machine)
 boot_info.fdt = high + (8192 * 2);
 boot_info.fdt &= ~8191;
 
-xilinx_load_device_tree(boot_info.fdt, machine->ram_size,
-initrd_base, initrd_size,
-kernel_cmdline);
+xilinx_load_device_tree(machine, boot_info.fdt,
+initrd_base, initrd_size);
 }
 env->load_info = &boot_info;
 }
-- 
2.37.2

[PATCH v7 02/14] hw/microblaze: set machine->fdt in microblaze_load_dtb()

This will enable support for 'dumpdtb' QMP/HMP command for all
microblaze machines that uses microblaze_load_dtb().

Cc: Edgar E. Iglesias 
Signed-off-by: Daniel Henrique Barboza 
---
 hw/microblaze/boot.c  | 8 +++-
 hw/microblaze/meson.build | 2 +-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 8b92a9801a..c8eff7b6fc 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -39,6 +39,8 @@
 
 #include "boot.h"
 
+#include 
+
 static struct
 {
 void (*machine_cpu_reset)(MicroBlazeCPU *);
@@ -72,6 +74,7 @@ static int microblaze_load_dtb(hwaddr addr,
const char *kernel_cmdline,
const char *dtb_filename)
 {
+MachineState *machine = MACHINE(qdev_get_machine());
 int fdt_size;
 void *fdt = NULL;
 int r;
@@ -100,7 +103,10 @@ static int microblaze_load_dtb(hwaddr addr,
 }
 
 cpu_physical_memory_write(addr, fdt, fdt_size);
-g_free(fdt);
+
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = fdt;
+
 return fdt_size;
 }
 
diff --git a/hw/microblaze/meson.build b/hw/microblaze/meson.build
index bb9e4eb8f4..a38a397872 100644
--- a/hw/microblaze/meson.build
+++ b/hw/microblaze/meson.build
@@ -1,5 +1,5 @@
 microblaze_ss = ss.source_set()
-microblaze_ss.add(files('boot.c'))
+microblaze_ss.add(files('boot.c'), fdt)
 microblaze_ss.add(when: 'CONFIG_PETALOGIX_S3ADSP1800', if_true: 
files('petalogix_s3adsp1800_mmu.c'))
 microblaze_ss.add(when: 'CONFIG_PETALOGIX_ML605', if_true: 
files('petalogix_ml605_mmu.c'))
 microblaze_ss.add(when: 'CONFIG_XLNX_ZYNQMP_PMU', if_true: 
files('xlnx-zynqmp-pmu.c'))
-- 
2.37.2

[PATCH v7 09/14] hw/ppc: set machine->fdt in pnv_reset()

This will enable support for the 'dumpdtb' QMP/HMP command for
all powernv machines.

Reviewed-by: Cédric Le Goater 
Reviewed-by: Frederic Barrat 
Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/pnv.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 354aa289d1..6a20c4811f 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -678,7 +678,13 @@ static void pnv_reset(MachineState *machine)
 qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
 cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt));
 
-g_free(fdt);
+/*
+ * Set machine->fdt for 'dumpdtb' QMP/HMP command. Free
+ * the existing machine->fdt to avoid leaking it during
+ * a reset.
+ */
+g_free(machine->fdt);
+machine->fdt = fdt;
 }
 
 static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp)
-- 
2.37.2

[PATCH v7 03/14] hw/nios2: set machine->fdt in nios2_load_dtb()

This will enable support for 'dumpdtb' QMP/HMP command for all nios2
machines that uses nios2_load_dtb().

Cc: Chris Wulff 
Cc: Marek Vasut 
Signed-off-by: Daniel Henrique Barboza 
---
 hw/nios2/boot.c  | 8 +++-
 hw/nios2/meson.build | 2 +-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/nios2/boot.c b/hw/nios2/boot.c
index 21cb47..b30a7b1efb 100644
--- a/hw/nios2/boot.c
+++ b/hw/nios2/boot.c
@@ -43,6 +43,8 @@
 
 #include "boot.h"
 
+#include 
+
 #define NIOS2_MAGIC0x534f494e
 
 static struct nios2_boot_info {
@@ -81,6 +83,7 @@ static uint64_t translate_kernel_address(void *opaque, 
uint64_t addr)
 static int nios2_load_dtb(struct nios2_boot_info bi, const uint32_t ramsize,
   const char *kernel_cmdline, const char *dtb_filename)
 {
+MachineState *machine = MACHINE(qdev_get_machine());
 int fdt_size;
 void *fdt = NULL;
 int r;
@@ -113,7 +116,10 @@ static int nios2_load_dtb(struct nios2_boot_info bi, const 
uint32_t ramsize,
 }
 
 cpu_physical_memory_write(bi.fdt, fdt, fdt_size);
-g_free(fdt);
+
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
+machine->fdt = fdt;
+
 return fdt_size;
 }
 
diff --git a/hw/nios2/meson.build b/hw/nios2/meson.build
index 6c58e8082b..22277bd6c5 100644
--- a/hw/nios2/meson.build
+++ b/hw/nios2/meson.build
@@ -1,5 +1,5 @@
 nios2_ss = ss.source_set()
-nios2_ss.add(files('boot.c'))
+nios2_ss.add(files('boot.c'), fdt)
 nios2_ss.add(when: 'CONFIG_NIOS2_10M50', if_true: files('10m50_devboard.c'))
 nios2_ss.add(when: 'CONFIG_NIOS2_GENERIC_NOMMU', if_true: 
files('generic_nommu.c'))
 
-- 
2.37.2

[PATCH v7 01/14] hw/arm: do not free machine->fdt in arm_load_dtb()

At this moment, arm_load_dtb() can free machine->fdt when
binfo->dtb_filename is NULL. If there's no 'dtb_filename', 'fdt' will be
retrieved by binfo->get_dtb(). If get_dtb() returns machine->fdt, as is
the case of machvirt_dtb() from hw/arm/virt.c, fdt now has a pointer to
machine->fdt. And, in that case, the existing g_free(fdt) at the end of
arm_load_dtb() will make machine->fdt point to an invalid memory region.

This is not an issue right now because there's no code that access
machine->fdt after arm_load_dtb(), but we're going to add a QMP/HMP
FDT command that will rely on machine->fdt being valid.

Instead of freeing 'fdt' at the end of arm_load_dtb(), assign it to
machine->fdt. This will allow the FDT of ARM machines that relies on
arm_load_dtb() to be accessed later on.

Since all ARM machines allocates the FDT only once, we don't need to
worry about leaking the existing FDT during a machine reset (which is
something that other machines have to look after, e.g. the ppc64 pSeries
machine).

Cc: Peter Maydell 
Cc: qemu-...@nongnu.org
Signed-off-by: Daniel Henrique Barboza 
---
 hw/arm/boot.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index ada2717f76..60bbfba37f 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -684,7 +684,8 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info 
*binfo,
  */
 rom_add_blob_fixed_as("dtb", fdt, size, addr, as);
 
-g_free(fdt);
+/* Set ms->fdt for 'dumpdtb' QMP/HMP command */
+ms->fdt = fdt;
 
 return size;
 
-- 
2.37.2

[PATCH v7 00/14] QMP/HMP: introduce 'dumpdtb'