Re: [PATCH v6 5/9] target/riscv: remove 'cpu_vl' global

[Philippe Mathieu-Daudé]

On 21/2/24 22:31, Daniel Henrique Barboza wrote:

At this moment the global is used only in do_vsetvl(). Do a direct env
load in do_vsetvl() to read 'vl' and remove the global.

Suggested-by: Richard Henderson 
Signed-off-by: Daniel Henrique Barboza 
---
  target/riscv/insn_trans/trans_rvv.c.inc | 2 +-
  target/riscv/translate.c| 3 +--
  2 files changed, 2 insertions(+), 3 deletions(-)


I was expecting this to be squashed in previous patch, but it
is indeed clearer as a separate one.

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v4 2/3] virtio: Declare the decoding functions to static

[Markus Armbruster]

Hyman Huang  writes:

> qmp_decode_protocols(), qmp_decode_status(), and qmp_decode_features()
> are now only used in virtio-hmp-cmds.c.  So move them into there,
> redeclare them to static, and replace the qmp_ prefix with hmp_.
>
> Signed-off-by: Hyman Huang 

Reviewed-by: Markus Armbruster 

Re: [PATCH v3 1/3] qdev: Add a granule_mode property

[Eric Auger]

Hi Richard,

On 2/21/24 22:58, Richard Henderson wrote:
> On 2/21/24 10:58, Eric Auger wrote:
>> Introduce a new enum type property allowing to set an
>> IOMMU granule. Values are 4K, 16K, 64K and host. This
>> latter indicates the vIOMMU granule will matches the
>> host page size.
>>
>> A subsequent patch will add such a property to the
>> virtio-iommu device.
>>
>> Signed-off-by: Eric Auger 
>> ---
>>   include/hw/qdev-properties-system.h |  3 +++
>>   include/hw/virtio/virtio-iommu.h    | 11 +++
>>   hw/core/qdev-properties-system.c    | 15 +++
>>   hw/virtio/virtio-iommu.c    | 10 ++
>>   4 files changed, 39 insertions(+)
>>
>> diff --git a/include/hw/qdev-properties-system.h
>> b/include/hw/qdev-properties-system.h
>> index 06c359c190..626be87dd3 100644
>> --- a/include/hw/qdev-properties-system.h
>> +++ b/include/hw/qdev-properties-system.h
>> @@ -8,6 +8,7 @@ extern const PropertyInfo qdev_prop_macaddr;
>>   extern const PropertyInfo qdev_prop_reserved_region;
>>   extern const PropertyInfo qdev_prop_multifd_compression;
>>   extern const PropertyInfo qdev_prop_mig_mode;
>> +extern const PropertyInfo qdev_prop_granule_mode;
>>   extern const PropertyInfo qdev_prop_losttickpolicy;
>>   extern const PropertyInfo qdev_prop_blockdev_on_error;
>>   extern const PropertyInfo qdev_prop_bios_chs_trans;
>> @@ -47,6 +48,8 @@ extern const PropertyInfo
>> qdev_prop_iothread_vq_mapping_list;
>>   #define DEFINE_PROP_MIG_MODE(_n, _s, _f, _d) \
>>   DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_mig_mode, \
>>  MigMode)
>> +#define DEFINE_PROP_GRANULE_MODE(_n, _s, _f, _d) \
>> +    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_granule_mode,
>> GranuleMode)
>>   #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
>>   DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
>>   LostTickPolicy)
>> diff --git a/include/hw/virtio/virtio-iommu.h
>> b/include/hw/virtio/virtio-iommu.h
>> index 5fbe4677c2..a82c4fa471 100644
>> --- a/include/hw/virtio/virtio-iommu.h
>> +++ b/include/hw/virtio/virtio-iommu.h
>> @@ -31,6 +31,16 @@ OBJECT_DECLARE_SIMPLE_TYPE(VirtIOIOMMU, VIRTIO_IOMMU)
>>     #define TYPE_VIRTIO_IOMMU_MEMORY_REGION "virtio-iommu-memory-region"
>>   +typedef enum GranuleMode {
>> +    GRANULE_MODE_4K,
>> +    GRANULE_MODE_16K,
>> +    GRANULE_MODE_64K,
>> +    GRANULE_MODE_HOST,
>> +    GRANULE_MODE__MAX,
>> +} GranuleMode;
>> +
>> +extern const QEnumLookup GranuleMode_lookup;
>> +
>>   typedef struct IOMMUDevice {
>>   void *viommu;
>>   PCIBus   *bus;
>> @@ -67,6 +77,7 @@ struct VirtIOIOMMU {
>>   Notifier machine_done;
>>   bool granule_frozen;
>>   uint8_t aw_bits;
>> +    GranuleMode granule_mode;
>>   };
>>     #endif
>> diff --git a/hw/core/qdev-properties-system.c
>> b/hw/core/qdev-properties-system.c
>> index 1a396521d5..3a0b36a7a7 100644
>> --- a/hw/core/qdev-properties-system.c
>> +++ b/hw/core/qdev-properties-system.c
>> @@ -34,6 +34,7 @@
>>   #include "net/net.h"
>>   #include "hw/pci/pci.h"
>>   #include "hw/pci/pcie.h"
>> +#include "hw/virtio/virtio-iommu.h"
>>   #include "hw/i386/x86.h"
>>   #include "util/block-helpers.h"
>>   @@ -679,6 +680,20 @@ const PropertyInfo qdev_prop_mig_mode = {
>>   .set_default_value = qdev_propinfo_set_default_value_enum,
>>   };
>>   +/* --- GranuleMode --- */
>> +
>> +QEMU_BUILD_BUG_ON(sizeof(GranuleMode) != sizeof(int));
>> +
>> +const PropertyInfo qdev_prop_granule_mode = {
>> +    .name = "GranuleMode",
>> +    .description = "granule_mode values, "
>> +   "4K, 16K, 64K, host",
>> +    .enum_table = _lookup,
>> +    .get = qdev_propinfo_get_enum,
>> +    .set = qdev_propinfo_set_enum,
>> +    .set_default_value = qdev_propinfo_set_default_value_enum,
>> +};
>> +
>>   /* --- Reserved Region --- */
>>     /*
>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>> index 2ec5ef3cd1..2ce5839c60 100644
>> --- a/hw/virtio/virtio-iommu.c
>> +++ b/hw/virtio/virtio-iommu.c
>> @@ -44,6 +44,16 @@
>>   #define VIOMMU_DEFAULT_QUEUE_SIZE 256
>>   #define VIOMMU_PROBE_SIZE 512
>>   +const QEnumLookup GranuleMode_lookup = {
>> +    .array = (const char *const[]) {
>> +    [GRANULE_MODE_4K]  = "4K",
>> +    [GRANULE_MODE_16K] = "16K",
>> +    [GRANULE_MODE_64K] = "64K",
>> +    [GRANULE_MODE_HOST] = "host",
>> +    },
>> +    .size = GRANULE_MODE__MAX
>> +};
>
> Does it ever make sense to use anything other than host?
> Especially since 4k fails on a 64k host, as you report.
Let me reuse Jean-Philippe's support matrix:
Before this series, 4KB granule was always used by the virtio-iommu.
The support matrix was:

 Host | Guest | virtio-net | IGB passthrough
  4k  | 4k| Y  | Y
  64k | 64k   | Y  | N
  64k | 4k| Y  | N
  4k  | 64k   | Y  | Y

with this series, host becomes the default. It fixes the 64KB/64KB config by 
default. It allows 64KB/4KB (virtio only) with 

Re: [PATCH] hw/intc/Kconfig: Fix GIC settings when using "--without-default-devices"

[Philippe Mathieu-Daudé]

On 21/2/24 12:00, Thomas Huth wrote:

When using "--without-default-devices", the ARM_GICV3_TCG and ARM_GIC_KVM
settings currently get disabled, though the arm virt machine is only of
very limited use in that case. This also causes the migration-test to
fail in such builds. Let's make sure that we always keep the GIC switches
enabled in the --without-default-devices builds, too.

Signed-off-by: Thomas Huth 
---
  hw/intc/Kconfig | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index 97d550b06b..2b5b2d2301 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -12,10 +12,6 @@ config IOAPIC
  bool
  select I8259
  
-config ARM_GIC

-bool
-select MSI_NONBROKEN
-
  config OPENPIC
  bool
  select MSI_NONBROKEN
@@ -25,14 +21,18 @@ config APIC
  select MSI_NONBROKEN
  select I8259
  
+config ARM_GIC

+bool
+select ARM_GICV3_TCG if TCG
+select ARM_GIC_KVM if KVM


This is odd, we usually 'select' dependencies.


+select MSI_NONBROKEN
+
  config ARM_GICV3_TCG
  bool
-default y


Don't we want instead:

   default y if TCG


  depends on ARM_GIC && TCG
  
  config ARM_GIC_KVM

  bool
-default y


and:

   default y if KVM

?


  depends on ARM_GIC && KVM
  
  config XICS

Re: [PATCH] hw/sparc/leon3: Fix wrong usage of DO_UPCAST macro

[Philippe Mathieu-Daudé]

On 21/2/24 19:49, Philippe Mathieu-Daudé wrote:

On 21/2/24 19:47, Philippe Mathieu-Daudé wrote:

On 21/2/24 19:07, Thomas Huth wrote:

leon3.c currently fails to compile with some compilers when the -Wvla
option has been enabled:

  ../hw/sparc/leon3.c: In function ‘leon3_cpu_reset’:
  ../hw/sparc/leon3.c:153:5: error: ISO C90 forbids variable length 
array

   ‘offset_must_be_zero’ [-Werror=vla]
    153 | ResetData *s = (ResetData *)DO_UPCAST(ResetData, 
info[id], info);

    | ^
  cc1: all warnings being treated as errors

Looking at this code, the DO_UPCAST macro is indeed used in a wrong way
here: DO_UPCAST is supposed to check that the second parameter is the
first entry of the struct that the first parameter indicates, but since
we use and index into the info[] array, this of course cannot work.

The intention here was likely rather to use the container_of() macro
instead, so switch the code accordingly.


Fixes: d65aba8286 ("hw/sparc/leon3: implement multiprocessor")


Signed-off-by: Thomas Huth 
---
  hw/sparc/leon3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 



Patch queued!

RE: [EXT] Re: [PATCH] vhost_net: add NOTIFICATION_DATA and IN_ORDER feature bits to vdpa_feature_bits

[Srujana Challa]

Ping.

> Subject: RE: [EXT] Re: [PATCH] vhost_net: add NOTIFICATION_DATA and
> IN_ORDER feature bits to vdpa_feature_bits
> 
> 
> 
> > -Original Message-
> > From: Michael S. Tsirkin 
> > Sent: Monday, February 19, 2024 3:15 PM
> > To: Srujana Challa 
> > Cc: qemu-devel@nongnu.org; Vamsi Krishna Attunuru
> > ; Jerin Jacob ; Jason Wang
> > 
> > Subject: [EXT] Re: [PATCH] vhost_net: add NOTIFICATION_DATA and
> > IN_ORDER feature bits to vdpa_feature_bits
> >
> > External Email
> >
> > --
> > On Tue, Jan 02, 2024 at 04:44:32PM +0530, Srujana Challa wrote:
> > > Enables VIRTIO_F_NOTIFICATION_DATA and VIRTIO_F_IN_ORDER feature
> > bits
> > > for vhost vdpa backend. Also adds code to consider all feature bits
> > > supported by vhost net client type for feature negotiation, so that
> > > vhost backend device supported features can be negotiated with guest.
> > >
> > > Signed-off-by: Srujana Challa 
> > > ---
> > >  hw/net/vhost_net.c | 10 ++
> > >  net/vhost-vdpa.c   |  2 ++
> > >  2 files changed, 12 insertions(+)
> > >
> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index
> > > e8e1661646..65ae8bcece 100644
> > > --- a/hw/net/vhost_net.c
> > > +++ b/hw/net/vhost_net.c
> > > @@ -117,6 +117,16 @@ static const int
> > > *vhost_net_get_feature_bits(struct vhost_net *net)
> > >
> > >  uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t
> > > features)  {
> > > +const int *bit = vhost_net_get_feature_bits(net);
> > > +
> > > +/*
> > > + * Consider all feature bits for feature negotiation with vhost 
> > > backend,
> > > + * so that all backend device supported features can be negotiated.
> > > + */
> > > +while (*bit != VHOST_INVALID_FEATURE_BIT) {
> > > +features |= (1ULL << *bit);
> > > +bit++;
> > > +}
> > >  return vhost_get_features(>dev, vhost_net_get_feature_bits(net),
> > >  features);
> > >  }
> >
> > I don't think we should do this part. With vdpa QEMU is in control of
> > which features are exposed and that is intentional since features are
> > often tied to other behaviour.
> 
> Vdpa Qemu can negotiate all the features which vdpa backend device supports
> with the guest right?
> Guest drivers (it could be userspace or kernel drivers) will negotiate their 
> own
> features, so that
> frontend supported features will get the precedence.
> 
> >
> > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index
> > > 3726ee5d67..51334fcfe2 100644
> > > --- a/net/vhost-vdpa.c
> > > +++ b/net/vhost-vdpa.c
> > > @@ -57,7 +57,9 @@ typedef struct VhostVDPAState {
> > >   */
> > >  const int vdpa_feature_bits[] = {
> > >  VIRTIO_F_ANY_LAYOUT,
> > > +VIRTIO_F_IN_ORDER,
> > >  VIRTIO_F_IOMMU_PLATFORM,
> > > +VIRTIO_F_NOTIFICATION_DATA,
> > >  VIRTIO_F_NOTIFY_ON_EMPTY,
> > >  VIRTIO_F_RING_PACKED,
> > >  VIRTIO_F_RING_RESET,
> > > --
> > > 2.25.1

Re: [PATCH 0/6] [PATCH 0/6] Fix missing ERRP_GUARD() when dereference @errp

[Zhao Liu]

On Thu, Feb 22, 2024 at 09:04:14AM +0300, Michael Tokarev wrote:
> Date: Thu, 22 Feb 2024 09:04:14 +0300
> From: Michael Tokarev 
> Subject: Re: [PATCH 0/6] [PATCH 0/6] Fix missing ERRP_GUARD() when
>  dereference @errp
> 
> 21.02.2024 12:43, Zhao Liu wrote:
> > From: Zhao Liu 
> > 
> > Hi all,
> > 
> > Thanks to Markus's explanation about ERRP_GUARD() on my previsou
> > patch [1],
> > 
> > I realize that perhaps more @errp dereference cases need to be
> > double-checked to ensure that ERRP_GUARD() is being used correctly.
> > 
> > Therefore, there're the patches to add more missing ERRP_GUARD().
> > 
> > [1]: https://lore.kernel.org/qemu-devel/875xz0ojg7@pond.sub.org/
> 
> Hi!
> 
> Since you're going to respin (it looks like), please include the initial
> patch (hw/intc one) into the series as well, so it's all in one go.
> 
> I'm not sure this should come via trivial-patches tree though.  Looks
> trivial enough :)
> 

Hi Michael,

Sure, I'll add that one into this series.
I understand that such small fixes across multiple parts are suited for
trivial-tree.

Thanks,
Zhao

Re: [PATCH v2 0/2] Field 'reason' for MIGRATION event

[Markus Armbruster]

Fabiano Rosas  writes:

> Roman Khapov  writes:
>
> Hi Roman,
>
>> This is resending of series 20240215082659.1378342-1-rkha...@yandex-team.ru,
>> where patch subjects numbers were broken in patch 2/2.
>>
>> Sometimes, when migration fails, it is hard to find out
>> the cause of the problems: you have to grep qemu logs.
>> At the same time, there is MIGRATION event, which looks like
>> suitable place to hold such error descriptions.
>
> query-migrate after the event is received should be enough for giving
> you the failure reason. We have that in error-desc. See commit
> c94143e587 ("migration: Display error in query-migrate irrelevant of
> status").
>
>>
>> To handle situation like this (maybe one day it will be useful
>> for other MIGRATION statuses to have additional 'reason' strings),
>
> I find it unlikely. There's no "reason" for making progress except
> that's how things work. Only the exceptional (i.e. failure) statuses
> would have a reason. Today that's FAILED only, maybe also
> POSTCOPY_PAUSED.

I can't see a need for the proposed feature then.

>> the general optional field 'reason' can be added.

[...]

Re: [PATCH v4 32/34] monitor: fdset: Match against O_DIRECT

[Markus Armbruster]

Fabiano Rosas  writes:

> Markus Armbruster  writes:
>
>> Fabiano Rosas  writes:
>>
>>> We're about to enable the use of O_DIRECT in the migration code and
>>> due to the alignment restrictions imposed by filesystems we need to
>>> make sure the flag is only used when doing aligned IO.
>>>
>>> The migration will do parallel IO to different regions of a file, so
>>> we need to use more than one file descriptor. Those cannot be obtained
>>> by duplicating (dup()) since duplicated file descriptors share the
>>> file status flags, including O_DIRECT. If one migration channel does
>>> unaligned IO while another sets O_DIRECT to do aligned IO, the
>>> filesystem would fail the unaligned operation.
>>>
>>> The add-fd QMP command along with the fdset code are specifically
>>> designed to allow the user to pass a set of file descriptors with
>>> different access flags into QEMU to be later fetched by code that
>>> needs to alternate between those flags when doing IO.
>>>
>>> Extend the fdset matching function to behave the same with the
>>> O_DIRECT flag.
>>>
>>> Signed-off-by: Fabiano Rosas 
>>> ---
>>>  monitor/fds.c | 6 ++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/monitor/fds.c b/monitor/fds.c
>>> index 9a28e4b72b..42bf3eb982 100644
>>> --- a/monitor/fds.c
>>> +++ b/monitor/fds.c
>>> @@ -413,6 +413,12 @@ static bool monitor_fdset_flags_match(int flags, int 
>>> fd_flags)
>>static bool monitor_fdset_flags_match(int flags, int fd_flags)
>>{
>>bool match = false;
>>
>>>  if ((flags & O_ACCMODE) == (fd_flags & O_ACCMODE)) {
>>>  match = true;
>>> +
>>> +#ifdef O_DIRECT
>>> +if ((flags & O_DIRECT) != (fd_flags & O_DIRECT)) {
>>> +match = false;
>>> +}
>>> +#endif
>>>  }
>>>  
>>>  return match;
>>}
>>
>> I'd prefer something like
>>
>>static bool monitor_fdset_flags_match(int flags, int fd_flags)
>>{
>>#ifdef O_DIRECT
>>if ((flags & O_DIRECT) != (fd_flags & O_DIRECT)) {
>>return false;
>>}
>>#endif
>>
>>if ((flags & O_ACCMODE) != (fd_flags & O_ACCMODE)) {
>>return false;
>>
>>}
>>
>>return true;
>>}
>
> This makes the O_DIRECT flag dictate the outcome when it's present. I
> want O_DIRECT to be considered only when all other flags have matched.
>
> Otherwise we regress the original use-case if the user happened to have
> put O_DIRECT in the flags. A non-match due to different O_ACCMODE would
> become a match due to (possibly) matching O_DIRECT.

The fact that I missed this signifies one of two things: either was
suffering from code review brain (quite possible!), or this needs a
comment and/or clearer coding.

If I understand you correctly, you want to return true when the bits
selected by the two masks together match.

If we didn't need ifdeffery, we wouldn't use nested conditionals for
comparing bits under a mask.  We'd use something like

int mask = O_ACCMODE | O_DIRECT;

return (flags & mask) == (fd_flags & mask);

Bring back the ifdeffery:

int mask = O_ACCMODE;

#ifdef O_DIRECT
mask |= O_DIRECT;
#endif

return (flags & mask) == (fd_flags & mask);

Or maybe even

#ifndef O_DIRECT
#define O_DIRECT 0
#endif

int mask = O_ACCMODE | O_DIRECT;

return (flags & mask) == (fd_flags & mask);

Not sure this is even worth a helper function.

Or am I stull suffering from code review brain?

Re: [PATCH v6 13/15] hw/pci: Determine if rombar is explicitly enabled

[Akihiko Odaki]

On 2024/02/21 17:15, Markus Armbruster wrote:

Akihiko Odaki  writes:


vfio determines if rombar is explicitly enabled by inspecting QDict.
Inspecting QDict is not nice because QDict is untyped and depends on the
details on the external interface. Add an infrastructure to determine if
rombar is explicitly enabled to hw/pci.

Signed-off-by: Akihiko Odaki 
---
  include/hw/pci/pci_device.h | 5 +
  1 file changed, 5 insertions(+)

diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index ca151325085d..c4fdc96ef50d 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -205,6 +205,11 @@ static inline uint16_t pci_get_bdf(PCIDevice *dev)
  return PCI_BUILD_BDF(pci_bus_num(pci_get_bus(dev)), dev->devfn);
  }
  
+static inline bool pci_rom_bar_explicitly_enabled(PCIDevice *dev)

+{
+return dev->rom_bar && dev->rom_bar != -1;


Compares signed with unsigned: rom_bar is uint32_t, -1 is int.

If int is at most 32 bits, the comparison converts -1 to
(uint32_t)0x.

If int is wider, it converts dev->rom_bar to int without changing its
value.  In particular, it converts the default value 0x (written
as -1 in the previous patch) to (int)0x.  Oops.

Best not to mix signed and unsigned in comparisons.  Easy enough to
avoid here.

Still, we don't actually test "rom_bar has not been set".  We test
"rom_bar has the default value".  Nothing stops a user from passing
rombar=0x to -device.  See my review of the next patch.


I followed addr and romsize properties that already use -1 as the 
default value. addr is int32_t, but romsize is uint32_t. I'll change 
romsize and rom_bar to use UINT32_MAX as the default value.


This changes the behavior of 0x, but that should be fine. Most 
people should only set 0 or 1. Maybe someone types wrongly and sets a 
number like 2 or 12, but nobody types 0x by mistake.

Re: [PATCH v6 12/15] hw/pci: Use -1 as a default value for rombar

[Akihiko Odaki]

On 2024/02/21 16:59, Markus Armbruster wrote:

Akihiko Odaki  writes:


Currently there is no way to distinguish the case that rombar is
explicitly specified as 1 and the case that rombar is not specified.

Set rombar -1 by default to distinguish these cases just as it is done
for addr and romsize. It was confirmed that changing the default value
to -1 will not change the behavior by looking at occurences of rom_bar.

$ git grep -w rom_bar
hw/display/qxl.c:328:QXLRom *rom = memory_region_get_ram_ptr(>rom_bar);
hw/display/qxl.c:431:qxl_set_dirty(>rom_bar, 0, qxl->rom_size);
hw/display/qxl.c:1048:QXLRom *rom = 
memory_region_get_ram_ptr(>rom_bar);
hw/display/qxl.c:2131:memory_region_init_rom(>rom_bar, OBJECT(qxl), 
"qxl.vrom",
hw/display/qxl.c:2154: PCI_BASE_ADDRESS_SPACE_MEMORY, >rom_bar);
hw/display/qxl.h:101:MemoryRegion   rom_bar;
hw/pci/pci.c:74:DEFINE_PROP_UINT32("rombar",  PCIDevice, rom_bar, 1),
hw/pci/pci.c:2329:if (!pdev->rom_bar) {
hw/vfio/pci.c:1019:if (vdev->pdev.romfile || !vdev->pdev.rom_bar) {
hw/xen/xen_pt_load_rom.c:29:if (dev->romfile || !dev->rom_bar) {
include/hw/pci/pci_device.h:150:uint32_t rom_bar;

rom_bar refers to a different variable in qxl. It is only tested if
the value is 0 or not in the other places.


Makes me wonder why it's uint32_t.  Not this patch's problem.


Indeed. It should have been OnOffAuto. I'm not changing it now though 
because I'm too worried if it is too disruptive.





Signed-off-by: Akihiko Odaki 
---
  hw/pci/pci.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 54b375da2d26..909c5b3ee4ee 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -71,7 +71,7 @@ static Property pci_props[] = {
  DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
  DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
  DEFINE_PROP_UINT32("romsize", PCIDevice, romsize, -1),
-DEFINE_PROP_UINT32("rombar",  PCIDevice, rom_bar, 1),
+DEFINE_PROP_UINT32("rombar",  PCIDevice, rom_bar, -1),
  DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
  QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
  DEFINE_PROP_BIT("x-pcie-lnksta-dllla", PCIDevice, cap_present,

Re: [PATCH 18/23] plugins: add an API to read registers

[Akihiko Odaki]

On 2024/02/21 23:14, Alex Bennée wrote:

Akihiko Odaki  writes:


On 2024/02/21 19:02, Alex Bennée wrote:

Akihiko Odaki  writes:


On 2024/02/20 23:14, Alex Bennée wrote:

Akihiko Odaki  writes:


On 2024/02/17 1:30, Alex Bennée wrote:

We can only request a list of registers once the vCPU has been
initialised so the user needs to use either call the get function on
vCPU initialisation or during the translation phase.
We don't expose the reg number to the plugin instead hiding it
behind
an opaque handle. This allows for a bit of future proofing should the
internals need to be changed while also being hashed against the
CPUClass so we can handle different register sets per-vCPU in
hetrogenous situations.
Having an internal state within the plugins also allows us to expand
the interface in future (for example providing callbacks on register
change if the translator can track changes).
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1706
Cc: Akihiko Odaki 
Message-Id: <20240103173349.398526-39-alex.ben...@linaro.org>
Based-on: <20231025093128.33116-18-akihiko.od...@daynix.com>
Signed-off-by: Alex Bennée 
Reviewed-by: Pierrick Bouvier 



+/*
+ * Register handles
+ *
+ * The plugin infrastructure keeps hold of these internal data
+ * structures which are presented to plugins as opaque handles. They
+ * are global to the system and therefor additions to the hash table
+ * must be protected by the @reg_handle_lock.
+ *
+ * In order to future proof for up-coming heterogeneous work we want
+ * different entries for each CPU type while sharing them in the
+ * common case of multiple cores of the same type.
+ */
+
+static QemuMutex reg_handle_lock;
+
+struct qemu_plugin_register {
+const char *name;
+int gdb_reg_num;
+};
+
+static GHashTable *reg_handles; /* hash table of PluginReg */
+
+/* Generate a stable key - would xxhash be overkill? */
+static gpointer cpu_plus_reg_to_key(CPUState *cs, int gdb_regnum)
+{
+uintptr_t key = (uintptr_t) cs->cc;
+key ^= gdb_regnum;
+return GUINT_TO_POINTER(key);
+}


I have pointed out this is theoretically prone to collisions and
unsafe.

How is it unsafe? The aim is to share handles for the same CPUClass
rather than having a unique handle per register/cpu combo.


THe intention is legitimate, but the implementation is not safe. It
assumes (uintptr)cs->cc ^ gdb_regnum is unique, but there is no such
guarantee. The key of GHashTable must be unique; generating hashes of
keys should be done with hash_func given to g_hash_table_new().

This isn't a hash its a non-unique key. It is however unique for
the same register on the same class of CPU so for each vCPU in a system
can share the same opaque handles.
The hashing is done internally by glib. We would assert if there was
a
duplicate key referring to a different register.
I'm unsure what you want here? Do you have a suggestion for the key
generation algorithm? As the comment notes I did consider a more complex
mixing algorithm using xxhash but that wouldn't guarantee no clash
either.


I suggest using a struct that holds both of cs->cc and gdb_regnum, and
pass g_direct_equal() and g_direct_hash() to g_hash_table_new().


We already do:

 if (!reg_handles) {
 reg_handles = g_hash_table_new(g_direct_hash, g_direct_equal);
 }

But we can't use g_direct_equal with something that exceeds the width of
gpointer as it is a straight equality test of the key. What you are
suggesting requires allocating memory for each key and de-referencing
with a custom GEqualFunc.


My bad. I wrongly remembered g_direct_equal() and g_direct_hash(). It 
indeed seems to need a more complicated solution.


It is possible to write a GEqualFunc and a GHashFunc that consumes a 
struct but it is a chore. How about having a two-level GHashTable? 
reg_handles will be a GHashTable keyed with cs->cc, and another 
GHashTable will be keyed with gdb_regnum.

Re: [PATCH] ui/cocoa: Fix incorrect window clipping on macOS Sonoma

[Michael Tokarev]

[Adding a few more Ccs]

17.02.2024 18:58, David Parsons :

macOS Sonoma changes the NSView.clipsToBounds to false by default where it was 
true in
earlier version of macOS. This causes the window contents to be obscured by the 
window
frame. This fixes the issue by conditionally setting the clipping on Sonoma to 
true.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1994
Signed-off-by: David Parsons 

diff --git a/ui/cocoa.m b/ui/cocoa.m
index eb99064bee..c9e3b96004 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -365,6 +365,9 @@ - (id)initWithFrame:(NSRect)frameRect
  screen.width = frameRect.size.width;
  screen.height = frameRect.size.height;
  kbd = qkbd_state_init(dcl.con);
+if (@available(macOS 14, *)) {
+[self setClipsToBounds:YES];
+}
  
  }

  return self;



Hi David!

While the code change is tiny, I for one know nothing about MacOS and
its cocoa thing, so to me (with my trivial-patches hat on) this is a
no-go.  I'd love to have a review from someone more knowlegeable in
this area.

Thanks,

/mjt

Re: [PATCH 0/6] [PATCH 0/6] Fix missing ERRP_GUARD() when dereference @errp

[Michael Tokarev]

21.02.2024 12:43, Zhao Liu wrote:

From: Zhao Liu 

Hi all,

Thanks to Markus's explanation about ERRP_GUARD() on my previsou
patch [1],

I realize that perhaps more @errp dereference cases need to be
double-checked to ensure that ERRP_GUARD() is being used correctly.

Therefore, there're the patches to add more missing ERRP_GUARD().

[1]: https://lore.kernel.org/qemu-devel/875xz0ojg7@pond.sub.org/


Hi!

Since you're going to respin (it looks like), please include the initial
patch (hw/intc one) into the series as well, so it's all in one go.

I'm not sure this should come via trivial-patches tree though.  Looks
trivial enough :)

/mjt

Re: Support Android hypervisors

[Trilok Soni]

On 2/21/2024 9:37 PM, RR NN wrote:
> Android Virtualization Framework (AVF) supports "KVM(pKVM)" also Qualcomm's
> "Gunyah" and MediaTek's "GenieZone" as the hypervisor. Please Add these
> hypervisors to QEMU.

I don't understand this comment. Do you want QEMU to work as VMM as well
for these Hypervisors? AVF works w/ CrosVM as VMM. 

Recently Vatsa had submitted RFC for supporting QEMU w/ Gunyah. You can check
that as an example. 

-- 
---Trilok Soni

Re: [PATCH v2 0/2] Update description for input grab key

[Michael Tokarev]

21.02.2024 22:52, Tianlan Zhou :

Input grab key should be Ctrl-Alt-g, not just Ctrl-Alt.

v2:
- Update help message in system/vl.c

v1:
- Initial patch

Tianlan Zhou (2):
   docs/system: Update description for input grab key
   system/vl: Update description for input grab key

  docs/system/keys.rst.inc | 2 +-
  system/vl.c  | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)


Applied to trivial-patches, thanks!

/mjt

Support Android hypervisors

[RR NN]

Android Virtualization Framework (AVF) supports "KVM(pKVM)" also Qualcomm's
"Gunyah" and MediaTek's "GenieZone" as the hypervisor. Please Add these
hypervisors to QEMU.

Re: [PATCH v2 1/2] docs/system: Update description for input grab key

[Thomas Huth]

On 21/02/2024 20.52, Tianlan Zhou wrote:

Input grab key should be Ctrl-Alt-g, not just Ctrl-Alt.

Signed-off-by: Tianlan Zhou 
---
  docs/system/keys.rst.inc | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/system/keys.rst.inc b/docs/system/keys.rst.inc
index 2e2c97aa23..59966a3fe7 100644
--- a/docs/system/keys.rst.inc
+++ b/docs/system/keys.rst.inc
@@ -29,7 +29,7 @@ Ctrl-Alt-n
 *3*
Serial port
  
-Ctrl-Alt

+Ctrl-Alt-g
 Toggle mouse and keyboard grab.
  
  In the virtual consoles, you can use Ctrl-Up, Ctrl-Down, Ctrl-PageUp and


Fixes: f8d2c9369b ("sdl: use ctrl-alt-g as grab hotkey")

Reviewed-by: Thomas Huth 

Re: [PATCH v2 2/2] system/vl: Update description for input grab key

[Thomas Huth]

On 21/02/2024 20.52, Tianlan Zhou wrote:

Input grab key should be Ctrl-Alt-g, not just Ctrl-Alt.

Signed-off-by: Tianlan Zhou 
---
  system/vl.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/system/vl.c b/system/vl.c
index a82555ae15..b8469d9965 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -891,7 +891,7 @@ static void help(int exitcode)
  printf("\nDuring emulation, the following keys are useful:\n"
 "ctrl-alt-f  toggle full screen\n"
 "ctrl-alt-n  switch to virtual console 'n'\n"
-   "ctrl-alttoggle mouse and keyboard grab\n"
+   "ctrl-alt-g  toggle mouse and keyboard grab\n"
 "\n"
 "When using -nographic, press 'ctrl-a h' to get some help.\n"
 "\n"


Fixes: f8d2c9369b ("sdl: use ctrl-alt-g as grab hotkey")

Reviewed-by: Thomas Huth 

Re: [RFC PATCH v2 05/22] target/arm: Support MSR access to ALLINT

[Jinjie Ruan via]

On 2024/2/22 3:28, Richard Henderson wrote:
> On 2/21/24 03:08, Jinjie Ruan via wrote:
>> Support ALLINT msr access as follow:
>> mrs , ALLINT    // read allint
>> msr ALLINT,     // write allint with imm
>>
>> Signed-off-by: Jinjie Ruan 
>> ---
>>   target/arm/helper.c | 32 
>>   1 file changed, 32 insertions(+)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index a3062cb2ad..211156d640 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -4618,6 +4618,31 @@ static void aa64_daif_write(CPUARMState *env,
>> const ARMCPRegInfo *ri,
>>   env->daif = value & PSTATE_DAIF;
>>   }
>>   +static void aa64_allint_write(CPUARMState *env, const ARMCPRegInfo
>> *ri,
>> +  uint64_t value)
>> +{
>> +    env->allint = value & PSTATE_ALLINT;
>> +}
>> +
>> +static uint64_t aa64_allint_read(CPUARMState *env, const ARMCPRegInfo
>> *ri)
>> +{
>> +    return env->allint & PSTATE_ALLINT;
>> +}
>> +
>> +static CPAccessResult aa64_allint_access(CPUARMState *env,
>> + const ARMCPRegInfo *ri, bool
>> isread)
>> +{
>> +    if (arm_current_el(env) == 0) {
>> +    return CP_ACCESS_TRAP_UNCATEGORIZED;
>> +    }
> 
> This is handled by .access PL1_RW.
> 
>> +
>> +    if (arm_current_el(env) == 1 && arm_is_el2_enabled(env) &&
>> +    cpu_isar_feature(aa64_hcx, env_archcpu(env)) &&
>> +    (env->cp15.hcrx_el2 & HCRX_TALLINT))
>> +    return CP_ACCESS_TRAP_EL2;
> 
> You should be using arm_hcrx_el2_eff(env).
> Missing braces.

I'll fix it, thank you!

> 
>> @@ -5437,6 +5462,13 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
>>     .access = PL0_RW, .accessfn = aa64_daif_access,
>>     .fieldoffset = offsetof(CPUARMState, daif),
>>     .writefn = aa64_daif_write, .resetfn = arm_cp_reset_ignore },
>> +    { .name = "ALLINT", .state = ARM_CP_STATE_AA64,
>> +  .opc0 = 3, .opc1 = 0, .opc2 = 0, .crn = 4, .crm = 3,
>> +  .type = ARM_CP_NO_RAW,
>> +  .access = PL1_RW, .accessfn = aa64_allint_access,
>> +  .fieldoffset = offsetof(CPUARMState, allint),
>> +  .writefn = aa64_allint_write, .readfn = aa64_allint_read,
>> +  .resetfn = arm_cp_reset_ignore },
> 
> You cannot add ALLINT here in v8_cp_reginfo[].
> Compare fgt_reginfo[], and how it is registered.

I'll fix it, thank you!

> 
> 
> r~

[QEMU PATCH v6 0/1] S3 support

[Jiqian Chen]

Hi all,
This is the v6 patch to support S3.
In current code, when guest does S3, virtio devices are reset during that
process, that causes the display resources of virtio-gpu are destroyed,
then the display can't come back after resuming.
This v6 patch implement the No_Soft_Reset bit of PCI_PM_CTRL register,
when this bit is set, the resetting will not be done, so that the display
can work after resuming.
This version abandons all previous version implementations and is a new
different solution according to the outcome of the discussion and
suggestions in the mailing thread of virtio-spec.
(https://lists.oasis-open.org/archives/virtio-comment/202401/msg00077.html)

Best regards,
Jiqian Chen

V5:
Hi all,
v5 makes below changes:
* Since this series patches add a new mechanism that let virtgpu and Qemu can 
negotiate
  their reset behavior, and other guys hope me can improve this mechanism to 
virtio pci
  level, so that other virtio devices can also benefit from it. So instead of 
adding
  new feature flag VIRTIO_GPU_F_FREEZE_S3 only serves for virtgpu, v5 add a new 
parameter
  named freeze_mode to struct VirtIODevice, when guest begin suspending, set 
freeze_mode
  to VIRTIO_PCI_FREEZE_MODE_FREEZE_S3, and then all virtio devices can get this 
status,
  and notice that guest is suspending, then they can change their reset 
behavior . See
  the new commit "virtio-pci: Add freeze_mode case for virtio pci"
* The second commit is just for virtgpu, when freeze_mode is 
VIRTIO_PCI_FREEZE_MODE_FREEZE_S3,
  prevent Qemu destroying render resources, so that the display can come back 
after resuming.

V5 of kernel patch:
https://lore.kernel.org/lkml/20230919104607.2282248-1-jiqian.c...@amd.com/T/#t

The link to trace this issue:
https://gitlab.com/qemu-project/qemu/-/issues/1860

v4:
Hi all,
Thanks for Gerd Hoffmann's advice. V4 makes below changes:
* Use enum for freeze mode, so this can be extended with more
  modes in the future.
* Rename functions and paratemers with "_S3" postfix.
And no functional changes.
Link:
https://lore.kernel.org/qemu-devel/20230720120816.8751-1-jiqian.c...@amd.com/
No v4 patch on kernel side.


v3:
Hi all,
Thanks for Michael S. Tsirkin's advice. V3 makes below changes:
* Remove changes in file include/standard-headers/linux/virtio_gpu.h
  I am not supposed to edit this file and it will be imported after
  the patches of linux kernel was merged.
Link:
https://lore.kernel.org/qemu-devel/20230719074726.1613088-1-jiqian.c...@amd.com/T/#t
V3 of kernel patch:
https://lore.kernel.org/lkml/20230720115805.8206-1-jiqian.c...@amd.com/T/#t


v2:
makes below changes:
* Change VIRTIO_CPU_CMD_STATUS_FREEZING to 0x0400 (<0x1000)
* Add virtio_gpu_device_unrealize to destroy resources to solve
  potential memory leak problem. This also needs hot-plug support.
* Add a new feature flag VIRTIO_GPU_F_FREEZING, so that guest and
  host can negotiate whenever freezing is supported or not.
Link:
https://lore.kernel.org/qemu-devel/20230630070016.841459-1-jiqian.c...@amd.com/T/#t
V2 of kernel patch:
https://lore.kernel.org/lkml/20230630073448.842767-1-jiqian.c...@amd.com/T/#t


v1:
Hi all,
I am working to implement virtgpu S3 function on Xen.

Currently on Xen, if we start a guest who enables virtgpu, and then run "echo 
mem > /sys/power/state" to suspend guest. And run "sudo xl trigger  
s3resume" to resume guest. We can find that the guest kernel comes back, but 
the display doesn't.
It just shown a black screen.

Through reading codes, I founded that when guest was during suspending, it 
called into Qemu to call virtio_gpu_gl_reset. In virtio_gpu_gl_reset, it 
destroyed all resources and reset renderer. This made the display gone after 
guest resumed.

I think we should keep resources or prevent they being destroyed when guest is 
suspending. So, I add a new status named freezing to virtgpu, and add a new 
ctrl message VIRTIO_GPU_CMD_STATUS_FREEZING to get notification from guest. If 
freezing is set to true, and then Qemu will realize that guest is suspending, 
it will not destroy resources and will not reset renderer. If freezing is set 
to false, Qemu will do its origin actions, and has no other impaction.

And now, display can come back and applications can continue their status after 
guest resumes.
Link:
https://lore.kernel.org/qemu-devel/20230608025655.1674357-1-jiqian.c...@amd.com/
V1 of kernel patch:
https://lore.kernel.org/lkml/20230608063857.1677973-1-jiqian.c...@amd.com/


Jiqian Chen (1):
  virtio-pci: implement No_Soft_Reset bit

 hw/virtio/virtio-pci.c | 37 +-
 include/hw/virtio/virtio-pci.h |  5 +
 2 files changed, 41 insertions(+), 1 deletion(-)

-- 
2.34.1

[QEMU PATCH v6 1/1] virtio-pci: implement No_Soft_Reset bit

[Jiqian Chen]

In current code, when guest does S3, virtio devices are reset due to
the bit No_Soft_Reset is not set. After resetting, the display resources
of virtio-gpu are destroyed, then the display can't come back and only
show blank after resuming.

Implement No_Soft_Reset bit of PCI_PM_CTRL register, then guest can check
this bit, if this bit is set, the devices resetting will not be done, and
then the display can work after resuming.

Signed-off-by: Jiqian Chen 
---
 hw/virtio/virtio-pci.c | 37 +-
 include/hw/virtio/virtio-pci.h |  5 +
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 1a7039fb0c68..da5312010345 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2197,6 +2197,11 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error 
**errp)
 pcie_cap_lnkctl_init(pci_dev);
 }
 
+if (proxy->flags & VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET) {
+pci_set_word(pci_dev->config + pos + PCI_PM_CTRL,
+ PCI_PM_CTRL_NO_SOFT_RESET);
+}
+
 if (proxy->flags & VIRTIO_PCI_FLAG_INIT_PM) {
 /* Init Power Management Control Register */
 pci_set_word(pci_dev->wmask + pos + PCI_PM_CTRL,
@@ -2259,18 +2264,46 @@ static void virtio_pci_reset(DeviceState *qdev)
 }
 }
 
+static bool device_no_need_reset(PCIDevice *dev)
+{
+if (pci_is_express(dev)) {
+uint16_t pmcsr;
+
+pmcsr = pci_get_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL);
+/*
+ * When No_Soft_Reset bit is set and device
+ * is in D3hot state, can't reset device
+ */
+if ((pmcsr & PCI_PM_CTRL_NO_SOFT_RESET) &&
+(pmcsr & PCI_PM_CTRL_STATE_MASK) == 3)
+return true;
+}
+
+return false;
+}
+
 static void virtio_pci_bus_reset_hold(Object *obj)
 {
 PCIDevice *dev = PCI_DEVICE(obj);
 DeviceState *qdev = DEVICE(obj);
 
+if (device_no_need_reset(dev))
+return;
+
 virtio_pci_reset(qdev);
 
 if (pci_is_express(dev)) {
+uint16_t pmcsr;
+uint16_t val = 0;
+
 pcie_cap_deverr_reset(dev);
 pcie_cap_lnkctl_reset(dev);
 
-pci_set_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL, 0);
+/* don't reset the RO bits */
+pmcsr = pci_get_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL);
+val = val | (pmcsr & PCI_PM_CTRL_NO_SOFT_RESET) |
+(pmcsr & PCI_PM_CTRL_DATA_SCALE_MASK);
+pci_set_word(dev->config + dev->exp.pm_cap + PCI_PM_CTRL, val);
 }
 }
 
@@ -2297,6 +2330,8 @@ static Property virtio_pci_properties[] = {
 VIRTIO_PCI_FLAG_INIT_LNKCTL_BIT, true),
 DEFINE_PROP_BIT("x-pcie-pm-init", VirtIOPCIProxy, flags,
 VIRTIO_PCI_FLAG_INIT_PM_BIT, true),
+DEFINE_PROP_BIT("x-pcie-pm-no-soft-reset", VirtIOPCIProxy, flags,
+VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET_BIT, true),
 DEFINE_PROP_BIT("x-pcie-flr-init", VirtIOPCIProxy, flags,
 VIRTIO_PCI_FLAG_INIT_FLR_BIT, true),
 DEFINE_PROP_BIT("aer", VirtIOPCIProxy, flags,
diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 59d88018c16a..9e67ba38c748 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -43,6 +43,7 @@ enum {
 VIRTIO_PCI_FLAG_INIT_FLR_BIT,
 VIRTIO_PCI_FLAG_AER_BIT,
 VIRTIO_PCI_FLAG_ATS_PAGE_ALIGNED_BIT,
+VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET_BIT,
 };
 
 /* Need to activate work-arounds for buggy guests at vmstate load. */
@@ -79,6 +80,10 @@ enum {
 /* Init Power Management */
 #define VIRTIO_PCI_FLAG_INIT_PM (1 << VIRTIO_PCI_FLAG_INIT_PM_BIT)
 
+/* Init The No_Soft_Reset bit of Power Management */
+#define VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET \
+  (1 << VIRTIO_PCI_FLAG_PM_NO_SOFT_RESET_BIT)
+
 /* Init Function Level Reset capability */
 #define VIRTIO_PCI_FLAG_INIT_FLR (1 << VIRTIO_PCI_FLAG_INIT_FLR_BIT)
 
-- 
2.34.1

Re: [PATCH] qga-win: Add support of Windows Server 2025 in get-osinfo command

[Dehan Meng]

Done. thanks.

On Wed, Feb 21, 2024 at 6:00 PM Konstantin Kostiuk 
wrote:

>
>
> On Wed, Feb 21, 2024 at 11:51 AM Dehan Meng  wrote:
>
>> Add support of Windows Server 2025 in get-osinfo command
>>
>> Signed-off-by: Dehan Meng 
>> ---
>>  qga/commands-win32.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
>> index 697c65507c..f3c7e604c9 100644
>> --- a/qga/commands-win32.c
>> +++ b/qga/commands-win32.c
>> @@ -2154,6 +2154,7 @@ static ga_win_10_0_t const
>> WIN_10_0_SERVER_VERSION_MATRIX[4] = {
>>
>
> You don't update the array size, there are out of range elements
>
>
>>  {14393, "Microsoft Windows Server 2016","2016"},
>>  {17763, "Microsoft Windows Server 2019","2019"},
>>  {20344, "Microsoft Windows Server 2022","2022"},
>> +{26040, "MIcrosoft Windows Server 2025","2025"},
>>  {0, 0}
>>  };
>>
>> --
>> 2.35.1
>>
>>

[PATCH v2 0/1] update the array size

[Dehan Meng]

v1 -> v2
update the array size "WIN_10_0_SERVER_VERSION_MATRIX" in case 
array out of range elements.

Dehan Meng (1):
  qga-win: Add support of Windows Server 2025 in get-osinfo command

 qga/commands-win32.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.35.1

[PATCH v2 1/1] qga-win: Add support of Windows Server 2025 in get-osinfo command

[Dehan Meng]

Add support of Windows Server 2025 in get-osinfo command

Signed-off-by: Dehan Meng 
---
 qga/commands-win32.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 697c65507c..b37fa7b5ba 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2150,10 +2150,11 @@ typedef struct _ga_win_10_0_t {
 char const *version_id;
 } ga_win_10_0_t;
 
-static ga_win_10_0_t const WIN_10_0_SERVER_VERSION_MATRIX[4] = {
+static ga_win_10_0_t const WIN_10_0_SERVER_VERSION_MATRIX[5] = {
 {14393, "Microsoft Windows Server 2016","2016"},
 {17763, "Microsoft Windows Server 2019","2019"},
 {20344, "Microsoft Windows Server 2022","2022"},
+{26040, "MIcrosoft Windows Server 2025","2025"},
 {0, 0}
 };
 
-- 
2.35.1

[PATCH v2 1/1] qga-win: Add support of Windows Server 2025 in get-osinfo command

[Dehan Meng]

Add support of Windows Server 2025 in get-osinfo command

Signed-off-by: Dehan Meng 
---
 qga/commands-win32.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 697c65507c..b37fa7b5ba 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2150,10 +2150,11 @@ typedef struct _ga_win_10_0_t {
 char const *version_id;
 } ga_win_10_0_t;
 
-static ga_win_10_0_t const WIN_10_0_SERVER_VERSION_MATRIX[4] = {
+static ga_win_10_0_t const WIN_10_0_SERVER_VERSION_MATRIX[5] = {
 {14393, "Microsoft Windows Server 2016","2016"},
 {17763, "Microsoft Windows Server 2019","2019"},
 {20344, "Microsoft Windows Server 2022","2022"},
+{26040, "MIcrosoft Windows Server 2025","2025"},
 {0, 0}
 };
 
-- 
2.35.1

[PATCH v2 0/1] update the array size

[Dehan Meng]

v1 -> v2
update the array size "WIN_10_0_SERVER_VERSION_MATRIX" in case 
array out of range elements.

Dehan Meng (1):
  qga-win: Add support of Windows Server 2025 in get-osinfo command

 qga/commands-win32.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.35.1

Re: [RFC PATCH v2 06/22] target/arm: Add support for Non-maskable Interrupt

[Jinjie Ruan via]

On 2024/2/22 4:06, Richard Henderson wrote:
> On 2/21/24 03:08, Jinjie Ruan via wrote:
>> This only implements the external delivery method via the GICv3.
>>
>> Signed-off-by: Jinjie Ruan 
>> ---
>>   target/arm/cpu-qom.h |  3 ++-
>>   target/arm/cpu.c | 39 ++-
>>   target/arm/cpu.h |  2 ++
>>   target/arm/helper.c  |  1 +
>>   4 files changed, 39 insertions(+), 6 deletions(-)
>>
>> diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
>> index 8e032691db..66d555a605 100644
>> --- a/target/arm/cpu-qom.h
>> +++ b/target/arm/cpu-qom.h
>> @@ -36,11 +36,12 @@ DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU,
>>   #define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU
>>   #define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX)
>>   -/* Meanings of the ARMCPU object's four inbound GPIO lines */
>> +/* Meanings of the ARMCPU object's five inbound GPIO lines */
>>   #define ARM_CPU_IRQ 0
>>   #define ARM_CPU_FIQ 1
>>   #define ARM_CPU_VIRQ 2
>>   #define ARM_CPU_VFIQ 3
>> +#define ARM_CPU_NMI 4
>>     /* For M profile, some registers are banked secure vs non-secure;
>>    * these are represented as a 2-element array where the first element
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index 5e5978c302..055670343e 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -128,7 +128,7 @@ static bool arm_cpu_has_work(CPUState *cs)
>>     return (cpu->power_state != PSCI_OFF)
>>   && cs->interrupt_request &
>> -    (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD
>> +    (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI
>>    | CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ |
>> CPU_INTERRUPT_VSERR
>>    | CPU_INTERRUPT_EXITTB);
>>   }
> 
> I think you should not include CPU_INTERRUPT_NMI when it cannot be
> delivered, e.g. FEAT_NMI not enabled.

OK! I'll fix it.
> 
> 
>> @@ -668,6 +668,7 @@ static inline bool arm_excp_unmasked(CPUState *cs,
>> unsigned int excp_idx,
>>   CPUARMState *env = cpu_env(cs);
>>   bool pstate_unmasked;
>>   bool unmasked = false;
>> +    bool nmi_unmasked = false;
>>     /*
>>    * Don't take exceptions if they target a lower EL.
>> @@ -678,13 +679,29 @@ static inline bool arm_excp_unmasked(CPUState
>> *cs, unsigned int excp_idx,
>>   return false;
>>   }
>>   +    nmi_unmasked = (!(env->allint & PSTATE_ALLINT)) &
>> +   (!((env->cp15.sctlr_el[target_el] &
>> SCTLR_SPINTMASK) &&
>> +   (env->pstate & PSTATE_SP) && cur_el == target_el));
> 
> I don't see SCTLR_ELx.NMI being tested anywhere, which is required to
> enable everything else.

OK! I'll fix it.

> 
>>   case EXCP_FIQ:
>> -    pstate_unmasked = !(env->daif & PSTATE_F);
>> +    if (cpu_isar_feature(aa64_nmi, env_archcpu(env))) {
>> +    pstate_unmasked = (!(env->daif & PSTATE_F)) & nmi_unmasked;
>> +    } else {
>> +    pstate_unmasked = !(env->daif & PSTATE_F);
>> +    }
>>   break;
>>     case EXCP_IRQ:
>> -    pstate_unmasked = !(env->daif & PSTATE_I);
>> +    if (cpu_isar_feature(aa64_nmi, env_archcpu(env))) {
>> +    pstate_unmasked = (!(env->daif & PSTATE_I)) & nmi_unmasked;
>> +    } else {
>> +    pstate_unmasked = !(env->daif & PSTATE_I);
>> +    }
>>   break;
> 
> I don't see what this is doing.  While Superpriority is IMPLEMENTATION
> DEFINED, how are you defining it for QEMU?  Is there a definition from
> real hw which makes sense under emulation?

These code add allint mask for IRQ or FIQ,since ALLINT also mask them.

The superpriority is DEFINED in  gicv3 code and it is a NMI exception
for PE.

> 
> 
> r~

Re: [RFC PATCH v2 04/22] target/arm: Implement ALLINT MSR (immediate)

[Jinjie Ruan via]

On 2024/2/22 4:41, Richard Henderson wrote:
> On 2/21/24 09:09, Richard Henderson wrote:
>> On 2/21/24 03:08, Jinjie Ruan via wrote:
>>> Add ALLINT MSR (immediate) to decodetree. And the EL0 check is necessary
>>> to ALLINT. Avoid the unconditional write to pc and use
>>> raise_exception_ra
>>> to unwind.
>>>
>>> Signed-off-by: Jinjie Ruan 
>>> ---
>>>   target/arm/tcg/a64.decode  |  1 +
>>>   target/arm/tcg/helper-a64.c    | 24 
>>>   target/arm/tcg/helper-a64.h    |  1 +
>>>   target/arm/tcg/translate-a64.c | 10 ++
>>>   4 files changed, 36 insertions(+)
>>>
>>> diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
>>> index 8a20dce3c8..3588080024 100644
>>> --- a/target/arm/tcg/a64.decode
>>> +++ b/target/arm/tcg/a64.decode
>>> @@ -207,6 +207,7 @@ MSR_i_DIT   1101 0101  0 011 0100 
>>> 010 1 @msr_i
>>>   MSR_i_TCO   1101 0101  0 011 0100  100 1 @msr_i
>>>   MSR_i_DAIFSET   1101 0101  0 011 0100  110 1 @msr_i
>>>   MSR_i_DAIFCLEAR 1101 0101  0 011 0100  111 1 @msr_i
>>> +MSR_i_ALLINT    1101 0101  0 001 0100  000 1 @msr_i
>>>   MSR_i_SVCR  1101 0101  0 011 0100 0 mask:2 imm:1 011 1
>>>   # MRS, MSR (register), SYS, SYSL. These are all essentially the
>>> diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
>>> index ebaa7f00df..3686926ada 100644
>>> --- a/target/arm/tcg/helper-a64.c
>>> +++ b/target/arm/tcg/helper-a64.c
>>> @@ -66,6 +66,30 @@ void HELPER(msr_i_spsel)(CPUARMState *env,
>>> uint32_t imm)
>>>   update_spsel(env, imm);
>>>   }
>>> +static void allint_check(CPUARMState *env, uint32_t op,
>>> +   uint32_t imm, uintptr_t ra)
>>> +{
>>> +    /* ALLINT update to PSTATE. */
>>> +    if (arm_current_el(env) == 0) {
>>> +    raise_exception_ra(env, EXCP_UDEF,
>>> +   syn_aa64_sysregtrap(0, extract32(op, 0, 3),
>>> +   extract32(op, 3, 3), 4,
>>> +   imm, 0x1f, 0),
>>> +   exception_target_el(env), ra);
>>> +    }
>>> +}
>>
>> A runtime check for EL0 is not necessary; you've already handled that
>> in trans_MSR_i_ALLINT().  However, what *is* missing here is the test
>> against TALLINT for EL1.
>>
>>> +
>>> +void HELPER(msr_i_allint)(CPUARMState *env, uint32_t imm)
>>> +{
>>> +    allint_check(env, 0x8, imm, GETPC());
>>> +    if (imm == 1) {
>>> +    env->allint |= PSTATE_ALLINT;
>>> +    } else {
>>> +    env->allint &= ~PSTATE_ALLINT;
>>> +    }
>>
>> I think you should not write an immediate-specific helper, but one
>> which can also handle the variable "MSR allint, ".  This is no
>> more difficult than
>>
>> void HELPER(msr_allint)(CPUARMState *env, target_ulong val)
>> {
>>  ... check ...
>>  env->pstate = (env->pstate & ~PSTATE_ALLINT) | (val &
>> PSTATE_ALLINT);
>> }
> 
> Ho hum..  I just noticed that TALLINT only traps immediate write of 1,
> not also immediate write of 0.  So one helper for both MSR Xt and MSR
> imm is not practical.

it is a real problem.

> 
> 
> r~

Re: [RFC PATCH v2 04/22] target/arm: Implement ALLINT MSR (immediate)

[Jinjie Ruan via]

On 2024/2/22 3:09, Richard Henderson wrote:
> On 2/21/24 03:08, Jinjie Ruan via wrote:
>> Add ALLINT MSR (immediate) to decodetree. And the EL0 check is necessary
>> to ALLINT. Avoid the unconditional write to pc and use raise_exception_ra
>> to unwind.
>>
>> Signed-off-by: Jinjie Ruan 
>> ---
>>   target/arm/tcg/a64.decode  |  1 +
>>   target/arm/tcg/helper-a64.c    | 24 
>>   target/arm/tcg/helper-a64.h    |  1 +
>>   target/arm/tcg/translate-a64.c | 10 ++
>>   4 files changed, 36 insertions(+)
>>
>> diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
>> index 8a20dce3c8..3588080024 100644
>> --- a/target/arm/tcg/a64.decode
>> +++ b/target/arm/tcg/a64.decode
>> @@ -207,6 +207,7 @@ MSR_i_DIT   1101 0101  0 011 0100  010
>> 1 @msr_i
>>   MSR_i_TCO   1101 0101  0 011 0100  100 1 @msr_i
>>   MSR_i_DAIFSET   1101 0101  0 011 0100  110 1 @msr_i
>>   MSR_i_DAIFCLEAR 1101 0101  0 011 0100  111 1 @msr_i
>> +MSR_i_ALLINT    1101 0101  0 001 0100  000 1 @msr_i
>>   MSR_i_SVCR  1101 0101  0 011 0100 0 mask:2 imm:1 011 1
>>     # MRS, MSR (register), SYS, SYSL. These are all essentially the
>> diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
>> index ebaa7f00df..3686926ada 100644
>> --- a/target/arm/tcg/helper-a64.c
>> +++ b/target/arm/tcg/helper-a64.c
>> @@ -66,6 +66,30 @@ void HELPER(msr_i_spsel)(CPUARMState *env, uint32_t
>> imm)
>>   update_spsel(env, imm);
>>   }
>>   +static void allint_check(CPUARMState *env, uint32_t op,
>> +   uint32_t imm, uintptr_t ra)
>> +{
>> +    /* ALLINT update to PSTATE. */
>> +    if (arm_current_el(env) == 0) {
>> +    raise_exception_ra(env, EXCP_UDEF,
>> +   syn_aa64_sysregtrap(0, extract32(op, 0, 3),
>> +   extract32(op, 3, 3), 4,
>> +   imm, 0x1f, 0),
>> +   exception_target_el(env), ra);
>> +    }
>> +}
> 
> A runtime check for EL0 is not necessary; you've already handled that in
> trans_MSR_i_ALLINT().  However, what *is* missing here is the test
> against TALLINT for EL1.

Thank you! I'll fix it.

> 
>> +
>> +void HELPER(msr_i_allint)(CPUARMState *env, uint32_t imm)
>> +{
>> +    allint_check(env, 0x8, imm, GETPC());
>> +    if (imm == 1) {
>> +    env->allint |= PSTATE_ALLINT;
>> +    } else {
>> +    env->allint &= ~PSTATE_ALLINT;
>> +    }
> 
> I think you should not write an immediate-specific helper, but one which
> can also handle the variable "MSR allint, ".  This is no more
> difficult than

The kernel patches now uses the immediate-specific helper only, so it is
necessary for test.

> 
> void HELPER(msr_allint)(CPUARMState *env, target_ulong val)
> {
>     ... check ...
>     env->pstate = (env->pstate & ~PSTATE_ALLINT) | (val & PSTATE_ALLINT);
> }
> 
>> +    arm_rebuild_hflags(env);
>> +}
> 
> allint does not affect hflags; no rebuild required.

Thank you! I'll fix it.

> 
>> +static bool trans_MSR_i_ALLINT(DisasContext *s, arg_i *a)
>> +{
>> +    if (!dc_isar_feature(aa64_nmi, s) || s->current_el == 0) {
>> +    return false;
>> +    }
>> +    gen_helper_msr_i_allint(tcg_env, tcg_constant_i32(a->imm));
> 
> You're passing all of #imm4, not #imm1, which meant the test in your
> msr_i_allint helper was wrong.

In fact, it's either 0 or 1 for CRm according to the arm manual, so it
is not necessary.

> 
> To work with the generalized helper above, this would be
> 
>     tcg_constant_tl((a->imm & 1) * PSTATE_ALLINT);
> 
> 
> r~

Re: [RFC PATCH 1/5] cxl/core: correct length of DPA field masks

[Dan Williams]

[ add Ira and Davidlohr ]

Shiyang Ruan wrote:
> 
> 
> 在 2024/2/10 14:34, Dan Williams 写道:
> > Shiyang Ruan wrote:
> >> The length of Physical Address in General Media Event Record/DRAM Event
> >> Record is 64-bit, so the field mask should be defined as such length.
> > 
> > Can you include this user visible side-effect of this change. Looks like
> > this could cause usages of CXL_DPA_FLAGS_MASK to return an incorrect
> > result?
> 
> Ok.  Will change it to this:
> 
> The length of Physical Address in General Media Event Record/DRAM Event 
> Record is 64bit, per CXL Spec r3.0 - 8.2.9.2.1.1, Table 8-43.  Currently 
> CXL_DPA_FLAGS_MASK is defined as int (32bit), then CXL_DPA_MASK is a int 
> too, it will be 0xFFC0 while using "->dpa & CXL_DPA_MASK" to 
> obtain real physical address (to drop flags in lower bits), in this case 
> the higher 32bit of ->dpa will be lost.
> 
> To avoid this, define CXL_DPA_FLAGS_MASK as 64bit: 0x3FULL.

That part was clear, what is missing is the impact. For example, this
bug only impacts the output of the dpa field in the cxl_general_media
and cxl_dram tracepoints, but no other part of the CXL code.

So in this case the impact of the bug is low, but in the worst case an
wrong size mask could compromise the security / integrity of the system.

So I would expect a changelog like this to make the importance of the
fix clear and to notify the original stakeholders where the bug was
introduced.

---
The length of Physical Address in General Media Event Record/DRAM Event
Record is 64-bit, so the field mask should be defined as such length.
Otherwise, this causes cxl_general_media and cxl_dram tracepoints to
mask off the upper-32-bits of DPA addresses. The cxl_poison event is
unaffected.

If userspace was doing its own DPA-to-HPA translation this could lead to
incorrect page retirement decisions, but there is no known consumer
(like rasdaemon) of this event today.

Fixes: d54a531a430b ("cxl/mem: Trace General Media Event Record")
Cc: 
Cc: Dan Williams 
Cc: Davidlohr Bueso 
Cc: Jonathan Cameron 
Cc: Ira Weiny 
---

Re: [PATCH v4 0/2] Add support for LAM in QEMU

[Binbin Wu]

Ping...

Hi Paolo,
do you have time to have a look at this patchset?


On 1/22/2024 4:55 PM, Binbin Wu wrote:

Gentle ping...
Please help to review and consider applying the patch series. (The KVM
part has been merged).


On 1/12/2024 2:00 PM, Binbin Wu wrote:
Linear-address masking (LAM) [1], modifies the checking that is 
applied to

*64-bit* linear addresses, allowing software to use of the untranslated
address bits for metadata and masks the metadata bits before using 
them as

linear addresses to access memory.

When the feature is virtualized and exposed to guest, it can be used for
efficient
address sanitizers (ASAN) implementation and for optimizations in 
JITs and

virtual machines.

The KVM patch series can be found in [2].

[1] Intel ISE https://cdrdv2.intel.com/v1/dl/getContent/671368
 Chapter Linear Address Masking (LAM)
[2] 
https://lore.kernel.org/kvm/20230913124227.12574-1-binbin...@linux.intel.com


---
Changelog
v4:
- Add a reviewed-by from Xiaoyao for patch 1.
- Mask out LAM bit on CR4 if vcpu doesn't support LAM in 
cpu_x86_update_cr4() (Xiaoyao)


v3:
- https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg04160.html

Binbin Wu (1):
   target/i386: add control bits support for LAM

Robert Hoo (1):
   target/i386: add support for LAM in CPUID enumeration

  target/i386/cpu.c    | 2 +-
  target/i386/cpu.h    | 9 -
  target/i386/helper.c | 4 
  3 files changed, 13 insertions(+), 2 deletions(-)


base-commit: f614acb7450282a119d85d759f27eae190476058

RE: [PATCH 4/4] target/riscv: Apply modularized matching conditions for icount trigger

[張哲嘉]

Hi Daniel,

> -Original Message-
> From: Daniel Henrique Barboza 
> Sent: Thursday, February 22, 2024 2:06 AM
> To: Alvin Che-Chia Chang(張哲嘉) ;
> qemu-ri...@nongnu.org; qemu-devel@nongnu.org
> Cc: alistair.fran...@wdc.com; bin.m...@windriver.com;
> liwei1...@gmail.com; zhiwei_...@linux.alibaba.com
> Subject: Re: [PATCH 4/4] target/riscv: Apply modularized matching conditions
> for icount trigger
> 
> 
> 
> On 2/19/24 00:25, Alvin Chang wrote:
> > We have implemented trigger_common_match(), which checks if the
> > enabled privilege levels of the trigger match CPU's current privilege
> > level. We can invoke trigger_common_match() to check the privilege
> > levels of the type 3 triggers.
> >
> > Signed-off-by: Alvin Chang 
> > ---
> >   target/riscv/debug.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/riscv/debug.c b/target/riscv/debug.c index
> > 67ba19c966..de996a393c 100644
> > --- a/target/riscv/debug.c
> > +++ b/target/riscv/debug.c
> > @@ -624,7 +624,7 @@ void helper_itrigger_match(CPURISCVState *env)
> >   if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
> >   continue;
> >   }
> > -if (check_itrigger_priv(env, i)) {
> > +if (!trigger_common_match(env, TRIGGER_TYPE_INST_CNT, i)) {
> >   continue;
> >   }
> 
> 
> Looks good. Shouldn't we also change riscv_itrigger_enabled() to also use
> trigger_common_match()? riscv_itrigger_enabled() is remarkably similar to
> helper_itrigger_match() so I believe we can also use the new function there.

I think we might not want to apply trigger_common_match() into 
riscv_itrigger_enabled().
The trigger_common_match() is used to check if the trigger can be matched in 
current privilege level.
It will check many conditions: trigger privilege levels, textra, tcontrol, etc.
The riscv_itrigger_enabled() is used to check if any icount trigger is enabled 
by checking vs/vu/count/s/u fields of tdata1 only.

In fact, we found the comparisons between tdata1 bit-fields and env->priv in 
check_itrigger_priv() are bugs.
And we have a patch to fix that:

bool riscv_itrigger_enabled(CPURISCVState *env)
{
int count;
for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
continue;
}
-if (check_itrigger_priv(env, i)) {
+if ((env->tdata1[i] & ITRIGGER_VS) == 0 &&
+(env->tdata1[i] & ITRIGGER_VU) == 0 &&
+(env->tdata1[i] & ITRIGGER_U)  == 0 &&
+(env->tdata1[i] & ITRIGGER_S)  == 0 &&
+(env->tdata1[i] & ITRIGGER_M)  == 0 ) {
continue;
}
count = itrigger_get_count(env, i);
if (!count) {
continue;
}
return true;
}

return false;
}


Sincerely,
Alvin Chang

> 
> 
> Thanks,
> 
> Daniel
> 
> 
> 
> 
> 
> >   count = itrigger_get_count(env, i);

Re: [PATCH 5/5] tests: Add migration test for loongarch64

[maobibo]

On 2024/2/22 上午5:24, Fabiano Rosas wrote:

Bibo Mao  writes:


This patch adds migration test support for loongarch64. The test code
comes from aarch64 mostly, only that it it booted as bios in qemu since
kernel requires elf format and bios uses binary format.

In addition to providing the binary, this patch also includes the source
code and the build script in tests/migration/loongarch64. So users can
change the source and/or re-compile the binary as they wish.

Signed-off-by: Bibo Mao 


Just a nit below.

Reviewed-by: Fabiano Rosas 


---
  tests/migration/Makefile |  2 +-
  tests/migration/loongarch64/Makefile | 18 ++
  tests/migration/loongarch64/a-b-kernel.S | 46 
  tests/migration/loongarch64/a-b-kernel.h | 13 +++
  tests/migration/migration-test.h |  3 ++
  tests/qtest/meson.build  |  4 +++
  tests/qtest/migration-test.c | 10 ++
  7 files changed, 95 insertions(+), 1 deletion(-)
  create mode 100644 tests/migration/loongarch64/Makefile
  create mode 100644 tests/migration/loongarch64/a-b-kernel.S
  create mode 100644 tests/migration/loongarch64/a-b-kernel.h

diff --git a/tests/migration/Makefile b/tests/migration/Makefile
index 13e99b1692..cfebfe23f8 100644
--- a/tests/migration/Makefile
+++ b/tests/migration/Makefile
@@ -5,7 +5,7 @@
  # See the COPYING file in the top-level directory.
  #
  
-TARGET_LIST = i386 aarch64 s390x

+TARGET_LIST = i386 aarch64 s390x loongarch64
  
  SRC_PATH = ../..
  
diff --git a/tests/migration/loongarch64/Makefile b/tests/migration/loongarch64/Makefile

new file mode 100644
index 00..5d8719205f
--- /dev/null
+++ b/tests/migration/loongarch64/Makefile
@@ -0,0 +1,18 @@
+# To specify cross compiler prefix, use CROSS_PREFIX=
+#   $ make CROSS_PREFIX=loongarch64-linux-gnu-
+
+.PHONY: all clean
+all: a-b-kernel.h
+
+a-b-kernel.h: loongarch64.kernel
+   echo "$$__note" > $@
+   xxd -i $< | sed -e 's/.*int.*//' >> $@
+
+loongarch64.kernel: loongarch64.elf
+   $(CROSS_PREFIX)objcopy -j .text -O binary $< $@
+
+loongarch64.elf: a-b-kernel.S
+   $(CROSS_PREFIX)gcc -o $@ -nostdlib -Wl,--build-id=none $<
+
+clean:
+   $(RM) *.kernel *.elf
diff --git a/tests/migration/loongarch64/a-b-kernel.S 
b/tests/migration/loongarch64/a-b-kernel.S
new file mode 100644
index 00..078f91b306
--- /dev/null
+++ b/tests/migration/loongarch64/a-b-kernel.S
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2024 Loongson Technology Corporation Limited
+ */
+#include "../migration-test.h"
+
+#define LOONGARCH_CSR_CRMD  0
+#define LOONGARCH_VIRT_UART 0x1FE001E0
+.section .text
+
+.globl  _start
+_start:
+/* output char 'A' to UART16550 */
+li.d$t0, LOONGARCH_VIRT_UART
+li.w$t1, 'A'
+st.b$t1, $t0, 0
+
+/* traverse test memory region */
+   li.d$t0, LOONGARCH_TEST_MEM_START


Stray tab here.

Good catch, there is tab here. Will fix it in next version.

Regards
Bibo Mao



+li.d$t1, LOONGARCH_TEST_MEM_END
+li.d$t2, TEST_MEM_PAGE_SIZE
+
+clean:
+st.b$zero, $t0, 0
+add.d   $t0,   $t0, $t2
+bne $t0,   $t1, clean
+
+mainloop:
+li.d$t0, LOONGARCH_TEST_MEM_START
+li.d$t1, LOONGARCH_TEST_MEM_END
+li.d$t2, TEST_MEM_PAGE_SIZE
+
+li.d$t4, LOONGARCH_VIRT_UART
+li.w$t5, 'B'
+
+innerloop:
+ld.bu   $t3, $t0, 0
+addi.w  $t3, $t3, 1
+ext.w.b $t3, $t3
+st.b$t3, $t0, 0
+add.d   $t0, $t0, $t2
+bne $t0, $t1, innerloop
+
+st.b$t5, $t4, 0
+b   mainloop
+nop
diff --git a/tests/migration/loongarch64/a-b-kernel.h 
b/tests/migration/loongarch64/a-b-kernel.h
new file mode 100644
index 00..6019450229
--- /dev/null
+++ b/tests/migration/loongarch64/a-b-kernel.h
@@ -0,0 +1,13 @@
+
+unsigned char loongarch64_kernel[] = {
+  0x0c, 0xc0, 0x3f, 0x14, 0x8c, 0x81, 0x87, 0x03, 0x0d, 0x04, 0x81, 0x03,
+  0x8d, 0x01, 0x00, 0x29, 0x0c, 0x00, 0x01, 0x14, 0x0d, 0x80, 0x0c, 0x14,
+  0x2e, 0x00, 0x00, 0x14, 0x80, 0x01, 0x00, 0x29, 0x8c, 0xb9, 0x10, 0x00,
+  0x8d, 0xf9, 0xff, 0x5f, 0x0c, 0x00, 0x01, 0x14, 0x0d, 0x80, 0x0c, 0x14,
+  0x2e, 0x00, 0x00, 0x14, 0x10, 0xc0, 0x3f, 0x14, 0x10, 0x82, 0x87, 0x03,
+  0x11, 0x08, 0x81, 0x03, 0x8f, 0x01, 0x00, 0x2a, 0xef, 0x05, 0x80, 0x02,
+  0xef, 0x5d, 0x00, 0x00, 0x8f, 0x01, 0x00, 0x29, 0x8c, 0xb9, 0x10, 0x00,
+  0x8d, 0xed, 0xff, 0x5f, 0x11, 0x02, 0x00, 0x29, 0xff, 0xcf, 0xff, 0x53,
+  0x00, 0x00, 0x40, 0x03
+};
+
diff --git a/tests/migration/migration-test.h b/tests/migration/migration-test.h
index 68512c0b1b..b6e9914f9c 100644
--- a/tests/migration/migration-test.h
+++ b/tests/migration/migration-test.h
@@ -32,4 +32,7 @@
   */
  #define ARM_TEST_MAX_KERNEL_SIZE (512 * 1024)
  
+/* LoongArch64 */

+#define LOONGARCH_TEST_MEM_START (8 * 1024 * 1024)

Re: [RFC PATCH v2 01/22] target/arm: Add FEAT_NMI to max

[Jinjie Ruan via]

On 2024/2/22 5:22, Richard Henderson wrote:
> On 2/21/24 03:08, Jinjie Ruan via wrote:
>> Enable FEAT_NMI on the 'max' CPU.
>>
>> Signed-off-by: Jinjie Ruan 
>> ---
>>   docs/system/arm/emulation.rst | 1 +
>>   target/arm/tcg/cpu64.c    | 1 +
>>   2 files changed, 2 insertions(+)
> 
> Reviewed-by: Richard Henderson 
> 
> However, this patch must be sorted last, after all support for FEAT_NMI
> is present.

Good suggestion! Placed it last is more reasonable.

> 
> 
> r~
> 
>>
>> diff --git a/docs/system/arm/emulation.rst
>> b/docs/system/arm/emulation.rst
>> index f67aea2d83..91baf7ad69 100644
>> --- a/docs/system/arm/emulation.rst
>> +++ b/docs/system/arm/emulation.rst
>> @@ -63,6 +63,7 @@ the following architecture extensions:
>>   - FEAT_MTE (Memory Tagging Extension)
>>   - FEAT_MTE2 (Memory Tagging Extension)
>>   - FEAT_MTE3 (MTE Asymmetric Fault Handling)
>> +- FEAT_NMI (Non-maskable Interrupt)
>>   - FEAT_NV (Nested Virtualization)
>>   - FEAT_NV2 (Enhanced nested virtualization support)
>>   - FEAT_PACIMP (Pointer authentication - IMPLEMENTATION DEFINED
>> algorithm)
>> diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c
>> index 5fba2c0f04..60f0dcd799 100644
>> --- a/target/arm/tcg/cpu64.c
>> +++ b/target/arm/tcg/cpu64.c
>> @@ -1175,6 +1175,7 @@ void aarch64_max_tcg_initfn(Object *obj)
>>   t = FIELD_DP64(t, ID_AA64PFR1, RAS_FRAC, 0);  /* FEAT_RASv1p1 +
>> FEAT_DoubleFault */
>>   t = FIELD_DP64(t, ID_AA64PFR1, SME, 1);   /* FEAT_SME */
>>   t = FIELD_DP64(t, ID_AA64PFR1, CSV2_FRAC, 0); /* FEAT_CSV2_2 */
>> +    t = FIELD_DP64(t, ID_AA64PFR1, NMI, 1);   /* FEAT_NMI */
>>   cpu->isar.id_aa64pfr1 = t;
>>     t = cpu->isar.id_aa64mmfr0;
> 

Re: [RFC PATCH v2 03/22] target/arm: Add PSTATE.ALLINT

[Jinjie Ruan via]

On 2024/2/22 2:50, Richard Henderson wrote:
> On 2/21/24 03:08, Jinjie Ruan via wrote:
>> The ALLINT bit in PSTATE is used to mask all IRQ or FIQ interrupts.
>>
>> Place this in its own field within ENV, as that will
>> make it easier to reset from within TCG generated code.
>>
>> With the change to pstate_read/write, exception entry
>> and return are automatically handled.
>>
>> Signed-off-by: Jinjie Ruan 
>> ---
>>   target/arm/cpu.c | 3 +++
>>   target/arm/cpu.h | 9 +++--
>>   2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index 5fa86bc8d5..5e5978c302 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -1104,6 +1104,9 @@ static void aarch64_cpu_dump_state(CPUState *cs,
>> FILE *f, int flags)
>>   if (cpu_isar_feature(aa64_bti, cpu)) {
>>   qemu_fprintf(f, "  BTYPE=%d", (psr & PSTATE_BTYPE) >> 10);
>>   }
>> +    if (cpu_isar_feature(aa64_nmi, cpu)) {
>> +    qemu_fprintf(f, "  ALLINT=%d", (psr & PSTATE_ALLINT) >> 13);
>> +    }
> 
> This is one bit -- !!(psr & ALLINT) is better
> 
> We don't individually print DAIF either; why is this bit more special?

That makes sense, it should be removed.

> 
>> @@ -224,6 +224,7 @@ typedef struct CPUArchState {
>>    *    semantics as for AArch32, as described in the comments on
>> each field)
>>    *  nRW (also known as M[4]) is kept, inverted, in env->aarch64
>>    *  DAIF (exception masks) are kept in env->daif
>> + *  ALLINT (all IRQ or FIQ interrupts masks) are kept in env->allint
>>    *  BTYPE is kept in env->btype
>>    *  SM and ZA are kept in env->svcr
>>    *  all other bits are stored in their correct places in
>> env->pstate
>> @@ -261,6 +262,7 @@ typedef struct CPUArchState {
>>   uint32_t btype;  /* BTI branch type.  spsr[11:10].  */
>>   uint64_t daif; /* exception masks, in the bits they are in
>> PSTATE */
>>   uint64_t svcr; /* PSTATE.{SM,ZA} in the bits they are in SVCR */
>> +    uint64_t allint; /* All IRQ or FIQ interrupt mask, in the bit in
>> PSTATE */
> 
> Why is this split out from env->pstate?
> 
> The allint bit matches the documentation for SPSR_EL1, which is how
> env->pstate is documented.  The other exclusions have some performance
> imperative which I don't see for allint.

It seems to me that allint is a bit like daif, so it is reasonable to
maintain it separately in ENV as what daif do it.

> 
> 
> r~

RE: [PATCH 2/4] target/riscv: Apply modularized matching conditions for breakpoint

[張哲嘉]

Hi Daniel,

> -Original Message-
> From: Daniel Henrique Barboza 
> Sent: Thursday, February 22, 2024 1:26 AM
> To: Alvin Che-Chia Chang(張哲嘉) ;
> qemu-ri...@nongnu.org; qemu-devel@nongnu.org
> Cc: alistair.fran...@wdc.com; bin.m...@windriver.com;
> liwei1...@gmail.com; zhiwei_...@linux.alibaba.com
> Subject: Re: [PATCH 2/4] target/riscv: Apply modularized matching conditions
> for breakpoint
> 
> 
> 
> On 2/19/24 00:25, Alvin Chang wrote:
> > We have implemented trigger_common_match(), which checks if the
> > enabled privilege levels of the trigger match CPU's current privilege level.
> > Remove the related code in riscv_cpu_debug_check_breakpoint() and
> > invoke
> > trigger_common_match() to check the privilege levels of the type 2 and
> > type 6 triggers for the breakpoints.
> >
> > Only the execution bit and the executed PC should be futher checked in
> 
> typo: further

Thanks! Will fix it.

> 
> > riscv_cpu_debug_check_breakpoint().
> >
> > Signed-off-by: Alvin Chang 
> > ---
> >   target/riscv/debug.c | 26 ++
> >   1 file changed, 6 insertions(+), 20 deletions(-)
> >
> > diff --git a/target/riscv/debug.c b/target/riscv/debug.c index
> > 7932233073..b971ed5d7a 100644
> > --- a/target/riscv/debug.c
> > +++ b/target/riscv/debug.c
> > @@ -855,21 +855,17 @@ bool riscv_cpu_debug_check_breakpoint(CPUState
> *cs)
> >   for (i = 0; i < RV_MAX_TRIGGERS; i++) {
> >   trigger_type = get_trigger_type(env, i);
> >
> > +if (!trigger_common_match(env, trigger_type, i)) {
> > +continue;
> > +}
> > +
> 
> I believe this will change how the function behaves. Before this patch, we
> would 'return false' if we have a TRIGGER_TYPE_AD_MATCH and
> env->virt_enabled is true.
> 
> Now, for the same case, we'll keep looping until we found a match, or return
> 'false'
> if we run out of triggers.

Oh! I didn't notice that the behavior is changed.. thank you.

Image that CPU supports both type 2 and type 6 triggers, and the debugger sets 
two identical breakpoints.(this should be a mistake, but we should not restrict 
the debugger)
One of them is type 2 breakpoint at trigger index 0, and the other is type 6 
breakpoint at trigger index 1.
Now if the system runs in VS/VU modes, it could match type 6 breakpoint, so the 
looping is necessary.
If the system runs in M/HS/U modes, it could match type 2 breakpoint.
Is my understanding correct?


Sincerely,
Alvin

> 
> This seems ok to do, but we should state in the commit msg that we're
> intentionally change how the function works. If that's not the idea and we 
> want
> to preserve the existing behavior, we would need to do:
> 
> >
> > +if (!trigger_common_match(env, trigger_type, i)) {
> > +return false;
> > +}
> 
> Instead of just continue looping. Thanks,
> 
> 
> Daniel
> 
> >   switch (trigger_type) {
> >   case TRIGGER_TYPE_AD_MATCH:
> > -/* type 2 trigger cannot be fired in VU/VS mode */
> > -if (env->virt_enabled) {
> > -return false;
> > -}
> > -
> >   ctrl = env->tdata1[i];
> >   pc = env->tdata2[i];
> >
> >   if ((ctrl & TYPE2_EXEC) && (bp->pc == pc)) {
> > -/* check U/S/M bit against current privilege level
> */
> > -if ((ctrl >> 3) & BIT(env->priv)) {
> > -return true;
> > -}
> > +return true;
> >   }
> >   break;
> >   case TRIGGER_TYPE_AD_MATCH6:
> > @@ -877,17 +873,7 @@ bool riscv_cpu_debug_check_breakpoint(CPUState
> *cs)
> >   pc = env->tdata2[i];
> >
> >   if ((ctrl & TYPE6_EXEC) && (bp->pc == pc)) {
> > -if (env->virt_enabled) {
> > -/* check VU/VS bit against current privilege
> level */
> > -if ((ctrl >> 23) & BIT(env->priv)) {
> > -return true;
> > -}
> > -} else {
> > -/* check U/S/M bit against current privilege
> level */
> > -if ((ctrl >> 3) & BIT(env->priv)) {
> > -return true;
> > -}
> > -}
> > +return true;
> >   }
> >   break;
> >   default:

Re: [PULL 05/13] linux-user: Use walk_memory_regions for open_self_maps

[Richard Henderson]

On 2/12/24 10:43, Ilya Leoshkevich wrote:

int main(void)
{
 shmat(shmget(IPC_PRIVATE, 1836016, IPC_CREAT | 0600), (void 
*)0x2804000, 0);
 open("/proc/self/maps", O_RDONLY);
}

Apparently an mmap() is missing for shmat() when g>h and shmaddr is
specified. The mismatch between the host's and the guest's view of the
mapping's tail appears to be causing the SEGV.


Yes, shmat() is handling none of the h != g page size issues;
it is in fact fairly broken.


r~

[PATCH] pl031: Update last RTCLR value on write in case it's read back

[Jessica Clarke]

The PL031 allows you to read RTCLR, which is meant to give you the last
value written. PL031State has an lr field which is used when reading
from RTCLR, and is present in the VM migration state, but we never
actually update it, so it always reads as its initial 0 value.

Signed-off-by: Jessica Clarke 
---
 hw/rtc/pl031.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/rtc/pl031.c b/hw/rtc/pl031.c
index 837b0bdf9b..563bb4b446 100644
--- a/hw/rtc/pl031.c
+++ b/hw/rtc/pl031.c
@@ -141,6 +141,7 @@ static void pl031_write(void * opaque, hwaddr offset,
 g_autofree const char *qom_path = object_get_canonical_path(opaque);
 struct tm tm;
 
+s->lr = value;
 s->tick_offset += value - pl031_get_count(s);
 
 qemu_get_timedate(, s->tick_offset);
-- 
2.34.1

Re: [PATCH] hw/cxl/cxl-mailbox-utils: remove unneeded mailbox output payload space zeroing

[fan]

On Wed, Feb 21, 2024 at 09:59:49PM +0100, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 21/2/24 19:53, nifan@gmail.com wrote:
> > From: Fan Ni 
> > 
> > The whole mailbox output payload space is already zeroed after copying
> > out the input payload, which happens before processing the specific mailbox
> > command:
> > https://elixir.bootlin.com/qemu/latest/source/hw/cxl/cxl-device-utils.c#L204
> 
> Since "latest" isn't stable, this link won't be accurate in 6 months.
> 
> Please use the current release:
> https://elixir.bootlin.com/qemu/v8.2.1/source/hw/cxl/cxl-device-utils.c#L204
> 

Thanks, Philippe. 

Just sent out v2 as you suggested.
https://lore.kernel.org/linux-cxl/20240221221824.1092966-1-nifan@gmail.com/T/#u

Fan

> > 
> > Signed-off-by: Fan Ni 
> > ---
> >   hw/cxl/cxl-mailbox-utils.c | 7 ---
> >   1 file changed, 7 deletions(-)
> 

[PATCH v2] hw/cxl/cxl-mailbox-utils: remove unneeded mailbox output payload space zeroing

[nifan . cxl]

From: Fan Ni 

The whole mailbox output payload space is already zeroed after copying
out the input payload, which happens before processing the specific mailbox
command:
https://elixir.bootlin.com/qemu/v8.2.1/source/hw/cxl/cxl-device-utils.c#L204

Signed-off-by: Fan Ni 
---
 hw/cxl/cxl-mailbox-utils.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index e5eb97cb91..fda88470a3 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -226,7 +226,6 @@ static CXLRetCode cmd_events_get_records(const struct 
cxl_cmd *cmd,
 log_type = payload_in[0];
 
 pl = (CXLGetEventPayload *)payload_out;
-memset(pl, 0, sizeof(*pl));
 
 max_recs = (cxlds->payload_size - CXL_EVENT_PAYLOAD_HDR_SIZE) /
 CXL_EVENT_RECORD_SIZE;
@@ -264,7 +263,6 @@ static CXLRetCode cmd_events_get_interrupt_policy(const 
struct cxl_cmd *cmd,
 CXLEventLog *log;
 
 policy = (CXLEventInterruptPolicy *)payload_out;
-memset(policy, 0, sizeof(*policy));
 
 log = >event_logs[CXL_EVENT_TYPE_INFO];
 if (log->irq_enabled) {
@@ -363,7 +361,6 @@ static CXLRetCode cmd_infostat_identify(const struct 
cxl_cmd *cmd,
 QEMU_BUILD_BUG_ON(sizeof(*is_identify) != 18);
 
 is_identify = (void *)payload_out;
-memset(is_identify, 0, sizeof(*is_identify));
 is_identify->pcie_vid = class->vendor_id;
 is_identify->pcie_did = class->device_id;
 if (object_dynamic_cast(OBJECT(cci->d), TYPE_CXL_USP)) {
@@ -597,7 +594,6 @@ static CXLRetCode cmd_infostat_bg_op_sts(const struct 
cxl_cmd *cmd,
 QEMU_BUILD_BUG_ON(sizeof(*bg_op_status) != 8);
 
 bg_op_status = (void *)payload_out;
-memset(bg_op_status, 0, sizeof(*bg_op_status));
 bg_op_status->status = cci->bg.complete_pct << 1;
 if (cci->bg.runtime > 0) {
 bg_op_status->status |= 1U << 0;
@@ -636,7 +632,6 @@ static CXLRetCode cmd_firmware_update_get_info(const struct 
cxl_cmd *cmd,
 }
 
 fw_info = (void *)payload_out;
-memset(fw_info, 0, sizeof(*fw_info));
 
 fw_info->slots_supported = 2;
 fw_info->slot_info = BIT(0) | BIT(3);
@@ -792,7 +787,6 @@ static CXLRetCode cmd_identify_memory_device(const struct 
cxl_cmd *cmd,
 }
 
 id = (void *)payload_out;
-memset(id, 0, sizeof(*id));
 
 snprintf(id->fw_revision, 0x10, "BWFW VERSION %02d", 0);
 
@@ -1079,7 +1073,6 @@ static CXLRetCode cmd_media_get_poison_list(const struct 
cxl_cmd *cmd,
 out_pl_len = sizeof(*out) + record_count * sizeof(out->records[0]);
 assert(out_pl_len <= CXL_MAILBOX_MAX_PAYLOAD_SIZE);
 
-memset(out, 0, out_pl_len);
 QLIST_FOREACH(ent, poison_list, node) {
 uint64_t start, stop;
 
-- 
2.43.0

Re: [PATCH v6 9/9] target/riscv/vector_helper.c: optimize loops in ldst helpers

[Richard Henderson]

On 2/21/24 11:31, Daniel Henrique Barboza wrote:

Change the for loops in ldst helpers to do a single increment in the
counter, and assign it env->vstart, to avoid re-reading from vstart
every time.

Suggested-by: Richard Henderson
Signed-off-by: Daniel Henrique Barboza
---
  target/riscv/vector_helper.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v6 5/9] target/riscv: remove 'cpu_vl' global

[Richard Henderson]

On 2/21/24 11:31, Daniel Henrique Barboza wrote:

At this moment the global is used only in do_vsetvl(). Do a direct env
load in do_vsetvl() to read 'vl' and remove the global.

Suggested-by: Richard Henderson
Signed-off-by: Daniel Henrique Barboza
---
  target/riscv/insn_trans/trans_rvv.c.inc | 2 +-
  target/riscv/translate.c| 3 +--
  2 files changed, 2 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v3 1/3] qdev: Add a granule_mode property

[Richard Henderson]

On 2/21/24 10:58, Eric Auger wrote:

Introduce a new enum type property allowing to set an
IOMMU granule. Values are 4K, 16K, 64K and host. This
latter indicates the vIOMMU granule will matches the
host page size.

A subsequent patch will add such a property to the
virtio-iommu device.

Signed-off-by: Eric Auger 
---
  include/hw/qdev-properties-system.h |  3 +++
  include/hw/virtio/virtio-iommu.h| 11 +++
  hw/core/qdev-properties-system.c| 15 +++
  hw/virtio/virtio-iommu.c| 10 ++
  4 files changed, 39 insertions(+)

diff --git a/include/hw/qdev-properties-system.h 
b/include/hw/qdev-properties-system.h
index 06c359c190..626be87dd3 100644
--- a/include/hw/qdev-properties-system.h
+++ b/include/hw/qdev-properties-system.h
@@ -8,6 +8,7 @@ extern const PropertyInfo qdev_prop_macaddr;
  extern const PropertyInfo qdev_prop_reserved_region;
  extern const PropertyInfo qdev_prop_multifd_compression;
  extern const PropertyInfo qdev_prop_mig_mode;
+extern const PropertyInfo qdev_prop_granule_mode;
  extern const PropertyInfo qdev_prop_losttickpolicy;
  extern const PropertyInfo qdev_prop_blockdev_on_error;
  extern const PropertyInfo qdev_prop_bios_chs_trans;
@@ -47,6 +48,8 @@ extern const PropertyInfo qdev_prop_iothread_vq_mapping_list;
  #define DEFINE_PROP_MIG_MODE(_n, _s, _f, _d) \
  DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_mig_mode, \
 MigMode)
+#define DEFINE_PROP_GRANULE_MODE(_n, _s, _f, _d) \
+DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_granule_mode, GranuleMode)
  #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
  DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
  LostTickPolicy)
diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
index 5fbe4677c2..a82c4fa471 100644
--- a/include/hw/virtio/virtio-iommu.h
+++ b/include/hw/virtio/virtio-iommu.h
@@ -31,6 +31,16 @@ OBJECT_DECLARE_SIMPLE_TYPE(VirtIOIOMMU, VIRTIO_IOMMU)
  
  #define TYPE_VIRTIO_IOMMU_MEMORY_REGION "virtio-iommu-memory-region"
  
+typedef enum GranuleMode {

+GRANULE_MODE_4K,
+GRANULE_MODE_16K,
+GRANULE_MODE_64K,
+GRANULE_MODE_HOST,
+GRANULE_MODE__MAX,
+} GranuleMode;
+
+extern const QEnumLookup GranuleMode_lookup;
+
  typedef struct IOMMUDevice {
  void *viommu;
  PCIBus   *bus;
@@ -67,6 +77,7 @@ struct VirtIOIOMMU {
  Notifier machine_done;
  bool granule_frozen;
  uint8_t aw_bits;
+GranuleMode granule_mode;
  };
  
  #endif

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 1a396521d5..3a0b36a7a7 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -34,6 +34,7 @@
  #include "net/net.h"
  #include "hw/pci/pci.h"
  #include "hw/pci/pcie.h"
+#include "hw/virtio/virtio-iommu.h"
  #include "hw/i386/x86.h"
  #include "util/block-helpers.h"
  
@@ -679,6 +680,20 @@ const PropertyInfo qdev_prop_mig_mode = {

  .set_default_value = qdev_propinfo_set_default_value_enum,
  };
  
+/* --- GranuleMode --- */

+
+QEMU_BUILD_BUG_ON(sizeof(GranuleMode) != sizeof(int));
+
+const PropertyInfo qdev_prop_granule_mode = {
+.name = "GranuleMode",
+.description = "granule_mode values, "
+   "4K, 16K, 64K, host",
+.enum_table = _lookup,
+.get = qdev_propinfo_get_enum,
+.set = qdev_propinfo_set_enum,
+.set_default_value = qdev_propinfo_set_default_value_enum,
+};
+
  /* --- Reserved Region --- */
  
  /*

diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 2ec5ef3cd1..2ce5839c60 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -44,6 +44,16 @@
  #define VIOMMU_DEFAULT_QUEUE_SIZE 256
  #define VIOMMU_PROBE_SIZE 512
  
+const QEnumLookup GranuleMode_lookup = {

+.array = (const char *const[]) {
+[GRANULE_MODE_4K]  = "4K",
+[GRANULE_MODE_16K] = "16K",
+[GRANULE_MODE_64K] = "64K",
+[GRANULE_MODE_HOST] = "host",
+},
+.size = GRANULE_MODE__MAX
+};


Does it ever make sense to use anything other than host?
Especially since 4k fails on a 64k host, as you report.
Why would we want to be able to select a granule that doesn't work?

There are a few older hosts with 8k pages, e.g. Alpha, MIPS, Sparc.


r~

[Stable-7.2.10 14/33] virtio_iommu: Clear IOMMUPciBus pointer cache when system reset

[Michael Tokarev]

From: Zhenzhong Duan 

s->iommu_pcibus_by_bus_num is a IOMMUPciBus pointer cache indexed
by bus number, bus number may not always be a fixed value,
i.e., guest reboot to different kernel which set bus number with
different algorithm.

This could lead to endpoint binding to wrong iommu MR in
virtio_iommu_get_endpoint(), then vfio device setup wrong
mapping from other device.

Remove the memset in virtio_iommu_device_realize() to avoid
redundancy with memset in system reset.

Signed-off-by: Zhenzhong Duan 
Message-Id: <20240125073706.339369-2-zhenzhong.d...@intel.com>
Reviewed-by: Eric Auger 
Tested-by: Eric Auger 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit 9a457383ce9d309d4679b079fafb51f0a2d949aa)
Signed-off-by: Michael Tokarev 

diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index eb82462c95..95db19f144 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -1140,6 +1140,8 @@ static void virtio_iommu_system_reset(void *opaque)
 
 trace_virtio_iommu_system_reset();
 
+memset(s->iommu_pcibus_by_bus_num, 0, sizeof(s->iommu_pcibus_by_bus_num));
+
 /*
  * config.bypass is sticky across device reset, but should be restored on
  * system reset
@@ -1156,8 +1158,6 @@ static void virtio_iommu_device_realize(DeviceState *dev, 
Error **errp)
 
 virtio_init(vdev, VIRTIO_ID_IOMMU, sizeof(struct virtio_iommu_config));
 
-memset(s->iommu_pcibus_by_bus_num, 0, sizeof(s->iommu_pcibus_by_bus_num));
-
 s->req_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
  virtio_iommu_handle_command);
 s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
-- 
2.39.2

[Stable-7.2.10 28/33] target/i386: Generate an illegal opcode exception on cmp instructions with lock prefix

[Michael Tokarev]

From: Ziqiao Kong 

target/i386: As specified by Intel Manual Vol2 3-180, cmp instructions
are not allowed to have lock prefix and a `UD` should be raised. Without
this patch, s1->T0 will be uninitialized and used in the case OP_CMPL.

Signed-off-by: Ziqiao Kong 
Message-ID: <20240215095015.570748-2-ziqiaok...@gmail.com>
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
(cherry picked from commit 99d0dcd7f102c07a510200d768cae65e5db25d23)
Signed-off-by: Michael Tokarev 

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 68c42fd9ff..abacb91ddf 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -1501,12 +1501,13 @@ static bool check_iopl(DisasContext *s)
 /* if d == OR_TMP0, it means memory operand (address in A0) */
 static void gen_op(DisasContext *s1, int op, MemOp ot, int d)
 {
+/* Invalid lock prefix when destination is not memory or OP_CMPL. */
+if ((d != OR_TMP0 || op == OP_CMPL) && s1->prefix & PREFIX_LOCK) {
+gen_illegal_opcode(s1);
+return;
+}
+
 if (d != OR_TMP0) {
-if (s1->prefix & PREFIX_LOCK) {
-/* Lock prefix when destination is not memory.  */
-gen_illegal_opcode(s1);
-return;
-}
 gen_op_mov_v_reg(s1, ot, s1->T0, d);
 } else if (!(s1->prefix & PREFIX_LOCK)) {
 gen_op_ld_v(s1, ot, s1->T0, s1->A0);
-- 
2.39.2

[Stable-7.2.10 26/33] i386/cpuid: Decrease cpuid_i when skipping CPUID leaf 1F

[Michael Tokarev]

From: Xiaoyao Li 

Existing code misses a decrement of cpuid_i when skip leaf 0x1F.
There's a blank CPUID entry(with leaf, subleaf as 0, and all fields
stuffed 0s) left in the CPUID array.

It conflicts with correct CPUID leaf 0.

Signed-off-by: Xiaoyao Li 
Reviewed-by:Yang Weijiang 
Message-ID: <20240125024016.2521244-2-xiaoyao...@intel.com>
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
(cherry picked from commit 10f92799af8ba3c3cef2352adcd4780f13fbab31)
Signed-off-by: Michael Tokarev 

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 002b699030..5779b80ecb 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1859,6 +1859,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
 }
 case 0x1f:
 if (env->nr_dies < 2) {
+cpuid_i--;
 break;
 }
 /* fallthrough */
-- 
2.39.2

[Stable-7.2.10 29/33] ui: reject extended clipboard message if not activated

[Michael Tokarev]

From: Daniel P. Berrangé 

The extended clipboard message protocol requires that the client
activate the extension by requesting a psuedo encoding. If this
is not done, then any extended clipboard messages from the client
should be considered invalid and the client dropped.

Signed-off-by: Daniel P. Berrangé 
Reviewed-by: Marc-André Lureau 
Message-Id: <20240115095119.654271-1-berra...@redhat.com>
(cherry picked from commit 4cba8388968b70fe20e290221dc421c717051fdd)
Signed-off-by: Michael Tokarev 

diff --git a/ui/vnc.c b/ui/vnc.c
index 1ca16c0ff6..629a500adc 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2456,6 +2456,11 @@ static int protocol_client_msg(VncState *vs, uint8_t 
*data, size_t len)
 }
 
 if (read_s32(data, 4) < 0) {
+if (!vnc_has_feature(vs, VNC_FEATURE_CLIPBOARD_EXT)) {
+error_report("vnc: extended clipboard message while disabled");
+vnc_client_error(vs);
+break;
+}
 if (dlen < 4) {
 error_report("vnc: malformed payload (header less than 4 
bytes)"
  " in extended clipboard pseudo-encoding.");
-- 
2.39.2

[Stable-7.2.10 22/33] target/arm: Don't get MDCR_EL2 in pmu_counter_enabled() before checking ARM_FEATURE_PMU

[Michael Tokarev]

From: Peter Maydell 

It doesn't make sense to read the value of MDCR_EL2 on a non-A-profile
CPU, and in fact if you try to do it we will assert:

#6  0x74b95e96 in __GI___assert_fail
(assertion=0x565a8c70 "!arm_feature(env, ARM_FEATURE_M)", 
file=0x565a6e5c "../../target/arm/helper.c", line=12600, 
function=0x565a9560 <__PRETTY_FUNCTION__.0> "arm_security_space_below_el3") 
at ./assert/assert.c:101
#7  0x55ebf412 in arm_security_space_below_el3 (env=0x57bc8190) at 
../../target/arm/helper.c:12600
#8  0x55ea6f89 in arm_is_el2_enabled (env=0x57bc8190) at 
../../target/arm/cpu.h:2595
#9  0x55ea942f in arm_mdcr_el2_eff (env=0x57bc8190) at 
../../target/arm/internals.h:1512

We might call pmu_counter_enabled() on an M-profile CPU (for example
from the migration pre/post hooks in machine.c); this should always
return false because these CPUs don't set ARM_FEATURE_PMU.

Avoid the assertion by not calling arm_mdcr_el2_eff() before we
have done the early return for "PMU not present".

This fixes an assertion failure if you try to do a loadvm or
savevm for an M-profile board.

Cc: qemu-sta...@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2155
Signed-off-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Message-id: 20240208153346.970021-1-peter.mayd...@linaro.org
(cherry picked from commit ac1d88e9e7ca0bed83e91e07ce6d0597f10cc77d)
Signed-off-by: Michael Tokarev 

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 343acfab3a..2e284e048c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1125,13 +1125,21 @@ static bool pmu_counter_enabled(CPUARMState *env, 
uint8_t counter)
 bool enabled, prohibited = false, filtered;
 bool secure = arm_is_secure(env);
 int el = arm_current_el(env);
-uint64_t mdcr_el2 = arm_mdcr_el2_eff(env);
-uint8_t hpmn = mdcr_el2 & MDCR_HPMN;
+uint64_t mdcr_el2;
+uint8_t hpmn;
 
+/*
+ * We might be called for M-profile cores where MDCR_EL2 doesn't
+ * exist and arm_mdcr_el2_eff() will assert, so this early-exit check
+ * must be before we read that value.
+ */
 if (!arm_feature(env, ARM_FEATURE_PMU)) {
 return false;
 }
 
+mdcr_el2 = arm_mdcr_el2_eff(env);
+hpmn = mdcr_el2 & MDCR_HPMN;
+
 if (!arm_feature(env, ARM_FEATURE_EL2) ||
 (counter < hpmn || counter == 31)) {
 e = env->cp15.c9_pmcr & PMCRE;
-- 
2.39.2

[Stable-7.2.10 11/33] cxl/cdat: Handle cdat table build errors

[Michael Tokarev]

From: Ira Weiny 

The callback for building CDAT tables may return negative error codes.
This was previously unhandled and will result in potentially huge
allocations later on in ct3_build_cdat()

Detect the negative error code and defer cdat building.

Fixes: f5ee7413d592 ("hw/mem/cxl-type3: Add CXL CDAT Data Object Exchange")
Cc: Huai-Cheng Kuo 
Reviewed-by: Dave Jiang 
Reviewed-by: Fan Ni 
Signed-off-by: Ira Weiny 
Signed-off-by: Jonathan Cameron 
Message-Id: <20240126120132.24248-2-jonathan.came...@huawei.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit c62926f730d08450502d36548e28dd727c998ace)
Signed-off-by: Michael Tokarev 

diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c
index 3653aa56f0..931693f02d 100644
--- a/hw/cxl/cxl-cdat.c
+++ b/hw/cxl/cxl-cdat.c
@@ -62,7 +62,7 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp)
 
 cdat->built_buf_len = cdat->build_cdat_table(>built_buf, 
cdat->private);
 
-if (!cdat->built_buf_len) {
+if (cdat->built_buf_len <= 0) {
 /* Build later as not all data available yet */
 cdat->to_update = true;
 return;
-- 
2.39.2

[Stable-7.2.10 31/33] ui/clipboard: add asserts for update and request

[Michael Tokarev]

From: Fiona Ebner 

Should an issue like CVE-2023-6683 ever appear again in the future,
it will be more obvious which assumption was violated.

Suggested-by: Marc-André Lureau 
Signed-off-by: Fiona Ebner 
Reviewed-by: Marc-André Lureau 
Message-ID: <20240124105749.204610-2-f.eb...@proxmox.com>
(cherry picked from commit 9c416582611b7495bdddb4c5456c7acb64b78938)
Signed-off-by: Michael Tokarev 

diff --git a/ui/clipboard.c b/ui/clipboard.c
index b3f6fa3c9e..4264884a6c 100644
--- a/ui/clipboard.c
+++ b/ui/clipboard.c
@@ -65,12 +65,24 @@ bool qemu_clipboard_check_serial(QemuClipboardInfo *info, 
bool client)
 
 void qemu_clipboard_update(QemuClipboardInfo *info)
 {
+uint32_t type;
 QemuClipboardNotify notify = {
 .type = QEMU_CLIPBOARD_UPDATE_INFO,
 .info = info,
 };
 assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
 
+for (type = 0; type < QEMU_CLIPBOARD_TYPE__COUNT; type++) {
+/*
+ * If data is missing, the clipboard owner's 'request' callback needs 
to
+ * be set. Otherwise, there is no way to get the clipboard data and
+ * qemu_clipboard_request() cannot be called.
+ */
+if (info->types[type].available && !info->types[type].data) {
+assert(info->owner && info->owner->request);
+}
+}
+
 notifier_list_notify(_notifiers, );
 
 if (cbinfo[info->selection] != info) {
@@ -132,6 +144,8 @@ void qemu_clipboard_request(QemuClipboardInfo *info,
 !info->owner)
 return;
 
+assert(info->owner->request);
+
 info->types[type].requested = true;
 info->owner->request(info, type);
 }
-- 
2.39.2

[Stable-7.2.10 24/33] i386/cpu: Clear FEAT_XSAVE_XSS_LO/HI leafs when CPUID_EXT_XSAVE is not available

[Michael Tokarev]

From: Xiaoyao Li 

Leaf FEAT_XSAVE_XSS_LO and FEAT_XSAVE_XSS_HI also need to be cleared
when CPUID_EXT_XSAVE is not set.

Fixes: 301e90675c3f ("target/i386: Enable support for XSAVES based features")
Signed-off-by: Xiaoyao Li 
Reviewed-by: Yang Weijiang 
Message-ID: <20240115091325.1904229-2-xiaoyao...@intel.com>
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
(cherry picked from commit 81f5cad3858f27623b1b14467926032d229b76cc)
Signed-off-by: Michael Tokarev 

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 0f71ff9fea..952fa5780f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6114,6 +6114,8 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
 if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) {
 env->features[FEAT_XSAVE_XCR0_LO] = 0;
 env->features[FEAT_XSAVE_XCR0_HI] = 0;
+env->features[FEAT_XSAVE_XSS_LO] = 0;
+env->features[FEAT_XSAVE_XSS_HI] = 0;
 return;
 }
 
-- 
2.39.2

[Stable-7.2.10 30/33] ui/clipboard: mark type as not available when there is no data

[Michael Tokarev]

From: Fiona Ebner 

With VNC, a client can send a non-extended VNC_MSG_CLIENT_CUT_TEXT
message with len=0. In qemu_clipboard_set_data(), the clipboard info
will be updated setting data to NULL (because g_memdup(data, size)
returns NULL when size is 0). If the client does not set the
VNC_ENCODING_CLIPBOARD_EXT feature when setting up the encodings, then
the 'request' callback for the clipboard peer is not initialized.
Later, because data is NULL, qemu_clipboard_request() can be reached
via vdagent_chr_write() and vdagent_clipboard_recv_request() and
there, the clipboard owner's 'request' callback will be attempted to
be called, but that is a NULL pointer.

In particular, this can happen when using the KRDC (22.12.3) VNC
client.

Another scenario leading to the same issue is with two clients (say
noVNC and KRDC):

The noVNC client sets the extension VNC_FEATURE_CLIPBOARD_EXT and
initializes its cbpeer.

The KRDC client does not, but triggers a vnc_client_cut_text() (note
it's not the _ext variant)). There, a new clipboard info with it as
the 'owner' is created and via qemu_clipboard_set_data() is called,
which in turn calls qemu_clipboard_update() with that info.

In qemu_clipboard_update(), the notifier for the noVNC client will be
called, i.e. vnc_clipboard_notify() and also set vs->cbinfo for the
noVNC client. The 'owner' in that clipboard info is the clipboard peer
for the KRDC client, which did not initialize the 'request' function.
That sounds correct to me, it is the owner of that clipboard info.

Then when noVNC sends a VNC_MSG_CLIENT_CUT_TEXT message (it did set
the VNC_FEATURE_CLIPBOARD_EXT feature correctly, so a check for it
passes), that clipboard info is passed to qemu_clipboard_request() and
the original segfault still happens.

Fix the issue by handling updates with size 0 differently. In
particular, mark in the clipboard info that the type is not available.

While at it, switch to g_memdup2(), because g_memdup() is deprecated.

Cc: qemu-sta...@nongnu.org
Fixes: CVE-2023-6683
Reported-by: Markus Frank 
Suggested-by: Marc-André Lureau 
Signed-off-by: Fiona Ebner 
Reviewed-by: Marc-André Lureau 
Tested-by: Markus Frank 
Message-ID: <20240124105749.204610-1-f.eb...@proxmox.com>
(cherry picked from commit 405484b29f6548c7b86549b0f961b906337aa68a)
Signed-off-by: Michael Tokarev 

diff --git a/ui/clipboard.c b/ui/clipboard.c
index 3d14bffaf8..b3f6fa3c9e 100644
--- a/ui/clipboard.c
+++ b/ui/clipboard.c
@@ -163,9 +163,15 @@ void qemu_clipboard_set_data(QemuClipboardPeer *peer,
 }
 
 g_free(info->types[type].data);
-info->types[type].data = g_memdup(data, size);
-info->types[type].size = size;
-info->types[type].available = true;
+if (size) {
+info->types[type].data = g_memdup2(data, size);
+info->types[type].size = size;
+info->types[type].available = true;
+} else {
+info->types[type].data = NULL;
+info->types[type].size = 0;
+info->types[type].available = false;
+}
 
 if (update) {
 qemu_clipboard_update(info);
-- 
2.39.2

[Stable-7.2.10 27/33] i386/cpuid: Move leaf 7 to correct group

[Michael Tokarev]

From: Xiaoyao Li 

CPUID leaf 7 was grouped together with SGX leaf 0x12 by commit
b9edbadefb9e ("i386: Propagate SGX CPUID sub-leafs to KVM") by mistake.

SGX leaf 0x12 has its specific logic to check if subleaf (starting from 2)
is valid or not by checking the bit 0:3 of corresponding EAX is 1 or
not.

Leaf 7 follows the logic that EAX of subleaf 0 enumerates the maximum
valid subleaf.

Fixes: b9edbadefb9e ("i386: Propagate SGX CPUID sub-leafs to KVM")
Signed-off-by: Xiaoyao Li 
Message-ID: <20240125024016.2521244-4-xiaoyao...@intel.com>
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
(cherry picked from commit 0729857c707535847d7fe31d3d91eb8b2a118e3c)
Signed-off-by: Michael Tokarev 

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 5779b80ecb..4d83bb5784 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1900,7 +1900,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
 c = _data.entries[cpuid_i++];
 }
 break;
-case 0x7:
 case 0x12:
 for (j = 0; ; j++) {
 c->function = i;
@@ -1920,6 +1919,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
 c = _data.entries[cpuid_i++];
 }
 break;
+case 0x7:
 case 0x14:
 case 0x1d:
 case 0x1e: {
-- 
2.39.2

[Stable-7.2.10 32/33] ui/console: Fix console resize with placeholder surface

[Michael Tokarev]

From: Tianlan Zhou 

In `qemu_console_resize()`, the old surface of the console is keeped if the new
console size is the same as the old one. If the old surface is a placeholder,
and the new size of console is the same as the placeholder surface (640*480),
the surface won't be replace.
In this situation, the surface's `QEMU_PLACEHOLDER_FLAG` flag is still set, so
the console won't be displayed in SDL display mode.
This patch fixes this problem by forcing a new surface if the old one is a
placeholder.

Signed-off-by: Tianlan Zhou 
Reviewed-by: Marc-André Lureau 
Message-ID: <20240207172024.8-1-bobby...@126.com>
(cherry picked from commit 95b08fee8f68d284a5028d37fd28be7a70c8e92b)
Signed-off-by: Michael Tokarev 

diff --git a/ui/console.c b/ui/console.c
index 52414d6aa3..269cf27163 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2583,7 +2583,7 @@ void qemu_console_resize(QemuConsole *s, int width, int 
height)
 assert(s->console_type == GRAPHIC_CONSOLE);
 
 if ((s->scanout.kind != SCANOUT_SURFACE ||
- (surface && surface->flags & QEMU_ALLOCATED_FLAG)) &&
+ (surface && !is_buffer_shared(surface) && !is_placeholder(surface))) 
&&
 qemu_console_get_width(s, -1) == width &&
 qemu_console_get_height(s, -1) == height) {
 return;
-- 
2.39.2

[Stable-7.2.10 16/33] tests/acpi: Allow update of DSDT.cxl

[Michael Tokarev]

From: Jonathan Cameron 

The _STA value returned currently indicates the ACPI0017 device
is not enabled.  Whilst this isn't a real device, setting _STA
like this may prevent an OS from enumerating it correctly and
hence from parsing the CEDT table.

Signed-off-by: Jonathan Cameron 
Message-Id: <20240126120132.24248-11-jonathan.came...@huawei.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit 14ec4ff3e4293635240ba5a7afe7a0f3ba447d31)
Signed-off-by: Michael Tokarev 

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h 
b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..9ce0f596cc 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,2 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/q35/DSDT.cxl",
-- 
2.39.2

[Stable-7.2.10 21/33] target/arm: Fix SVE/SME gross MTE suppression checks

[Michael Tokarev]

From: Richard Henderson 

The TBI and TCMA bits are located within mtedesc, not desc.

Cc: qemu-sta...@nongnu.org
Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
Tested-by: Gustavo Romero 
Message-id: 20240207025210.8837-7-richard.hender...@linaro.org
Signed-off-by: Peter Maydell 
(cherry picked from commit 855f94eca80c85a99f459e36684ea2f98f6a3243)
Signed-off-by: Michael Tokarev 

diff --git a/target/arm/sme_helper.c b/target/arm/sme_helper.c
index 8856773635..d592c78ec9 100644
--- a/target/arm/sme_helper.c
+++ b/target/arm/sme_helper.c
@@ -606,8 +606,8 @@ void sme_ld1_mte(CPUARMState *env, void *za, uint64_t *vg,
 desc = extract32(desc, 0, SIMD_DATA_SHIFT + SVE_MTEDESC_SHIFT);
 
 /* Perform gross MTE suppression early. */
-if (!tbi_check(desc, bit55) ||
-tcma_check(desc, bit55, allocation_tag_from_addr(addr))) {
+if (!tbi_check(mtedesc, bit55) ||
+tcma_check(mtedesc, bit55, allocation_tag_from_addr(addr))) {
 mtedesc = 0;
 }
 
@@ -783,8 +783,8 @@ void sme_st1_mte(CPUARMState *env, void *za, uint64_t *vg, 
target_ulong addr,
 desc = extract32(desc, 0, SIMD_DATA_SHIFT + SVE_MTEDESC_SHIFT);
 
 /* Perform gross MTE suppression early. */
-if (!tbi_check(desc, bit55) ||
-tcma_check(desc, bit55, allocation_tag_from_addr(addr))) {
+if (!tbi_check(mtedesc, bit55) ||
+tcma_check(mtedesc, bit55, allocation_tag_from_addr(addr))) {
 mtedesc = 0;
 }
 
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 27838fb6e2..45a93755fe 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -5803,8 +5803,8 @@ void sve_ldN_r_mte(CPUARMState *env, uint64_t *vg, 
target_ulong addr,
 desc = extract32(desc, 0, SIMD_DATA_SHIFT + SVE_MTEDESC_SHIFT);
 
 /* Perform gross MTE suppression early. */
-if (!tbi_check(desc, bit55) ||
-tcma_check(desc, bit55, allocation_tag_from_addr(addr))) {
+if (!tbi_check(mtedesc, bit55) ||
+tcma_check(mtedesc, bit55, allocation_tag_from_addr(addr))) {
 mtedesc = 0;
 }
 
@@ -6159,8 +6159,8 @@ void sve_ldnfff1_r_mte(CPUARMState *env, void *vg, 
target_ulong addr,
 desc = extract32(desc, 0, SIMD_DATA_SHIFT + SVE_MTEDESC_SHIFT);
 
 /* Perform gross MTE suppression early. */
-if (!tbi_check(desc, bit55) ||
-tcma_check(desc, bit55, allocation_tag_from_addr(addr))) {
+if (!tbi_check(mtedesc, bit55) ||
+tcma_check(mtedesc, bit55, allocation_tag_from_addr(addr))) {
 mtedesc = 0;
 }
 
@@ -6413,8 +6413,8 @@ void sve_stN_r_mte(CPUARMState *env, uint64_t *vg, 
target_ulong addr,
 desc = extract32(desc, 0, SIMD_DATA_SHIFT + SVE_MTEDESC_SHIFT);
 
 /* Perform gross MTE suppression early. */
-if (!tbi_check(desc, bit55) ||
-tcma_check(desc, bit55, allocation_tag_from_addr(addr))) {
+if (!tbi_check(mtedesc, bit55) ||
+tcma_check(mtedesc, bit55, allocation_tag_from_addr(addr))) {
 mtedesc = 0;
 }
 
-- 
2.39.2

[Stable-7.2.10 07/33] pci-host: designware: Limit value range of iATU viewport register

[Michael Tokarev]

From: Guenter Roeck 

The latest version of qemu (v8.2.0-869-g7a1dc45af5) crashes when booting
the mcimx7d-sabre emulation with Linux v5.11 and later.

qemu-system-arm: ../system/memory.c:2750: memory_region_set_alias_offset: 
Assertion `mr->alias' failed.

Problem is that the Designware PCIe emulation accepts the full value range
for the iATU Viewport Register. However, both hardware and emulation only
support four inbound and four outbound viewports.

The Linux kernel determines the number of supported viewports by writing
0xff into the viewport register and reading the value back. The expected
value when reading the register is the highest supported viewport index.
Match that code by masking the supported viewport value range when the
register is written. With this change, the Linux kernel reports

imx6q-pcie 3380.pcie: iATU: unroll F, 4 ob, 4 ib, align 0K, limit 4G

as expected and supported.

Fixes: d64e5eabc4c7 ("pci: Add support for Designware IP block")
Cc: Andrey Smirnov 
Cc: Nikita Ostrenkov 
Signed-off-by: Guenter Roeck 
Message-id: 20240129060055.2616989-1-li...@roeck-us.net
Reviewed-by: Peter Maydell 
Signed-off-by: Peter Maydell 
(cherry picked from commit 8a73152020337a7fbf34daf0a006d4d89ec1494e)
Signed-off-by: Michael Tokarev 

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index bde3a343a2..c235b9daa3 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -340,6 +340,8 @@ static void designware_pcie_root_config_write(PCIDevice *d, 
uint32_t address,
 break;
 
 case DESIGNWARE_PCIE_ATU_VIEWPORT:
+val &= DESIGNWARE_PCIE_ATU_REGION_INBOUND |
+(DESIGNWARE_PCIE_NUM_VIEWPORTS - 1);
 root->atu_viewport = val;
 break;
 
-- 
2.39.2

[Stable-7.2.10 03/33] block/blkio: Make s->mem_region_alignment be 64 bits

[Michael Tokarev]

From: "Richard W.M. Jones" 

With GCC 14 the code failed to compile on i686 (and was wrong for any
version of GCC):

../block/blkio.c: In function ‘blkio_file_open’:
../block/blkio.c:857:28: error: passing argument 3 of ‘blkio_get_uint64’ from 
incompatible pointer type [-Wincompatible-pointer-types]
  857 |>mem_region_alignment);
  |^~~~
  ||
  |size_t * {aka unsigned int *}
In file included from ../block/blkio.c:12:
/usr/include/blkio.h:49:67: note: expected ‘uint64_t *’ {aka ‘long long 
unsigned int *’} but argument is of type ‘size_t *’ {aka ‘unsigned int *’}
   49 | int blkio_get_uint64(struct blkio *b, const char *name, uint64_t 
*value);
  | ~~^

Signed-off-by: Richard W.M. Jones 
Message-id: 20240130122006.2977938-1-rjo...@redhat.com
Signed-off-by: Stefan Hajnoczi 
(cherry picked from commit 615eaeab3d318ba239d54141a4251746782f65c1)
Signed-off-by: Michael Tokarev 

diff --git a/block/blkio.c b/block/blkio.c
index 5eae3adfaf..cb66160268 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -74,7 +74,7 @@ typedef struct {
 CoQueue bounce_available;
 
 /* The value of the "mem-region-alignment" property */
-size_t mem_region_alignment;
+uint64_t mem_region_alignment;
 
 /* Can we skip adding/deleting blkio_mem_regions? */
 bool needs_mem_regions;
-- 
2.39.2

[Stable-7.2.10 33/33] audio: Depend on dbus_display1_dep

[Michael Tokarev]

From: Akihiko Odaki 

dbusaudio needs dbus_display1_dep.

Fixes: 739362d4205c ("audio: add "dbus" audio backend")
Signed-off-by: Akihiko Odaki 
Reviewed-by: Marc-André Lureau 
Message-Id: <20240214-dbus-v7-1-7eff29f04...@daynix.com>
(cherry picked from commit d67611907590a1e6c998b7c5a5cb4394acf84329)
Signed-off-by: Michael Tokarev 
(Mjt: fixup in audio/meson.build due to missing v8.0.0-2306-ga95a464777
 "audio: dbus requires pixman")

diff --git a/audio/meson.build b/audio/meson.build
index 34aed78342..ce171f710d 100644
--- a/audio/meson.build
+++ b/audio/meson.build
@@ -29,7 +29,8 @@ endforeach
 
 if dbus_display
 module_ss = ss.source_set()
-module_ss.add(when: gio, if_true: files('dbusaudio.c'))
+module_ss.add(when: [gio, dbus_display1_dep],
+  if_true: files('dbusaudio.c'))
 audio_modules += {'dbus': module_ss}
 endif
 
-- 
2.39.2

[Stable-7.2.10 20/33] target/arm: Fix nregs computation in do_{ld, st}_zpa

[Michael Tokarev]

From: Richard Henderson 

The field is encoded as [0-3], which is convenient for
indexing our array of function pointers, but the true
value is [1-4].  Adjust before calling do_mem_zpa.

Add an assert, and move the comment re passing ZT to
the helper back next to the relevant code.

Cc: qemu-sta...@nongnu.org
Fixes: 206adacfb8d ("target/arm: Add mte helpers for sve scalar + int loads")
Signed-off-by: Richard Henderson 
Tested-by: Gustavo Romero 
Message-id: 20240207025210.8837-3-richard.hender...@linaro.org
Reviewed-by: Peter Maydell 
Signed-off-by: Peter Maydell 
(cherry picked from commit 64c6e7444dff64b42d11b836b9aec9acfbe8ecc2)
Signed-off-by: Michael Tokarev 

diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index 621a2abb22..7388e1dbc7 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -4587,11 +4587,7 @@ static void do_mem_zpa(DisasContext *s, int zt, int pg, 
TCGv_i64 addr,
 TCGv_ptr t_pg;
 int desc = 0;
 
-/*
- * For e.g. LD4, there are not enough arguments to pass all 4
- * registers as pointers, so encode the regno into the data field.
- * For consistency, do this even for LD1.
- */
+assert(mte_n >= 1 && mte_n <= 4);
 if (s->mte_active[0]) {
 int msz = dtype_msz(dtype);
 
@@ -4605,6 +4601,11 @@ static void do_mem_zpa(DisasContext *s, int zt, int pg, 
TCGv_i64 addr,
 addr = clean_data_tbi(s, addr);
 }
 
+/*
+ * For e.g. LD4, there are not enough arguments to pass all 4
+ * registers as pointers, so encode the regno into the data field.
+ * For consistency, do this even for LD1.
+ */
 desc = simd_desc(vsz, vsz, zt | desc);
 t_pg = tcg_temp_new_ptr();
 
@@ -4744,7 +4745,7 @@ static void do_ld_zpa(DisasContext *s, int zt, int pg,
  * accessible via the instruction encoding.
  */
 assert(fn != NULL);
-do_mem_zpa(s, zt, pg, addr, dtype, nreg, false, fn);
+do_mem_zpa(s, zt, pg, addr, dtype, nreg + 1, false, fn);
 }
 
 static bool trans_LD_zprr(DisasContext *s, arg_rprr_load *a)
@@ -5320,14 +5321,13 @@ static void do_st_zpa(DisasContext *s, int zt, int pg, 
TCGv_i64 addr,
 if (nreg == 0) {
 /* ST1 */
 fn = fn_single[s->mte_active[0]][be][msz][esz];
-nreg = 1;
 } else {
 /* ST2, ST3, ST4 -- msz == esz, enforced by encoding */
 assert(msz == esz);
 fn = fn_multiple[s->mte_active[0]][be][nreg - 1][msz];
 }
 assert(fn != NULL);
-do_mem_zpa(s, zt, pg, addr, msz_dtype(s, msz), nreg, true, fn);
+do_mem_zpa(s, zt, pg, addr, msz_dtype(s, msz), nreg + 1, true, fn);
 }
 
 static bool trans_ST_zprr(DisasContext *s, arg_rprr_store *a)
-- 
2.39.2

[Stable-7.2.10 23/33] iotests: Make 144 deterministic again

[Michael Tokarev]

From: Kevin Wolf 

Since commit effd60c8 changed how QMP commands are processed, the order
of the block-commit return value and job events in iotests 144 wasn't
fixed and more and caused the test to fail intermittently.

Change the test to cache events first and then print them in a
predefined order.

Waiting three times for JOB_STATUS_CHANGE is a bit uglier than just
waiting for the JOB_STATUS_CHANGE that has "status": "ready", but the
tooling we have doesn't seem to allow the latter easily.

Fixes: effd60c878176bcaf97fa7ce2b12d04bb8ead6f7
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2126
Signed-off-by: Kevin Wolf 
Reviewed-by: Stefan Hajnoczi 
Message-id: 20240209173103.239994-1-kw...@redhat.com
Signed-off-by: Peter Maydell 
(cherry picked from commit cc29c12ec629ba68a4a6cb7d165c94cc8502815a)
Signed-off-by: Michael Tokarev 

diff --git a/tests/qemu-iotests/144 b/tests/qemu-iotests/144
index bdcc498fa2..d284a0e442 100755
--- a/tests/qemu-iotests/144
+++ b/tests/qemu-iotests/144
@@ -83,12 +83,22 @@ echo
 echo === Performing block-commit on active layer ===
 echo
 
+capture_events="BLOCK_JOB_READY JOB_STATUS_CHANGE"
+
 # Block commit on active layer, push the new overlay into base
 _send_qemu_cmd $h "{ 'execute': 'block-commit',
 'arguments': {
  'device': 'virtio0'
   }
-}" "READY"
+}" "return"
+
+_wait_event $h "JOB_STATUS_CHANGE"
+_wait_event $h "JOB_STATUS_CHANGE"
+_wait_event $h "JOB_STATUS_CHANGE"
+
+_wait_event $h "BLOCK_JOB_READY"
+
+capture_events=
 
 _send_qemu_cmd $h "{ 'execute': 'block-job-complete',
 'arguments': {
diff --git a/tests/qemu-iotests/144.out b/tests/qemu-iotests/144.out
index b3b4812015..2245ddfa10 100644
--- a/tests/qemu-iotests/144.out
+++ b/tests/qemu-iotests/144.out
@@ -25,9 +25,9 @@ Formatting 'TEST_DIR/tmp.qcow2', fmt=qcow2 cluster_size=65536 
extended_l2=off co
  'device': 'virtio0'
   }
 }
+{"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"JOB_STATUS_CHANGE", "data": {"status": "created", "id": "virtio0"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"JOB_STATUS_CHANGE", "data": {"status": "running", "id": "virtio0"}}
-{"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"JOB_STATUS_CHANGE", "data": {"status": "ready", "id": "virtio0"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_READY", "data": {"device": "virtio0", "len": 0, "offset": 0, 
"speed": 0, "type": "commit"}}
 { 'execute': 'block-job-complete',
-- 
2.39.2

[Stable-7.2.10 25/33] i386/cpu: Mask with XCR0/XSS mask for FEAT_XSAVE_XCR0_HI and FEAT_XSAVE_XSS_HI leafs

[Michael Tokarev]

From: Xiaoyao Li 

The value of FEAT_XSAVE_XCR0_HI leaf and FEAT_XSAVE_XSS_HI leaf also
need to be masked by XCR0 and XSS mask respectively, to make it
logically correct.

Fixes: 301e90675c3f ("target/i386: Enable support for XSAVES based features")
Signed-off-by: Xiaoyao Li 
Reviewed-by: Yang Weijiang 
Message-ID: <20240115091325.1904229-3-xiaoyao...@intel.com>
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
(cherry picked from commit a11a365159b944e05be76f3ec3b98c8b38cb70fd)
Signed-off-by: Michael Tokarev 

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 952fa5780f..52a3020032 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6134,9 +6134,9 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
 }
 
 env->features[FEAT_XSAVE_XCR0_LO] = mask & CPUID_XSTATE_XCR0_MASK;
-env->features[FEAT_XSAVE_XCR0_HI] = mask >> 32;
+env->features[FEAT_XSAVE_XCR0_HI] = (mask & CPUID_XSTATE_XCR0_MASK) >> 32;
 env->features[FEAT_XSAVE_XSS_LO] = mask & CPUID_XSTATE_XSS_MASK;
-env->features[FEAT_XSAVE_XSS_HI] = mask >> 32;
+env->features[FEAT_XSAVE_XSS_HI] = (mask & CPUID_XSTATE_XSS_MASK) >> 32;
 }
 
 /* Steps involved on loading and filtering CPUID data
-- 
2.39.2

[Stable-7.2.10 12/33] cxl/cdat: Fix header sum value in CDAT checksum

[Michael Tokarev]

From: Ira Weiny 

The addition of the DCD support for CXL type-3 devices extended the CDAT
table large enough that the checksum being returned was incorrect.[1]

This was because the checksum value was using the header length field
rather than each of the 4 bytes of the length field.  This was
previously not seen because the length of the CDAT data was less than
256 thus resulting in an equivalent checksum value.

Properly calculate the checksum for the CDAT header.

[1] 
https://lore.kernel.org/all/20231116-fix-cdat-devm-free-v1-1-b148b4070...@intel.com/

Fixes: aba578bdace5 ("hw/cxl/cdat: CXL CDAT Data Object Exchange 
implementation")
Cc: Huai-Cheng Kuo 
Signed-off-by: Ira Weiny 
Reviewed-by: Dave Jiang 
Reviewed-by: Fan Ni 
Signed-off-by: Jonathan Cameron 

Message-Id: <20240126120132.24248-5-jonathan.came...@huawei.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit 64fdad5e67587e88c2f1d8f294e89403856a4a31)
Signed-off-by: Michael Tokarev 

diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c
index 931693f02d..0cde11854e 100644
--- a/hw/cxl/cxl-cdat.c
+++ b/hw/cxl/cxl-cdat.c
@@ -49,6 +49,7 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp)
 g_autofree CDATTableHeader *cdat_header = NULL;
 g_autofree CDATEntry *cdat_st = NULL;
 uint8_t sum = 0;
+uint8_t *hdr_buf;
 int ent, i;
 
 /* Use default table if fopen == NULL */
@@ -94,8 +95,12 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp)
 /* For now, no runtime updates */
 cdat_header->sequence = 0;
 cdat_header->length += sizeof(CDATTableHeader);
-sum += cdat_header->revision + cdat_header->sequence +
-cdat_header->length;
+
+hdr_buf = (uint8_t *)cdat_header;
+for (i = 0; i < sizeof(*cdat_header); i++) {
+sum += hdr_buf[i];
+}
+
 /* Sum of all bytes including checksum must be 0 */
 cdat_header->checksum = ~sum + 1;
 
-- 
2.39.2

[Stable-7.2.10 05/33] system/vl.c: Fix handling of '-serial none -serial something'

[Michael Tokarev]

From: Peter Maydell 

Currently if the user passes multiple -serial options on the command
line, we mostly treat those as applying to the different serial
devices in order, so that for example
 -serial stdio -serial file:filename
will connect the first serial port to stdio and the second to the
named file.

The exception to this is the '-serial none' serial device type.  This
means "don't allocate this serial device", but a bug means that
following -serial options are not correctly handled, so that
 -serial none -serial stdio
has the unexpected effect that stdio is connected to the first serial
port, not the second.

This is a very long-standing bug that dates back at least as far as
commit 998bbd74b9d81 from 2009.

Make the 'none' serial type move forward in the indexing of serial
devices like all the other serial types, so that any subsequent
-serial options are correctly handled.

Note that if your commandline mistakenly had a '-serial none' that
was being overridden by a following '-serial something' option, you
should delete the unnecessary '-serial none'.  This will give you the
same behaviour as before, on QEMU versions both with and without this
bug fix.

Cc: qemu-sta...@nongnu.org
Reported-by: Bohdan Kostiv 
Signed-off-by: Peter Maydell 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Richard Henderson 
Message-id: 20240122163607.459769-2-peter.mayd...@linaro.org
Fixes: 998bbd74b9d81 ("default devices: core code & serial lines")
Signed-off-by: Peter Maydell 
(cherry picked from commit d2019a9d0c34a4fdcb5b5df550d73040dc0637d9)
Signed-off-by: Michael Tokarev 

diff --git a/softmmu/vl.c b/softmmu/vl.c
index ce88869618..ab4394c53d 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1363,18 +1363,22 @@ static void qemu_create_default_devices(void)
 static int serial_parse(const char *devname)
 {
 int index = num_serial_hds;
-char label[32];
 
-if (strcmp(devname, "none") == 0)
-return 0;
-snprintf(label, sizeof(label), "serial%d", index);
 serial_hds = g_renew(Chardev *, serial_hds, index + 1);
 
-serial_hds[index] = qemu_chr_new_mux_mon(label, devname, NULL);
-if (!serial_hds[index]) {
-error_report("could not connect serial device"
- " to character backend '%s'", devname);
-return -1;
+if (strcmp(devname, "none") == 0) {
+/* Don't allocate a serial device for this index */
+serial_hds[index] = NULL;
+} else {
+char label[32];
+snprintf(label, sizeof(label), "serial%d", index);
+
+serial_hds[index] = qemu_chr_new_mux_mon(label, devname, NULL);
+if (!serial_hds[index]) {
+error_report("could not connect serial device"
+ " to character backend '%s'", devname);
+return -1;
+}
 }
 num_serial_hds++;
 return 0;
-- 
2.39.2

[Stable-7.2.10 18/33] tests/acpi: Update DSDT.cxl to reflect change _STA return value.

[Michael Tokarev]

From: Jonathan Cameron 

_STA will now return 0xB (in common with most other devices)
rather than not setting the bits to indicate this fake device
has not been enabled, and self tests haven't passed.

Signed-off-by: Jonathan Cameron 
Message-Id: <20240126120132.24248-13-jonathan.came...@huawei.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit b24a981b9f1c4767aaea815e504a2c7aeb405d72)
Signed-off-by: Michael Tokarev 
(Mjt: rebuild tests/data/acpi/q35/DSDT.cxl for 7.2.x)

diff --git a/tests/data/acpi/q35/DSDT.cxl b/tests/data/acpi/q35/DSDT.cxl
index f9c6dd4ee0..267709e4e4 100644
Binary files a/tests/data/acpi/q35/DSDT.cxl and b/tests/data/acpi/q35/DSDT.cxl 
differ
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h 
b/tests/qtest/bios-tables-test-allowed-diff.h
index 9ce0f596cc..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,2 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/q35/DSDT.cxl",
-- 
2.39.2

[Stable-7.2.10 15/33] smmu: Clear SMMUPciBus pointer cache when system reset

[Michael Tokarev]

From: Zhenzhong Duan 

s->smmu_pcibus_by_bus_num is a SMMUPciBus pointer cache indexed
by bus number, bus number may not always be a fixed value,
i.e., guest reboot to different kernel which set bus number with
different algorithm.

This could lead to smmu_iommu_mr() providing the wrong iommu MR.

Suggested-by: Eric Auger 
Signed-off-by: Zhenzhong Duan 
Message-Id: <20240125073706.339369-3-zhenzhong.d...@intel.com>
Reviewed-by: Eric Auger 
Tested-by: Eric Auger 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit 8a6b3f4dc95a064e88adaca86374108da0ecb38d)
Signed-off-by: Michael Tokarev 

diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index bbca3a8db3..7abc166eb3 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -529,6 +529,8 @@ static void smmu_base_reset(DeviceState *dev)
 {
 SMMUState *s = ARM_SMMU(dev);
 
+memset(s->smmu_pcibus_by_bus_num, 0, sizeof(s->smmu_pcibus_by_bus_num));
+
 g_hash_table_remove_all(s->configs);
 g_hash_table_remove_all(s->iotlb);
 }
-- 
2.39.2

[Stable-7.2.10 19/33] linux-user/aarch64: Choose SYNC as the preferred MTE mode

[Michael Tokarev]

From: Richard Henderson 

The API does not generate an error for setting ASYNC | SYNC; that merely
constrains the selection vs the per-cpu default.  For qemu linux-user,
choose SYNC as the default.

Cc: qemu-sta...@nongnu.org
Reported-by: Gustavo Romero 
Signed-off-by: Richard Henderson 
Tested-by: Gustavo Romero 
Message-id: 20240207025210.8837-2-richard.hender...@linaro.org
Signed-off-by: Peter Maydell 
(cherry picked from commit 681dfc0d552963d4d598350d26097a692900b408)
Signed-off-by: Michael Tokarev 

diff --git a/linux-user/aarch64/target_prctl.h 
b/linux-user/aarch64/target_prctl.h
index 907c314146..d9f6648e27 100644
--- a/linux-user/aarch64/target_prctl.h
+++ b/linux-user/aarch64/target_prctl.h
@@ -171,21 +171,26 @@ static abi_long 
do_prctl_set_tagged_addr_ctrl(CPUArchState *env, abi_long arg2)
 env->tagged_addr_enable = arg2 & PR_TAGGED_ADDR_ENABLE;
 
 if (cpu_isar_feature(aa64_mte, cpu)) {
-switch (arg2 & PR_MTE_TCF_MASK) {
-case PR_MTE_TCF_NONE:
-case PR_MTE_TCF_SYNC:
-case PR_MTE_TCF_ASYNC:
-break;
-default:
-return -EINVAL;
-}
-
 /*
  * Write PR_MTE_TCF to SCTLR_EL1[TCF0].
- * Note that the syscall values are consistent with hw.
+ *
+ * The kernel has a per-cpu configuration for the sysadmin,
+ * /sys/devices/system/cpu/cpu/mte_tcf_preferred,
+ * which qemu does not implement.
+ *
+ * Because there is no performance difference between the modes, and
+ * because SYNC is most useful for debugging MTE errors, choose SYNC
+ * as the preferred mode.  With this preference, and the way the API
+ * uses only two bits, there is no way for the program to select
+ * ASYMM mode.
  */
-env->cp15.sctlr_el[1] =
-deposit64(env->cp15.sctlr_el[1], 38, 2, arg2 >> PR_MTE_TCF_SHIFT);
+unsigned tcf = 0;
+if (arg2 & PR_MTE_TCF_SYNC) {
+tcf = 1;
+} else if (arg2 & PR_MTE_TCF_ASYNC) {
+tcf = 2;
+}
+env->cp15.sctlr_el[1] = deposit64(env->cp15.sctlr_el[1], 38, 2, tcf);
 
 /*
  * Write PR_MTE_TAG to GCR_EL1[Exclude].
-- 
2.39.2

[Stable-7.2.10 v0 00/33] Patch Round-up for stable 7.2.10, freeze on 2024-03-02

[Michael Tokarev]

The following patches are queued for QEMU stable v7.2.10:

  https://gitlab.com/qemu-project/qemu/-/commits/staging-7.2

Patch freeze is 2024-03-02, and the release is planned for 2024-03-04:

  https://wiki.qemu.org/Planning/7.2

Please respond here or CC qemu-sta...@nongnu.org on any additional patches
you think should (or shouldn't) be included in the release.

The changes which are staging for inclusion, with the original commit hash
from master branch, are given below the bottom line.

Thanks!

/mjt

--
01 27eb8499edb2 Fabiano Rosas:
   migration: Fix use-after-free of migration state object
02 db101376af52 Yihuan Pan:
   qemu-docs: Update options for graphical frontends
03 615eaeab3d31 Richard W.M. Jones:
   block/blkio: Make s->mem_region_alignment be 64 bits
04 f670be1aad33 Jan Klötzke:
   target/arm: fix exception syndrome for AArch32 bkpt insn
05 d2019a9d0c34 Peter Maydell:
   system/vl.c: Fix handling of '-serial none -serial something'
06 747bfaf3a9d2 Peter Maydell:
   qemu-options.hx: Improve -serial option documentation
07 8a7315202033 Guenter Roeck:
   pci-host: designware: Limit value range of iATU viewport register
08 cd8a35b913c2 Akihiko Odaki:
   hw/smbios: Fix OEM strings table option validation
09 196578c9d051 Akihiko Odaki:
   hw/smbios: Fix port connector option validation
10 aa05bd9ef407 Andrey Ignatov:
   vhost-user.rst: Fix vring address description
11 c62926f730d0 Ira Weiny:
   cxl/cdat: Handle cdat table build errors
12 64fdad5e6758 Ira Weiny:
   cxl/cdat: Fix header sum value in CDAT checksum
13 729d45a6af06 Li Zhijian:
   hw/cxl: Pass CXLComponentState to cache_mem_ops
14 9a457383ce9d Zhenzhong Duan:
   virtio_iommu: Clear IOMMUPciBus pointer cache when system reset
15 8a6b3f4dc95a Zhenzhong Duan:
   smmu: Clear SMMUPciBus pointer cache when system reset
16 14ec4ff3e429 Jonathan Cameron:
   tests/acpi: Allow update of DSDT.cxl
17 d9ae5802f656 Jonathan Cameron:
   hw/i386: Fix _STA return value for ACPI0017
18 b24a981b9f1c Jonathan Cameron:
   tests/acpi: Update DSDT.cxl to reflect change _STA return value.
19 681dfc0d5529 Richard Henderson:
   linux-user/aarch64: Choose SYNC as the preferred MTE mode
20 64c6e7444dff Richard Henderson:
   target/arm: Fix nregs computation in do_{ld,st}_zpa
21 855f94eca80c Richard Henderson:
   target/arm: Fix SVE/SME gross MTE suppression checks
22 ac1d88e9e7ca Peter Maydell:
   target/arm: Don't get MDCR_EL2 in pmu_counter_enabled() before checking 
   ARM_FEATURE_PMU
23 cc29c12ec629 Kevin Wolf:
   iotests: Make 144 deterministic again
24 81f5cad3858f Xiaoyao Li:
   i386/cpu: Clear FEAT_XSAVE_XSS_LO/HI leafs when CPUID_EXT_XSAVE is not 
   available
25 a11a365159b9 Xiaoyao Li:
   i386/cpu: Mask with XCR0/XSS mask for FEAT_XSAVE_XCR0_HI and 
   FEAT_XSAVE_XSS_HI leafs
26 10f92799af8b Xiaoyao Li:
   i386/cpuid: Decrease cpuid_i when skipping CPUID leaf 1F
27 0729857c7075 Xiaoyao Li:
   i386/cpuid: Move leaf 7 to correct group
28 99d0dcd7f102 Ziqiao Kong:
   target/i386: Generate an illegal opcode exception on cmp instructions 
   with lock prefix
29 4cba8388968b Daniel P. Berrangé:
   ui: reject extended clipboard message if not activated
30 405484b29f65 Fiona Ebner:
   ui/clipboard: mark type as not available when there is no data
31 9c416582611b Fiona Ebner:
   ui/clipboard: add asserts for update and request
32 95b08fee8f68 Tianlan Zhou:
   ui/console: Fix console resize with placeholder surface
33 d67611907590 Akihiko Odaki:
   audio: Depend on dbus_display1_dep

[Stable-7.2.10 10/33] vhost-user.rst: Fix vring address description

[Michael Tokarev]

From: Andrey Ignatov 

There is no "size" field in vring address structure. Remove it.

Fixes: 5fc0e00291 ("Add vhost-user protocol documentation")
Signed-off-by: Andrey Ignatov 
Message-Id: <20240112004555.64900-1-r...@apple.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit aa05bd9ef4073ccb72d04ad78de32916af31c7c3)
Signed-off-by: Michael Tokarev 

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 3f18ab424e..936de705e1 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -111,9 +111,9 @@ A vring state description
 A vring address description
 ^^^
 
-+---+---+--++--+---+-+
-| index | flags | size | descriptor | used | available | log |
-+---+---+--++--+---+-+
++---+---++--+---+-+
+| index | flags | descriptor | used | available | log |
++---+---++--+---+-+
 
 :index: a 32-bit vring index
 
-- 
2.39.2

Re: [RFC PATCH v2 19/22] hw/intc/arm_gicv3: Add irq superpriority information

[Richard Henderson]

On 2/21/24 03:08, Jinjie Ruan via wrote:

A SPI, PPI or SGI interrupt can have a superpriority property. So
maintain superpriority information in PendingIrq and GICR/GICD.

Signed-off-by: Jinjie Ruan
---
  include/hw/intc/arm_gicv3_common.h | 4 
  1 file changed, 4 insertions(+)


Acked-by: Richard Henderson 

Though this patch is mis-ordered compared to its uses.


r~

[Stable-7.2.10 06/33] qemu-options.hx: Improve -serial option documentation

[Michael Tokarev]

From: Peter Maydell 

The -serial option documentation is a bit brief about '-serial none'
and '-serial null'. In particular it's not very clear about the
difference between them, and it doesn't mention that it's up to
the machine model whether '-serial none' means "don't create the
serial port" or "don't wire the serial port up to anything".

Expand on these points.

Signed-off-by: Peter Maydell 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20240122163607.459769-3-peter.mayd...@linaro.org
(cherry picked from commit 747bfaf3a9d2f3cd51674763dc1f7575100cd200)
Signed-off-by: Michael Tokarev 

diff --git a/qemu-options.hx b/qemu-options.hx
index 379692da86..7f798ce47e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3968,7 +3968,8 @@ SRST
 This option can be used several times to simulate up to 4 serial
 ports.
 
-Use ``-serial none`` to disable all serial ports.
+You can use ``-serial none`` to suppress the creation of default
+serial devices.
 
 Available character devices are:
 
@@ -3990,10 +3991,17 @@ SRST
 [Linux only] Pseudo TTY (a new PTY is automatically allocated)
 
 ``none``
-No device is allocated.
+No device is allocated. Note that for machine types which
+emulate systems where a serial device is always present in
+real hardware, this may be equivalent to the ``null`` option,
+in that the serial device is still present but all output
+is discarded. For boards where the number of serial ports is
+truly variable, this suppresses the creation of the device.
 
 ``null``
-void device
+A guest will see the UART or serial device as present in the
+machine, but all output is discarded, and there is no input.
+Conceptually equivalent to redirecting the output to ``/dev/null``.
 
 ``chardev:id``
 Use a named character device defined with the ``-chardev``
-- 
2.39.2

[Stable-7.2.10 01/33] migration: Fix use-after-free of migration state object

[Michael Tokarev]

From: Fabiano Rosas 

We're currently allowing the process_incoming_migration_bh bottom-half
to run without holding a reference to the 'current_migration' object,
which leads to a segmentation fault if the BH is still live after
migration_shutdown() has dropped the last reference to
current_migration.

In my system the bug manifests as migrate_multifd() returning true
when it shouldn't and multifd_load_shutdown() calling
multifd_recv_terminate_threads() which crashes due to an uninitialized
multifd_recv_state.

Fix the issue by holding a reference to the object when scheduling the
BH and dropping it before returning from the BH. The same is already
done for the cleanup_bh at migrate_fd_cleanup_schedule().

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1969
Signed-off-by: Fabiano Rosas 
Link: https://lore.kernel.org/r/20240119233922.32588-2-faro...@suse.de
Signed-off-by: Peter Xu 
(cherry picked from commit 27eb8499edb2bc952c29ddae0bdac9fc959bf7b1)
Signed-off-by: Michael Tokarev 

diff --git a/migration/migration.c b/migration/migration.c
index c8ca7927b4..9b496cce1d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -572,6 +572,7 @@ static void process_incoming_migration_bh(void *opaque)
   MIGRATION_STATUS_COMPLETED);
 qemu_bh_delete(mis->bh);
 migration_incoming_state_destroy();
+object_unref(OBJECT(migrate_get_current()));
 }
 
 static void coroutine_fn
@@ -638,6 +639,7 @@ process_incoming_migration_co(void *opaque)
 goto fail;
 }
 mis->bh = qemu_bh_new(process_incoming_migration_bh, mis);
+object_ref(OBJECT(migrate_get_current()));
 qemu_bh_schedule(mis->bh);
 mis->migration_incoming_co = NULL;
 return;
-- 
2.39.2

[Stable-7.2.10 13/33] hw/cxl: Pass CXLComponentState to cache_mem_ops

[Michael Tokarev]

From: Li Zhijian 

cache_mem_ops.{read,write}() interprets opaque as
CXLComponentState(cxl_cstate) instead of ComponentRegisters(cregs).

Fortunately, cregs is the first member of cxl_cstate, so their values are
the same.

Fixes: 9e58f52d3f8 ("hw/cxl/component: Introduce CXL components (8.1.x, 8.2.5)")
Reviewed-by: Fan Ni 
Signed-off-by: Li Zhijian 
Signed-off-by: Jonathan Cameron 
Message-Id: <20240126120132.24248-8-jonathan.came...@huawei.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit 729d45a6af06753d3e330f589c248fe9687c5cd5)
Signed-off-by: Michael Tokarev 

diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
index 3edd303a33..5934b95848 100644
--- a/hw/cxl/cxl-component-utils.c
+++ b/hw/cxl/cxl-component-utils.c
@@ -126,7 +126,7 @@ void cxl_component_register_block_init(Object *obj,
 /* io registers controls link which we don't care about in QEMU */
 memory_region_init_io(>io, obj, NULL, cregs, ".io",
   CXL2_COMPONENT_IO_REGION_SIZE);
-memory_region_init_io(>cache_mem, obj, _mem_ops, cregs,
+memory_region_init_io(>cache_mem, obj, _mem_ops, cxl_cstate,
   ".cache_mem", CXL2_COMPONENT_CM_REGION_SIZE);
 
 memory_region_add_subregion(>component_registers, 0, >io);
-- 
2.39.2

[Stable-7.2.10 17/33] hw/i386: Fix _STA return value for ACPI0017

[Michael Tokarev]

From: Jonathan Cameron 

Found whilst testing a series for the linux kernel that actually
bothers to check if enabled is set. 0xB is the option used
for vast majority of DSDT entries in QEMU.
It is a little odd for a device that doesn't really exist and
is simply a hook to tell the OS there is a CEDT table but 0xB
seems a reasonable choice and avoids need to special case
this device in the OS.

Means:
* Device present.
* Device enabled and decoding it's resources.
* Not shown in UI
* Functioning properly
* No battery (on this device!)

Signed-off-by: Jonathan Cameron 
Message-Id: <20240126120132.24248-12-jonathan.came...@huawei.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit d9ae5802f656f6fb53b788747ba557a826b6e740)
Signed-off-by: Michael Tokarev 

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index d9eaa5fc4d..f9cdacadb1 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1311,7 +1311,7 @@ static void build_acpi0017(Aml *table)
 aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0017")));
 
 method = aml_method("_STA", 0, AML_NOTSERIALIZED);
-aml_append(method, aml_return(aml_int(0x01)));
+aml_append(method, aml_return(aml_int(0x0B)));
 aml_append(dev, method);
 
 aml_append(scope, dev);
-- 
2.39.2

[Stable-7.2.10 08/33] hw/smbios: Fix OEM strings table option validation

[Michael Tokarev]

From: Akihiko Odaki 

qemu_smbios_type11_opts did not have the list terminator and that
resulted in out-of-bound memory access. It also needs to have an element
for the type option.

Cc: qemu-sta...@nongnu.org
Fixes: 2d6dcbf93fb0 ("smbios: support setting OEM strings table")
Signed-off-by: Akihiko Odaki 
Reviewed-by: Michael Tokarev 
Reviewed-by: Ani Sinha 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael Tokarev 
(cherry picked from commit cd8a35b913c24248267c682cb9a348461c106139)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index cd43185417..7a58d50d80 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -368,6 +368,11 @@ static const QemuOptDesc qemu_smbios_type8_opts[] = {
 };
 
 static const QemuOptDesc qemu_smbios_type11_opts[] = {
+{
+.name = "type",
+.type = QEMU_OPT_NUMBER,
+.help = "SMBIOS element type",
+},
 {
 .name = "value",
 .type = QEMU_OPT_STRING,
@@ -378,6 +383,7 @@ static const QemuOptDesc qemu_smbios_type11_opts[] = {
 .type = QEMU_OPT_STRING,
 .help = "OEM string data from file",
 },
+{ /* end of list */ }
 };
 
 static const QemuOptDesc qemu_smbios_type17_opts[] = {
-- 
2.39.2

[Stable-7.2.10 02/33] qemu-docs: Update options for graphical frontends

[Michael Tokarev]

From: Yihuan Pan 

The command line options `-ctrl-grab` and `-alt-grab` have been removed
in QEMU 7.1. Instead, use the `-display sdl,grab-mod=` option
to specify the grab modifiers.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2103
Signed-off-by: Yihuan Pan 
Signed-off-by: Michael Tokarev 
(cherry picked from commit db101376af52e81f740a27f5fa38260ad171323c)

diff --git a/docs/system/keys.rst.inc b/docs/system/keys.rst.inc
index bd9b8e5f6f..2e2c97aa23 100644
--- a/docs/system/keys.rst.inc
+++ b/docs/system/keys.rst.inc
@@ -1,8 +1,9 @@
-During the graphical emulation, you can use special key combinations to
-change modes. The default key mappings are shown below, but if you use
-``-alt-grab`` then the modifier is Ctrl-Alt-Shift (instead of Ctrl-Alt)
-and if you use ``-ctrl-grab`` then the modifier is the right Ctrl key
-(instead of Ctrl-Alt):
+During the graphical emulation, you can use special key combinations from
+the following table to change modes. By default the modifier is Ctrl-Alt
+(used in the table below) which can be changed with ``-display`` suboption
+``mod=`` where appropriate. For example, ``-display sdl,
+grab-mod=lshift-lctrl-lalt`` changes the modifier key to Ctrl-Alt-Shift,
+while ``-display sdl,grab-mod=rctrl`` changes it to the right Ctrl key.
 
 Ctrl-Alt-f
Toggle full screen
-- 
2.39.2

[Stable-7.2.10 04/33] target/arm: fix exception syndrome for AArch32 bkpt insn

[Michael Tokarev]

From: Jan Klötzke 

Debug exceptions that target AArch32 Hyp mode are reported differently
than on AAarch64. Internally, Qemu uses the AArch64 syndromes. Therefore
such exceptions need to be either converted to a prefetch abort
(breakpoints, vector catch) or a data abort (watchpoints).

Cc: qemu-sta...@nongnu.org
Signed-off-by: Jan Klötzke 
Reviewed-by: Richard Henderson 
Message-id: 20240127202758.3326381-1-jan.kloet...@kernkonzept.com
Signed-off-by: Peter Maydell 
(cherry picked from commit f670be1aad33e801779af580398895b9455747ee)
Signed-off-by: Michael Tokarev 

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 02cfeece45..343acfab3a 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9836,6 +9836,24 @@ static void arm_cpu_do_interrupt_aarch32(CPUState *cs)
 }
 
 if (env->exception.target_el == 2) {
+/* Debug exceptions are reported differently on AArch32 */
+switch (syn_get_ec(env->exception.syndrome)) {
+case EC_BREAKPOINT:
+case EC_BREAKPOINT_SAME_EL:
+case EC_AA32_BKPT:
+case EC_VECTORCATCH:
+env->exception.syndrome = syn_insn_abort(arm_current_el(env) == 2,
+ 0, 0, 0x22);
+break;
+case EC_WATCHPOINT:
+env->exception.syndrome = syn_set_ec(env->exception.syndrome,
+ EC_DATAABORT);
+break;
+case EC_WATCHPOINT_SAME_EL:
+env->exception.syndrome = syn_set_ec(env->exception.syndrome,
+ EC_DATAABORT_SAME_EL);
+break;
+}
 arm_cpu_do_interrupt_aarch32_hyp(cs);
 return;
 }
diff --git a/target/arm/syndrome.h b/target/arm/syndrome.h
index 15334a3d15..75a3327a30 100644
--- a/target/arm/syndrome.h
+++ b/target/arm/syndrome.h
@@ -25,6 +25,8 @@
 #ifndef TARGET_ARM_SYNDROME_H
 #define TARGET_ARM_SYNDROME_H
 
+#include "qemu/bitops.h"
+
 /* Valid Syndrome Register EC field values */
 enum arm_exception_class {
 EC_UNCATEGORIZED  = 0x00,
@@ -76,6 +78,7 @@ typedef enum {
 SME_ET_InactiveZA,
 } SMEExceptionType;
 
+#define ARM_EL_EC_LENGTH 6
 #define ARM_EL_EC_SHIFT 26
 #define ARM_EL_IL_SHIFT 25
 #define ARM_EL_ISV_SHIFT 24
@@ -87,6 +90,11 @@ static inline uint32_t syn_get_ec(uint32_t syn)
 return syn >> ARM_EL_EC_SHIFT;
 }
 
+static inline uint32_t syn_set_ec(uint32_t syn, uint32_t ec)
+{
+return deposit32(syn, ARM_EL_EC_SHIFT, ARM_EL_EC_LENGTH, ec);
+}
+
 /*
  * Utility functions for constructing various kinds of syndrome value.
  * Note that in general we follow the AArch64 syndrome values; in a
-- 
2.39.2

[Stable-7.2.10 09/33] hw/smbios: Fix port connector option validation

[Michael Tokarev]

From: Akihiko Odaki 

qemu_smbios_type8_opts did not have the list terminator and that
resulted in out-of-bound memory access. It also needs to have an element
for the type option.

Cc: qemu-sta...@nongnu.org
Fixes: fd8caa253c56 ("hw/smbios: support for type 8 (port connector)")
Signed-off-by: Akihiko Odaki 
Reviewed-by: Michael Tokarev 
Reviewed-by: Ani Sinha 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael Tokarev 
(cherry picked from commit 196578c9d051d19c23e6c13e97b791a41b318315)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 7a58d50d80..9f4d007d96 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -345,6 +345,11 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = {
 };
 
 static const QemuOptDesc qemu_smbios_type8_opts[] = {
+{
+.name = "type",
+.type = QEMU_OPT_NUMBER,
+.help = "SMBIOS element type",
+},
 {
 .name = "internal_reference",
 .type = QEMU_OPT_STRING,
@@ -365,6 +370,7 @@ static const QemuOptDesc qemu_smbios_type8_opts[] = {
 .type = QEMU_OPT_NUMBER,
 .help = "port type",
 },
+{ /* end of list */ }
 };
 
 static const QemuOptDesc qemu_smbios_type11_opts[] = {
-- 
2.39.2

Re: [RFC PATCH v2 18/22] hw/arm/virt: Add FEAT_GICv3_NMI feature support in virt GIC

[Richard Henderson]

On 2/21/24 03:08, Jinjie Ruan via wrote:

Included support FEAT_GICv3_NMI feature as part of virt platform
GIC initialization.

Signed-off-by: Jinjie Ruan 
---
  hw/arm/virt.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index c442652d0f..0359dbd8bd 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -772,6 +772,8 @@ static void create_gic(VirtMachineState *vms, MemoryRegion 
*mem)
  qdev_prop_set_array(vms->gic, "redist-region-count",
  redist_region_count);
  
+qdev_prop_set_bit(vms->gic, "has-nmi", true);


This should be set based on whether the cpu class created has FEAT_NMI.


r~

Re: [RFC PATCH v2 16/22] hw/intc/arm_gicv3: Implement GICD_INMIR

[Richard Henderson]

On 2/21/24 03:08, Jinjie Ruan via wrote:

Add GICD_INMIR0, GICD_INMIRnE register and support access GICD_INMIR0.

Signed-off-by: Jinjie Ruan
---
  hw/intc/arm_gicv3_dist.c | 38 ++
  hw/intc/gicv3_internal.h |  2 ++
  2 files changed, 40 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [RFC PATCH v2 08/22] target/arm: Handle IS/FS in ISR_EL1 for NMI

[Richard Henderson]

On 2/21/24 03:08, Jinjie Ruan via wrote:

Add IS and FS bit in ISR_EL1 and handle the read according to whether the
NMI is IRQ or FIQ.

Signed-off-by: Jinjie Ruan 
---
  target/arm/cpu.h| 2 ++
  target/arm/helper.c | 9 +
  2 files changed, 11 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 051e589e19..e2d07e3312 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1476,6 +1476,8 @@ FIELD(CPTR_EL3, TCPAC, 31, 1)
  #define CPSR_N (1U << 31)
  #define CPSR_NZCV (CPSR_N | CPSR_Z | CPSR_C | CPSR_V)
  #define CPSR_AIF (CPSR_A | CPSR_I | CPSR_F)
+#define ISR_FS (1U << 9)
+#define ISR_IS (1U << 10)
  
  #define CPSR_IT (CPSR_IT_0_1 | CPSR_IT_2_7)

  #define CACHED_CPSR_BITS (CPSR_T | CPSR_AIF | CPSR_GE | CPSR_IT | CPSR_Q \
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 0bd7a87e51..62c8e5d611 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2022,6 +2022,10 @@ static uint64_t isr_read(CPUARMState *env, const 
ARMCPRegInfo *ri)
  if (cs->interrupt_request & CPU_INTERRUPT_HARD) {
  ret |= CPSR_I;
  }
+
+if ((cs->interrupt_request & CPU_INTERRUPT_NMI) && env->nmi_is_irq) {
+ret |= ISR_IS;
+}
  }
  
  if (hcr_el2 & HCR_FMO) {

@@ -2032,6 +2036,11 @@ static uint64_t isr_read(CPUARMState *env, const 
ARMCPRegInfo *ri)
  if (cs->interrupt_request & CPU_INTERRUPT_FIQ) {
  ret |= CPSR_F;
  }
+
+if ((cs->interrupt_request & CPU_INTERRUPT_NMI) &&
+(!env->nmi_is_irq)) {
+ret |= ISR_FS;
+}
  }


The external CPU_INTERRUPT_NMI will never signal FIQ.

With CPU_INTERRUPT_NMI, both CPSR_I and ISR_IS must be set.

Missing is the handling of HCRX_EL2.{VFNMI,VINMI} to signal vFIQ and vIRQ with 
superpriority.  Unless I missed it, I don't see HCRX_EL2 adjusted for FEAT_NMI at all.



r~

[PATCH v6 2/9] trans_rvv.c.inc: remove 'is_store' bool from load/store fns

[Daniel Henrique Barboza]

After the 'mark_vs_dirty' changes from the previous patch the 'is_store'
bool is unused in all load/store functions that were changed. Remove it.

Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
---
 target/riscv/insn_trans/trans_rvv.c.inc | 69 -
 1 file changed, 34 insertions(+), 35 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index 7a98f1caa6..15ccebf3fc 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -609,8 +609,7 @@ typedef void gen_helper_ldst_us(TCGv_ptr, TCGv_ptr, TCGv,
 TCGv_env, TCGv_i32);
 
 static bool ldst_us_trans(uint32_t vd, uint32_t rs1, uint32_t data,
-  gen_helper_ldst_us *fn, DisasContext *s,
-  bool is_store)
+  gen_helper_ldst_us *fn, DisasContext *s)
 {
 TCGv_ptr dest, mask;
 TCGv base;
@@ -673,7 +672,7 @@ static bool ld_us_op(DisasContext *s, arg_r2nfvm *a, 
uint8_t eew)
 data = FIELD_DP32(data, VDATA, NF, a->nf);
 data = FIELD_DP32(data, VDATA, VTA, s->vta);
 data = FIELD_DP32(data, VDATA, VMA, s->vma);
-return ldst_us_trans(a->rd, a->rs1, data, fn, s, false);
+return ldst_us_trans(a->rd, a->rs1, data, fn, s);
 }
 
 static bool ld_us_check(DisasContext *s, arg_r2nfvm* a, uint8_t eew)
@@ -710,7 +709,7 @@ static bool st_us_op(DisasContext *s, arg_r2nfvm *a, 
uint8_t eew)
 data = FIELD_DP32(data, VDATA, VM, a->vm);
 data = FIELD_DP32(data, VDATA, LMUL, emul);
 data = FIELD_DP32(data, VDATA, NF, a->nf);
-return ldst_us_trans(a->rd, a->rs1, data, fn, s, true);
+return ldst_us_trans(a->rd, a->rs1, data, fn, s);
 }
 
 static bool st_us_check(DisasContext *s, arg_r2nfvm* a, uint8_t eew)
@@ -739,7 +738,7 @@ static bool ld_us_mask_op(DisasContext *s, arg_vlm_v *a, 
uint8_t eew)
 /* Mask destination register are always tail-agnostic */
 data = FIELD_DP32(data, VDATA, VTA, s->cfg_vta_all_1s);
 data = FIELD_DP32(data, VDATA, VMA, s->vma);
-return ldst_us_trans(a->rd, a->rs1, data, fn, s, false);
+return ldst_us_trans(a->rd, a->rs1, data, fn, s);
 }
 
 static bool ld_us_mask_check(DisasContext *s, arg_vlm_v *a, uint8_t eew)
@@ -756,7 +755,7 @@ static bool st_us_mask_op(DisasContext *s, arg_vsm_v *a, 
uint8_t eew)
 /* EMUL = 1, NFIELDS = 1 */
 data = FIELD_DP32(data, VDATA, LMUL, 0);
 data = FIELD_DP32(data, VDATA, NF, 1);
-return ldst_us_trans(a->rd, a->rs1, data, fn, s, true);
+return ldst_us_trans(a->rd, a->rs1, data, fn, s);
 }
 
 static bool st_us_mask_check(DisasContext *s, arg_vsm_v *a, uint8_t eew)
@@ -776,7 +775,7 @@ typedef void gen_helper_ldst_stride(TCGv_ptr, TCGv_ptr, 
TCGv,
 
 static bool ldst_stride_trans(uint32_t vd, uint32_t rs1, uint32_t rs2,
   uint32_t data, gen_helper_ldst_stride *fn,
-  DisasContext *s, bool is_store)
+  DisasContext *s)
 {
 TCGv_ptr dest, mask;
 TCGv base, stride;
@@ -823,7 +822,7 @@ static bool ld_stride_op(DisasContext *s, arg_rnfvm *a, 
uint8_t eew)
 data = FIELD_DP32(data, VDATA, NF, a->nf);
 data = FIELD_DP32(data, VDATA, VTA, s->vta);
 data = FIELD_DP32(data, VDATA, VMA, s->vma);
-return ldst_stride_trans(a->rd, a->rs1, a->rs2, data, fn, s, false);
+return ldst_stride_trans(a->rd, a->rs1, a->rs2, data, fn, s);
 }
 
 static bool ld_stride_check(DisasContext *s, arg_rnfvm* a, uint8_t eew)
@@ -857,7 +856,7 @@ static bool st_stride_op(DisasContext *s, arg_rnfvm *a, 
uint8_t eew)
 return false;
 }
 
-return ldst_stride_trans(a->rd, a->rs1, a->rs2, data, fn, s, true);
+return ldst_stride_trans(a->rd, a->rs1, a->rs2, data, fn, s);
 }
 
 static bool st_stride_check(DisasContext *s, arg_rnfvm* a, uint8_t eew)
@@ -880,7 +879,7 @@ typedef void gen_helper_ldst_index(TCGv_ptr, TCGv_ptr, TCGv,
 
 static bool ldst_index_trans(uint32_t vd, uint32_t rs1, uint32_t vs2,
  uint32_t data, gen_helper_ldst_index *fn,
- DisasContext *s, bool is_store)
+ DisasContext *s)
 {
 TCGv_ptr dest, mask, index;
 TCGv base;
@@ -947,7 +946,7 @@ static bool ld_index_op(DisasContext *s, arg_rnfvm *a, 
uint8_t eew)
 data = FIELD_DP32(data, VDATA, NF, a->nf);
 data = FIELD_DP32(data, VDATA, VTA, s->vta);
 data = FIELD_DP32(data, VDATA, VMA, s->vma);
-return ldst_index_trans(a->rd, a->rs1, a->rs2, data, fn, s, false);
+return ldst_index_trans(a->rd, a->rs1, a->rs2, data, fn, s);
 }
 
 static bool ld_index_check(DisasContext *s, arg_rnfvm* a, uint8_t eew)
@@ -999,7 +998,7 @@ static bool st_index_op(DisasContext *s, arg_rnfvm *a, 
uint8_t eew)
 data = FIELD_DP32(data, VDATA, VM, a->vm);
 data = FIELD_DP32(data, VDATA, LMUL, emul);
 data = 

[PATCH v6 8/9] target/riscv: Clear vstart_qe_zero flag

[Daniel Henrique Barboza]

From: Ivan Klokov 

The vstart_qe_zero flag is set at the beginning of the translation
phase from the env->vstart variable. During the execution phase all
functions will set env->vstart = 0 after a successful execution,
but the vstart_eq_zero flag remains the same as at the start of the
block. This will wrongly cause SIGILLs in translations that requires
env->vstart = 0 and might be reading vstart_eq_zero = false.

This patch adds a new finalize_rvv_inst() helper that is called at the
end of each vector instruction that will both update vstart_eq_zero and
do a mark_vs_dirty().

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1976
Signed-off-by: Ivan Klokov 
Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Richard Henderson 
---
 target/riscv/insn_trans/trans_rvbf16.c.inc |  6 +-
 target/riscv/insn_trans/trans_rvv.c.inc| 78 --
 target/riscv/insn_trans/trans_rvvk.c.inc   | 12 ++--
 target/riscv/translate.c   |  6 ++
 4 files changed, 56 insertions(+), 46 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvbf16.c.inc 
b/target/riscv/insn_trans/trans_rvbf16.c.inc
index a842e76a6b..0a9cd1ec31 100644
--- a/target/riscv/insn_trans/trans_rvbf16.c.inc
+++ b/target/riscv/insn_trans/trans_rvbf16.c.inc
@@ -83,7 +83,7 @@ static bool trans_vfncvtbf16_f_f_w(DisasContext *ctx, 
arg_vfncvtbf16_f_f_w *a)
ctx->cfg_ptr->vlenb,
ctx->cfg_ptr->vlenb, data,
gen_helper_vfncvtbf16_f_f_w);
-mark_vs_dirty(ctx);
+finalize_rvv_inst(ctx);
 return true;
 }
 return false;
@@ -108,7 +108,7 @@ static bool trans_vfwcvtbf16_f_f_v(DisasContext *ctx, 
arg_vfwcvtbf16_f_f_v *a)
ctx->cfg_ptr->vlenb,
ctx->cfg_ptr->vlenb, data,
gen_helper_vfwcvtbf16_f_f_v);
-mark_vs_dirty(ctx);
+finalize_rvv_inst(ctx);
 return true;
 }
 return false;
@@ -135,7 +135,7 @@ static bool trans_vfwmaccbf16_vv(DisasContext *ctx, 
arg_vfwmaccbf16_vv *a)
ctx->cfg_ptr->vlenb,
ctx->cfg_ptr->vlenb, data,
gen_helper_vfwmaccbf16_vv);
-mark_vs_dirty(ctx);
+finalize_rvv_inst(ctx);
 return true;
 }
 return false;
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index db08efa278..1933a6f5c2 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -167,7 +167,7 @@ static bool do_vsetvl(DisasContext *s, int rd, int rs1, 
TCGv s2)
 
 gen_helper_vsetvl(dst, tcg_env, s1, s2);
 gen_set_gpr(s, rd, dst);
-mark_vs_dirty(s);
+finalize_rvv_inst(s);
 
 gen_update_pc(s, s->cur_insn_len);
 lookup_and_goto_ptr(s);
@@ -187,7 +187,7 @@ static bool do_vsetivli(DisasContext *s, int rd, TCGv s1, 
TCGv s2)
 
 gen_helper_vsetvl(dst, tcg_env, s1, s2);
 gen_set_gpr(s, rd, dst);
-mark_vs_dirty(s);
+finalize_rvv_inst(s);
 gen_update_pc(s, s->cur_insn_len);
 lookup_and_goto_ptr(s);
 s->base.is_jmp = DISAS_NORETURN;
@@ -636,6 +636,7 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1, 
uint32_t data,
 
 fn(dest, mask, base, tcg_env, desc);
 
+finalize_rvv_inst(s);
 return true;
 }
 
@@ -791,6 +792,7 @@ static bool ldst_stride_trans(uint32_t vd, uint32_t rs1, 
uint32_t rs2,
 
 fn(dest, mask, base, stride, tcg_env, desc);
 
+finalize_rvv_inst(s);
 return true;
 }
 
@@ -892,6 +894,7 @@ static bool ldst_index_trans(uint32_t vd, uint32_t rs1, 
uint32_t vs2,
 
 fn(dest, mask, base, index, tcg_env, desc);
 
+finalize_rvv_inst(s);
 return true;
 }
 
@@ -1022,7 +1025,7 @@ static bool ldff_trans(uint32_t vd, uint32_t rs1, 
uint32_t data,
 
 fn(dest, mask, base, tcg_env, desc);
 
-mark_vs_dirty(s);
+finalize_rvv_inst(s);
 return true;
 }
 
@@ -1079,6 +1082,7 @@ static bool ldst_whole_trans(uint32_t vd, uint32_t rs1, 
uint32_t nf,
 
 fn(dest, base, tcg_env, desc);
 
+finalize_rvv_inst(s);
 return true;
 }
 
@@ -1168,7 +1172,7 @@ do_opivv_gvec(DisasContext *s, arg_rmrr *a, GVecGen3Fn 
*gvec_fn,
tcg_env, s->cfg_ptr->vlenb,
s->cfg_ptr->vlenb, data, fn);
 }
-mark_vs_dirty(s);
+finalize_rvv_inst(s);
 return true;
 }
 
@@ -1219,7 +1223,7 @@ static bool opivx_trans(uint32_t vd, uint32_t rs1, 
uint32_t vs2, uint32_t vm,
 
 fn(dest, mask, src1, src2, tcg_env, desc);
 
-mark_vs_dirty(s);
+finalize_rvv_inst(s);
 return true;
 }
 
@@ -1244,7 +1248,7 @@ do_opivx_gvec(DisasContext *s, arg_rmrr *a, GVecGen2sFn 
*gvec_fn,
 gvec_fn(s->sew, vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2),
 src1, MAXSZ(s), MAXSZ(s));
 
-mark_vs_dirty(s);
+finalize_rvv_inst(s);
 return true;
 }
 return 

[PATCH v6 5/9] target/riscv: remove 'cpu_vl' global

[Daniel Henrique Barboza]

At this moment the global is used only in do_vsetvl(). Do a direct env
load in do_vsetvl() to read 'vl' and remove the global.

Suggested-by: Richard Henderson 
Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/insn_trans/trans_rvv.c.inc | 2 +-
 target/riscv/translate.c| 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index 58299d9bb8..69f32d081e 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -157,7 +157,7 @@ static bool do_vsetvl(DisasContext *s, int rd, int rs1, 
TCGv s2)
 
 if (rd == 0 && rs1 == 0) {
 s1 = tcg_temp_new();
-tcg_gen_mov_tl(s1, cpu_vl);
+tcg_gen_ld_tl(s1, tcg_env, offsetof(CPURISCVState, vl));
 } else if (rs1 == 0) {
 /* As the mask is at least one bit, RV_VLEN_MAX is >= VLMAX */
 s1 = tcg_constant_tl(RV_VLEN_MAX);
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index f2f0593830..3040f5e0e4 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -38,7 +38,7 @@
 #undef  HELPER_H
 
 /* global register indices */
-static TCGv cpu_gpr[32], cpu_gprh[32], cpu_pc, cpu_vl;
+static TCGv cpu_gpr[32], cpu_gprh[32], cpu_pc;
 static TCGv_i64 cpu_fpr[32]; /* assume F and D extensions */
 static TCGv load_res;
 static TCGv load_val;
@@ -1320,7 +1320,6 @@ void riscv_translate_init(void)
 }
 
 cpu_pc = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, pc), "pc");
-cpu_vl = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, vl), "vl");
 load_res = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, load_res),
  "load_res");
 load_val = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, load_val),
-- 
2.43.2

[PATCH v6 6/9] target/riscv/vector_helper.c: set vstart = 0 in GEN_VEXT_VSLIDEUP_VX()

[Daniel Henrique Barboza]

The helper isn't setting env->vstart = 0 after its execution, as it is
expected from every vector instruction that completes successfully.

Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Richard Henderson 
---
 target/riscv/vector_helper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 84cec73eb2..cc7290a1bb 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -4782,6 +4782,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, 
void *vs2, \
 } \
 *((ETYPE *)vd + H(i)) = *((ETYPE *)vs2 + H(i - offset));  \
 } \
+env->vstart = 0;  \
 /* set tail elements to 1s */ \
 vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz);  \
 }
-- 
2.43.2

[PATCH v6 7/9] trans_rvv.c.inc: remove redundant mark_vs_dirty() calls

[Daniel Henrique Barboza]

trans_vmv_v_i , trans_vfmv_v_f and the trans_##NAME macro from
GEN_VMV_WHOLE_TRANS() are calling mark_vs_dirty() in both branches of
their 'ifs'. conditionals.

Call it just once in the end like other functions are doing.

Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Richard Henderson 
---
 target/riscv/insn_trans/trans_rvv.c.inc | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index 69f32d081e..db08efa278 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -2044,7 +2044,6 @@ static bool trans_vmv_v_i(DisasContext *s, arg_vmv_v_i *a)
 if (s->vl_eq_vlmax && !(s->vta && s->lmul < 0)) {
 tcg_gen_gvec_dup_imm(s->sew, vreg_ofs(s, a->rd),
  MAXSZ(s), MAXSZ(s), simm);
-mark_vs_dirty(s);
 } else {
 TCGv_i32 desc;
 TCGv_i64 s1;
@@ -2062,9 +2061,8 @@ static bool trans_vmv_v_i(DisasContext *s, arg_vmv_v_i *a)
   s->cfg_ptr->vlenb, data));
 tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, a->rd));
 fns[s->sew](dest, s1, tcg_env, desc);
-
-mark_vs_dirty(s);
 }
+mark_vs_dirty(s);
 return true;
 }
 return false;
@@ -2591,7 +2589,6 @@ static bool trans_vfmv_v_f(DisasContext *s, arg_vfmv_v_f 
*a)
 
 tcg_gen_gvec_dup_i64(s->sew, vreg_ofs(s, a->rd),
  MAXSZ(s), MAXSZ(s), t1);
-mark_vs_dirty(s);
 } else {
 TCGv_ptr dest;
 TCGv_i32 desc;
@@ -2614,9 +2611,8 @@ static bool trans_vfmv_v_f(DisasContext *s, arg_vfmv_v_f 
*a)
 tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, a->rd));
 
 fns[s->sew - 1](dest, t1, tcg_env, desc);
-
-mark_vs_dirty(s);
 }
+mark_vs_dirty(s);
 return true;
 }
 return false;
@@ -3524,12 +3520,11 @@ static bool trans_##NAME(DisasContext *s, arg_##NAME * 
a)   \
 if (s->vstart_eq_zero) {\
 tcg_gen_gvec_mov(s->sew, vreg_ofs(s, a->rd),\
  vreg_ofs(s, a->rs2), maxsz, maxsz);\
-mark_vs_dirty(s);   \
 } else {\
 tcg_gen_gvec_2_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2), \
tcg_env, maxsz, maxsz, 0, gen_helper_vmvr_v); \
-mark_vs_dirty(s);   \
 }   \
+mark_vs_dirty(s);   \
 return true;\
 }   \
 return false;   \
-- 
2.43.2

[PATCH v6 0/9] riscv: set vstart_eq_zero on mark_vs_dirty

[Daniel Henrique Barboza]

Hi,

In this version 2 new patches were added:

- patch 5 eliminates the 'cpu_vl' global, and do_vsetvl() now loads 'vl'
  directly from env. This was suggested by Richard in the v5 review;

- patch 9 does a change in how we're doing the loops in ldst helpers.
  This was also proposed by Richard but back in v2. 

Patch 9 is not related to what we're fixing here but let's fold it in
and avoid leaving any code suggestions behind.

Series based on alistair/riscv-to-apply.next. 

Patches missing acks/reviews: 5 and 9

Changes from v5:
- patch 5 (new): remove 'cpu_vl' global
- patch 9 (new): change the loop in ldst helpers
- v5 link: 
https://lore.kernel.org/qemu-riscv/20240221022252.252872-1-dbarb...@ventanamicro.com/

Daniel Henrique Barboza (8):
  trans_rvv.c.inc: mark_vs_dirty() before loads and stores
  trans_rvv.c.inc: remove 'is_store' bool from load/store fns
  target/riscv: remove 'over' brconds from vector trans
  target/riscv/translate.c: remove 'cpu_vstart' global
  target/riscv: remove 'cpu_vl' global
  target/riscv/vector_helper.c: set vstart = 0 in GEN_VEXT_VSLIDEUP_VX()
  trans_rvv.c.inc: remove redundant mark_vs_dirty() calls
  target/riscv/vector_helper.c: optimize loops in ldst helpers

Ivan Klokov (1):
  target/riscv: Clear vstart_qe_zero flag

 target/riscv/insn_trans/trans_rvbf16.c.inc |  18 +-
 target/riscv/insn_trans/trans_rvv.c.inc| 294 ++---
 target/riscv/insn_trans/trans_rvvk.c.inc   |  30 +--
 target/riscv/translate.c   |  11 +-
 target/riscv/vector_helper.c   |   7 +-
 5 files changed, 104 insertions(+), 256 deletions(-)

-- 
2.43.2

[PATCH v6 9/9] target/riscv/vector_helper.c: optimize loops in ldst helpers

[Daniel Henrique Barboza]

Change the for loops in ldst helpers to do a single increment in the
counter, and assign it env->vstart, to avoid re-reading from vstart
every time.

Suggested-by: Richard Henderson 
Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/vector_helper.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index cc7290a1bb..1ab386830a 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -208,7 +208,7 @@ vext_ldst_stride(void *vd, void *v0, target_ulong base,
 uint32_t esz = 1 << log2_esz;
 uint32_t vma = vext_vma(desc);
 
-for (i = env->vstart; i < env->vl; i++, env->vstart++) {
+for (i = env->vstart; i < env->vl; env->vstart = ++i) {
 k = 0;
 while (k < nf) {
 if (!vm && !vext_elem_mask(v0, i)) {
@@ -274,7 +274,7 @@ vext_ldst_us(void *vd, target_ulong base, CPURISCVState 
*env, uint32_t desc,
 uint32_t esz = 1 << log2_esz;
 
 /* load bytes from guest memory */
-for (i = env->vstart; i < evl; i++, env->vstart++) {
+for (i = env->vstart; i < evl; env->vstart = ++i) {
 k = 0;
 while (k < nf) {
 target_ulong addr = base + ((i * nf + k) << log2_esz);
@@ -388,7 +388,7 @@ vext_ldst_index(void *vd, void *v0, target_ulong base,
 uint32_t vma = vext_vma(desc);
 
 /* load bytes from guest memory */
-for (i = env->vstart; i < env->vl; i++, env->vstart++) {
+for (i = env->vstart; i < env->vl; env->vstart = ++i) {
 k = 0;
 while (k < nf) {
 if (!vm && !vext_elem_mask(v0, i)) {
-- 
2.43.2

[PATCH v6 4/9] target/riscv/translate.c: remove 'cpu_vstart' global

[Daniel Henrique Barboza]

The global is unused after recent changes.

Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Richard Henderson 
---
 target/riscv/translate.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 177418b2b9..f2f0593830 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -38,7 +38,7 @@
 #undef  HELPER_H
 
 /* global register indices */
-static TCGv cpu_gpr[32], cpu_gprh[32], cpu_pc, cpu_vl, cpu_vstart;
+static TCGv cpu_gpr[32], cpu_gprh[32], cpu_pc, cpu_vl;
 static TCGv_i64 cpu_fpr[32]; /* assume F and D extensions */
 static TCGv load_res;
 static TCGv load_val;
@@ -1321,8 +1321,6 @@ void riscv_translate_init(void)
 
 cpu_pc = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, pc), "pc");
 cpu_vl = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, vl), "vl");
-cpu_vstart = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, vstart),
-"vstart");
 load_res = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, load_res),
  "load_res");
 load_val = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, load_val),
-- 
2.43.2

[PATCH v6 3/9] target/riscv: remove 'over' brconds from vector trans

[Daniel Henrique Barboza]

Most of the vector translations has this following pattern at the start:

TCGLabel *over = gen_new_label();
tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);

And then right at the end:

 gen_set_label(over);
 return true;

This means that if vstart >= vl we'll not set vstart = 0 at the end of
the insns - this is done inside the helper that is being skipped.  The
reason why this pattern hasn't been a bigger problem is because the
conditional vstart >= vl is very rare.

Checking all the helpers in vector_helper.c we see all of them with a
pattern like this:

for (i = env->vstart; i < vl; i++) {
(...)
}
env->vstart = 0;

Thus they can handle vstart >= vl case gracefully, with the benefit of
setting env->vstart = 0 during the process.

Remove all 'over' conditionals and let the helper set env->vstart = 0
every time.

While we're at it, remove the (vl == 0) brconds from trans_rvbf16.c.inc
too since they're unneeded.

Suggested-by: Richard Henderson 
Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Richard Henderson 
---
 target/riscv/insn_trans/trans_rvbf16.c.inc |  12 ---
 target/riscv/insn_trans/trans_rvv.c.inc| 117 -
 target/riscv/insn_trans/trans_rvvk.c.inc   |  18 
 3 files changed, 147 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvbf16.c.inc 
b/target/riscv/insn_trans/trans_rvbf16.c.inc
index 8ee99df3f3..a842e76a6b 100644
--- a/target/riscv/insn_trans/trans_rvbf16.c.inc
+++ b/target/riscv/insn_trans/trans_rvbf16.c.inc
@@ -71,11 +71,8 @@ static bool trans_vfncvtbf16_f_f_w(DisasContext *ctx, 
arg_vfncvtbf16_f_f_w *a)
 
 if (opfv_narrow_check(ctx, a) && (ctx->sew == MO_16)) {
 uint32_t data = 0;
-TCGLabel *over = gen_new_label();
 
 gen_set_rm_chkfrm(ctx, RISCV_FRM_DYN);
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 data = FIELD_DP32(data, VDATA, VM, a->vm);
 data = FIELD_DP32(data, VDATA, LMUL, ctx->lmul);
@@ -87,7 +84,6 @@ static bool trans_vfncvtbf16_f_f_w(DisasContext *ctx, 
arg_vfncvtbf16_f_f_w *a)
ctx->cfg_ptr->vlenb, data,
gen_helper_vfncvtbf16_f_f_w);
 mark_vs_dirty(ctx);
-gen_set_label(over);
 return true;
 }
 return false;
@@ -100,11 +96,8 @@ static bool trans_vfwcvtbf16_f_f_v(DisasContext *ctx, 
arg_vfwcvtbf16_f_f_v *a)
 
 if (opfv_widen_check(ctx, a) && (ctx->sew == MO_16)) {
 uint32_t data = 0;
-TCGLabel *over = gen_new_label();
 
 gen_set_rm_chkfrm(ctx, RISCV_FRM_DYN);
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 data = FIELD_DP32(data, VDATA, VM, a->vm);
 data = FIELD_DP32(data, VDATA, LMUL, ctx->lmul);
@@ -116,7 +109,6 @@ static bool trans_vfwcvtbf16_f_f_v(DisasContext *ctx, 
arg_vfwcvtbf16_f_f_v *a)
ctx->cfg_ptr->vlenb, data,
gen_helper_vfwcvtbf16_f_f_v);
 mark_vs_dirty(ctx);
-gen_set_label(over);
 return true;
 }
 return false;
@@ -130,11 +122,8 @@ static bool trans_vfwmaccbf16_vv(DisasContext *ctx, 
arg_vfwmaccbf16_vv *a)
 if (require_rvv(ctx) && vext_check_isa_ill(ctx) && (ctx->sew == MO_16) &&
 vext_check_dss(ctx, a->rd, a->rs1, a->rs2, a->vm)) {
 uint32_t data = 0;
-TCGLabel *over = gen_new_label();
 
 gen_set_rm_chkfrm(ctx, RISCV_FRM_DYN);
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 data = FIELD_DP32(data, VDATA, VM, a->vm);
 data = FIELD_DP32(data, VDATA, LMUL, ctx->lmul);
@@ -147,7 +136,6 @@ static bool trans_vfwmaccbf16_vv(DisasContext *ctx, 
arg_vfwmaccbf16_vv *a)
ctx->cfg_ptr->vlenb, data,
gen_helper_vfwmaccbf16_vv);
 mark_vs_dirty(ctx);
-gen_set_label(over);
 return true;
 }
 return false;
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index 15ccebf3fc..58299d9bb8 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -615,9 +615,6 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1, 
uint32_t data,
 TCGv base;
 TCGv_i32 desc;
 
-TCGLabel *over = gen_new_label();
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
-
 dest = tcg_temp_new_ptr();
 mask = tcg_temp_new_ptr();
 base = get_gpr(s, rs1, EXT_NONE);
@@ -639,7 +636,6 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1, 
uint32_t data,
 
 fn(dest, mask, base, tcg_env, desc);
 
-gen_set_label(over);
 return true;
 }
 
@@ -781,9 +777,6 @@ static bool ldst_stride_trans(uint32_t vd, uint32_t rs1, 
uint32_t rs2,
 TCGv base, stride;
 

[PATCH v6 1/9] trans_rvv.c.inc: mark_vs_dirty() before loads and stores

[Daniel Henrique Barboza]

While discussing a problem with how we're (not) setting vstart_eq_zero
Richard had the following to say w.r.t the conditional mark_vs_dirty()
calls on load/store functions [1]:

"I think it's required to have stores set dirty unconditionally, before
the operation.

Consider a store that traps on the 2nd element, leaving vstart = 2, and
exiting to the main loop via exception. The exception enters the kernel
page fault handler. The kernel may need to fault in the page for the
process, and in the meantime task switch.

If vs dirty is not already set, the kernel won't know to save vector
state on task switch."

Do a mark_vs_dirty() before both loads and stores.

[1] 
https://lore.kernel.org/qemu-riscv/72c7503b-0f43-44b8-aa82-fbafed2aa...@linaro.org/

Suggested-by: Richard Henderson 
Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Richard Henderson 
---
 target/riscv/insn_trans/trans_rvv.c.inc | 23 ---
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index 9e101ab434..7a98f1caa6 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -636,11 +636,9 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1, 
uint32_t data,
 tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, vd));
 tcg_gen_addi_ptr(mask, tcg_env, vreg_ofs(s, 0));
 
-fn(dest, mask, base, tcg_env, desc);
+mark_vs_dirty(s);
 
-if (!is_store) {
-mark_vs_dirty(s);
-}
+fn(dest, mask, base, tcg_env, desc);
 
 gen_set_label(over);
 return true;
@@ -797,11 +795,9 @@ static bool ldst_stride_trans(uint32_t vd, uint32_t rs1, 
uint32_t rs2,
 tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, vd));
 tcg_gen_addi_ptr(mask, tcg_env, vreg_ofs(s, 0));
 
-fn(dest, mask, base, stride, tcg_env, desc);
+mark_vs_dirty(s);
 
-if (!is_store) {
-mark_vs_dirty(s);
-}
+fn(dest, mask, base, stride, tcg_env, desc);
 
 gen_set_label(over);
 return true;
@@ -904,11 +900,9 @@ static bool ldst_index_trans(uint32_t vd, uint32_t rs1, 
uint32_t vs2,
 tcg_gen_addi_ptr(index, tcg_env, vreg_ofs(s, vs2));
 tcg_gen_addi_ptr(mask, tcg_env, vreg_ofs(s, 0));
 
-fn(dest, mask, base, index, tcg_env, desc);
+mark_vs_dirty(s);
 
-if (!is_store) {
-mark_vs_dirty(s);
-}
+fn(dest, mask, base, index, tcg_env, desc);
 
 gen_set_label(over);
 return true;
@@ -1102,11 +1096,10 @@ static bool ldst_whole_trans(uint32_t vd, uint32_t rs1, 
uint32_t nf,
 base = get_gpr(s, rs1, EXT_NONE);
 tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, vd));
 
+mark_vs_dirty(s);
+
 fn(dest, base, tcg_env, desc);
 
-if (!is_store) {
-mark_vs_dirty(s);
-}
 gen_set_label(over);
 
 return true;
-- 
2.43.2

[PULL 06/25] hw/ppc/ppc440_pcix: Move ppc440_pcix.c to hw/pci-host/

[Philippe Mathieu-Daudé]

ppc440_pcix.c is moved from the target specific ppc_ss[] meson
source set to pci_ss[] which is common to all targets: the
object is built once.

Reviewed-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20240215105017.57748-5-phi...@linaro.org>
---
 MAINTAINERS| 2 +-
 hw/{ppc => pci-host}/ppc440_pcix.c | 0
 hw/pci-host/Kconfig| 4 
 hw/pci-host/meson.build| 1 +
 hw/pci-host/trace-events   | 8 
 hw/ppc/Kconfig | 1 +
 hw/ppc/meson.build | 2 +-
 hw/ppc/trace-events| 8 
 8 files changed, 16 insertions(+), 10 deletions(-)
 rename hw/{ppc => pci-host}/ppc440_pcix.c (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8d9ccd5073..5535df4487 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1556,7 +1556,7 @@ L: qemu-...@nongnu.org
 S: Maintained
 F: hw/ppc/sam460ex.c
 F: hw/ppc/ppc440_uc.c
-F: hw/ppc/ppc440_pcix.c
+F: hw/pci-host/ppc440_pcix.c
 F: hw/display/sm501*
 F: hw/ide/sii3112.c
 F: hw/rtc/m41t80.c
diff --git a/hw/ppc/ppc440_pcix.c b/hw/pci-host/ppc440_pcix.c
similarity index 100%
rename from hw/ppc/ppc440_pcix.c
rename to hw/pci-host/ppc440_pcix.c
diff --git a/hw/pci-host/Kconfig b/hw/pci-host/Kconfig
index 0a221e719e..c91880b237 100644
--- a/hw/pci-host/Kconfig
+++ b/hw/pci-host/Kconfig
@@ -10,6 +10,10 @@ config PPC4XX_PCI
 bool
 select PCI
 
+config PPC440_PCIX
+bool
+select PCI
+
 config RAVEN_PCI
 bool
 select PCI
diff --git a/hw/pci-host/meson.build b/hw/pci-host/meson.build
index eb6dc71c88..3001e93a43 100644
--- a/hw/pci-host/meson.build
+++ b/hw/pci-host/meson.build
@@ -15,6 +15,7 @@ pci_ss.add(when: 'CONFIG_SH_PCI', if_true: files('sh_pci.c'))
 
 # PPC devices
 pci_ss.add(when: 'CONFIG_PPC4XX_PCI', if_true: files('ppc4xx_pci.c'))
+pci_ss.add(when: 'CONFIG_PPC440_PCIX', if_true: files('ppc440_pcix.c'))
 pci_ss.add(when: 'CONFIG_RAVEN_PCI', if_true: files('raven.c'))
 pci_ss.add(when: 'CONFIG_GRACKLE_PCI', if_true: files('grackle.c'))
 # NewWorld PowerMac
diff --git a/hw/pci-host/trace-events b/hw/pci-host/trace-events
index 90a37ebff0..0a816b9aa1 100644
--- a/hw/pci-host/trace-events
+++ b/hw/pci-host/trace-events
@@ -41,6 +41,14 @@ unin_read(uint64_t addr, uint64_t value) "addr=0x%" PRIx64 " 
val=0x%"PRIx64
 ppc4xx_pci_map_irq(int32_t devfn, int irq_num, int slot) "devfn 0x%x irq %d -> 
%d"
 ppc4xx_pci_set_irq(int irq_num) "PCI irq %d"
 
+# ppc440_pcix.c
+ppc440_pcix_map_irq(int32_t devfn, int irq_num, int slot) "devfn 0x%x irq %d 
-> %d"
+ppc440_pcix_set_irq(int irq_num) "PCI irq %d"
+ppc440_pcix_update_pim(int idx, uint64_t size, uint64_t la) "Added window %d 
of size=0x%" PRIx64 " to CPU=0x%" PRIx64
+ppc440_pcix_update_pom(int idx, uint32_t size, uint64_t la, uint64_t pcia) 
"Added window %d of size=0x%x from CPU=0x%" PRIx64 " to PCI=0x%" PRIx64
+ppc440_pcix_reg_read(uint64_t addr, uint32_t val) "addr 0x%" PRIx64 " = 0x%" 
PRIx32
+ppc440_pcix_reg_write(uint64_t addr, uint32_t val, uint32_t size) "addr 0x%" 
PRIx64 " = 0x%" PRIx32 " size 0x%" PRIx32
+
 # pnv_phb4.c
 pnv_phb4_xive_notify(uint64_t notif_port, uint64_t data) "notif=@0x%"PRIx64" 
data=0x%"PRIx64
 pnv_phb4_xive_notify_ic(uint64_t addr, uint64_t data) "addr=@0x%"PRIx64" 
data=0x%"PRIx64
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 82e847d22c..99d571fa20 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -46,6 +46,7 @@ config PPC440
 imply TEST_DEVICES
 imply E1000_PCI
 select PCI_EXPRESS
+select PPC440_PCIX
 select PPC4XX
 select SERIAL
 select FDT_PPC
diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index d0efc0aba5..da14fccce5 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -60,7 +60,7 @@ ppc_ss.add(when: 'CONFIG_PPC405', if_true: files(
   'ppc405_uc.c'))
 ppc_ss.add(when: 'CONFIG_PPC440', if_true: files(
   'ppc440_bamboo.c',
-  'ppc440_pcix.c', 'ppc440_uc.c'))
+  'ppc440_uc.c'))
 ppc_ss.add(when: 'CONFIG_PPC4XX', if_true: files(
   'ppc4xx_devs.c',
   'ppc4xx_sdram.c'))
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index b59fbf340f..157ea756e9 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -146,14 +146,6 @@ rs6000mc_size_read(uint32_t addr, uint32_t val) "read 
addr=0x%x val=0x%x"
 rs6000mc_size_write(uint32_t addr, uint32_t val) "write addr=0x%x val=0x%x"
 rs6000mc_parity_read(uint32_t addr, uint32_t val) "read addr=0x%x val=0x%x"
 
-# ppc440_pcix.c
-ppc440_pcix_map_irq(int32_t devfn, int irq_num, int slot) "devfn 0x%x irq %d 
-> %d"
-ppc440_pcix_set_irq(int irq_num) "PCI irq %d"
-ppc440_pcix_update_pim(int idx, uint64_t size, uint64_t la) "Added window %d 
of size=0x%" PRIx64 " to CPU=0x%" PRIx64
-ppc440_pcix_update_pom(int idx, uint32_t size, uint64_t la, uint64_t pcia) 
"Added window %d of size=0x%x from CPU=0x%" PRIx64 " to PCI=0x%" PRIx64
-ppc440_pcix_reg_read(uint64_t addr, uint32_t val) "addr 0x%" PRIx64 " = 0x%" 
PRIx32
-ppc440_pcix_reg_write(uint64_t addr, uint32_t val, 

Re: [RFC PATCH v2 06/22] target/arm: Add support for Non-maskable Interrupt

[Richard Henderson]

On 2/21/24 03:08, Jinjie Ruan via wrote:

This only implements the external delivery method via the GICv3.

Signed-off-by: Jinjie Ruan 
---
  target/arm/cpu-qom.h |  3 ++-
  target/arm/cpu.c | 39 ++-
  target/arm/cpu.h |  2 ++
  target/arm/helper.c  |  1 +
  4 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index 8e032691db..66d555a605 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -36,11 +36,12 @@ DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU,
  #define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU
  #define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX)
  
-/* Meanings of the ARMCPU object's four inbound GPIO lines */

+/* Meanings of the ARMCPU object's five inbound GPIO lines */
  #define ARM_CPU_IRQ 0
  #define ARM_CPU_FIQ 1
  #define ARM_CPU_VIRQ 2
  #define ARM_CPU_VFIQ 3
+#define ARM_CPU_NMI 4


You need a 6th GPIO for vNMI.


r~

Re: [RFC PATCH v2 07/22] target/arm: Add support for NMI event state

[Richard Henderson]

On 2/21/24 10:10, Richard Henderson wrote:

On 2/21/24 03:08, Jinjie Ruan via wrote:

The NMI exception state include whether the interrupt with super priority
is IRQ or FIQ, so add a nmi_is_irq flag in CPUARMState to distinguish it.

Signed-off-by: Jinjie Ruan 
---
  target/arm/cpu.h    | 2 ++
  target/arm/helper.c | 9 +
  2 files changed, 11 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 5257343bcb..051e589e19 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -603,6 +603,8 @@ typedef struct CPUArchState {
  /* State of our input IRQ/FIQ/VIRQ/VFIQ lines */
  uint32_t irq_line_state;
+    bool nmi_is_irq;


Why would you need to add this to CPUARMState?
This has the appearance of requiring only a local variable.
But it is hard to tell since you do not set it within this patch at all.


According to Arm GIC section 4.6.3 Interrupt superpriority, NMI is always IRQ, never FIQ, 
so this is never required.



r~

Re: [PATCH] target/riscv: Add missing include guard in pmu.h

[Atish Patra]

On 2/20/24 08:20, Daniel Henrique Barboza wrote:



On 2/20/24 08:08, frank.ch...@sifive.com wrote:

From: Frank Chang 

Add missing include guard in pmu.h to avoid the problem of double
inclusion.

Signed-off-by: Frank Chang 
---


Reviewed-by: Daniel Henrique Barboza 


  target/riscv/pmu.h | 5 +
  1 file changed, 5 insertions(+)

diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h
index 505fc850d3..7c0ad661e0 100644
--- a/target/riscv/pmu.h
+++ b/target/riscv/pmu.h
@@ -16,6 +16,9 @@
   * this program.  If not, see .
   */
+#ifndef RISCV_PMU_H
+#define RISCV_PMU_H
+
  #include "cpu.h"
  #include "qapi/error.h"
@@ -31,3 +34,5 @@ int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum 
riscv_pmu_event_idx event_idx);
  void riscv_pmu_generate_fdt_node(void *fdt, uint32_t cmask, char 
*pmu_name);

  int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value,
    uint32_t ctr_idx);
+
+#endif /* RISCV_PMU_H */




Oops. Thanks for the fix.

Reviewed-by: Atish Patra 

[PATCH 17/28] qemu-img: snapshot: refresh options/--help

[Michael Tokarev]

Add missing long options and --help output.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 45 -
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index ee35768af8..ce939708d4 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3608,26 +3608,51 @@ static int img_snapshot(const img_cmd_t *ccmd, int 
argc, char **argv)
 for(;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
 {"object", required_argument, 0, OPTION_OBJECT},
+{"format", required_argument, 0, 'f'},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {"force-share", no_argument, 0, 'U'},
+{"list", no_argument, 0, SNAPSHOT_LIST},
+{"apply", no_argument, 0, SNAPSHOT_APPLY},
+{"create", no_argument, 0, SNAPSHOT_CREATE},
+{"delete", no_argument, 0, SNAPSHOT_DELETE},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":la:c:d:f:hqU",
+c = getopt_long(argc, argv, "la:c:d:f:hqU",
 long_options, NULL);
 if (c == -1) {
 break;
 }
 switch(c) {
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
 case 'h':
-help();
-return 0;
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [-l | -a|-c|-d SNAPSHOT]\n"
+"[-U] [--object OBJDEF] FILENAME\n"
+,
+"  -q, --quiet\n"
+"  quiet operations\n"
+"  -f, --format FMT\n"
+"  specify FILENAME format explicitly\n"
+"  --image-opts\n"
+"  indicates that FILENAME is a complete image specification\n"
+"   instead of a file name (incompatible with --format)\n"
+"  -U, --force-share\n"
+"  open image in shared mode for concurrent access\n"
+"  --object OBJDEF\n"
+"  QEMU user-creatable object (eg encryption key)\n"
+"  Operation, one of:\n"
+"-l, --list\n"
+"   list snapshots in FILENAME (the default)\n"
+"-c, --create SNAPSHOT\n"
+"   create named snapshot\n"
+"-a, --apply SNAPSHOT\n"
+"   apply named snapshot to the base\n"
+"-d, --delete SNAPSHOT\n"
+"   delete named snapshot\n"
+"  FILENAME - image file name (or specification with --image-opts)\n"
+);
+break;
 case 'f':
 fmt = optarg;
 break;
@@ -3654,6 +3679,8 @@ static int img_snapshot(const img_cmd_t *ccmd, int argc, 
char **argv)
 case OPTION_IMAGE_OPTS:
 image_opts = true;
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 
-- 
2.39.2

[PATCH 18/28] qemu-img: rebase: refresh options/--help

[Michael Tokarev]

Add missing long options and --help output.

Options added:
 --format, --cache - for the image in question
 --backing, --backing-format, --backing-cache, --backing-unsafe -
   for the new backing file
(was eg CACHE vs SRC_CACHE, which is unclear).

Probably should rename local variables.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 55 +-
 1 file changed, 46 insertions(+), 9 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index ce939708d4..2a4bff2872 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3792,26 +3792,61 @@ static int img_rebase(const img_cmd_t *ccmd, int argc, 
char **argv)
 for(;;) {
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
+{"quiet", no_argument, 0, 'q'},
+{"progress", no_argument, 0, 'p'},
 {"object", required_argument, 0, OPTION_OBJECT},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {"force-share", no_argument, 0, 'U'},
+{"format", required_argument, 0, 'f'},
+{"cache", required_argument, 0, 't'},
 {"compress", no_argument, 0, 'c'},
+{"backing", required_argument, 0, 'b'},
+{"backing-format", required_argument, 0, 'F'},
+{"backing-cache", required_argument, 0, 'T'},
+{"backing-unsafe", no_argument, 0, 'u'},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, ":hf:F:b:upt:T:qUc",
+c = getopt_long(argc, argv, "hf:F:b:upt:T:qUc",
 long_options, NULL);
 if (c == -1) {
 break;
 }
-switch(c) {
-case ':':
-missing_argument(argv[optind - 1]);
-break;
-case '?':
-unrecognized_option(argv[optind - 1]);
-break;
+switch (c) {
 case 'h':
-help();
+cmd_help(ccmd,
+"[-f FMT | --image-opts] [-t CACHE] [-q] [-U] [-p]\n"
+"[-b BACKING_FILENAME [-F BACKING_FMT] [-T BACKING_CACHE]] [-u]\n"
+"[--object OBJDEF] [-c] FILENAME\n"
+"Rebases FILENAME on top of BACKING_FILENAME or no backing file\n"
+,
+"  -q, --quiet\n"
+" quiet operation\n"
+"  -p, --progress\n"
+" show progress indicator\n"
+"  -f, --format FMT\n"
+" specify FILENAME format explicitly\n"
+"  --image-opts\n"
+" indicates that FILENAME is a complete image specification\n"
+" instead of a file name (incompatible with --format)\n"
+"  -t, --cache CACHE\n"
+" cache mode for FILENAME (" BDRV_DEFAULT_CACHE ")\n"
+"  -b, --backing BACKING_FILENAME|\"\"\n"
+" rebase onto this file (or no backing file)\n"
+"  -F, --backing-format BACKING_FMT\n"
+" specify format for BACKING_FILENAME\n"
+"  -T, --backing-cache CACHE\n"
+" BACKING_FILENAME cache mode (" BDRV_DEFAULT_CACHE ")\n"
+"  -u, --backing-unsafe\n"
+" do not fail if BACKING_FILENAME can not be read\n"
+"  -c, --compress\n"
+" compress image (when image supports this)\n"
+"  -U, --force-share\n"
+" open image in shared mode for concurrent access\n"
+"  --object OBJDEF\n"
+" QEMU user-creatable object (eg encryption key)\n"
+"  FILENAME\n"
+" image file name (or specification with --image-opts)\n"
+);
 return 0;
 case 'f':
 fmt = optarg;
@@ -3849,6 +3884,8 @@ static int img_rebase(const img_cmd_t *ccmd, int argc, 
char **argv)
 case 'c':
 compress = true;
 break;
+default:
+tryhelp(argv[0]);
 }
 }
 
-- 
2.39.2

[PATCH 01/28] qemu-img: stop printing error twice in a few places

[Michael Tokarev]

Currently we have:

  ./qemu-img resize none +10
  qemu-img: Could not open 'none': Could not open 'none': No such file or 
directory

stop printing the message twice, - local_err already has
all the info, no need to prepend additional text there.

There are a few other places like this, but I'm unsure
about these.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 7668f86769..5a756be600 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -404,7 +404,7 @@ static BlockBackend *img_open_file(const char *filename,
 }
 blk = blk_new_open(filename, NULL, options, flags, _err);
 if (!blk) {
-error_reportf_err(local_err, "Could not open '%s': ", filename);
+error_report_err(local_err);
 return NULL;
 }
 blk_set_enable_write_cache(blk, !writethrough);
@@ -597,7 +597,7 @@ static int img_create(int argc, char **argv)
 bdrv_img_create(filename, fmt, base_filename, base_fmt,
 options, img_size, flags, quiet, _err);
 if (local_err) {
-error_reportf_err(local_err, "%s: ", filename);
+error_report_err(local_err);
 goto fail;
 }
 
@@ -5253,9 +5253,7 @@ static int img_dd(int argc, char **argv)
 
 ret = bdrv_create(drv, out.filename, opts, _err);
 if (ret < 0) {
-error_reportf_err(local_err,
-  "%s: error while creating output image: ",
-  out.filename);
+error_report_err(local_err);
 ret = -1;
 goto out;
 }
-- 
2.39.2

[PATCH 04/28] qemu-img: global option processing and error printing

[Michael Tokarev]

In order to correctly print executable name in various
error messages, pass argv[0] to error_exit() function.
This way, error messages will refer to actual executable
name, which may be different from 'qemu-img'.

For subcommands, pass whole argv[] array, so argv[0] is
the executable name, not subcommand name.  In order to
do that, avoid resetting optind but continue with the
next option.  Also don't require at least 3 options on
the command line: it makes no sense with options before
subcommand.

Before invoking a subcommand, replace argv[0] to include
the subcommand name.

Introduce tryhelp() function which just prints

 try 'command-name --help' for more info

and exits.  When tryhelp() is called from within a subcommand
handler, the message will look like:

 try 'command-name subcommand --help' for more info

qemu-img uses getopt_long() with ':' as the first char in
optstring parameter, which means it doesn't print error
messages but return ':' or '?' instead, and qemu-img uses
unrecognized_option() or missing_argument() function to
print error messages.  But it doesn't quite work:

 $ ./qemu-img -xx
 qemu-img: unrecognized option './qemu-img'

so the aim is to let getopt_long() to print regular error
messages instead (removing ':' prefix from optstring) and
remove handling of '?' and ':' "options" entirely.  With
concatenated argv[0] and the subcommand, it all finally
does the right thing in all cases.  This will be done in
subsequent changes command by command, with main() done
last.

unrecognized_option() and missing_argument() functions
prototypes aren't changed by this patch, since they're
called from many places and will be removed a few patches
later.  Only artifical "qemu-img" argv0 is provided in
there for now.

Signed-off-by: Michael Tokarev 
---
 qemu-img.c | 75 +++---
 1 file changed, 38 insertions(+), 37 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index df425b2517..44dbf5be4f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -101,8 +101,15 @@ static void format_print(void *opaque, const char *name)
 printf(" %s", name);
 }
 
-static G_NORETURN G_GNUC_PRINTF(1, 2)
-void error_exit(const char *fmt, ...)
+static G_NORETURN
+void tryhelp(const char *argv0)
+{
+error_printf("Try '%s --help' for more info\n", argv0);
+exit(EXIT_FAILURE);
+}
+
+static G_NORETURN G_GNUC_PRINTF(2, 3)
+void error_exit(const char *argv0, const char *fmt, ...)
 {
 va_list ap;
 
@@ -110,20 +117,19 @@ void error_exit(const char *fmt, ...)
 error_vreport(fmt, ap);
 va_end(ap);
 
-error_printf("Try 'qemu-img --help' for more information\n");
-exit(EXIT_FAILURE);
+tryhelp(argv0);
 }
 
 static G_NORETURN
 void missing_argument(const char *option)
 {
-error_exit("missing argument for option '%s'", option);
+error_exit("qemu-img", "missing argument for option '%s'", option);
 }
 
 static G_NORETURN
 void unrecognized_option(const char *option)
 {
-error_exit("unrecognized option '%s'", option);
+error_exit("qemu-img", "unrecognized option '%s'", option);
 }
 
 /* Please keep in synch with docs/tools/qemu-img.rst */
@@ -576,7 +582,7 @@ static int img_create(int argc, char **argv)
 }
 
 if (optind >= argc) {
-error_exit("Expecting image file name");
+error_exit(argv[0], "Expecting image file name");
 }
 optind++;
 
@@ -588,7 +594,7 @@ static int img_create(int argc, char **argv)
 }
 }
 if (optind != argc) {
-error_exit("Unexpected argument: %s", argv[optind]);
+error_exit(argv[0], "Unexpected argument: %s", argv[optind]);
 }
 
 bdrv_img_create(filename, fmt, base_filename, base_fmt,
@@ -770,7 +776,7 @@ static int img_check(int argc, char **argv)
 } else if (!strcmp(optarg, "all")) {
 fix = BDRV_FIX_LEAKS | BDRV_FIX_ERRORS;
 } else {
-error_exit("Unknown option value for -r "
+error_exit(argv[0], "Unknown option value for -r "
"(expecting 'leaks' or 'all'): %s", optarg);
 }
 break;
@@ -795,7 +801,7 @@ static int img_check(int argc, char **argv)
 }
 }
 if (optind != argc - 1) {
-error_exit("Expecting one image file name");
+error_exit(argv[0], "Expecting one image file name");
 }
 filename = argv[optind++];
 
@@ -1025,7 +1031,7 @@ static int img_commit(int argc, char **argv)
 }
 
 if (optind != argc - 1) {
-error_exit("Expecting one image file name");
+error_exit(argv[0], "Expecting one image file name");
 }
 filename = argv[optind++];
 
@@ -1446,7 +1452,7 @@ static int img_compare(int argc, char **argv)
 
 
 if (optind != argc - 2) {
-error_exit("Expecting two image file names");
+error_exit(argv[0], "Expecting two image file names");
 }
 filename1 = argv[optind++];
 filename2 = argv[optind++];
@@ -3056,7 +3062,7 @@ static int