Re: [PATCH v7 10/16] i386/cpu: Introduce cluster-id to X86CPU

[Zhao Liu]

On Wed, Jan 17, 2024 at 12:40:12AM +0800, Xiaoyao Li wrote:
> Date: Wed, 17 Jan 2024 00:40:12 +0800
> From: Xiaoyao Li 
> Subject: Re: [PATCH v7 10/16] i386/cpu: Introduce cluster-id to X86CPU
> 
> On 1/15/2024 11:18 PM, Zhao Liu wrote:
> > Hi Xiaoyao,
> > 
> > On Mon, Jan 15, 2024 at 03:45:58PM +0800, Xiaoyao Li wrote:
> > > Date: Mon, 15 Jan 2024 15:45:58 +0800
> > > From: Xiaoyao Li 
> > > Subject: Re: [PATCH v7 10/16] i386/cpu: Introduce cluster-id to X86CPU
> > > 
> > > On 1/15/2024 1:59 PM, Zhao Liu wrote:
> > > > (Also cc "machine core" maintainers.)
> > > > u
> > > > Hi Xiaoyao,
> > > > 
> > > > On Mon, Jan 15, 2024 at 12:18:17PM +0800, Xiaoyao Li wrote:
> > > > > Date: Mon, 15 Jan 2024 12:18:17 +0800
> > > > > From: Xiaoyao Li 
> > > > > Subject: Re: [PATCH v7 10/16] i386/cpu: Introduce cluster-id to X86CPU
> > > > > 
> > > > > On 1/15/2024 11:27 AM, Zhao Liu wrote:
> > > > > > On Sun, Jan 14, 2024 at 09:49:18PM +0800, Xiaoyao Li wrote:
> > > > > > > Date: Sun, 14 Jan 2024 21:49:18 +0800
> > > > > > > From: Xiaoyao Li 
> > > > > > > Subject: Re: [PATCH v7 10/16] i386/cpu: Introduce cluster-id to 
> > > > > > > X86CPU
> > > > > > > 
> > > > > > > On 1/8/2024 4:27 PM, Zhao Liu wrote:
> > > > > > > > From: Zhuocheng Ding 
> > > > > > > > 
> > > > > > > > Introduce cluster-id other than module-id to be consistent with
> > > > > > > > CpuInstanceProperties.cluster-id, and this avoids the confusion
> > > > > > > > of parameter names when hotplugging.
> > > > > > > 
> > > > > > > I don't think reusing 'cluster' from arm for x86's 'module' is a 
> > > > > > > good idea.
> > > > > > > It introduces confusion around the code.
> > > > > > 
> > > > > > There is a precedent: generic "socket" v.s. i386 "package".
> > > > > 
> > > > > It's not the same thing. "socket" vs "package" is just software 
> > > > > people and
> > > > > hardware people chose different name. It's just different naming 
> > > > > issue.
> > > > 
> > > > No, it's a similar issue. Same physical device, different name only.
> > > > 
> > > > Furthermore, the topology was introduced for resource layout and silicon
> > > > fabrication, and similar design ideas and fabrication processes are 
> > > > fairly
> > > > consistent across common current arches. Therefore, it is possible to
> > > > abstract similar topological hierarchies for different arches.
> > > > 
> > > > > 
> > > > > however, here it's reusing name issue while 'cluster' has been 
> > > > > defined for
> > > > > x86. It does introduce confusion.
> > > > 
> > > > There's nothing fundamentally different between the x86 module and the
> > > > generic cluster, is there? This is the reason that I don't agree with
> > > > introducing "modules" in -smp.
> > > 
> > > generic cluster just means the cluster of processors, i.e, a group of
> > > cpus/lps. It is just a middle level between die and core.
> > 
> > Not sure if you mean the "cluster" device for TCG GDB? "cluster" device
> > is different with "cluster" option in -smp.
> 
> No, I just mean the word 'cluster'. And I thought what you called "generic
> cluster" means "a cluster of logical processors"
> 
> Below I quote the description of Yanan's commit 864c3b5c32f0:
> 
> A cluster generally means a group of CPU cores which share L2 cache
> or other mid-level resources, and it is the shared resources that
> is used to improve scheduler's behavior. From the point of view of
> the size range, it's between CPU die and CPU core. For example, on
> some ARM64 Kunpeng servers, we have 6 clusters in each NUMA node,
> and 4 CPU cores in each cluster. The 4 CPU cores share a separate
> L2 cache and a L3 cache tag, which brings cache affinity advantage.
> 
> What I get from it, is, cluster is just a middle level between CPU die and
> CPU core.

Here the words "a group of CPU" is not the software concept, but a hardware
topology.

> The cpu cores inside one cluster shares some mid-level resource.
> L2 cache is just one example of the shared mid-level resource. So it can be
> either module level, or tile level in x86, or even the diegrp level you
> mentioned below.

In actual hardware design, ARM's cluster is close to intel's module.

I'm not seeing any examples of clusters being similar to tile or diegrp.
Even within intel, the hardware architects have agreed definitions for each
level.

> 
> > When Yanan introduced the "cluster" option in -smp, he mentioned that it
> > is for sharing L2 and L3 tags, which roughly corresponds to our module.
> > 
> > > 
> > > It can be the module level in intel, or tile level. Further, if per die lp
> > > number increases in the future, there might be more middle levels in intel
> > > between die and core. Then at that time, how to decide what level should
> > > cluster be mapped to?
> > 
> > Currently, there're 3 levels defined in SDM which are between die and
> > core: diegrp, tile and module. In our products, L2 is just sharing on the
> > module, so the intel's module and the general 

RE: [PATCH rfcv1 1/6] backends/iommufd_device: introduce IOMMUFDDevice

[Duan, Zhenzhong]

>-Original Message-
>From: Eric Auger 
>Subject: Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce
>IOMMUFDDevice
>
>
>
>On 1/15/24 11:13, Zhenzhong Duan wrote:
>> IOMMUFDDevice represents a device in iommufd and can be used as
>> a communication interface between devices (i.e., VFIO, VDPA) and
>> vIOMMU.
>>
>> Currently it includes iommufd handler and device id information
>> which could be used by vIOMMU to get hw IOMMU information.
>>
>> In future nested translation support, vIOMMU is going to have
>> more iommufd related operations like allocate hwpt for a device,
>> attach/detach hwpt, etc. So IOMMUFDDevice will be further expanded.
>>
>> IOMMUFDDevice is willingly not a QOM object because we don't want
>> it to be visible from the user interface.
>>
>> Introduce a helper iommufd_device_init to initialize IOMMUFDDevice.
>>
>> Originally-by: Yi Liu 
>> Signed-off-by: Yi Sun 
>> Signed-off-by: Zhenzhong Duan 
>> ---
>>  MAINTAINERS |  4 +--
>>  include/sysemu/iommufd_device.h | 31 
>>  backends/iommufd_device.c   | 50
>+
>Maybe it is still time to move the iommufd files in a sepate dir, under
>hw at the same level as vfio.
>
>Thoughts?

Any reason for the movement? Hw dir contains entries to emulate different
Devices. Iommufd is not a real device. It's more a backend.

Thanks
Zhenzhong

RE: [PATCH rfcv1 3/6] intel_iommu: add set/unset_iommu_device callback

[Duan, Zhenzhong]

>-Original Message-
>From: Eric Auger 
>Subject: Re: [PATCH rfcv1 3/6] intel_iommu: add set/unset_iommu_device
>callback
>
>
>
>On 1/18/24 09:43, Duan, Zhenzhong wrote:
>>
>>> -Original Message-
>>> From: Eric Auger 
>>> Subject: Re: [PATCH rfcv1 3/6] intel_iommu: add
>set/unset_iommu_device
>>> callback
>>>
>>> Hi Zhenzhong,
>>>
>>> On 1/15/24 11:13, Zhenzhong Duan wrote:
 From: Yi Liu 

 This adds set/unset_iommu_device() implementation in Intel vIOMMU.
 In set call, IOMMUFDDevice is recorded in hash table indexed by
 PCI BDF.

 Signed-off-by: Yi Liu 
 Signed-off-by: Yi Sun 
 Signed-off-by: Zhenzhong Duan 
 ---
  include/hw/i386/intel_iommu.h | 10 +
  hw/i386/intel_iommu.c | 79
>>> +++
  2 files changed, 89 insertions(+)

 diff --git a/include/hw/i386/intel_iommu.h
>>> b/include/hw/i386/intel_iommu.h
 index 7fa0a695c8..c65fdde56f 100644
 --- a/include/hw/i386/intel_iommu.h
 +++ b/include/hw/i386/intel_iommu.h
 @@ -62,6 +62,7 @@ typedef union VTD_IR_TableEntry
>VTD_IR_TableEntry;
  typedef union VTD_IR_MSIAddress VTD_IR_MSIAddress;
  typedef struct VTDPASIDDirEntry VTDPASIDDirEntry;
  typedef struct VTDPASIDEntry VTDPASIDEntry;
 +typedef struct VTDIOMMUFDDevice VTDIOMMUFDDevice;

  /* Context-Entry */
  struct VTDContextEntry {
 @@ -148,6 +149,13 @@ struct VTDAddressSpace {
  IOVATree *iova_tree;
  };

 +struct VTDIOMMUFDDevice {
 +PCIBus *bus;
 +uint8_t devfn;
 +IOMMUFDDevice *idev;
 +IntelIOMMUState *iommu_state;
 +};
 +
>>> Just wondering whether we shouldn't reuse the VTDAddressSpace to
>store
>>> the idev, if any. How have you made your choice. What will it become
>>> when PASID gets added?
>> VTDAddressSpace is indexed by aliased BDF, but VTDIOMMUFDDevice is
>indexed
>> by device's BDF. So we can't just store VTDIOMMUFDDevice as a pointer in
>> VTDAddressSpace, may need a list in case more than one device in same
>address
>> space. Then a global VTDIOMMUFDDevice list is better for lookup.
>
>OK but if several devices are hidden under an aliased BDF, can't they
>share the host properties (DMAR ecap/cap)?

It depends on that if the vfio devices are under same aliased BDF in host.
If vfio devices are configured under same aliased BDF in qemu but they are
not in same aliased BDF in host, their host cap/ecap may be different.

Thanks
Zhenzhong

Re: [PATCH] target/i386: Add new CPU model SierraForest

[Tao Su]

Kindly ping for any comments.

Thanks,
Tao

Re: [PATCH v2] tests/docker: Add sqlite3 module to openSUSE Leap container

[Michael Tokarev]

Sorry for the previous dummy post, - hit the wrong button :)

17.01.2024 19:42, Fabiano Rosas :

Avocado needs sqlite3:



--- a/tests/docker/dockerfiles/opensuse-leap.docker
+++ b/tests/docker/dockerfiles/opensuse-leap.docker
@@ -90,6 +90,7 @@ RUN zypper update -y && \
 pcre-devel-static \
 pipewire-devel \
 pkgconfig \
+   python311 \
 python311-base \
 python311-pip \


Isn't python311 already pulls in python311-base?

/mjt

Re: [PATCH v2] tests/docker: Add sqlite3 module to openSUSE Leap container

[Michael Tokarev]

 python311-setuptools \
diff --git a/tests/lcitool/mappings.yml b/tests/lcitool/mappings.yml
index 0b908882f1..407c03301b 100644
--- a/tests/lcitool/mappings.yml
+++ b/tests/lcitool/mappings.yml
@@ -59,6 +59,10 @@ mappings:
  CentOSStream8:
  OpenSUSELeap15:
  
+  python3-sqlite3:

+CentOSStream8: python38
+OpenSUSELeap15: python311
+
python3-tomli:
  # test using tomllib
  apk:
diff --git a/tests/lcitool/projects/qemu.yml b/tests/lcitool/projects/qemu.yml
index 82092c9f17..149b15de57 100644
--- a/tests/lcitool/projects/qemu.yml
+++ b/tests/lcitool/projects/qemu.yml
@@ -97,6 +97,7 @@ packages:
   - python3-pip
   - python3-sphinx
   - python3-sphinx-rtd-theme
+ - python3-sqlite3
   - python3-tomli
   - python3-venv
   - rpm2cpio

RE: [RFC 4/7] virtio-iommu: Implement PCIIOMMUOps set_host_resv_regions

[Duan, Zhenzhong]

>-Original Message-
>From: Eric Auger 
>Subject: Re: [RFC 4/7] virtio-iommu: Implement PCIIOMMUOps
>set_host_resv_regions
>
>Hi Zhenzhong,
>On 1/18/24 08:43, Duan, Zhenzhong wrote:
>> Hi Eric,
>>
>>> -Original Message-
>>> From: Eric Auger 
>>> Subject: [RFC 4/7] virtio-iommu: Implement PCIIOMMUOps
>>> set_host_resv_regions
>>>
>>> Reuse the implementation of virtio_iommu_set_iova_ranges() which
>>> will be removed in subsequent patches.
>>>
>>> Signed-off-by: Eric Auger 
>>> ---
>>> hw/virtio/virtio-iommu.c | 134 +--
>---
>>> -
>>> 1 file changed, 101 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>>> index 8a4bd933c6..716a3fcfbf 100644
>>> --- a/hw/virtio/virtio-iommu.c
>>> +++ b/hw/virtio/virtio-iommu.c
>>> @@ -461,8 +461,109 @@ static AddressSpace
>>> *virtio_iommu_find_add_as(PCIBus *bus, void *opaque,
>>> return >as;
>>> }
>>>
>>> +/**
>>> + * rebuild_resv_regions: rebuild resv regions with both the
>>> + * info of host resv ranges and property set resv ranges
>>> + */
>>> +static int rebuild_resv_regions(IOMMUDevice *sdev)
>>> +{
>>> +GList *l;
>>> +int i = 0;
>>> +
>>> +/* free the existing list and rebuild it from scratch */
>>> +g_list_free_full(sdev->resv_regions, g_free);
>>> +sdev->resv_regions = NULL;
>>> +
>>> +/* First add host reserved regions if any, all tagged as RESERVED */
>>> +for (l = sdev->host_resv_ranges; l; l = l->next) {
>>> +ReservedRegion *reg = g_new0(ReservedRegion, 1);
>>> +Range *r = (Range *)l->data;
>>> +
>>> +reg->type = VIRTIO_IOMMU_RESV_MEM_T_RESERVED;
>>> +range_set_bounds(>range, range_lob(r), range_upb(r));
>>> +sdev->resv_regions = resv_region_list_insert(sdev->resv_regions,
>reg);
>>> +trace_virtio_iommu_host_resv_regions(sdev-
 iommu_mr.parent_obj.name, i,
>>> + range_lob(>range),
>>> + range_upb(>range));
>>> +i++;
>>> +}
>>> +/*
>>> + * then add higher priority reserved regions set by the machine
>>> + * through properties
>>> + */
>>> +add_prop_resv_regions(sdev);
>>> +return 0;
>>> +}
>>> +
>>> +static int virtio_iommu_set_host_iova_ranges(PCIBus *bus, void
>*opaque,
>>> + int devfn, GList *iova_ranges,
>>> + Error **errp)
>>> +{
>>> +VirtIOIOMMU *s = opaque;
>>> +IOMMUPciBus *sbus = g_hash_table_lookup(s->as_by_busptr, bus);
>>> +IOMMUDevice *sdev;
>>> +GList *current_ranges;
>>> +GList *l, *tmp, *new_ranges = NULL;
>>> +int ret = -EINVAL;
>>> +
>>> +if (!sbus) {
>>> +error_report("%s no sbus", __func__);
>>> +}
>> Do we plan to support multiple devices in same iommu group?
>> as_by_busptr is hashed by bus which is an aliased bus of the device.
>> So sbus may be NULL if device's bus is different from aliased bus.
>If I understand you remark properly this means that
>
>virtio_iommu_set_host_iova_ranges should take as arg the aliased bus and
>not the bus, right?
>I think we shall support non singleton groups too.

Not exactly. I think we should pass device's real BDF, not aliased BDF. Or else
we setup reserved ranges of all devices in same group to aliased BDF.

I'm just suspecting that the hash lookup with real BDF index will return NULL if
multiple devices are in same group. If that’s true, then iova_ranges is never
passed to guest.

>
>>
>>> +
>>> +sdev = sbus->pbdev[devfn];
>>> +
>>> +current_ranges = sdev->host_resv_ranges;
>>> +
>>> +warn_report("%s: host_resv_regions=%d", __func__, !!sdev-
 host_resv_ranges);
>>> +/* check that each new resv region is included in an existing one */
>>> +if (sdev->host_resv_ranges) {
>> May be we could just error out as vfio_realize should not call
>> set_host_iova_ranges() twice.
>so if we have several devices in the group,
>
>set_host_iova_ranges()

Yes,

>
> may be called several times. Right?

but should be on different sdev due to different real BDF, not only on aliased 
BDF.

Thanks
Zhenzhong

>
>Eric
>>
>> Thanks
>> Zhenzhong
>>> +range_inverse_array(iova_ranges,
>>> +_ranges,
>>> +0, UINT64_MAX);
>>> +
>>> +for (tmp = new_ranges; tmp; tmp = tmp->next) {
>>> +Range *newr = (Range *)tmp->data;
>>> +bool included = false;
>>> +
>>> +for (l = current_ranges; l; l = l->next) {
>>> +Range * r = (Range *)l->data;
>>> +
>>> +if (range_contains_range(r, newr)) {
>>> +included = true;
>>> +break;
>>> +}
>>> +}
>>> +if (!included) {
>>> +goto error;
>>> +}
>>> +}
>>> +/* all new reserved ranges are 

Why invtsc (CPUID_APM_INVTSC) is unmigratable?

[Xiaoyao Li]

I'm wondering why CPUID_APM_INVTSC is set as unmigratable_flags. Could 
anyone explain it?


When the host supports invtsc, it can be exposed to guest.
When the src VM has invtsc exposed, what will forbid it to be migrated 
to a dest that also supports VMs with invtsc exposed?

RE: [RFC 0/7] VIRTIO-IOMMU/VFIO: Fix host iommu geometry handling for hotplugged devices

[Duan, Zhenzhong]

>-Original Message-
>From: Eric Auger 
>Subject: Re: [RFC 0/7] VIRTIO-IOMMU/VFIO: Fix host iommu geometry
>handling for hotplugged devices
>
>Hi Zhenzhong,
>On 1/18/24 08:10, Duan, Zhenzhong wrote:
>> Hi Eric,
>>
>>> -Original Message-
>>> From: Eric Auger 
>>> Cc: m...@redhat.com; c...@redhat.com
>>> Subject: [RFC 0/7] VIRTIO-IOMMU/VFIO: Fix host iommu geometry
>handling
>>> for hotplugged devices
>>>
>>> In [1] we attempted to fix a case where a VFIO-PCI device protected
>>> with a virtio-iommu was assigned to an x86 guest. On x86 the physical
>>> IOMMU may have an address width (gaw) of 39 or 48 bits whereas the
>>> virtio-iommu used to expose a 64b address space by default.
>>> Hence the guest was trying to use the full 64b space and we hit
>>> DMA MAP failures. To work around this issue we managed to pass
>>> usable IOVA regions (excluding the out of range space) from VFIO
>>> to the virtio-iommu device. This was made feasible by introducing
>>> a new IOMMU Memory Region callback dubbed
>iommu_set_iova_regions().
>>> This latter gets called when the IOMMU MR is enabled which
>>> causes the vfio_listener_region_add() to be called.
>>>
>>> However with VFIO-PCI hotplug, this technique fails due to the
>>> race between the call to the callback in the add memory listener
>>> and the virtio-iommu probe request. Indeed the probe request gets
>>> called before the attach to the domain. So in that case the usable
>>> regions are communicated after the probe request and fail to be
>>> conveyed to the guest. To be honest the problem was hinted by
>>> Jean-Philippe in [1] and I should have been more careful at
>>> listening to him and testing with hotplug :-(
>> It looks the global virtio_iommu_config.bypass is never cleared in guest.
>> When guest virtio_iommu driver enable IOMMU, should it clear this
>> bypass attribute?
>> If it could be cleared in viommu_probe(), then qemu will call
>> virtio_iommu_set_config() then virtio_iommu_switch_address_space_all()
>> to enable IOMMU MR. Then both coldplugged and hotplugged devices will
>work.
>
>this field is iommu wide while the probe applies on a one device.In
>general I would prefer not to be dependent on the MR enablement. We know
>that the device is likely to be protected and we can collect its
>requirements beforehand.   

Agree that your new patch is cleaner.

>>
>> Intel iommu has a similar bit in register GCMD_REG.TE, when guest
>> intel_iommu driver probe set it, on qemu side,
>vtd_address_space_refresh_all()
>> is called to enable IOMMU MRs.
>interesting.
>
>Would be curious to get Jean Philippe's pov.
>>
>>> For coldplugged device the technique works because we make sure all
>>> the IOMMU MR are enabled once on the machine init done: 94df5b2180
>>> ("virtio-iommu: Fix 64kB host page size VFIO device assignment")
>>> for granule freeze. But I would be keen to get rid of this trick.
>>>
>>> Using an IOMMU MR Ops is unpractical because this relies on the
>IOMMU
>>> MR to have been enabled and the corresponding
>vfio_listener_region_add()
>>> to be executed. Instead this series proposes to replace the usage of this
>>> API by the recently introduced PCIIOMMUOps: ba7d12eb8c  ("hw/pci:
>>> modify
>>> pci_setup_iommu() to set PCIIOMMUOps"). That way, the callback can be
>>> called earlier, once the usable IOVA regions have been collected by
>>> VFIO, without the need for the IOMMU MR to be enabled.
>>>
>>> This looks cleaner. In the short term this may also be used for
>>> passing the page size mask, which would allow to get rid of the
>>> hacky transient IOMMU MR enablement mentionned above.
>>>
>>> [1] [PATCH v4 00/12] VIRTIO-IOMMU/VFIO: Don't assume 64b IOVA
>space
>>>https://lore.kernel.org/all/20231019134651.842175-1-
>>> eric.au...@redhat.com/
>>>
>>> [2] https://lore.kernel.org/all/20230929161547.GB2957297@myrica/
>>>
>>> Extra Notes:
>>> With that series, the reserved memory regions are communicated on
>time
>>> so that the virtio-iommu probe request grabs them. However this is not
>>> sufficient. In some cases (my case), I still see some DMA MAP failures
>>> and the guest keeps on using IOVA ranges outside the geometry of the
>>> physical IOMMU. This is due to the fact the VFIO-PCI device is in the
>>> same iommu group as the pcie root port. Normally the kernel
>>> iova_reserve_iommu_regions (dma-iommu.c) is supposed to call
>>> reserve_iova()
>>> for each reserved IOVA, which carves them out of the allocator. When
>>> iommu_dma_init_domain() gets called for the hotplugged vfio-pci device
>>> the iova domain is already allocated and set and we don't call
>>> iova_reserve_iommu_regions() again for the vfio-pci device. So its
>>> corresponding reserved regions are not properly taken into account.
>> I suspect there is same issue with coldplugged devices. If those devices
>> are in same group, get iova_reserve_iommu_regions() is only called
>> for first device. But other devices's reserved regions are missed.
>
>Correct
>>
>> 

RE: FW: [PATCH] vhost-user: add VIRTIO_F_IN_ORDER and VIRTIO_F_NOTIFICATION_DATA feature

[Wentao Jia]

VIRTIO_F_IN_ORDER and VIRTIO_F_NOTIFICATION_DATA feature are important feature
for dpdk vdpa packets transmitting performance, add the 2 features at vhost-user
front-end to negotiation with backend.

Signed-off-by: Kyle Xu 
Signed-off-by: Wentao Jia 
Reviewed-by:   Xinying Yu 
Reviewed-by:   Shujing Dong 
Reviewed-by:   Rick Zhong 
---
 hw/core/machine.c   | 2 ++
 hw/net/vhost_net.c  | 2 ++
 hw/net/virtio-net.c | 4 
 3 files changed, 8 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index fb5afdcae4..e620f5e7d0 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -40,6 +40,7 @@ GlobalProperty hw_compat_8_1[] = {
 { "ramfb", "x-migrate", "off" },
 { "vfio-pci-nohotplug", "x-ramfb-migrate", "off" },
 { "igb", "x-pcie-flr-init", "off" },
+{ TYPE_VIRTIO_NET, "notification_data", "off"},
 };
 const size_t hw_compat_8_1_len = G_N_ELEMENTS(hw_compat_8_1);

@@ -65,6 +66,7 @@ GlobalProperty hw_compat_7_1[] = {
 { "virtio-rng-pci", "vectors", "0" },
 { "virtio-rng-pci-transitional", "vectors", "0" },
 { "virtio-rng-pci-non-transitional", "vectors", "0" },
+{ TYPE_VIRTIO_NET, "in_order", "off"},
 };
 const size_t hw_compat_7_1_len = G_N_ELEMENTS(hw_compat_7_1);

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index e8e1661646..211ca859a6 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -76,6 +76,8 @@ static const int user_feature_bits[] = {
 VIRTIO_F_IOMMU_PLATFORM,
 VIRTIO_F_RING_PACKED,
 VIRTIO_F_RING_RESET,
+VIRTIO_F_IN_ORDER,
+VIRTIO_F_NOTIFICATION_DATA,
 VIRTIO_NET_F_RSS,
 VIRTIO_NET_F_HASH_REPORT,
 VIRTIO_NET_F_GUEST_USO4,
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 7a2846fa1c..dc0a028934 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3949,6 +3949,10 @@ static Property virtio_net_properties[] = {
   VIRTIO_NET_F_GUEST_USO6, true),
 DEFINE_PROP_BIT64("host_uso", VirtIONet, host_features,
   VIRTIO_NET_F_HOST_USO, true),
+DEFINE_PROP_BIT64("in_order", VirtIONet, host_features,
+  VIRTIO_F_IN_ORDER, true),
+DEFINE_PROP_BIT64("notification_data", VirtIONet, host_features,
+  VIRTIO_F_NOTIFICATION_DATA, true),
 DEFINE_PROP_END_OF_LIST(),
 };

--

Re: [PATCH 1/2] ppc/spapr: change pseries machine default to POWER10 CPU

[Harsh Prateek Bora]

On 1/18/24 19:44, Nicholas Piggin wrote:

POWER10 is the latest pseries CPU.

Signed-off-by: Nicholas Piggin 


Reviewed-by: Harsh Prateek Bora 


---
  hw/ppc/spapr.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e8dabc8614..021b1a00e1 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4675,7 +4675,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
void *data)
  
  smc->dr_lmb_enabled = true;

  smc->update_dt_enabled = true;
-mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.2");
+mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power10_v2.0");
  mc->has_hotpluggable_cpus = true;
  mc->nvdimm_supported = true;
  smc->resize_hpt_default = SPAPR_RESIZE_HPT_ENABLED;

Re: [PATCH v4 3/3] ci: Disable migration compatibility tests for aarch64

[Peter Xu]

On Thu, Jan 18, 2024 at 01:49:51PM -0300, Fabiano Rosas wrote:
> Until 9.0 is out, we need to keep the aarch64 job disabled because the
> tests always use the n-1 version of migration-test. That happens to be
> broken for aarch64 in 8.2. Once 9.0 is out, it will become the n-1
> version and it will bring the fixed tests.
> 
> We can revert this patch when 9.0 releases.
> 
> Signed-off-by: Fabiano Rosas 
> ---
>  .gitlab-ci.d/buildtest.yml | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index f0b0edc634..b344a4685f 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -217,10 +217,14 @@ build-previous-qemu:
>  - QTEST_QEMU_BINARY_DST=./qemu-system-${TARGET}
>QTEST_QEMU_BINARY=../build/qemu-system-${TARGET} 
> ./tests/qtest/migration-test
>  
> +# This job is disabled until we release 9.0. The existing
> +# migration-test in 8.2 is broken on aarch64. The fix was already
> +# commited, but it will only take effect once 9.0 is out.
>  migration-compat-aarch64:
>extends: .migration-compat-common
>variables:
>  TARGET: aarch64
> +QEMU_JOB_OPTIONAL: 1

Optionally you can move the whole migration-compat-aarch64 from previous
patch to here, then even if someone kicks off CI for previous commit it
won't fail.

Would QEMU_JOB_SKIPPED suits more here?

https://www.qemu.org/docs/master/devel/ci.html

QEMU_JOB_OPTIONAL

The job is expected to be successful in general, but is not run by
default due to need to conserve limited CI resources. It is
available to be started manually by the contributor in the CI
pipelines UI.

QEMU_JOB_SKIPPED

The job is not reliably successsful in general, so is not currently
suitable to be run by default. Ideally this should be a temporary
marker until the problems can be addressed, or the job permanently
removed.

I suppose they all fall into "manual trigger" as a result, but just in case
it'll behave differently in the future.

>  
>  migration-compat-x86_64:
>extends: .migration-compat-common
> -- 
> 2.35.3
> 

-- 
Peter Xu

Re: [PATCH v2 1/2] target/riscv: Convert sdtrig functionality from property to an extension

[Anup Patel]

On Wed, Jan 17, 2024 at 7:54 PM Himanshu Chauhan
 wrote:
>
> The debug trigger (sdtrig) capability is controlled using the debug property.
> The sdtrig is an ISA extension and should be treated so. The sdtrig extension
> may or may not be implemented in a system. Therefore, it must raise an illegal
> instruction exception when it is disabled and its CSRs are accessed.
>
> This patch removes the "debug" property and replaces it with ext_sdtrig 
> extension.
> It also raises an illegal instruction exception when the extension is 
> disabled and
> its CSRs are accessed.
>
> Signed-off-by: Himanshu Chauhan 
> ---
>  target/riscv/cpu.c| 7 +++
>  target/riscv/cpu_cfg.h| 2 +-
>  target/riscv/cpu_helper.c | 2 +-
>  target/riscv/csr.c| 2 +-
>  target/riscv/machine.c| 2 +-
>  5 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index b07a76ef6b..c770a7e506 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -909,7 +909,7 @@ static void riscv_cpu_reset_hold(Object *obj)
>  set_default_nan_mode(1, >fp_status);
>
>  #ifndef CONFIG_USER_ONLY
> -if (cpu->cfg.debug) {
> +if (cpu->cfg.ext_sdtrig) {
>  riscv_trigger_reset_hold(env);
>  }
>
> @@ -1068,7 +1068,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> **errp)
>  riscv_cpu_register_gdb_regs_for_features(cs);
>
>  #ifndef CONFIG_USER_ONLY
> -if (cpu->cfg.debug) {
> +if (cpu->cfg.ext_sdtrig) {
>  riscv_trigger_realize(>env);
>  }
>  #endif
> @@ -1393,6 +1393,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_vendor_exts[] = {
>
>  /* These are experimental so mark with 'x-' */
>  const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[] = {
> +MULTI_EXT_CFG_BOOL("x-sdtrig", ext_sdtrig, true),

Drop the "x-" because Sdtrig is already frozen and public review has started.

>  MULTI_EXT_CFG_BOOL("x-smaia", ext_smaia, false),
>  MULTI_EXT_CFG_BOOL("x-ssaia", ext_ssaia, false),
>
> @@ -1480,8 +1481,6 @@ Property riscv_cpu_options[] = {
>  };
>
>  static Property riscv_cpu_properties[] = {
> -DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
> -
>  #ifndef CONFIG_USER_ONLY
>  DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
>  #endif
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index f4605fb190..341ebf726a 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -109,6 +109,7 @@ struct RISCVCPUConfig {
>  bool ext_zvfbfwma;
>  bool ext_zvfh;
>  bool ext_zvfhmin;
> +bool ext_sdtrig;
>  bool ext_smaia;
>  bool ext_ssaia;
>  bool ext_sscofpmf;
> @@ -145,7 +146,6 @@ struct RISCVCPUConfig {
>  uint16_t cboz_blocksize;
>  bool mmu;
>  bool pmp;
> -bool debug;
>  bool misa_w;
>
>  bool short_isa_string;
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index e7e23b34f4..3f7c2f1315 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -126,7 +126,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>   ? EXT_STATUS_DIRTY : EXT_STATUS_DISABLED;
>  }
>
> -if (cpu->cfg.debug && !icount_enabled()) {
> +if (cpu->cfg.ext_sdtrig && !icount_enabled()) {
>  flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled);
>  }
>  #endif
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index c50a33397c..8dbb49aa88 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -543,7 +543,7 @@ static RISCVException have_mseccfg(CPURISCVState *env, 
> int csrno)
>
>  static RISCVException debug(CPURISCVState *env, int csrno)
>  {
> -if (riscv_cpu_cfg(env)->debug) {
> +if (riscv_cpu_cfg(env)->ext_sdtrig) {
>  return RISCV_EXCP_NONE;
>  }
>
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 72fe2374dc..8f9787a30f 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -231,7 +231,7 @@ static bool debug_needed(void *opaque)
>  {
>  RISCVCPU *cpu = opaque;
>
> -return cpu->cfg.debug;
> +return cpu->cfg.ext_sdtrig;
>  }
>
>  static int debug_post_load(void *opaque, int version_id)
> --
> 2.34.1
>
>

Regards,
Anup

Re: [RFC PATCH v3 19/30] migration/ram: Ignore multifd flush when doing fixed-ram migration

[Peter Xu]

On Wed, Jan 17, 2024 at 03:13:20PM -0300, Fabiano Rosas wrote:
> >> @@ -3242,8 +3243,11 @@ static int ram_save_iterate(QEMUFile *f, void 
> >> *opaque)
> >>  out:
> >>  if (ret >= 0
> >>  && migration_is_setup_or_active(migrate_get_current()->state)) {
> >> -if (migrate_multifd() && 
> >> migrate_multifd_flush_after_each_section()) {
> >> -ret = 
> >> multifd_send_sync_main(rs->pss[RAM_CHANNEL_PRECOPY].pss_channel);
> >> +if (migrate_multifd() &&
> >> +(migrate_multifd_flush_after_each_section() ||
> >> + migrate_fixed_ram())) {
> >> +ret = multifd_send_sync_main(
> >> +rs->pss[RAM_CHANNEL_PRECOPY].pss_channel);
> >
> > Why you want this one?  ram_save_iterate() can be called tens for each
> > second iiuc.
> >
> 
> AIUI, this is a requirement for live migration, so that we're not
> sending the new version of the page while the old version is still in
> transit.
> 
> > There's one more?  ram_save_complete():
> >
> > if (migrate_multifd() && !migrate_multifd_flush_after_each_section()) {
> > qemu_put_be64(f, RAM_SAVE_FLAG_MULTIFD_FLUSH);
> > }
> >
> > IIUC that's the one you referred to at [1] above, not sure why you modified
> > the code in ram_save_iterate() instead.
> >
> 
> I mentioned it in the commit message as well:
> 
> " - between iterations, to avoid a slow channel being overrun by a fast
>  channel in the subsequent iteration;"

IMHO you only need to flush all threads in find_dirty_block(). That's when
the "real iteration" happens (IOW, when the "real iteration" is defined to
be a full walk across all guest memories, rather than one single call to
ram_save_iterate()).

Multifd used to do too many flushes, and that's why we had the new
migrate_multifd_flush_after_each_section() and it's a bit of a mess if you
see..

To be super safe, you can also flush at ram_save_complete(), but I doubt
its necessity, and this same question applies to generic multifd: the
multifd_send_sync_main() in the ram_save_complete() can be redundant,
afaiu.  However we can leave that for later to not add even more dependency
to fixed-ram.  If that is justified, we can remove the sync_main for both
socket / file then.

-- 
Peter Xu

Re: [PATCH 2/8] ppc/spapr|pnv: Remove SAO from pa-features when running MTTCG

[David Gibson]

On Fri, Jan 19, 2024 at 12:09:36AM +1000, Nicholas Piggin wrote:
> SAO is a page table attribute that strengthens the memory ordering of
> accesses. QEMU with MTTCG does not implement this, so clear it in
> ibm,pa-features. There is a complication with spapr migration that is
> addressed with comments, it is not a new problem here.
> 
> Signed-off-by: Nicholas Piggin 
> ---
>  hw/ppc/pnv.c   |  5 +
>  hw/ppc/spapr.c | 15 +++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index b949398689..4969fbdb05 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -158,6 +158,11 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void 
> *fdt)
>  char *nodename;
>  int cpus_offset = get_cpus_node(fdt);
>  
> +if (qemu_tcg_mttcg_enabled()) {
> +/* SSO (SAO) ordering is not supported under MTTCG. */
> +pa_features[4 + 2] &= ~0x80;
> +}
> +
>  nodename = g_strdup_printf("%s@%x", dc->fw_name, pc->pir);
>  offset = fdt_add_subnode(fdt, cpus_offset, nodename);
>  _FDT(offset);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 021b1a00e1..1c79d5670d 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -284,6 +284,21 @@ static void spapr_dt_pa_features(SpaprMachineState 
> *spapr,
>  return;
>  }
>  
> +if (qemu_tcg_mttcg_enabled()) {
> +/*
> + * SSO (SAO) ordering is not supported under MTTCG, so disable it.
> + * There is no cap for this, so there is a migration bug here.
> + * However don't disable it entirely, to allow it to be used under
> + * KVM. This is a minor concern because:
> + * - SAO is an obscure an rarely (if ever) used feature.
> + * - SAO is removed from POWER10 / v3.1, so there is already a
> + *   migration problem today.
> + * - Linux does not test this pa-features bit today anyway, so it's
> + *   academic.
> + */
> +pa_features[4 + 2] &= ~0x80;

Oof.. I see the reasoning but modifying guest visible parameters based
on host capabilities without a cap really worries me nonetheless.

> +}
> +
>  if (ppc_hash64_has(cpu, PPC_HASH64_CI_LARGEPAGE)) {
>  /*
>   * Note: we keep CI large pages off by default because a 64K capable

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [RFC PATCH v3 15/30] io: Add a pwritev/preadv version that takes a discontiguous iovec

[Peter Xu]

On Thu, Jan 18, 2024 at 09:47:18AM -0300, Fabiano Rosas wrote:
> Peter Xu  writes:
> 
> > On Wed, Jan 17, 2024 at 03:06:15PM -0300, Fabiano Rosas wrote:
> >> Oh no, you're right. Because of p->pending_job. And thinking about
> >> p->pending_job, wouldn't a trylock to the same job while being more
> >> explicit?
> >> 
> >> next_channel %= migrate_multifd_channels();
> >> for (i = next_channel;; i = (i + 1) % migrate_multifd_channels()) {
> >> p = _send_state->params[i];
> >> 
> >> if(qemu_mutex_trylock(>mutex)) {
> >> if (p->quit) {
> >> error_report("%s: channel %d has already quit!", __func__, 
> >> i);
> >> qemu_mutex_unlock(>mutex);
> >> return -1;
> >> }
> >> next_channel = (i + 1) % migrate_multifd_channels();
> >> break;
> >> } else {
> >> /* channel still busy, try the next one */
> >> }
> >> }
> >> multifd_send_state->pages = p->pages;
> >> p->pages = pages;
> >> qemu_mutex_unlock(>mutex);
> >
> > We probably can't for now; multifd_send_thread() will unlock the mutex
> > before the iochannel write()s, while the write()s will need those fields.
> 
> Right, but we'd change that code to do the IO with the lock held. If no
> one is blocking, it should be ok to hold the lock. Anyway, food for
> thought.

I see what you meant.  Sounds possible.

-- 
Peter Xu

Re: [RFC PATCH v3 18/30] migration/multifd: Allow receiving pages without packets

[Peter Xu]

On Tue, Jan 16, 2024 at 05:25:03PM -0300, Fabiano Rosas wrote:
> Peter Xu  writes:
> 
> > On Mon, Nov 27, 2023 at 05:26:00PM -0300, Fabiano Rosas wrote:
> >> Currently multifd does not need to have knowledge of pages on the
> >> receiving side because all the information needed is within the
> >> packets that come in the stream.
> >> 
> >> We're about to add support to fixed-ram migration, which cannot use
> >> packets because it expects the ramblock section in the migration file
> >> to contain only the guest pages data.
> >> 
> >> Add a data structure to transfer pages between the ram migration code
> >> and the multifd receiving threads.
> >> 
> >> We don't want to reuse MultiFDPages_t for two reasons:
> >> 
> >> a) multifd threads don't really need to know about the data they're
> >>receiving.
> >> 
> >> b) the receiving side has to be stopped to load the pages, which means
> >>we can experiment with larger granularities than page size when
> >>transferring data.
> >> 
> >> Signed-off-by: Fabiano Rosas 
> >> ---
> >> - stopped using MultiFDPages_t and added a new structure which can
> >>   take offset + size
> >> ---
> >>  migration/multifd.c | 122 ++--
> >>  migration/multifd.h |  20 
> >>  2 files changed, 138 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/migration/multifd.c b/migration/multifd.c
> >> index c1381bdc21..7dfab2367a 100644
> >> --- a/migration/multifd.c
> >> +++ b/migration/multifd.c
> >> @@ -142,17 +142,36 @@ static void nocomp_recv_cleanup(MultiFDRecvParams *p)
> >>  static int nocomp_recv_data(MultiFDRecvParams *p, Error **errp)
> >>  {
> >>  uint32_t flags = p->flags & MULTIFD_FLAG_COMPRESSION_MASK;
> >> +ERRP_GUARD();
> >>  
> >>  if (flags != MULTIFD_FLAG_NOCOMP) {
> >>  error_setg(errp, "multifd %u: flags received %x flags expected 
> >> %x",
> >> p->id, flags, MULTIFD_FLAG_NOCOMP);
> >>  return -1;
> >>  }
> >> -for (int i = 0; i < p->normal_num; i++) {
> >> -p->iov[i].iov_base = p->host + p->normal[i];
> >> -p->iov[i].iov_len = p->page_size;
> >> +
> >> +if (!migrate_multifd_packets()) {
> >> +MultiFDRecvData *data = p->data;
> >> +size_t ret;
> >> +
> >> +ret = qio_channel_pread(p->c, (char *) data->opaque,
> >> +data->size, data->file_offset, errp);
> >> +if (ret != data->size) {
> >> +error_prepend(errp,
> >> +  "multifd recv (%u): read 0x%zx, expected 0x%zx",
> >> +  p->id, ret, data->size);
> >> +return -1;
> >> +}
> >> +
> >> +return 0;
> >> +} else {
> >> +for (int i = 0; i < p->normal_num; i++) {
> >> +p->iov[i].iov_base = p->host + p->normal[i];
> >> +p->iov[i].iov_len = p->page_size;
> >> +}
> >> +
> >> +return qio_channel_readv_all(p->c, p->iov, p->normal_num, errp);
> >>  }
> >
> > I guess you managed to squash the file loads into "no compression" handler
> > of multifd, but IMHO it's not as clean.
> >
> > Firstly, if to do so, we'd better make sure multifd-compression is not
> > enabled anywhere together with fixed-ram.  I didn't yet see such protection
> > in the series.  I think if it happens we should expect crashes because
> > they'll go into zlib/zstd paths for the file.
> 
> Yes, we need some checks around this.
> 
> >
> > IMHO the only model fixed-ram can share with multifd is the task management
> > part, mutexes, semaphores, etc..
> 
> AFAIU, that's what multifd *is*. Compression would just be another
> client of this task management code. This "nocomp" thing always felt off
> to me.
> 
> > IIRC I used to mention that it'll be nice
> > if we have simply a pool of threads so we can enqueue tasks.
> 
> Right, I don't disagree. However I don't think we can just show up with
> a thread pool and start moving stuff into it. I think the safest way to
> do this is to:
> 
> 1- Adapt multifd so that the client code is the sole responsible for the
>data being sent. No data knowledge by the multifd thread.
> 
>With this, nothing should need to touch multifd threads code
>anymore. New clients just define their methods and prepare the data
>as they please.
> 
> 2- Move everything that is left into multifd. Zero pages, postcopy, etc.
> 
> With 1 and 2 we'd have a pretty good picture of what kinds of operations
> we need to do and what are the requirements for the thread
> infrastructure.
> 
> 3- Try to use existing abstractions within QEMU to replace
>multifd. Write from scratch if none are suitable.
> 
> What do you think? We could put an action plan in place and start
> picking at it. My main concern is about what sorts of hidden assumptions
> are present in the current code that we'd start discovering if we just
> replaced it with something new.

You plan sounds good.  Generalization (3) can happen 

Re: [PATCH v4 3/6] target/riscv: Add helper functions to calculate current number of masked bits for pointer masking

[Deepak Gupta]

On Thu, Jan 18, 2024 at 12:50 PM Richard Henderson
 wrote:
>
> On 1/19/24 04:21, Deepak Gupta wrote:
> > On Tue, Jan 9, 2024 at 2:31 AM Alexey Baturo  
> > wrote:
> >>
> >> From: Alexey Baturo 
> >>
> >> Signed-off-by: Alexey Baturo 
> >> ---
> >
> >> +
> >> +bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
> >> +{
> >> +bool virt_mem_en = false;
> >> +#ifndef CONFIG_USER_ONLY
> >> +int satp_mode = 0;
> >> +int priv_mode = cpu_address_mode(env);
> >> +/* Get current PMM field */
> >> +if (riscv_cpu_mxl(env) == MXL_RV32) {
> >> +satp_mode = get_field(env->satp, SATP32_MODE);
> >> +} else {
> >> +satp_mode = get_field(env->satp, SATP64_MODE);
> >> +}
> >> +virt_mem_en = ((satp_mode != VM_1_10_MBARE) && (priv_mode != PRV_M));
> >> +#endif
> >> +return virt_mem_en;
> >
> > Obsessing a little bit on how to test PM enabled binaries with qemu-user.
> > If we return false above then we're not allowed to test binaries with
> > pointer masking enabled with qemu-user.
> > That use case is not required?
>
> In a previous round I suggested that the ifdefs are not necessary.
> But for now it will always be off for qemu-user.
>
> At some point pointer masking will be in hardware, and the kernel will gain 
> support for
> it, and there will likely be a prctl() added for it.  At the point the kernel 
> finalizes
> the API, you will be able to enable pointer masking for qemu-user.

I am sure I am missing some important detail here, BUT...

How is it different from aarch64 "top byte ignore".
I think commit: 16c8497 enables top byte ignore for user pointers and
by default for qemu-user for aarch64 target.

IIRC, user <--> kernel abi is only needed for pointers that are passed
to the kernel.

And in the case of qemu-user, we are talking about the host kernel.
Since arm64 had TBI enabled for qemu-user for a while and I imagine it
works on x86 host kernel
(assuming qemu-user is sanitizing pointers)
Same should work for risc-v qemu-user, right?

>
>
> r~

bug -net tap on qemu

[Kawaguchi Maeda]

hello qemu

I have given up on the documentation that you have provided, you provided
documentation but it did not match the reality when providing it.

are you not serious about giving the command "-net tap"?
I have 4 ethernets namely eth0, eth1, eth2 , and eth3 . My default
connection is eth0, but I want qemu to only access eth1, so I use this
command

qemu-system-x86_64\
  -m 3G\
  -cpu host\
  -enable-kvm\
  -drive file=window.img,format=raw,if=virtio\
  -net nic,model=virtio\
  -net tap,ifname=eth1,script=no,downscript=no \
  -vnc :0

then I get an error message
qemu-system-x86_64: could not configure /dev/net/tun (eth1): Invalid
argument

I've searched on Google and chatgpt can't help, wow this is the first time
I don't understand. usually if it's stuck I ask chatgpt and the results are
successful but this never works.

I don't understand anymore, help me

Re: [PATCH 00/20] arm: Rework target/ headers to build various hw/ files once

[Richard Henderson]

On 1/19/24 07:06, Philippe Mathieu-Daudé wrote:

Hi,

In order to fix a bug noticed [*] by Cédric and Fabiano in my
"Remove one use of qemu_get_cpu() in A7/A15 MPCore priv" series,
I ended reusing commits from other branches and it grew quite
a lot. This is the first "cleanup" part, unrelated on MPCorePriv.

Please review,

Phil.

Philippe Mathieu-Daudé (18):
   hw/arm/exynos4210: Include missing 'exec/tswap.h' header
   hw/arm/xilinx_zynq: Include missing 'exec/tswap.h' header
   hw/arm/smmuv3: Include missing 'hw/registerfields.h' header
   hw/arm/xlnx-versal: Include missing 'cpu.h' header
   target/arm/cpu-features: Include missing 'hw/registerfields.h' header
   target/arm/cpregs: Include missing 'hw/registerfields.h' header
   target/arm/cpregs: Include missing 'kvm-consts.h' header
   target/arm: Expose arm_cpu_mp_affinity() in 'multiprocessing.h' header
   target/arm: Declare ARM_CPU_TYPE_NAME/SUFFIX in 'cpu-qom.h'
   hw/cpu/a9mpcore: Build it only once
   hw/misc/xlnx-versal-crl: Include generic 'cpu-qom.h' instead of
 'cpu.h'
   hw/misc/xlnx-versal-crl: Build it only once
   target/arm: Expose M-profile register bank index definitions
   hw/arm/armv7m: Make 'hw/intc/armv7m_nvic.h' a target agnostic header
   target/arm: Move ARM_CPU_IRQ/FIQ definitions to 'cpu-qom.h' header
   target/arm: Move e2h_access() helper around
   target/arm: Move GTimer definitions to new 'gtimer.h' header
   hw/arm: Build various units only once


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v3 2/2] target/ppc: Implement attn instruction on BookS 64-bit processors

[Richard Henderson]

On 1/19/24 02:25, Nicholas Piggin wrote:

+/* attn enable check */
+static inline int check_attn_none(CPUPPCState *env)


Don't mark inline ...


@@ -2150,6 +2170,7 @@ POWERPC_FAMILY(405)(ObjectClass *oc, void *data)
  dc->desc = "PowerPC 405";
  pcc->init_proc = init_proc_405;
  pcc->check_pow = check_pow_nocheck;
+pcc->check_attn = check_attn_none;


... since it is never called directly.


r~

[PATCH v5 5/9] target/ppc: Simplify syscall exception handlers

[BALATON Zoltan]

After previous changes the hypercall handling in 7xx and 74xx
exception handlers can be folded into one if statement to simpilfy
this code. Also add "unlikely" to mark the less freqiently used branch
for the compiler.

Signed-off-by: BALATON Zoltan 
---
 target/ppc/excp_helper.c | 24 
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 411d67376c..035a9fd968 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -762,26 +762,22 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
 case POWERPC_EXCP_SYSCALL:   /* System call exception*/
 {
 int lev = env->error_code;
-
-if (lev == 1 && cpu->vhyp) {
-dump_hcall(env);
-} else {
-dump_syscall(env);
-}
 /*
  * The Virtual Open Firmware (VOF) relies on the 'sc 1'
  * instruction to communicate with QEMU. The pegasos2 machine
  * uses VOF and the 7xx CPUs, so although the 7xx don't have
  * HV mode, we need to keep hypercall support.
  */
-if (lev == 1 && cpu->vhyp) {
+if (unlikely(lev == 1 && cpu->vhyp)) {
 PPCVirtualHypervisorClass *vhc =
 PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+dump_hcall(env);
 vhc->hypercall(cpu->vhyp, cpu);
 powerpc_reset_excp_state(cpu);
 return;
+} else {
+dump_syscall(env);
 }
-
 break;
 }
 case POWERPC_EXCP_FPU:   /* Floating-point unavailable exception */
@@ -907,26 +903,22 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
 case POWERPC_EXCP_SYSCALL:   /* System call exception*/
 {
 int lev = env->error_code;
-
-if (lev == 1 && cpu->vhyp) {
-dump_hcall(env);
-} else {
-dump_syscall(env);
-}
 /*
  * The Virtual Open Firmware (VOF) relies on the 'sc 1'
  * instruction to communicate with QEMU. The pegasos2 machine
  * uses VOF and the 74xx CPUs, so although the 74xx don't have
  * HV mode, we need to keep hypercall support.
  */
-if (lev == 1 && cpu->vhyp) {
+if (unlikely(lev == 1 && cpu->vhyp)) {
 PPCVirtualHypervisorClass *vhc =
 PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+dump_hcall(env);
 vhc->hypercall(cpu->vhyp, cpu);
 powerpc_reset_excp_state(cpu);
 return;
+} else {
+dump_syscall(env);
 }
-
 break;
 }
 case POWERPC_EXCP_FPU:   /* Floating-point unavailable exception */
-- 
2.30.9

[PATCH v5 3/9] target/ppc: Fix gen_sc to use correct nip

[BALATON Zoltan]

Most exceptions are raised with nip pointing to the faulting
instruction but the sc instruction generating a syscall exception
leaves nip pointing to next instruction. Fix gen_sc to not use
gen_exception_err() which sets nip back but correctly set nip to
pc_next so we don't have to patch this in the exception handlers.

Signed-off-by: BALATON Zoltan 
Reviewed-by: Nicholas Piggin 
---
 target/ppc/excp_helper.c | 43 ++--
 target/ppc/translate.c   | 10 ++
 2 files changed, 8 insertions(+), 45 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 39eefc168a..1c07a11405 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -116,7 +116,7 @@ static void dump_syscall(CPUPPCState *env)
   ppc_dump_gpr(env, 0), ppc_dump_gpr(env, 3),
   ppc_dump_gpr(env, 4), ppc_dump_gpr(env, 5),
   ppc_dump_gpr(env, 6), ppc_dump_gpr(env, 7),
-  ppc_dump_gpr(env, 8), env->nip);
+  ppc_dump_gpr(env, 8), env->nip - 4);
 }
 
 static void dump_hcall(CPUPPCState *env)
@@ -131,7 +131,7 @@ static void dump_hcall(CPUPPCState *env)
   ppc_dump_gpr(env, 7), ppc_dump_gpr(env, 8),
   ppc_dump_gpr(env, 9), ppc_dump_gpr(env, 10),
   ppc_dump_gpr(env, 11), ppc_dump_gpr(env, 12),
-  env->nip);
+  env->nip - 4);
 }
 
 #ifdef CONFIG_TCG
@@ -516,12 +516,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
 break;
 case POWERPC_EXCP_SYSCALL:   /* System call exception*/
 dump_syscall(env);
-
-/*
- * We need to correct the NIP which in this case is supposed
- * to point to the next instruction
- */
-env->nip += 4;
 break;
 case POWERPC_EXCP_FIT:   /* Fixed-interval timer interrupt   */
 trace_ppc_excp_print("FIT");
@@ -632,12 +626,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
 break;
 case POWERPC_EXCP_SYSCALL:   /* System call exception*/
 dump_syscall(env);
-
-/*
- * We need to correct the NIP which in this case is supposed
- * to point to the next instruction
- */
-env->nip += 4;
 break;
 case POWERPC_EXCP_FPU:   /* Floating-point unavailable exception */
 case POWERPC_EXCP_DECR:  /* Decrementer exception*/
@@ -780,13 +768,6 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
 } else {
 dump_syscall(env);
 }
-
-/*
- * We need to correct the NIP which in this case is supposed
- * to point to the next instruction
- */
-env->nip += 4;
-
 /*
  * The Virtual Open Firmware (VOF) relies on the 'sc 1'
  * instruction to communicate with QEMU. The pegasos2 machine
@@ -932,13 +913,6 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
 } else {
 dump_syscall(env);
 }
-
-/*
- * We need to correct the NIP which in this case is supposed
- * to point to the next instruction
- */
-env->nip += 4;
-
 /*
  * The Virtual Open Firmware (VOF) relies on the 'sc 1'
  * instruction to communicate with QEMU. The pegasos2 machine
@@ -1098,12 +1072,6 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
 break;
 case POWERPC_EXCP_SYSCALL:   /* System call exception*/
 dump_syscall(env);
-
-/*
- * We need to correct the NIP which in this case is supposed
- * to point to the next instruction
- */
-env->nip += 4;
 break;
 case POWERPC_EXCP_FPU:   /* Floating-point unavailable exception */
 case POWERPC_EXCP_APU:   /* Auxiliary processor unavailable  */
@@ -1418,13 +1386,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
 } else {
 dump_syscall(env);
 }
-
-/*
- * We need to correct the NIP which in this case is supposed
- * to point to the next instruction
- */
-env->nip += 4;
-
 /* "PAPR mode" built-in hypercall emulation */
 if (lev == 1 && books_vhyp_handles_hcall(cpu)) {
 PPCVirtualHypervisorClass *vhc =
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 329da4d518..a80d24143e 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4535,15 +4535,17 @@ static void gen_hrfid(DisasContext *ctx)
 #endif
 static void gen_sc(DisasContext *ctx)
 {
-uint32_t lev;
-
 /*
  * LEV is a 7-bit field, but the top 6 bits are treated as a reserved
  * field (i.e., ignored). ISA v3.1 changes that to 5 bits, but that is
  * for Ultravisor which TCG does not support, so just ignore the top 6.
  */
-lev = 

[PATCH v5 1/9] target/ppc: Use env_cpu for cpu_abort in excp_helper

[BALATON Zoltan]

Use the env_cpu function to get the CPUState for cpu_abort. These are
only needed in case of fatal errors so this allows to avoid casting
and storing CPUState in a local variable wnen not needed.

Signed-off-by: BALATON Zoltan 
---
 target/ppc/excp_helper.c | 118 +--
 1 file changed, 63 insertions(+), 55 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 2ec6429e36..b8fd01d04c 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -445,7 +445,6 @@ static void powerpc_mcheck_checkstop(CPUPPCState *env)
 
 static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
 {
-CPUState *cs = CPU(cpu);
 CPUPPCState *env = >env;
 target_ulong msr, new_msr, vector;
 int srr0, srr1;
@@ -473,8 +472,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
 
 vector = env->excp_vectors[excp];
 if (vector == (target_ulong)-1ULL) {
-cpu_abort(cs, "Raised an exception without defined vector %d\n",
-  excp);
+cpu_abort(env_cpu(env),
+  "Raised an exception without defined vector %d\n", excp);
 }
 
 vector |= env->excp_prefix;
@@ -523,7 +522,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
 env->spr[SPR_40x_ESR] = ESR_PTR;
 break;
 default:
-cpu_abort(cs, "Invalid program exception %d. Aborting\n",
+cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
   env->error_code);
 break;
 }
@@ -550,11 +549,12 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
 trace_ppc_excp_print("PIT");
 break;
 case POWERPC_EXCP_DEBUG: /* Debug interrupt  */
-cpu_abort(cs, "%s exception not implemented\n",
+cpu_abort(env_cpu(env), "%s exception not implemented\n",
   powerpc_excp_name(excp));
 break;
 default:
-cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+  excp);
 break;
 }
 
@@ -569,7 +569,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
 
 static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
 {
-CPUState *cs = CPU(cpu);
 CPUPPCState *env = >env;
 target_ulong msr, new_msr, vector;
 
@@ -592,8 +591,8 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
 
 vector = env->excp_vectors[excp];
 if (vector == (target_ulong)-1ULL) {
-cpu_abort(cs, "Raised an exception without defined vector %d\n",
-  excp);
+cpu_abort(env_cpu(env),
+  "Raised an exception without defined vector %d\n", excp);
 }
 
 vector |= env->excp_prefix;
@@ -653,7 +652,7 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
 break;
 default:
 /* Should never occur */
-cpu_abort(cs, "Invalid program exception %d. Aborting\n",
+cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
   env->error_code);
 break;
 }
@@ -675,8 +674,9 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
 break;
 case POWERPC_EXCP_RESET: /* System reset exception   */
 if (FIELD_EX64(env->msr, MSR, POW)) {
-cpu_abort(cs, "Trying to deliver power-saving system reset "
-  "exception %d with no HV support\n", excp);
+cpu_abort(env_cpu(env),
+  "Trying to deliver power-saving system reset exception "
+  "%d with no HV support\n", excp);
 }
 break;
 case POWERPC_EXCP_TRACE: /* Trace exception  */
@@ -703,11 +703,12 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
 case POWERPC_EXCP_SMI:   /* System management interrupt  */
 case POWERPC_EXCP_MEXTBR:/* Maskable external breakpoint */
 case POWERPC_EXCP_NMEXTBR:   /* Non maskable external breakpoint */
-cpu_abort(cs, "%s exception not implemented\n",
+cpu_abort(env_cpu(env), "%s exception not implemented\n",
   powerpc_excp_name(excp));
 break;
 default:
-cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+  excp);
 break;
 }
 
@@ -730,7 +731,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
 
 static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
 {
-CPUState *cs = CPU(cpu);
 CPUPPCState *env = >env;
 target_ulong msr, new_msr, vector;
 
@@ -753,8 +753,8 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
 
 vector = env->excp_vectors[excp];
 if (vector == (target_ulong)-1ULL) {
-

[PATCH v5 6/9] target/ppc: Clean up ifdefs in excp_helper.c, part 1

[BALATON Zoltan]

Use #ifdef, #ifndef for brevity and add comments to #endif that are
more than a few lines apart for clarity.

Signed-off-by: BALATON Zoltan 
---
 target/ppc/excp_helper.c | 49 
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 035a9fd968..d8eab4ff6c 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -35,7 +35,7 @@
 
 /*/
 /* Exception processing */
-#if !defined(CONFIG_USER_ONLY)
+#ifndef CONFIG_USER_ONLY
 
 static const char *powerpc_excp_name(int excp)
 {
@@ -186,7 +186,7 @@ static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int 
excp)
  env->error_code);
 }
 
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
 static int powerpc_reset_wakeup(CPUPPCState *env, int excp, target_ulong *msr)
 {
 /* We no longer are in a PM state */
@@ -380,7 +380,7 @@ static void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp, 
target_ulong msr,
 }
 }
 }
-#endif
+#endif /* TARGET_PPC64 */
 
 static void powerpc_reset_excp_state(PowerPCCPU *cpu)
 {
@@ -1123,7 +1123,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
 break;
 }
 
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
 if (env->spr[SPR_BOOKE_EPCR] & EPCR_ICM) {
 /* Cat.64-bit: EPCR.ICM is copied to MSR.CM */
 new_msr |= (target_ulong)1 << MSR_CM;
@@ -1537,7 +1537,7 @@ static inline void powerpc_excp_books(PowerPCCPU *cpu, 
int excp)
 {
 g_assert_not_reached();
 }
-#endif
+#endif /* TARGET_PPC64 */
 
 static void powerpc_excp(PowerPCCPU *cpu, int excp)
 {
@@ -1588,7 +1588,7 @@ void ppc_cpu_do_interrupt(CPUState *cs)
 powerpc_excp(cpu, cs->exception_index);
 }
 
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
 #define P7_UNUSED_INTERRUPTS \
 (PPC_INTERRUPT_RESET | PPC_INTERRUPT_HVIRT | PPC_INTERRUPT_CEXT |   \
  PPC_INTERRUPT_WDT | PPC_INTERRUPT_CDOORBELL | PPC_INTERRUPT_FIT |  \
@@ -1919,7 +1919,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
 
 return 0;
 }
-#endif
+#endif /* TARGET_PPC64 */
 
 static int ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
 {
@@ -2036,7 +2036,7 @@ static int 
ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
 static int ppc_next_unmasked_interrupt(CPUPPCState *env)
 {
 switch (env->excp_model) {
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
 case POWERPC_EXCP_POWER7:
 return p7_next_unmasked_interrupt(env);
 case POWERPC_EXCP_POWER8:
@@ -2075,7 +2075,7 @@ void ppc_maybe_interrupt(CPUPPCState *env)
 }
 }
 
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
 static void p7_deliver_interrupt(CPUPPCState *env, int interrupt)
 {
 PowerPCCPU *cpu = env_archcpu(env);
@@ -2277,7 +2277,7 @@ static void p9_deliver_interrupt(CPUPPCState *env, int 
interrupt)
   interrupt);
 }
 }
-#endif
+#endif /* TARGET_PPC64 */
 
 static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
 {
@@ -2386,7 +2386,7 @@ static void ppc_deliver_interrupt_generic(CPUPPCState 
*env, int interrupt)
 static void ppc_deliver_interrupt(CPUPPCState *env, int interrupt)
 {
 switch (env->excp_model) {
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
 case POWERPC_EXCP_POWER7:
 p7_deliver_interrupt(env, interrupt);
 break;
@@ -2496,9 +2496,9 @@ void helper_raise_exception(CPUPPCState *env, uint32_t 
exception)
 {
 raise_exception_err_ra(env, exception, 0, 0);
 }
-#endif
+#endif /* CONFIG_TCG */
 
-#if !defined(CONFIG_USER_ONLY)
+#ifndef CONFIG_USER_ONLY
 #ifdef CONFIG_TCG
 void helper_store_msr(CPUPPCState *env, target_ulong val)
 {
@@ -2515,7 +2515,7 @@ void helper_ppc_maybe_interrupt(CPUPPCState *env)
 ppc_maybe_interrupt(env);
 }
 
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
 void helper_scv(CPUPPCState *env, uint32_t lev)
 {
 if (env->spr[SPR_FSCR] & (1ull << FSCR_SCV)) {
@@ -2544,7 +2544,7 @@ void helper_pminsn(CPUPPCState *env, uint32_t insn)
 
 ppc_maybe_interrupt(env);
 }
-#endif /* defined(TARGET_PPC64) */
+#endif /* TARGET_PPC64 */
 
 static void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr)
 {
@@ -2555,7 +2555,7 @@ static void do_rfi(CPUPPCState *env, target_ulong nip, 
target_ulong msr)
 if (env->flags & POWERPC_FLAG_TGPR)
 msr &= ~(1ULL << MSR_TGPR);
 
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
 /* Switching to 32-bit ? Crop the nip */
 if (!msr_is_64bit(env, msr)) {
 nip = (uint32_t)nip;
@@ -2584,7 +2584,7 @@ void helper_rfi(CPUPPCState *env)
 do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xul);
 }
 
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
 void helper_rfid(CPUPPCState *env)
 {
 /*
@@ -2605,7 +2605,7 @@ void helper_hrfid(CPUPPCState *env)
 {
 do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]);
 }
-#endif
+#endif /* TARGET_PPC64 */
 
 #if 

[PATCH v5 2/9] target/ppc: Readability improvements in exception handlers

[BALATON Zoltan]

Improve readability by shortening some long comments, removing
comments that state the obvious and dropping some empty lines so they
don't distract when reading the code.

Signed-off-by: BALATON Zoltan 
Acked-by: Nicholas Piggin 
---
 target/ppc/cpu.h |   1 +
 target/ppc/excp_helper.c | 179 +++
 2 files changed, 33 insertions(+), 147 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index f8101ffa29..2f9b610abc 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2902,6 +2902,7 @@ static inline bool ppc_has_spr(PowerPCCPU *cpu, int spr)
 }
 
 #if !defined(CONFIG_USER_ONLY)
+/* Sort out endianness of interrupt. Depends on the CPU, HV mode, etc. */
 static inline bool ppc_interrupts_little_endian(PowerPCCPU *cpu, bool hv)
 {
 PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index b8fd01d04c..39eefc168a 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -403,9 +403,8 @@ static void powerpc_set_excp_state(PowerPCCPU *cpu, 
target_ulong vector,
  * We don't use hreg_store_msr here as already have treated any
  * special case that could occur. Just store MSR and update hflags
  *
- * Note: We *MUST* not use hreg_store_msr() as-is anyway because it
- * will prevent setting of the HV bit which some exceptions might need
- * to do.
+ * Note: We *MUST* not use hreg_store_msr() as-is anyway because it will
+ * prevent setting of the HV bit which some exceptions might need to do.
  */
 env->nip = vector;
 env->msr = msr;
@@ -447,25 +446,15 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
 {
 CPUPPCState *env = >env;
 target_ulong msr, new_msr, vector;
-int srr0, srr1;
+int srr0 = SPR_SRR0, srr1 = SPR_SRR1;
 
 /* new srr1 value excluding must-be-zero bits */
 msr = env->msr & ~0x783fULL;
 
-/*
- * new interrupt handler msr preserves existing ME unless
- * explicitly overridden.
- */
+/* new interrupt handler msr preserves ME unless explicitly overriden */
 new_msr = env->msr & (((target_ulong)1 << MSR_ME));
 
-/* target registers */
-srr0 = SPR_SRR0;
-srr1 = SPR_SRR1;
-
-/*
- * Hypervisor emulation assistance interrupt only exists on server
- * arch 2.05 server or later.
- */
+/* HV emu assistance interrupt only exists on server arch 2.05 or later */
 if (excp == POWERPC_EXCP_HV_EMU) {
 excp = POWERPC_EXCP_PROGRAM;
 }
@@ -475,7 +464,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
 cpu_abort(env_cpu(env),
   "Raised an exception without defined vector %d\n", excp);
 }
-
 vector |= env->excp_prefix;
 
 switch (excp) {
@@ -487,7 +475,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
 powerpc_mcheck_checkstop(env);
 /* machine check exceptions don't have ME set */
 new_msr &= ~((target_ulong)1 << MSR_ME);
-
 srr0 = SPR_40x_SRR2;
 srr1 = SPR_40x_SRR3;
 break;
@@ -558,12 +545,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
 break;
 }
 
-/* Save PC */
 env->spr[srr0] = env->nip;
-
-/* Save MSR */
 env->spr[srr1] = msr;
-
 powerpc_set_excp_state(cpu, vector, new_msr);
 }
 
@@ -575,16 +558,10 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
 /* new srr1 value excluding must-be-zero bits */
 msr = env->msr & ~0x783fULL;
 
-/*
- * new interrupt handler msr preserves existing ME unless
- * explicitly overridden
- */
+/* new interrupt handler msr preserves ME unless explicitly overriden */
 new_msr = env->msr & ((target_ulong)1 << MSR_ME);
 
-/*
- * Hypervisor emulation assistance interrupt only exists on server
- * arch 2.05 server or later.
- */
+/* HV emu assistance interrupt only exists on server arch 2.05 or later */
 if (excp == POWERPC_EXCP_HV_EMU) {
 excp = POWERPC_EXCP_PROGRAM;
 }
@@ -594,7 +571,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
 cpu_abort(env_cpu(env),
   "Raised an exception without defined vector %d\n", excp);
 }
-
 vector |= env->excp_prefix;
 
 switch (excp) {
@@ -604,7 +580,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
 powerpc_mcheck_checkstop(env);
 /* machine check exceptions don't have ME set */
 new_msr &= ~((target_ulong)1 << MSR_ME);
-
 break;
 case POWERPC_EXCP_DSI:   /* Data storage exception   */
 trace_ppc_excp_dsi(env->spr[SPR_DSISR], env->spr[SPR_DAR]);
@@ -632,11 +607,9 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
 powerpc_reset_excp_state(cpu);
 return;
 }
-
 /*
- * FP exceptions always have NIP pointing to the faulting
- * instruction, 

[PATCH v5 0/9] Misc clean ups to target/ppc exception handling

[BALATON Zoltan]

These are some small clean ups for target/ppc/excp_helper.c trying to
make this code a bit simpler. No functional change is intended. This
series was submitted before but only partially merged due to freeze
and conflicting series os thia was postponed then to avoid conflicts.

v5:
- rebase on master
- keep logging nip pointing to the sc instruction
- add another patch

v4: Rebased on master dropping what was merged

BALATON Zoltan (9):
  target/ppc: Use env_cpu for cpu_abort in excp_helper
  target/ppc: Readability improvements in exception handlers
  target/ppc: Fix gen_sc to use correct nip
  target/ppc: Move patching nip from exception handler to helper_scv
  target/ppc: Simplify syscall exception handlers
  target/ppc: Clean up ifdefs in excp_helper.c, part 1
  target/ppc: Clean up ifdefs in excp_helper.c, part 2
  target/ppc: Clean up ifdefs in excp_helper.c, part 3
  target/ppc: Remove interrupt handler wrapper functions

 target/ppc/cpu.h |   1 +
 target/ppc/excp_helper.c | 490 +--
 target/ppc/translate.c   |  16 +-
 3 files changed, 170 insertions(+), 337 deletions(-)

-- 
2.30.9

[PATCH v5 9/9] target/ppc: Remove interrupt handler wrapper functions

[BALATON Zoltan]

These wrappers call out to handle POWER7 and newer in separate
functions but reduce to the generic case when TARGET_PPC64 is not
defined. It is easy enough to include the switch in the beginning of
the generic functions to branch out to the specific functions and get
rid of these wrappers. This avoids one indirection and entitely
compiles out the switch without TARGET_PPC64.

Signed-off-by: BALATON Zoltan 
---
 target/ppc/excp_helper.c | 70 ++--
 1 file changed, 31 insertions(+), 39 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 5124c3e6b5..de51627c4c 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1921,8 +1921,21 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
 }
 #endif /* TARGET_PPC64 */
 
-static int ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
+static int ppc_next_unmasked_interrupt(CPUPPCState *env)
 {
+#ifdef TARGET_PPC64
+switch (env->excp_model) {
+case POWERPC_EXCP_POWER7:
+return p7_next_unmasked_interrupt(env);
+case POWERPC_EXCP_POWER8:
+return p8_next_unmasked_interrupt(env);
+case POWERPC_EXCP_POWER9:
+case POWERPC_EXCP_POWER10:
+return p9_next_unmasked_interrupt(env);
+default:
+break;
+}
+#endif
 bool async_deliver;
 
 /* External reset */
@@ -2033,23 +2046,6 @@ static int 
ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
 return 0;
 }
 
-static int ppc_next_unmasked_interrupt(CPUPPCState *env)
-{
-switch (env->excp_model) {
-#ifdef TARGET_PPC64
-case POWERPC_EXCP_POWER7:
-return p7_next_unmasked_interrupt(env);
-case POWERPC_EXCP_POWER8:
-return p8_next_unmasked_interrupt(env);
-case POWERPC_EXCP_POWER9:
-case POWERPC_EXCP_POWER10:
-return p9_next_unmasked_interrupt(env);
-#endif
-default:
-return ppc_next_unmasked_interrupt_generic(env);
-}
-}
-
 /*
  * Sets CPU_INTERRUPT_HARD if there is at least one unmasked interrupt to be
  * delivered and clears CPU_INTERRUPT_HARD otherwise.
@@ -2279,8 +2275,24 @@ static void p9_deliver_interrupt(CPUPPCState *env, int 
interrupt)
 }
 #endif /* TARGET_PPC64 */
 
-static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
+static void ppc_deliver_interrupt(CPUPPCState *env, int interrupt)
 {
+#ifdef TARGET_PPC64
+switch (env->excp_model) {
+case POWERPC_EXCP_POWER7:
+p7_deliver_interrupt(env, interrupt);
+return;
+case POWERPC_EXCP_POWER8:
+p8_deliver_interrupt(env, interrupt);
+return;
+case POWERPC_EXCP_POWER9:
+case POWERPC_EXCP_POWER10:
+p9_deliver_interrupt(env, interrupt);
+return;
+default:
+break;
+}
+#endif
 PowerPCCPU *cpu = env_archcpu(env);
 
 switch (interrupt) {
@@ -2383,26 +2395,6 @@ static void ppc_deliver_interrupt_generic(CPUPPCState 
*env, int interrupt)
 }
 }
 
-static void ppc_deliver_interrupt(CPUPPCState *env, int interrupt)
-{
-switch (env->excp_model) {
-#ifdef TARGET_PPC64
-case POWERPC_EXCP_POWER7:
-p7_deliver_interrupt(env, interrupt);
-break;
-case POWERPC_EXCP_POWER8:
-p8_deliver_interrupt(env, interrupt);
-break;
-case POWERPC_EXCP_POWER9:
-case POWERPC_EXCP_POWER10:
-p9_deliver_interrupt(env, interrupt);
-break;
-#endif
-default:
-ppc_deliver_interrupt_generic(env, interrupt);
-}
-}
-
 void ppc_cpu_do_system_reset(CPUState *cs)
 {
 PowerPCCPU *cpu = POWERPC_CPU(cs);
-- 
2.30.9

[PATCH v5 4/9] target/ppc: Move patching nip from exception handler to helper_scv

[BALATON Zoltan]

From: Nicholas Piggin 

Unlike sc, for scv a facility unavailable interrupt must be generated
if FSCR[SCV]=0 so we can't raise the exception with nip set to next
instruction but we can move advancing nip if the FSCR check passes to
helper_scv so the exception handler does not need to change it.

[balaton: added commit message]
Signed-off-by: BALATON Zoltan 
---
 target/ppc/excp_helper.c | 2 +-
 target/ppc/translate.c   | 6 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 1c07a11405..411d67376c 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1405,7 +1405,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
 case POWERPC_EXCP_SYSCALL_VECTORED: /* scv exception */
 lev = env->error_code;
 dump_syscall(env);
-env->nip += 4;
 new_msr |= env->msr & ((target_ulong)1 << MSR_EE);
 new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
 
@@ -2528,6 +2527,7 @@ void helper_ppc_maybe_interrupt(CPUPPCState *env)
 void helper_scv(CPUPPCState *env, uint32_t lev)
 {
 if (env->spr[SPR_FSCR] & (1ull << FSCR_SCV)) {
+env->nip += 4;
 raise_exception_err(env, POWERPC_EXCP_SYSCALL_VECTORED, lev);
 } else {
 raise_exception_err(env, POWERPC_EXCP_FU, FSCR_IC_SCV);
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index a80d24143e..d8cd34721c 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4554,7 +4554,11 @@ static void gen_scv(DisasContext *ctx)
 {
 uint32_t lev = (ctx->opcode >> 5) & 0x7F;
 
-/* Set the PC back to the faulting instruction. */
+/*
+ * Set the PC back to the scv instruction (unlike sc), because a facility
+ * unavailable interrupt must be generated if FSCR[SCV]=0. The helper
+ * advances nip if the FSCR check passes.
+ */
 gen_update_nip(ctx, ctx->cia);
 gen_helper_scv(tcg_env, tcg_constant_i32(lev));
 
-- 
2.30.9

[PATCH v5 7/9] target/ppc: Clean up ifdefs in excp_helper.c, part 2

[BALATON Zoltan]

Remove check for !defined(CONFIG_USER_ONLY) as this is already within
an #ifndef CONFIG_USER_ONLY block.

Signed-off-by: BALATON Zoltan 
---
 target/ppc/excp_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index d8eab4ff6c..2d4a72883f 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2607,7 +2607,7 @@ void helper_hrfid(CPUPPCState *env)
 }
 #endif /* TARGET_PPC64 */
 
-#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
+#ifdef TARGET_PPC64
 void helper_rfebb(CPUPPCState *env, target_ulong s)
 {
 target_ulong msr = env->msr;
@@ -2682,7 +2682,7 @@ void raise_ebb_perfm_exception(CPUPPCState *env)
 
 do_ebb(env, POWERPC_EXCP_PERFM_EBB);
 }
-#endif /* defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) */
+#endif /* TARGET_PPC64 */
 
 /*/
 /* Embedded PowerPC specific helpers */
-- 
2.30.9

[PATCH v5 8/9] target/ppc: Clean up ifdefs in excp_helper.c, part 3

[BALATON Zoltan]

Concatenate #if blocks that are ending then beginning on the next line
again.

Signed-off-by: BALATON Zoltan 
---
 target/ppc/excp_helper.c | 15 ++-
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 2d4a72883f..5124c3e6b5 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2496,10 +2496,8 @@ void helper_raise_exception(CPUPPCState *env, uint32_t 
exception)
 {
 raise_exception_err_ra(env, exception, 0, 0);
 }
-#endif /* CONFIG_TCG */
 
 #ifndef CONFIG_USER_ONLY
-#ifdef CONFIG_TCG
 void helper_store_msr(CPUPPCState *env, target_ulong val)
 {
 uint32_t excp = hreg_store_msr(env, val, 0);
@@ -2605,9 +2603,7 @@ void helper_hrfid(CPUPPCState *env)
 {
 do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]);
 }
-#endif /* TARGET_PPC64 */
 
-#ifdef TARGET_PPC64
 void helper_rfebb(CPUPPCState *env, target_ulong s)
 {
 target_ulong msr = env->msr;
@@ -2707,10 +2703,8 @@ void helper_rfmci(CPUPPCState *env)
 /* FIXME: choose CSRR1 or MCSRR1 based on cpu type */
 do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1]);
 }
-#endif /* CONFIG_TCG */
-#endif /* !defined(CONFIG_USER_ONLY) */
+#endif /* !CONFIG_USER_ONLY */
 
-#ifdef CONFIG_TCG
 void helper_tw(CPUPPCState *env, target_ulong arg1, target_ulong arg2,
uint32_t flags)
 {
@@ -2738,9 +2732,7 @@ void helper_td(CPUPPCState *env, target_ulong arg1, 
target_ulong arg2,
 }
 }
 #endif /* TARGET_PPC64 */
-#endif /* CONFIG_TCG */
 
-#ifdef CONFIG_TCG
 static uint32_t helper_SIMON_LIKE_32_64(uint32_t x, uint64_t key, uint32_t 
lane)
 {
 const uint16_t c = 0xfffc;
@@ -2851,11 +2843,8 @@ HELPER_HASH(HASHST, env->spr[SPR_HASHKEYR], true, NPHIE)
 HELPER_HASH(HASHCHK, env->spr[SPR_HASHKEYR], false, NPHIE)
 HELPER_HASH(HASHSTP, env->spr[SPR_HASHPKEYR], true, PHIE)
 HELPER_HASH(HASHCHKP, env->spr[SPR_HASHPKEYR], false, PHIE)
-#endif /* CONFIG_TCG */
 
 #ifndef CONFIG_USER_ONLY
-#ifdef CONFIG_TCG
-
 /* Embedded.Processor Control */
 static int dbell2irq(target_ulong rb)
 {
@@ -3197,5 +3186,5 @@ bool ppc_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 return false;
 }
 
-#endif /* CONFIG_TCG */
 #endif /* !CONFIG_USER_ONLY */
+#endif /* CONFIG_TCG */
-- 
2.30.9

Re: [PATCH v4 4/4] virtio-blk: add iothread-vq-mapping parameter

[Stefan Hajnoczi]

On Thu, Dec 21, 2023 at 02:10:20PM +0100, Kevin Wolf wrote:
> Am 20.12.2023 um 14:47 hat Stefan Hajnoczi geschrieben:
> > Add the iothread-vq-mapping parameter to assign virtqueues to IOThreads.
> > Store the vq:AioContext mapping in the new struct
> > VirtIOBlockDataPlane->vq_aio_context[] field and refactor the code to
> > use the per-vq AioContext instead of the BlockDriverState's AioContext.
> > 
> > Reimplement --device virtio-blk-pci,iothread= and non-IOThread mode by
> > assigning all virtqueues to the IOThread and main loop's AioContext in
> > vq_aio_context[], respectively.
> > 
> > The comment in struct VirtIOBlockDataPlane about EventNotifiers is
> > stale. Remove it.
> > 
> > Signed-off-by: Stefan Hajnoczi 
> 
> I'm looking at virtio_blk_dma_restart_cb/bh(). It seems to run all
> queued requests in the iothread of the first vq, but when the requests
> complete, they will push the result to their original vq.
> 
> Do we know that the dataplane isn't started and won't be started until
> the requests complete? (I wouldn't expect so, because then moving to the
> AioContext of the BlockBackend wouldn't have been necessary either.) Or
> is there another reason why this is safe?

You are right. I overlooked this. I'll send a patch to make s->rq
multi-queue safe.

Stefan


signature.asc
Description: PGP signature

Re: [PATCH 1/2] target/s390x: Emulate CVDG

[Richard Henderson]

On 1/16/24 07:21, Ilya Leoshkevich wrote:

CVDG is the same as CVD, except that it converts 64 bits into 128,
rather than 32 into 64. Use larger data types in the CVD helper and
reuse it.

Reported-by: Ido Plat 
Signed-off-by: Ilya Leoshkevich 
---
  target/s390x/helper.h|  1 +
  target/s390x/tcg/insn-data.h.inc |  1 +
  target/s390x/tcg/int_helper.c| 11 ---
  target/s390x/tcg/translate.c |  8 
  4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 05102578fc9..332a9a9c632 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -89,6 +89,7 @@ DEF_HELPER_FLAGS_2(sqeb, TCG_CALL_NO_WG, i64, env, i64)
  DEF_HELPER_FLAGS_2(sqdb, TCG_CALL_NO_WG, i64, env, i64)
  DEF_HELPER_FLAGS_2(sqxb, TCG_CALL_NO_WG, i128, env, i128)
  DEF_HELPER_FLAGS_1(cvd, TCG_CALL_NO_RWG_SE, i64, s32)
+DEF_HELPER_FLAGS_1(cvdg, TCG_CALL_NO_RWG_SE, i128, s64)
  DEF_HELPER_FLAGS_4(pack, TCG_CALL_NO_WG, void, env, i32, i64, i64)
  DEF_HELPER_FLAGS_4(pka, TCG_CALL_NO_WG, void, env, i64, i64, i32)
  DEF_HELPER_FLAGS_4(pku, TCG_CALL_NO_WG, void, env, i64, i64, i32)
diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc
index 2f07f39d9cb..388dcb8dbbc 100644
--- a/target/s390x/tcg/insn-data.h.inc
+++ b/target/s390x/tcg/insn-data.h.inc
@@ -296,6 +296,7 @@
  /* CONVERT TO DECIMAL */
  C(0x4e00, CVD, RX_a,  Z,   r1_o, a2, 0, 0, cvd, 0)
  C(0xe326, CVDY,RXY_a, LD,  r1_o, a2, 0, 0, cvd, 0)
+C(0xe32e, CVDG,RXY_a, Z,   r1_o, a2, 0, 0, cvdg, 0)
  /* CONVERT TO FIXED */
  F(0xb398, CFEBR,   RRF_e, Z,   0, e2, new, r1_32, cfeb, 0, IF_BFP)
  F(0xb399, CFDBR,   RRF_e, Z,   0, f2, new, r1_32, cfdb, 0, IF_BFP)
diff --git a/target/s390x/tcg/int_helper.c b/target/s390x/tcg/int_helper.c
index eb8e6dd1b57..defb8fc7681 100644
--- a/target/s390x/tcg/int_helper.c
+++ b/target/s390x/tcg/int_helper.c
@@ -99,10 +99,15 @@ Int128 HELPER(divu64)(CPUS390XState *env, uint64_t ah, 
uint64_t al, uint64_t b)
  }
  
  uint64_t HELPER(cvd)(int32_t reg)

+{
+return helper_cvdg(reg);
+}
+
+Int128 HELPER(cvdg)(int64_t reg)
  {
  /* positive 0 */
-uint64_t dec = 0x0c;
-int64_t bin = reg;
+Int128 dec = 0x0c;
+Int128 bin = reg;
  int shift;
  
  if (bin < 0) {

@@ -110,7 +115,7 @@ uint64_t HELPER(cvd)(int32_t reg)
  dec = 0x0d;
  }
  
-for (shift = 4; (shift < 64) && bin; shift += 4) {

+for (shift = 4; (shift < 128) && bin; shift += 4) {
  dec |= (bin % 10) << shift;
  bin /= 10;
  }


None of this will work with the struct version of Int128 -- you need to use the int128_* 
functions for initialization and arithmetic.


I suggest you don't try to share code with CVD.


r~

Re: [PATCH v4 3/6] target/riscv: Add helper functions to calculate current number of masked bits for pointer masking

[Richard Henderson]

On 1/19/24 04:21, Deepak Gupta wrote:

On Tue, Jan 9, 2024 at 2:31 AM Alexey Baturo  wrote:


From: Alexey Baturo 

Signed-off-by: Alexey Baturo 
---



+
+bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
+{
+bool virt_mem_en = false;
+#ifndef CONFIG_USER_ONLY
+int satp_mode = 0;
+int priv_mode = cpu_address_mode(env);
+/* Get current PMM field */
+if (riscv_cpu_mxl(env) == MXL_RV32) {
+satp_mode = get_field(env->satp, SATP32_MODE);
+} else {
+satp_mode = get_field(env->satp, SATP64_MODE);
+}
+virt_mem_en = ((satp_mode != VM_1_10_MBARE) && (priv_mode != PRV_M));
+#endif
+return virt_mem_en;


Obsessing a little bit on how to test PM enabled binaries with qemu-user.
If we return false above then we're not allowed to test binaries with
pointer masking enabled with qemu-user.
That use case is not required?


In a previous round I suggested that the ifdefs are not necessary.
But for now it will always be off for qemu-user.

At some point pointer masking will be in hardware, and the kernel will gain support for 
it, and there will likely be a prctl() added for it.  At the point the kernel finalizes 
the API, you will be able to enable pointer masking for qemu-user.



r~

[PATCH 16/20] hw/arm/armv7m: Make 'hw/intc/armv7m_nvic.h' a target agnostic header

[Philippe Mathieu-Daudé]

Now than we can access the M-profile bank index
definitions from the target-agnostic "cpu-qom.h"
header, we don't need the huge "cpu.h" anymore
(except in hw/arm/armv7m.c). Reduce its inclusion
to the source unit.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/intc/armv7m_nvic.h | 2 +-
 hw/arm/armv7m.c   | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/intc/armv7m_nvic.h b/include/hw/intc/armv7m_nvic.h
index 6b4ae566c9..89fe8aedaa 100644
--- a/include/hw/intc/armv7m_nvic.h
+++ b/include/hw/intc/armv7m_nvic.h
@@ -10,7 +10,7 @@
 #ifndef HW_ARM_ARMV7M_NVIC_H
 #define HW_ARM_ARMV7M_NVIC_H
 
-#include "target/arm/cpu.h"
+#include "target/arm/cpu-qom.h"
 #include "hw/sysbus.h"
 #include "hw/timer/armv7m_systick.h"
 #include "qom/object.h"
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 1f21827773..edcd8adc74 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -21,6 +21,7 @@
 #include "qemu/module.h"
 #include "qemu/log.h"
 #include "target/arm/idau.h"
+#include "target/arm/cpu.h"
 #include "target/arm/cpu-features.h"
 #include "migration/vmstate.h"
 
-- 
2.41.0

[PATCH 19/20] target/arm: Move GTimer definitions to new 'gtimer.h' header

[Philippe Mathieu-Daudé]

Move Arm A-class Generic Timer definitions to the new
"target/arm/gtimer.h" header so units in hw/ which don't
need access to ARMCPU internals can use them without
having to include the huge "cpu.h".

Suggested-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/arm/cpu.h   |  8 +---
 target/arm/gtimer.h| 21 +
 hw/arm/allwinner-h3.c  |  1 +
 hw/arm/allwinner-r40.c |  1 +
 hw/arm/bcm2836.c   |  1 +
 hw/arm/sbsa-ref.c  |  1 +
 hw/arm/virt.c  |  1 +
 hw/arm/xlnx-versal.c   |  1 +
 hw/arm/xlnx-zynqmp.c   |  1 +
 hw/cpu/a15mpcore.c |  1 +
 target/arm/cpu.c   |  1 +
 target/arm/helper.c|  1 +
 target/arm/hvf/hvf.c   |  1 +
 target/arm/kvm.c   |  1 +
 target/arm/machine.c   |  1 +
 15 files changed, 35 insertions(+), 7 deletions(-)
 create mode 100644 target/arm/gtimer.h

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index e8df41d642..d3477b1601 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -27,6 +27,7 @@
 #include "exec/cpu-defs.h"
 #include "qapi/qapi-types-common.h"
 #include "target/arm/multiprocessing.h"
+#include "target/arm/gtimer.h"
 
 /* ARM processors have a weak memory model */
 #define TCG_GUEST_DEFAULT_MO  (0)
@@ -140,13 +141,6 @@ typedef struct ARMGenericTimer {
 uint64_t ctl; /* Timer Control register */
 } ARMGenericTimer;
 
-#define GTIMER_PHYS 0
-#define GTIMER_VIRT 1
-#define GTIMER_HYP  2
-#define GTIMER_SEC  3
-#define GTIMER_HYPVIRT  4
-#define NUM_GTIMERS 5
-
 #define VTCR_NSW (1u << 29)
 #define VTCR_NSA (1u << 30)
 #define VSTCR_SW VTCR_NSW
diff --git a/target/arm/gtimer.h b/target/arm/gtimer.h
new file mode 100644
index 00..b992941bef
--- /dev/null
+++ b/target/arm/gtimer.h
@@ -0,0 +1,21 @@
+/*
+ * ARM generic timer definitions for Arm A-class CPU
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef TARGET_ARM_GTIMER_H
+#define TARGET_ARM_GTIMER_H
+
+enum {
+GTIMER_PHYS = 0,
+GTIMER_VIRT = 1,
+GTIMER_HYP  = 2,
+GTIMER_SEC  = 3,
+GTIMER_HYPVIRT  = 4,
+#define NUM_GTIMERS   5
+};
+
+#endif
diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
index 2d684b5287..380e0ec11d 100644
--- a/hw/arm/allwinner-h3.c
+++ b/hw/arm/allwinner-h3.c
@@ -31,6 +31,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/arm/allwinner-h3.h"
 #include "target/arm/cpu-qom.h"
+#include "target/arm/gtimer.h"
 
 /* Memory map */
 const hwaddr allwinner_h3_memmap[] = {
diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
index 65392dbc23..898bef9d93 100644
--- a/hw/arm/allwinner-r40.c
+++ b/hw/arm/allwinner-r40.c
@@ -33,6 +33,7 @@
 #include "hw/arm/allwinner-r40.h"
 #include "hw/misc/allwinner-r40-dramc.h"
 #include "target/arm/cpu-qom.h"
+#include "target/arm/gtimer.h"
 
 /* Memory map */
 const hwaddr allwinner_r40_memmap[] = {
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 58a78780d2..e3ba18a8ec 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -16,6 +16,7 @@
 #include "hw/arm/raspi_platform.h"
 #include "hw/sysbus.h"
 #include "target/arm/cpu-qom.h"
+#include "target/arm/gtimer.h"
 
 struct BCM283XClass {
 /*< private >*/
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index d6081bfc41..85cb68d546 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -51,6 +51,7 @@
 #include "qapi/qmp/qlist.h"
 #include "qom/object.h"
 #include "target/arm/cpu-qom.h"
+#include "target/arm/gtimer.h"
 
 #define RAMLIMIT_GB 8192
 #define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 0ab5fd9477..bdfcf028a0 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -76,6 +76,7 @@
 #include "target/arm/cpu-qom.h"
 #include "target/arm/internals.h"
 #include "target/arm/multiprocessing.h"
+#include "target/arm/gtimer.h"
 #include "hw/mem/pc-dimm.h"
 #include "hw/mem/nvdimm.h"
 #include "hw/acpi/generic_event_device.h"
diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
index 87fdb39d43..2798df3730 100644
--- a/hw/arm/xlnx-versal.c
+++ b/hw/arm/xlnx-versal.c
@@ -24,6 +24,7 @@
 #include "hw/arm/xlnx-versal.h"
 #include "qemu/log.h"
 #include "target/arm/cpu-qom.h"
+#include "target/arm/gtimer.h"
 
 #define XLNX_VERSAL_ACPU_TYPE ARM_CPU_TYPE_NAME("cortex-a72")
 #define XLNX_VERSAL_RCPU_TYPE ARM_CPU_TYPE_NAME("cortex-r5f")
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 38cb34942f..65901c6e74 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -26,6 +26,7 @@
 #include "sysemu/sysemu.h"
 #include "kvm_arm.h"
 #include "target/arm/cpu-qom.h"
+#include "target/arm/gtimer.h"
 
 #define GIC_NUM_SPI_INTR 160
 
diff --git a/hw/cpu/a15mpcore.c b/hw/cpu/a15mpcore.c
index bfd8aa5644..967d8d3dd5 100644
--- a/hw/cpu/a15mpcore.c
+++ b/hw/cpu/a15mpcore.c
@@ -26,6 +26,7 @@
 #include "hw/qdev-properties.h"
 #include "sysemu/kvm.h"
 #include "kvm_arm.h"
+#include "target/arm/gtimer.h"
 
 static void a15mp_priv_set_irq(void *opaque, int irq, 

[PATCH 08/20] target/arm: Rename arm_cpu_mp_affinity

[Philippe Mathieu-Daudé]

From: Richard Henderson 

Rename to arm_build_mp_affinity.  This frees up the name for
other usage, and emphasizes that the cpu object is not involved.

Signed-off-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/arm/cpu.h  | 2 +-
 hw/arm/npcm7xx.c  | 2 +-
 hw/arm/sbsa-ref.c | 2 +-
 hw/arm/virt.c | 2 +-
 target/arm/cpu.c  | 6 +++---
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index ec276fcd57..55a19e8539 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1171,7 +1171,7 @@ void arm_cpu_post_init(Object *obj);
 (ARM_AFF0_MASK | ARM_AFF1_MASK | ARM_AFF2_MASK | ARM_AFF3_MASK)
 #define ARM64_AFFINITY_INVALID (~ARM64_AFFINITY_MASK)
 
-uint64_t arm_cpu_mp_affinity(int idx, uint8_t clustersz);
+uint64_t arm_build_mp_affinity(int idx, uint8_t clustersz);
 
 #ifndef CONFIG_USER_ONLY
 extern const VMStateDescription vmstate_arm_cpu;
diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
index 15ff21d047..7fb0a233b2 100644
--- a/hw/arm/npcm7xx.c
+++ b/hw/arm/npcm7xx.c
@@ -474,7 +474,7 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
 /* CPUs */
 for (i = 0; i < nc->num_cpus; i++) {
 object_property_set_int(OBJECT(>cpu[i]), "mp-affinity",
-arm_cpu_mp_affinity(i, NPCM7XX_MAX_NUM_CPUS),
+arm_build_mp_affinity(i, NPCM7XX_MAX_NUM_CPUS),
 _abort);
 object_property_set_int(OBJECT(>cpu[i]), "reset-cbar",
 NPCM7XX_GIC_CPU_IF_ADDR, _abort);
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 477dca0637..b8857d1e9e 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -148,7 +148,7 @@ static const int sbsa_ref_irqmap[] = {
 static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)
 {
 uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER;
-return arm_cpu_mp_affinity(idx, clustersz);
+return arm_build_mp_affinity(idx, clustersz);
 }
 
 static void sbsa_fdt_add_gic_node(SBSAMachineState *sms)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2793121cb4..3fc144236b 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1676,7 +1676,7 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState 
*vms, int idx)
 clustersz = GICV3_TARGETLIST_BITS;
 }
 }
-return arm_cpu_mp_affinity(idx, clustersz);
+return arm_build_mp_affinity(idx, clustersz);
 }
 
 static inline bool *virt_get_high_memmap_enabled(VirtMachineState *vms,
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 826ce842c0..0bbba48faa 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1307,7 +1307,7 @@ static void arm_cpu_dump_state(CPUState *cs, FILE *f, int 
flags)
 }
 }
 
-uint64_t arm_cpu_mp_affinity(int idx, uint8_t clustersz)
+uint64_t arm_build_mp_affinity(int idx, uint8_t clustersz)
 {
 uint32_t Aff1 = idx / clustersz;
 uint32_t Aff0 = idx % clustersz;
@@ -2113,8 +2113,8 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
**errp)
  * so these bits always RAZ.
  */
 if (cpu->mp_affinity == ARM64_AFFINITY_INVALID) {
-cpu->mp_affinity = arm_cpu_mp_affinity(cs->cpu_index,
-   ARM_DEFAULT_CPUS_PER_CLUSTER);
+cpu->mp_affinity = arm_build_mp_affinity(cs->cpu_index,
+ ARM_DEFAULT_CPUS_PER_CLUSTER);
 }
 
 if (cpu->reset_hivecs) {
-- 
2.41.0

[PATCH 11/20] target/arm: Declare ARM_CPU_TYPE_NAME/SUFFIX in 'cpu-qom.h'

[Philippe Mathieu-Daudé]

Missed in commit 2d56be5a29 ("target: Declare
FOO_CPU_TYPE_NAME/SUFFIX in 'cpu-qom.h'"). See
it for more details.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
---
 target/arm/cpu-qom.h | 3 +++
 target/arm/cpu.h | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index 02b914c876..f795994135 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -33,4 +33,7 @@ typedef struct AArch64CPUClass AArch64CPUClass;
 DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU,
TYPE_AARCH64_CPU)
 
+#define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU
+#define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX)
+
 #endif
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index cecac4c0a1..41659d0ef1 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2837,8 +2837,6 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync);
 #define ARM_CPUID_TI915T  0x54029152
 #define ARM_CPUID_TI925T  0x54029252
 
-#define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU
-#define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX)
 #define CPU_RESOLVING_TYPE TYPE_ARM_CPU
 
 #define TYPE_ARM_HOST_CPU "host-" TYPE_ARM_CPU
-- 
2.41.0

[PATCH 10/20] target/arm: Expose arm_cpu_mp_affinity() in 'multiprocessing.h' header

[Philippe Mathieu-Daudé]

Declare arm_cpu_mp_affinity() prototype in the new
 "target/arm/multiprocessing.h" header so units in
hw/arm/ can use it without having to include the huge
target-specific "cpu.h".

File list to include the new header generated using:

  $ git grep -lw arm_cpu_mp_affinity

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/arm/cpu.h |  6 +-
 target/arm/multiprocessing.h | 16 
 hw/arm/virt-acpi-build.c |  1 +
 hw/arm/virt.c|  1 +
 hw/arm/xlnx-versal-virt.c|  1 +
 hw/misc/xlnx-versal-crl.c|  1 +
 target/arm/arm-powerctl.c|  1 +
 target/arm/cpu.c |  5 +
 target/arm/hvf/hvf.c |  1 +
 target/arm/tcg/psci.c|  1 +
 10 files changed, 29 insertions(+), 5 deletions(-)
 create mode 100644 target/arm/multiprocessing.h

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index d1584bdb3b..cecac4c0a1 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -26,6 +26,7 @@
 #include "cpu-qom.h"
 #include "exec/cpu-defs.h"
 #include "qapi/qapi-types-common.h"
+#include "target/arm/multiprocessing.h"
 
 /* ARM processors have a weak memory model */
 #define TCG_GUEST_DEFAULT_MO  (0)
@@ -1173,11 +1174,6 @@ void arm_cpu_post_init(Object *obj);
 
 uint64_t arm_build_mp_affinity(int idx, uint8_t clustersz);
 
-static inline uint64_t arm_cpu_mp_affinity(ARMCPU *cpu)
-{
-return cpu->mp_affinity;
-}
-
 #ifndef CONFIG_USER_ONLY
 extern const VMStateDescription vmstate_arm_cpu;
 
diff --git a/target/arm/multiprocessing.h b/target/arm/multiprocessing.h
new file mode 100644
index 00..81715d345c
--- /dev/null
+++ b/target/arm/multiprocessing.h
@@ -0,0 +1,16 @@
+/*
+ * ARM multiprocessor CPU helpers
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef TARGET_ARM_MULTIPROCESSING_H
+#define TARGET_ARM_MULTIPROCESSING_H
+
+#include "target/arm/cpu-qom.h"
+
+uint64_t arm_cpu_mp_affinity(ARMCPU *cpu);
+
+#endif
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 2127778c1e..43ccc60f43 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -59,6 +59,7 @@
 #include "hw/acpi/ghes.h"
 #include "hw/acpi/viot.h"
 #include "hw/virtio/virtio-acpi.h"
+#include "target/arm/multiprocessing.h"
 
 #define ARM_SPI_BASE 32
 
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 34cba9ebd8..beba151620 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -74,6 +74,7 @@
 #include "hw/arm/smmuv3.h"
 #include "hw/acpi/acpi.h"
 #include "target/arm/internals.h"
+#include "target/arm/multiprocessing.h"
 #include "hw/mem/pc-dimm.h"
 #include "hw/mem/nvdimm.h"
 #include "hw/acpi/generic_event_device.h"
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 841ef69df6..29f4d2c2dc 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -20,6 +20,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/arm/xlnx-versal.h"
 #include "hw/arm/boot.h"
+#include "target/arm/multiprocessing.h"
 #include "qom/object.h"
 
 #define TYPE_XLNX_VERSAL_VIRT_MACHINE MACHINE_TYPE_NAME("xlnx-versal-virt")
diff --git a/hw/misc/xlnx-versal-crl.c b/hw/misc/xlnx-versal-crl.c
index 9bfa9baa15..1f1762ef16 100644
--- a/hw/misc/xlnx-versal-crl.c
+++ b/hw/misc/xlnx-versal-crl.c
@@ -19,6 +19,7 @@
 #include "hw/resettable.h"
 
 #include "target/arm/arm-powerctl.h"
+#include "target/arm/multiprocessing.h"
 #include "hw/misc/xlnx-versal-crl.h"
 
 #ifndef XLNX_VERSAL_CRL_ERR_DEBUG
diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
index 6c86e90102..2b2055c6ac 100644
--- a/target/arm/arm-powerctl.c
+++ b/target/arm/arm-powerctl.c
@@ -16,6 +16,7 @@
 #include "qemu/log.h"
 #include "qemu/main-loop.h"
 #include "sysemu/tcg.h"
+#include "target/arm/multiprocessing.h"
 
 #ifndef DEBUG_ARM_POWERCTL
 #define DEBUG_ARM_POWERCTL 0
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 0bbba48faa..89e44a31fd 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1314,6 +1314,11 @@ uint64_t arm_build_mp_affinity(int idx, uint8_t 
clustersz)
 return (Aff1 << ARM_AFF1_SHIFT) | Aff0;
 }
 
+uint64_t arm_cpu_mp_affinity(ARMCPU *cpu)
+{
+return cpu->mp_affinity;
+}
+
 static void arm_cpu_initfn(Object *obj)
 {
 ARMCPU *cpu = ARM_CPU(obj);
diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 659401e12c..71a26db188 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -28,6 +28,7 @@
 #include "arm-powerctl.h"
 #include "target/arm/cpu.h"
 #include "target/arm/internals.h"
+#include "target/arm/multiprocessing.h"
 #include "trace/trace-target_arm_hvf.h"
 #include "migration/vmstate.h"
 
diff --git a/target/arm/tcg/psci.c b/target/arm/tcg/psci.c
index 50d4b23d26..51d2ca3d30 100644
--- a/target/arm/tcg/psci.c
+++ b/target/arm/tcg/psci.c
@@ -24,6 +24,7 @@
 #include "sysemu/runstate.h"
 #include "internals.h"
 #include "arm-powerctl.h"
+#include "target/arm/multiprocessing.h"
 
 bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
 {
-- 
2.41.0

[PATCH 14/20] hw/misc/xlnx-versal-crl: Build it only once

[Philippe Mathieu-Daudé]

hw/misc/xlnx-versal-crl.c doesn't require "cpu.h"
anymore.  By removing it, the unit become target
agnostic: we can build it once. Update meson.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/misc/xlnx-versal-crl.c | 1 -
 hw/misc/meson.build   | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/misc/xlnx-versal-crl.c b/hw/misc/xlnx-versal-crl.c
index 1a596f1cf5..1f1762ef16 100644
--- a/hw/misc/xlnx-versal-crl.c
+++ b/hw/misc/xlnx-versal-crl.c
@@ -18,7 +18,6 @@
 #include "hw/register.h"
 #include "hw/resettable.h"
 
-#include "target/arm/cpu.h"
 #include "target/arm/arm-powerctl.h"
 #include "target/arm/multiprocessing.h"
 #include "hw/misc/xlnx-versal-crl.h"
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 36c20d5637..66820acac3 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -96,8 +96,8 @@ system_ss.add(when: 'CONFIG_SLAVIO', if_true: 
files('slavio_misc.c'))
 system_ss.add(when: 'CONFIG_ZYNQ', if_true: files('zynq_slcr.c'))
 system_ss.add(when: 'CONFIG_XLNX_ZYNQMP_ARM', if_true: 
files('xlnx-zynqmp-crf.c'))
 system_ss.add(when: 'CONFIG_XLNX_ZYNQMP_ARM', if_true: 
files('xlnx-zynqmp-apu-ctrl.c'))
-specific_ss.add(when: 'CONFIG_XLNX_VERSAL', if_true: 
files('xlnx-versal-crl.c'))
 system_ss.add(when: 'CONFIG_XLNX_VERSAL', if_true: files(
+  'xlnx-versal-crl.c',
   'xlnx-versal-xramc.c',
   'xlnx-versal-pmc-iou-slcr.c',
   'xlnx-versal-cfu.c',
-- 
2.41.0

Re: [PATCH 00/20] arm: Rework target/ headers to build various hw/ files once

[Philippe Mathieu-Daudé]

On 18/1/24 21:06, Philippe Mathieu-Daudé wrote:

Hi,

In order to fix a bug noticed [*] by Cédric and Fabiano in my
"Remove one use of qemu_get_cpu() in A7/A15 MPCore priv" series,
I ended reusing commits from other branches and it grew quite
a lot. This is the first "cleanup" part, unrelated on MPCorePriv.

Please review,

Phil.



[*] 
https://lore.kernel.org/qemu-devel/501c1bfe-fb26-42ab-a925-9888755c7...@linaro.org/



Philippe Mathieu-Daudé (18):
   hw/arm/exynos4210: Include missing 'exec/tswap.h' header
   hw/arm/xilinx_zynq: Include missing 'exec/tswap.h' header
   hw/arm/smmuv3: Include missing 'hw/registerfields.h' header
   hw/arm/xlnx-versal: Include missing 'cpu.h' header
   target/arm/cpu-features: Include missing 'hw/registerfields.h' header
   target/arm/cpregs: Include missing 'hw/registerfields.h' header
   target/arm/cpregs: Include missing 'kvm-consts.h' header
   target/arm: Expose arm_cpu_mp_affinity() in 'multiprocessing.h' header
   target/arm: Declare ARM_CPU_TYPE_NAME/SUFFIX in 'cpu-qom.h'
   hw/cpu/a9mpcore: Build it only once
   hw/misc/xlnx-versal-crl: Include generic 'cpu-qom.h' instead of
 'cpu.h'
   hw/misc/xlnx-versal-crl: Build it only once
   target/arm: Expose M-profile register bank index definitions
   hw/arm/armv7m: Make 'hw/intc/armv7m_nvic.h' a target agnostic header
   target/arm: Move ARM_CPU_IRQ/FIQ definitions to 'cpu-qom.h' header
   target/arm: Move e2h_access() helper around
   target/arm: Move GTimer definitions to new 'gtimer.h' header
   hw/arm: Build various units only once

[PATCH 02/20] hw/arm/xilinx_zynq: Include missing 'exec/tswap.h' header

[Philippe Mathieu-Daudé]

hw/arm/xilinx_zynq.c calls tswap32() which is declared
in "exec/tswap.h". Include it in order to avoid when
refactoring unrelated headers:

  hw/arm/xilinx_zynq.c:103:31: error: call to undeclared function 'tswap32';
  ISO C99 and later do not support implicit function declarations 
[-Wimplicit-function-declaration]
  board_setup_blob[n] = tswap32(board_setup_blob[n]);
^

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/arm/xilinx_zynq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index dbb9793aa1..d4c817ecdc 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -37,6 +37,7 @@
 #include "hw/qdev-clock.h"
 #include "sysemu/reset.h"
 #include "qom/object.h"
+#include "exec/tswap.h"
 
 #define TYPE_ZYNQ_MACHINE MACHINE_TYPE_NAME("xilinx-zynq-a9")
 OBJECT_DECLARE_SIMPLE_TYPE(ZynqMachineState, ZYNQ_MACHINE)
-- 
2.41.0

[PATCH 20/20] hw/arm: Build various units only once

[Philippe Mathieu-Daudé]

Various files in hw/arm/ don't require "cpu.h" anymore.
Except virt-acpi-build.c, all of them don't require any
ARM specific knowledge anymore and can be build once as
target agnostic units. Update meson accordingly.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/arm/collie.c   |  1 -
 hw/arm/gumstix.c  |  1 -
 hw/arm/integratorcp.c |  1 -
 hw/arm/mainstone.c|  1 -
 hw/arm/musicpal.c |  1 -
 hw/arm/omap2.c|  1 -
 hw/arm/omap_sx1.c |  1 -
 hw/arm/palm.c |  1 -
 hw/arm/spitz.c|  1 -
 hw/arm/strongarm.c|  1 -
 hw/arm/versatilepb.c  |  1 -
 hw/arm/vexpress.c |  1 -
 hw/arm/virt-acpi-build.c  |  1 -
 hw/arm/xilinx_zynq.c  |  1 -
 hw/arm/xlnx-versal-virt.c |  1 -
 hw/arm/z2.c   |  1 -
 hw/arm/meson.build| 23 ---
 17 files changed, 12 insertions(+), 27 deletions(-)

diff --git a/hw/arm/collie.c b/hw/arm/collie.c
index a0ad1b8dc7..eaa5c52d45 100644
--- a/hw/arm/collie.c
+++ b/hw/arm/collie.c
@@ -17,7 +17,6 @@
 #include "hw/arm/boot.h"
 #include "hw/block/flash.h"
 #include "exec/address-spaces.h"
-#include "cpu.h"
 #include "qom/object.h"
 #include "qemu/error-report.h"
 
diff --git a/hw/arm/gumstix.c b/hw/arm/gumstix.c
index 2ca4140c9f..3f2bcaa24e 100644
--- a/hw/arm/gumstix.c
+++ b/hw/arm/gumstix.c
@@ -44,7 +44,6 @@
 #include "hw/boards.h"
 #include "exec/address-spaces.h"
 #include "sysemu/qtest.h"
-#include "cpu.h"
 
 #define CONNEX_FLASH_SIZE   (16 * MiB)
 #define CONNEX_RAM_SIZE (64 * MiB)
diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 5600616a4d..793262eca8 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -9,7 +9,6 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
-#include "cpu.h"
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
 #include "hw/boards.h"
diff --git a/hw/arm/mainstone.c b/hw/arm/mainstone.c
index 68329c4617..fc14e05060 100644
--- a/hw/arm/mainstone.c
+++ b/hw/arm/mainstone.c
@@ -23,7 +23,6 @@
 #include "hw/block/flash.h"
 #include "hw/sysbus.h"
 #include "exec/address-spaces.h"
-#include "cpu.h"
 
 /* Device addresses */
 #define MST_FPGA_PHYS  0x0800
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index d89824f600..e46aa91807 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -12,7 +12,6 @@
 #include "qemu/osdep.h"
 #include "qemu/units.h"
 #include "qapi/error.h"
-#include "cpu.h"
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
 #include "hw/arm/boot.h"
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index f159fb73ea..d9683276c6 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -21,7 +21,6 @@
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
-#include "cpu.h"
 #include "exec/address-spaces.h"
 #include "sysemu/blockdev.h"
 #include "sysemu/qtest.h"
diff --git a/hw/arm/omap_sx1.c b/hw/arm/omap_sx1.c
index 4bf1579f8c..62d7915fb8 100644
--- a/hw/arm/omap_sx1.c
+++ b/hw/arm/omap_sx1.c
@@ -35,7 +35,6 @@
 #include "hw/block/flash.h"
 #include "sysemu/qtest.h"
 #include "exec/address-spaces.h"
-#include "cpu.h"
 #include "qemu/cutils.h"
 #include "qemu/error-report.h"
 
diff --git a/hw/arm/palm.c b/hw/arm/palm.c
index b86f2c331b..8c4c831614 100644
--- a/hw/arm/palm.c
+++ b/hw/arm/palm.c
@@ -29,7 +29,6 @@
 #include "hw/input/tsc2xxx.h"
 #include "hw/irq.h"
 #include "hw/loader.h"
-#include "cpu.h"
 #include "qemu/cutils.h"
 #include "qom/object.h"
 #include "qemu/error-report.h"
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 1d680b61e2..643a02b180 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -33,7 +33,6 @@
 #include "hw/adc/max111x.h"
 #include "migration/vmstate.h"
 #include "exec/address-spaces.h"
-#include "cpu.h"
 #include "qom/object.h"
 #include "audio/audio.h"
 
diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
index 75637869cb..7fd99a0f14 100644
--- a/hw/arm/strongarm.c
+++ b/hw/arm/strongarm.c
@@ -28,7 +28,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "cpu.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index 15b5ed0ced..1d813aa23b 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -9,7 +9,6 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
-#include "cpu.h"
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
 #include "hw/arm/boot.h"
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 49dbcdcbf0..f1b45245d5 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -24,7 +24,6 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/datadir.h"
-#include "cpu.h"
 #include "hw/sysbus.h"
 #include "hw/arm/boot.h"
 #include "hw/arm/primecell.h"
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 43ccc60f43..17aeec7a6f 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -32,7 +32,6 @@
 #include "qemu/error-report.h"
 #include "trace.h"
 #include 

[PATCH 17/20] target/arm: Move ARM_CPU_IRQ/FIQ definitions to 'cpu-qom.h' header

[Philippe Mathieu-Daudé]

The ARM_CPU_IRQ/FIQ definitions are used to index the GPIO
IRQ created calling qdev_init_gpio_in() in ARMCPU instance_init()
handler. To allow non-ARM code to raise interrupt on ARM cores,
move they to 'target/arm/cpu-qom.h' which is non-ARM specific and
can be included by any hw/ file.

File list to include the new header generated using:

  $ git grep -wEl 'ARM_CPU_(\w*IRQ|FIQ)'

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/arm/cpu-qom.h| 6 ++
 target/arm/cpu.h| 6 --
 hw/arm/allwinner-a10.c  | 1 +
 hw/arm/allwinner-h3.c   | 1 +
 hw/arm/allwinner-r40.c  | 1 +
 hw/arm/armv7m.c | 1 +
 hw/arm/aspeed_ast2400.c | 1 +
 hw/arm/aspeed_ast2600.c | 1 +
 hw/arm/bcm2836.c| 1 +
 hw/arm/exynos4210.c | 1 +
 hw/arm/fsl-imx25.c  | 1 +
 hw/arm/fsl-imx31.c  | 1 +
 hw/arm/fsl-imx6.c   | 1 +
 hw/arm/fsl-imx6ul.c | 1 +
 hw/arm/fsl-imx7.c   | 1 +
 hw/arm/highbank.c   | 1 +
 hw/arm/integratorcp.c   | 1 +
 hw/arm/musicpal.c   | 1 +
 hw/arm/npcm7xx.c| 1 +
 hw/arm/omap1.c  | 1 +
 hw/arm/omap2.c  | 1 +
 hw/arm/realview.c   | 1 +
 hw/arm/sbsa-ref.c   | 1 +
 hw/arm/strongarm.c  | 1 +
 hw/arm/versatilepb.c| 1 +
 hw/arm/vexpress.c   | 1 +
 hw/arm/virt.c   | 1 +
 hw/arm/xilinx_zynq.c| 1 +
 hw/arm/xlnx-versal.c| 1 +
 hw/arm/xlnx-zynqmp.c| 1 +
 target/arm/cpu.c| 1 +
 31 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index 77bbc1f13c..8e032691db 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -36,6 +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 */
+#define ARM_CPU_IRQ 0
+#define ARM_CPU_FIQ 1
+#define ARM_CPU_VIRQ 2
+#define ARM_CPU_VFIQ 3
+
 /* For M profile, some registers are banked secure vs non-secure;
  * these are represented as a 2-element array where the first element
  * is the non-secure copy and the second is the secure copy.
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index d6a79482ad..e8df41d642 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -93,12 +93,6 @@
 #define offsetofhigh32(S, M) (offsetof(S, M) + sizeof(uint32_t))
 #endif
 
-/* Meanings of the ARMCPU object's four inbound GPIO lines */
-#define ARM_CPU_IRQ 0
-#define ARM_CPU_FIQ 1
-#define ARM_CPU_VIRQ 2
-#define ARM_CPU_VFIQ 3
-
 /* ARM-specific extra insn start words:
  * 1: Conditional execution bits
  * 2: Partial exception syndrome for data aborts
diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index b0ea3f7f66..7e2ae7a15f 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -26,6 +26,7 @@
 #include "hw/boards.h"
 #include "hw/usb/hcd-ohci.h"
 #include "hw/loader.h"
+#include "target/arm/cpu-qom.h"
 
 #define AW_A10_SRAM_A_BASE  0x
 #define AW_A10_DRAMC_BASE   0x01c01000
diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
index f05afddf7e..2d684b5287 100644
--- a/hw/arm/allwinner-h3.c
+++ b/hw/arm/allwinner-h3.c
@@ -30,6 +30,7 @@
 #include "hw/loader.h"
 #include "sysemu/sysemu.h"
 #include "hw/arm/allwinner-h3.h"
+#include "target/arm/cpu-qom.h"
 
 /* Memory map */
 const hwaddr allwinner_h3_memmap[] = {
diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
index a0d367c60d..65392dbc23 100644
--- a/hw/arm/allwinner-r40.c
+++ b/hw/arm/allwinner-r40.c
@@ -32,6 +32,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/arm/allwinner-r40.h"
 #include "hw/misc/allwinner-r40-dramc.h"
+#include "target/arm/cpu-qom.h"
 
 /* Memory map */
 const hwaddr allwinner_r40_memmap[] = {
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index edcd8adc74..7c68525a9e 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -23,6 +23,7 @@
 #include "target/arm/idau.h"
 #include "target/arm/cpu.h"
 #include "target/arm/cpu-features.h"
+#include "target/arm/cpu-qom.h"
 #include "migration/vmstate.h"
 
 /* Bitbanded IO.  Each word corresponds to a single bit.  */
diff --git a/hw/arm/aspeed_ast2400.c b/hw/arm/aspeed_ast2400.c
index 0baa2ff96e..ad76035528 100644
--- a/hw/arm/aspeed_ast2400.c
+++ b/hw/arm/aspeed_ast2400.c
@@ -21,6 +21,7 @@
 #include "hw/i2c/aspeed_i2c.h"
 #include "net/net.h"
 #include "sysemu/sysemu.h"
+#include "target/arm/cpu-qom.h"
 
 #define ASPEED_SOC_IOMEM_SIZE   0x0020
 
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 3a9a303ab8..386a88d4e0 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -16,6 +16,7 @@
 #include "hw/i2c/aspeed_i2c.h"
 #include "net/net.h"
 #include "sysemu/sysemu.h"
+#include "target/arm/cpu-qom.h"
 
 #define ASPEED_SOC_IOMEM_SIZE   0x0020
 #define ASPEED_SOC_DPMCU_SIZE   0x0004
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index b0674a22a6..58a78780d2 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -15,6 +15,7 @@
 

[PATCH 13/20] hw/misc/xlnx-versal-crl: Include generic 'cpu-qom.h' instead of 'cpu.h'

[Philippe Mathieu-Daudé]

"target/arm/cpu.h" is target specific, any file including it
becomes target specific too, thus this is the same for any file
including "hw/misc/xlnx-versal-crl.h".

"hw/misc/xlnx-versal-crl.h" doesn't require any target specific
definition however, only the target-agnostic QOM definitions
from "target/arm/cpu-qom.h". Include the latter header to avoid
tainting unnecessary objects as target-specific.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/misc/xlnx-versal-crl.h | 2 +-
 hw/misc/xlnx-versal-crl.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/misc/xlnx-versal-crl.h 
b/include/hw/misc/xlnx-versal-crl.h
index dfb8dff197..dba6d3585d 100644
--- a/include/hw/misc/xlnx-versal-crl.h
+++ b/include/hw/misc/xlnx-versal-crl.h
@@ -11,7 +11,7 @@
 
 #include "hw/sysbus.h"
 #include "hw/register.h"
-#include "target/arm/cpu.h"
+#include "target/arm/cpu-qom.h"
 
 #define TYPE_XLNX_VERSAL_CRL "xlnx-versal-crl"
 OBJECT_DECLARE_SIMPLE_TYPE(XlnxVersalCRL, XLNX_VERSAL_CRL)
diff --git a/hw/misc/xlnx-versal-crl.c b/hw/misc/xlnx-versal-crl.c
index 1f1762ef16..1a596f1cf5 100644
--- a/hw/misc/xlnx-versal-crl.c
+++ b/hw/misc/xlnx-versal-crl.c
@@ -18,6 +18,7 @@
 #include "hw/register.h"
 #include "hw/resettable.h"
 
+#include "target/arm/cpu.h"
 #include "target/arm/arm-powerctl.h"
 #include "target/arm/multiprocessing.h"
 #include "hw/misc/xlnx-versal-crl.h"
-- 
2.41.0

[PATCH 03/20] hw/arm/smmuv3: Include missing 'hw/registerfields.h' header

[Philippe Mathieu-Daudé]

hw/arm/smmuv3-internal.h uses the REG32() and FIELD()
macros defined in "hw/registerfields.h". Include it in
order to avoid when refactoring unrelated headers:

  In file included from ../../hw/arm/smmuv3.c:34:
  hw/arm/smmuv3-internal.h:36:28: error: expected identifier
  REG32(IDR0,0x0)
 ^
  hw/arm/smmuv3-internal.h:37:5: error: expected function body after function 
declarator
  FIELD(IDR0, S2P, 0 , 1)
  ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/arm/smmuv3-internal.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index 6076025ad6..e987bc4686 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -21,6 +21,7 @@
 #ifndef HW_ARM_SMMUV3_INTERNAL_H
 #define HW_ARM_SMMUV3_INTERNAL_H
 
+#include "hw/registerfields.h"
 #include "hw/arm/smmu-common.h"
 
 typedef enum SMMUTranslationStatus {
-- 
2.41.0

[PATCH 09/20] target/arm: Create arm_cpu_mp_affinity

[Philippe Mathieu-Daudé]

From: Richard Henderson 

Wrapper to return the mp affinity bits from the cpu.

Signed-off-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/arm/cpu.h  | 5 +
 hw/arm/virt-acpi-build.c  | 2 +-
 hw/arm/virt.c | 6 +++---
 hw/arm/xlnx-versal-virt.c | 3 ++-
 hw/misc/xlnx-versal-crl.c | 4 ++--
 target/arm/arm-powerctl.c | 2 +-
 target/arm/hvf/hvf.c  | 4 ++--
 target/arm/tcg/psci.c | 2 +-
 8 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 55a19e8539..d1584bdb3b 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1173,6 +1173,11 @@ void arm_cpu_post_init(Object *obj);
 
 uint64_t arm_build_mp_affinity(int idx, uint8_t clustersz);
 
+static inline uint64_t arm_cpu_mp_affinity(ARMCPU *cpu)
+{
+return cpu->mp_affinity;
+}
+
 #ifndef CONFIG_USER_ONLY
 extern const VMStateDescription vmstate_arm_cpu;
 
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index a22a2f43a5..2127778c1e 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -720,7 +720,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
 build_append_int_noprefix(table_data, vgic_interrupt, 4);
 build_append_int_noprefix(table_data, 0, 8);/* GICR Base Address*/
 /* MPIDR */
-build_append_int_noprefix(table_data, armcpu->mp_affinity, 8);
+build_append_int_noprefix(table_data, arm_cpu_mp_affinity(armcpu), 8);
 /* Processor Power Efficiency Class */
 build_append_int_noprefix(table_data, 0, 1);
 /* Reserved */
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 3fc144236b..34cba9ebd8 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -370,7 +370,7 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms)
 for (cpu = 0; cpu < smp_cpus; cpu++) {
 ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu));
 
-if (armcpu->mp_affinity & ARM_AFF3_MASK) {
+if (arm_cpu_mp_affinity(armcpu) & ARM_AFF3_MASK) {
 addr_cells = 2;
 break;
 }
@@ -397,10 +397,10 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms)
 
 if (addr_cells == 2) {
 qemu_fdt_setprop_u64(ms->fdt, nodename, "reg",
- armcpu->mp_affinity);
+ arm_cpu_mp_affinity(armcpu));
 } else {
 qemu_fdt_setprop_cell(ms->fdt, nodename, "reg",
-  armcpu->mp_affinity);
+  arm_cpu_mp_affinity(armcpu));
 }
 
 if (ms->possible_cpus->cpus[cs->cpu_index].props.has_node_id) {
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 537118224f..841ef69df6 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -107,7 +107,8 @@ static void fdt_add_cpu_nodes(VersalVirt *s, uint32_t 
psci_conduit)
 ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i));
 
 qemu_fdt_add_subnode(s->fdt, name);
-qemu_fdt_setprop_cell(s->fdt, name, "reg", armcpu->mp_affinity);
+qemu_fdt_setprop_cell(s->fdt, name, "reg",
+  arm_cpu_mp_affinity(armcpu));
 if (psci_conduit != QEMU_PSCI_CONDUIT_DISABLED) {
 qemu_fdt_setprop_string(s->fdt, name, "enable-method", "psci");
 }
diff --git a/hw/misc/xlnx-versal-crl.c b/hw/misc/xlnx-versal-crl.c
index ac6889fcf2..9bfa9baa15 100644
--- a/hw/misc/xlnx-versal-crl.c
+++ b/hw/misc/xlnx-versal-crl.c
@@ -67,9 +67,9 @@ static void crl_reset_cpu(XlnxVersalCRL *s, ARMCPU *armcpu,
   bool rst_old, bool rst_new)
 {
 if (rst_new) {
-arm_set_cpu_off(armcpu->mp_affinity);
+arm_set_cpu_off(arm_cpu_mp_affinity(armcpu));
 } else {
-arm_set_cpu_on_and_reset(armcpu->mp_affinity);
+arm_set_cpu_on_and_reset(arm_cpu_mp_affinity(armcpu));
 }
 }
 
diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
index 8850381565..6c86e90102 100644
--- a/target/arm/arm-powerctl.c
+++ b/target/arm/arm-powerctl.c
@@ -37,7 +37,7 @@ CPUState *arm_get_cpu_by_id(uint64_t id)
 CPU_FOREACH(cpu) {
 ARMCPU *armcpu = ARM_CPU(cpu);
 
-if (armcpu->mp_affinity == id) {
+if (arm_cpu_mp_affinity(armcpu) == id) {
 return cpu;
 }
 }
diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index a537a5bc94..659401e12c 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1016,7 +1016,7 @@ static void hvf_raise_exception(CPUState *cpu, uint32_t 
excp,
 
 static void hvf_psci_cpu_off(ARMCPU *arm_cpu)
 {
-int32_t ret = arm_set_cpu_off(arm_cpu->mp_affinity);
+int32_t ret = arm_set_cpu_off(arm_cpu_mp_affinity(arm_cpu));
 assert(ret == QEMU_ARM_POWERCTL_RET_SUCCESS);
 }
 
@@ -1045,7 +1045,7 @@ static bool hvf_handle_psci_call(CPUState *cpu)
 int32_t ret = 0;
 
 trace_hvf_psci_call(param[0], param[1], param[2], param[3],
- 

[PATCH 15/20] target/arm: Expose M-profile register bank index definitions

[Philippe Mathieu-Daudé]

The ARMv7M QDev container accesses the QDev SysTickState
by its secure/non-secure bank index. In order to make
the "hw/intc/armv7m_nvic.h" header target-agnostic in
the next commit, first move the M-profile bank index
definitions to "target/arm/cpu-qom.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
Or do we want these in a more specific header?
---
 target/arm/cpu-qom.h | 15 +++
 target/arm/cpu.h | 15 ---
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index f795994135..77bbc1f13c 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -36,4 +36,19 @@ 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)
 
+/* For M profile, some registers are banked secure vs non-secure;
+ * these are represented as a 2-element array where the first element
+ * is the non-secure copy and the second is the secure copy.
+ * When the CPU does not have implement the security extension then
+ * only the first element is used.
+ * This means that the copy for the current security state can be
+ * accessed via env->registerfield[env->v7m.secure] (whether the security
+ * extension is implemented or not).
+ */
+enum {
+M_REG_NS = 0,
+M_REG_S = 1,
+M_REG_NUM_BANKS = 2,
+};
+
 #endif
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 41659d0ef1..d6a79482ad 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -73,21 +73,6 @@
 #define ARMV7M_EXCP_PENDSV  14
 #define ARMV7M_EXCP_SYSTICK 15
 
-/* For M profile, some registers are banked secure vs non-secure;
- * these are represented as a 2-element array where the first element
- * is the non-secure copy and the second is the secure copy.
- * When the CPU does not have implement the security extension then
- * only the first element is used.
- * This means that the copy for the current security state can be
- * accessed via env->registerfield[env->v7m.secure] (whether the security
- * extension is implemented or not).
- */
-enum {
-M_REG_NS = 0,
-M_REG_S = 1,
-M_REG_NUM_BANKS = 2,
-};
-
 /* ARM-specific interrupt pending bits.  */
 #define CPU_INTERRUPT_FIQ   CPU_INTERRUPT_TGT_EXT_1
 #define CPU_INTERRUPT_VIRQ  CPU_INTERRUPT_TGT_EXT_2
-- 
2.41.0

[PATCH 18/20] target/arm: Move e2h_access() helper around

[Philippe Mathieu-Daudé]

e2h_access() was added in commit bb5972e439 ("target/arm:
Add VHE timer register redirection and aliasing") close to
the generic_timer_cp_reginfo[] array, but isn't used until
vhe_reginfo[] definition. Move it closer to the other e2h
helpers.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/arm/helper.c | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index dc8f14f433..1ef00e50e4 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3342,20 +3342,6 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
 },
 };
 
-static CPAccessResult e2h_access(CPUARMState *env, const ARMCPRegInfo *ri,
- bool isread)
-{
-if (arm_current_el(env) == 1) {
-/* This must be a FEAT_NV access */
-/* TODO: FEAT_ECV will need to check CNTHCTL_EL2 here */
-return CP_ACCESS_OK;
-}
-if (!(arm_hcr_el2_eff(env) & HCR_E2H)) {
-return CP_ACCESS_TRAP;
-}
-return CP_ACCESS_OK;
-}
-
 #else
 
 /*
@@ -6543,6 +6529,21 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
 };
 
 #ifndef CONFIG_USER_ONLY
+
+static CPAccessResult e2h_access(CPUARMState *env, const ARMCPRegInfo *ri,
+ bool isread)
+{
+if (arm_current_el(env) == 1) {
+/* This must be a FEAT_NV access */
+/* TODO: FEAT_ECV will need to check CNTHCTL_EL2 here */
+return CP_ACCESS_OK;
+}
+if (!(arm_hcr_el2_eff(env) & HCR_E2H)) {
+return CP_ACCESS_TRAP;
+}
+return CP_ACCESS_OK;
+}
+
 /* Test if system register redirection is to occur in the current state.  */
 static bool redirect_for_e2h(CPUARMState *env)
 {
-- 
2.41.0

[PATCH 12/20] hw/cpu/a9mpcore: Build it only once

[Philippe Mathieu-Daudé]

hw/cpu/a9mpcore.c doesn't require "cpu.h" anymore.
By removing it, the unit become target agnostic:
we can build it once. Update meson.

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

diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
index d03f57e579..c30ef72c66 100644
--- a/hw/cpu/a9mpcore.c
+++ b/hw/cpu/a9mpcore.c
@@ -15,7 +15,7 @@
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
 #include "hw/core/cpu.h"
-#include "cpu.h"
+#include "target/arm/cpu-qom.h"
 
 #define A9_GIC_NUM_PRIORITY_BITS5
 
diff --git a/hw/cpu/meson.build b/hw/cpu/meson.build
index 6d319947ca..38cdcfbe57 100644
--- a/hw/cpu/meson.build
+++ b/hw/cpu/meson.build
@@ -2,5 +2,5 @@ system_ss.add(files('core.c', 'cluster.c'))
 
 system_ss.add(when: 'CONFIG_ARM11MPCORE', if_true: files('arm11mpcore.c'))
 system_ss.add(when: 'CONFIG_REALVIEW', if_true: files('realview_mpcore.c'))
-specific_ss.add(when: 'CONFIG_A9MPCORE', if_true: files('a9mpcore.c'))
+system_ss.add(when: 'CONFIG_A9MPCORE', if_true: files('a9mpcore.c'))
 specific_ss.add(when: 'CONFIG_A15MPCORE', if_true: files('a15mpcore.c'))
-- 
2.41.0

[PATCH 04/20] hw/arm/xlnx-versal: Include missing 'cpu.h' header

[Philippe Mathieu-Daudé]

include/hw/arm/xlnx-versal.h uses the ARMCPU structure which
is defined in the "target/arm/cpu.h" header. Include it in
order to avoid when refactoring unrelated headers:

  In file included from hw/arm/xlnx-versal-virt.c:20:
  include/hw/arm/xlnx-versal.h:62:23: error: array has incomplete element type 
'ARMCPU' (aka 'struct ArchCPU')
  ARMCPU cpu[XLNX_VERSAL_NR_ACPUS];
^

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/arm/xlnx-versal.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
index b24fa64557..025beb5532 100644
--- a/include/hw/arm/xlnx-versal.h
+++ b/include/hw/arm/xlnx-versal.h
@@ -34,6 +34,7 @@
 #include "hw/net/xlnx-versal-canfd.h"
 #include "hw/misc/xlnx-versal-cfu.h"
 #include "hw/misc/xlnx-versal-cframe-reg.h"
+#include "target/arm/cpu.h"
 
 #define TYPE_XLNX_VERSAL "xlnx-versal"
 OBJECT_DECLARE_SIMPLE_TYPE(Versal, XLNX_VERSAL)
-- 
2.41.0

[PATCH 07/20] target/arm/cpregs: Include missing 'kvm-consts.h' header

[Philippe Mathieu-Daudé]

target/arm/cpregs.h uses the CP_REG_ARCH_* definitions
from "target/arm/kvm-consts.h". Include it in order to
avoid when refactoring unrelated headers:

  target/arm/cpregs.h:191:18: error: use of undeclared identifier 
'CP_REG_ARCH_MASK'
  if ((kvmid & CP_REG_ARCH_MASK) == CP_REG_ARM64) {
   ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/arm/cpregs.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h
index ca2d6006ce..cc7c54378f 100644
--- a/target/arm/cpregs.h
+++ b/target/arm/cpregs.h
@@ -22,6 +22,7 @@
 #define TARGET_ARM_CPREGS_H
 
 #include "hw/registerfields.h"
+#include "target/arm/kvm-consts.h"
 
 /*
  * ARMCPRegInfo type field bits:
-- 
2.41.0

[PATCH 06/20] target/arm/cpregs: Include missing 'hw/registerfields.h' header

[Philippe Mathieu-Daudé]

target/arm/cpregs.h uses the FIELD() macro defined in
"hw/registerfields.h". Include it in order to avoid when
refactoring unrelated headers:

  target/arm/cpregs.h:347:30: error: expected identifier
  FIELD(HFGRTR_EL2, AFSR0_EL1, 0, 1)
   ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/arm/cpregs.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h
index b6fdd0f3eb..ca2d6006ce 100644
--- a/target/arm/cpregs.h
+++ b/target/arm/cpregs.h
@@ -21,6 +21,8 @@
 #ifndef TARGET_ARM_CPREGS_H
 #define TARGET_ARM_CPREGS_H
 
+#include "hw/registerfields.h"
+
 /*
  * ARMCPRegInfo type field bits:
  */
-- 
2.41.0

[PATCH 00/20] arm: Rework target/ headers to build various hw/ files once

[Philippe Mathieu-Daudé]

Hi,

In order to fix a bug noticed [*] by Cédric and Fabiano in my
"Remove one use of qemu_get_cpu() in A7/A15 MPCore priv" series,
I ended reusing commits from other branches and it grew quite
a lot. This is the first "cleanup" part, unrelated on MPCorePriv.

Please review,

Phil.

Philippe Mathieu-Daudé (18):
  hw/arm/exynos4210: Include missing 'exec/tswap.h' header
  hw/arm/xilinx_zynq: Include missing 'exec/tswap.h' header
  hw/arm/smmuv3: Include missing 'hw/registerfields.h' header
  hw/arm/xlnx-versal: Include missing 'cpu.h' header
  target/arm/cpu-features: Include missing 'hw/registerfields.h' header
  target/arm/cpregs: Include missing 'hw/registerfields.h' header
  target/arm/cpregs: Include missing 'kvm-consts.h' header
  target/arm: Expose arm_cpu_mp_affinity() in 'multiprocessing.h' header
  target/arm: Declare ARM_CPU_TYPE_NAME/SUFFIX in 'cpu-qom.h'
  hw/cpu/a9mpcore: Build it only once
  hw/misc/xlnx-versal-crl: Include generic 'cpu-qom.h' instead of
'cpu.h'
  hw/misc/xlnx-versal-crl: Build it only once
  target/arm: Expose M-profile register bank index definitions
  hw/arm/armv7m: Make 'hw/intc/armv7m_nvic.h' a target agnostic header
  target/arm: Move ARM_CPU_IRQ/FIQ definitions to 'cpu-qom.h' header
  target/arm: Move e2h_access() helper around
  target/arm: Move GTimer definitions to new 'gtimer.h' header
  hw/arm: Build various units only once

Richard Henderson (2):
  target/arm: Rename arm_cpu_mp_affinity
  target/arm: Create arm_cpu_mp_affinity

 hw/arm/smmuv3-internal.h  |  1 +
 include/hw/arm/xlnx-versal.h  |  1 +
 include/hw/intc/armv7m_nvic.h |  2 +-
 include/hw/misc/xlnx-versal-crl.h |  2 +-
 target/arm/cpregs.h   |  3 +++
 target/arm/cpu-features.h |  2 ++
 target/arm/cpu-qom.h  | 24 ++
 target/arm/cpu.h  | 34 +++
 target/arm/gtimer.h   | 21 +++
 target/arm/multiprocessing.h  | 16 +++
 hw/arm/allwinner-a10.c|  1 +
 hw/arm/allwinner-h3.c |  2 ++
 hw/arm/allwinner-r40.c|  2 ++
 hw/arm/armv7m.c   |  2 ++
 hw/arm/aspeed_ast2400.c   |  1 +
 hw/arm/aspeed_ast2600.c   |  1 +
 hw/arm/bcm2836.c  |  2 ++
 hw/arm/collie.c   |  1 -
 hw/arm/exynos4210.c   |  2 ++
 hw/arm/fsl-imx25.c|  1 +
 hw/arm/fsl-imx31.c|  1 +
 hw/arm/fsl-imx6.c |  1 +
 hw/arm/fsl-imx6ul.c   |  1 +
 hw/arm/fsl-imx7.c |  1 +
 hw/arm/gumstix.c  |  1 -
 hw/arm/highbank.c |  1 +
 hw/arm/integratorcp.c |  2 +-
 hw/arm/mainstone.c|  1 -
 hw/arm/musicpal.c |  2 +-
 hw/arm/npcm7xx.c  |  3 ++-
 hw/arm/omap1.c|  1 +
 hw/arm/omap2.c|  2 +-
 hw/arm/omap_sx1.c |  1 -
 hw/arm/palm.c |  1 -
 hw/arm/realview.c |  1 +
 hw/arm/sbsa-ref.c |  4 +++-
 hw/arm/spitz.c|  1 -
 hw/arm/strongarm.c|  2 +-
 hw/arm/versatilepb.c  |  2 +-
 hw/arm/vexpress.c |  2 +-
 hw/arm/virt-acpi-build.c  |  4 ++--
 hw/arm/virt.c | 11 ++
 hw/arm/xilinx_zynq.c  |  3 ++-
 hw/arm/xlnx-versal-virt.c |  5 +++--
 hw/arm/xlnx-versal.c  |  2 ++
 hw/arm/xlnx-zynqmp.c  |  2 ++
 hw/arm/z2.c   |  1 -
 hw/cpu/a15mpcore.c|  1 +
 hw/cpu/a9mpcore.c |  2 +-
 hw/misc/xlnx-versal-crl.c |  5 +++--
 target/arm/arm-powerctl.c |  3 ++-
 target/arm/cpu.c  | 13 +---
 target/arm/helper.c   | 30 ++-
 target/arm/hvf/hvf.c  |  6 --
 target/arm/kvm.c  |  1 +
 target/arm/machine.c  |  1 +
 target/arm/tcg/psci.c |  3 ++-
 hw/arm/meson.build| 23 +++--
 hw/cpu/meson.build|  2 +-
 hw/misc/meson.build   |  2 +-
 60 files changed, 178 insertions(+), 94 deletions(-)
 create mode 100644 target/arm/gtimer.h
 create mode 100644 target/arm/multiprocessing.h

-- 
2.41.0

[PATCH 01/20] hw/arm/exynos4210: Include missing 'exec/tswap.h' header

[Philippe Mathieu-Daudé]

hw/arm/exynos4210.c calls tswap32() which is declared
in "exec/tswap.h". Include it in order to avoid when
refactoring unrelated headers:

  hw/arm/exynos4210.c:499:22: error: call to undeclared function 'tswap32';
  ISO C99 and later do not support implicit function declarations 
[-Wimplicit-function-declaration]
  smpboot[n] = tswap32(smpboot[n]);
   ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/arm/exynos4210.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index de39fb0ece..af511a153d 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -23,6 +23,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "exec/tswap.h"
 #include "cpu.h"
 #include "hw/cpu/a9mpcore.h"
 #include "hw/irq.h"
-- 
2.41.0

[PATCH 05/20] target/arm/cpu-features: Include missing 'hw/registerfields.h' header

[Philippe Mathieu-Daudé]

target/arm/cpu-features.h uses the FIELD_EX32() macro
defined in "hw/registerfields.h". Include it in order
to avoid when refactoring unrelated headers:

  target/arm/cpu-features.h:44:12: error: call to undeclared function 
'FIELD_EX32';
  ISO C99 and later do not support implicit function declarations 
[-Wimplicit-function-declaration]
  return FIELD_EX32(id->id_isar0, ID_ISAR0, DIVIDE) != 0;
 ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/arm/cpu-features.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h
index 7a590c824c..028795ff23 100644
--- a/target/arm/cpu-features.h
+++ b/target/arm/cpu-features.h
@@ -20,6 +20,8 @@
 #ifndef TARGET_ARM_FEATURES_H
 #define TARGET_ARM_FEATURES_H
 
+#include "hw/registerfields.h"
+
 /*
  * Naming convention for isar_feature functions:
  * Functions which test 32-bit ID registers should have _aa32_ in
-- 
2.41.0

Re: [PATCH 2/2] target/riscv: Check 'A' and split extensions for atomic instructions

[Daniel Henrique Barboza]

On 1/15/24 13:25, Rob Bradford wrote:

Following the pattern for 'M' and Zmmul check if either the 'A'
extension is enabled or the appropriate split extension for the
instruction.

Also remove the assumption that only checking for 64-bit systems is
required for the double word variants.


Code LGTM but I don't understand what you mean in this sentence. The patch
is replacing REQUIRE_EXT(ctx, RVA) for either REQUIRE_A_OR_ZALRSC() or
REQUIRE_A_OR_ZAAMO(). There's no removal or change in any 64-bit line,
IIUC.


Thanks,

Daniel



Signed-off-by: Rob Bradford 
---
  target/riscv/insn_trans/trans_rva.c.inc | 56 +++--
  1 file changed, 34 insertions(+), 22 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rva.c.inc 
b/target/riscv/insn_trans/trans_rva.c.inc
index f0368de3e4..267930e5bc 100644
--- a/target/riscv/insn_trans/trans_rva.c.inc
+++ b/target/riscv/insn_trans/trans_rva.c.inc
@@ -18,6 +18,18 @@
   * this program.  If not, see .
   */
  
+#define REQUIRE_A_OR_ZAAMO(ctx) do {  \

+if (!ctx->cfg_ptr->ext_zaamo && !has_ext(ctx, RVA)) { \
+return false; \
+} \
+} while (0)
+
+#define REQUIRE_A_OR_ZALRSC(ctx) do {  \
+if (!ctx->cfg_ptr->ext_zalrsc && !has_ext(ctx, RVA)) { \
+return false; \
+} \
+} while (0)
+
  static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop)
  {
  TCGv src1;
@@ -96,143 +108,143 @@ static bool gen_amo(DisasContext *ctx, arg_atomic *a,
  
  static bool trans_lr_w(DisasContext *ctx, arg_lr_w *a)

  {
-REQUIRE_EXT(ctx, RVA);
+REQUIRE_A_OR_ZALRSC(ctx);
  return gen_lr(ctx, a, (MO_ALIGN | MO_TESL));
  }
  
  static bool trans_sc_w(DisasContext *ctx, arg_sc_w *a)

  {
-REQUIRE_EXT(ctx, RVA);
+REQUIRE_A_OR_ZALRSC(ctx);
  return gen_sc(ctx, a, (MO_ALIGN | MO_TESL));
  }
  
  static bool trans_amoswap_w(DisasContext *ctx, arg_amoswap_w *a)

  {
-REQUIRE_EXT(ctx, RVA);
+REQUIRE_A_OR_ZAAMO(ctx);
  return gen_amo(ctx, a, _gen_atomic_xchg_tl, (MO_ALIGN | MO_TESL));
  }
  
  static bool trans_amoadd_w(DisasContext *ctx, arg_amoadd_w *a)

  {
-REQUIRE_EXT(ctx, RVA);
+REQUIRE_A_OR_ZAAMO(ctx);
  return gen_amo(ctx, a, _gen_atomic_fetch_add_tl, (MO_ALIGN | 
MO_TESL));
  }
  
  static bool trans_amoxor_w(DisasContext *ctx, arg_amoxor_w *a)

  {
-REQUIRE_EXT(ctx, RVA);
+REQUIRE_A_OR_ZAAMO(ctx);
  return gen_amo(ctx, a, _gen_atomic_fetch_xor_tl, (MO_ALIGN | 
MO_TESL));
  }
  
  static bool trans_amoand_w(DisasContext *ctx, arg_amoand_w *a)

  {
-REQUIRE_EXT(ctx, RVA);
+REQUIRE_A_OR_ZAAMO(ctx);
  return gen_amo(ctx, a, _gen_atomic_fetch_and_tl, (MO_ALIGN | 
MO_TESL));
  }
  
  static bool trans_amoor_w(DisasContext *ctx, arg_amoor_w *a)

  {
-REQUIRE_EXT(ctx, RVA);
+REQUIRE_A_OR_ZAAMO(ctx);
  return gen_amo(ctx, a, _gen_atomic_fetch_or_tl, (MO_ALIGN | MO_TESL));
  }
  
  static bool trans_amomin_w(DisasContext *ctx, arg_amomin_w *a)

  {
-REQUIRE_EXT(ctx, RVA);
+REQUIRE_A_OR_ZAAMO(ctx);
  return gen_amo(ctx, a, _gen_atomic_fetch_smin_tl, (MO_ALIGN | 
MO_TESL));
  }
  
  static bool trans_amomax_w(DisasContext *ctx, arg_amomax_w *a)

  {
-REQUIRE_EXT(ctx, RVA);
+REQUIRE_A_OR_ZAAMO(ctx);
  return gen_amo(ctx, a, _gen_atomic_fetch_smax_tl, (MO_ALIGN | 
MO_TESL));
  }
  
  static bool trans_amominu_w(DisasContext *ctx, arg_amominu_w *a)

  {
-REQUIRE_EXT(ctx, RVA);
+REQUIRE_A_OR_ZAAMO(ctx);
  return gen_amo(ctx, a, _gen_atomic_fetch_umin_tl, (MO_ALIGN | 
MO_TESL));
  }
  
  static bool trans_amomaxu_w(DisasContext *ctx, arg_amomaxu_w *a)

  {
-REQUIRE_EXT(ctx, RVA);
+REQUIRE_A_OR_ZAAMO(ctx);
  return gen_amo(ctx, a, _gen_atomic_fetch_umax_tl, (MO_ALIGN | 
MO_TESL));
  }
  
  static bool trans_lr_d(DisasContext *ctx, arg_lr_d *a)

  {
  REQUIRE_64BIT(ctx);
-REQUIRE_EXT(ctx, RVA);
+REQUIRE_A_OR_ZALRSC(ctx);
  return gen_lr(ctx, a, MO_ALIGN | MO_TEUQ);
  }
  
  static bool trans_sc_d(DisasContext *ctx, arg_sc_d *a)

  {
  REQUIRE_64BIT(ctx);
-REQUIRE_EXT(ctx, RVA);
+REQUIRE_A_OR_ZALRSC(ctx);
  return gen_sc(ctx, a, (MO_ALIGN | MO_TEUQ));
  }
  
  static bool trans_amoswap_d(DisasContext *ctx, arg_amoswap_d *a)

  {
  REQUIRE_64BIT(ctx);
-REQUIRE_EXT(ctx, RVA);
+REQUIRE_A_OR_ZAAMO(ctx);
  return gen_amo(ctx, a, _gen_atomic_xchg_tl, (MO_ALIGN | MO_TEUQ));
  }
  
  static bool trans_amoadd_d(DisasContext *ctx, arg_amoadd_d *a)

  {
  REQUIRE_64BIT(ctx);
-REQUIRE_EXT(ctx, RVA);
+REQUIRE_A_OR_ZAAMO(ctx);
  return gen_amo(ctx, a, _gen_atomic_fetch_add_tl, (MO_ALIGN | 
MO_TEUQ));
  }
  
  static bool trans_amoxor_d(DisasContext *ctx, arg_amoxor_d *a)

  {
  REQUIRE_64BIT(ctx);
-REQUIRE_EXT(ctx, RVA);
+

[PATCH v2] target/i386/host-cpu: Use iommu phys_bits with VFIO assigned devices on Intel h/w

[Vivek Kasireddy]

Recent updates in OVMF and Seabios have resulted in MMIO regions
being placed at the upper end of the physical address space. As a
result, when a Host device is assigned to the Guest via VFIO, the
following mapping failures occur when VFIO tries to map the MMIO
regions of the device:
VFIO_MAP_DMA failed: Invalid argument
vfio_dma_map(0x557b2f2736d0, 0x3800, 0x100, 0x7f98ac40) = -22 
(Invalid argument)

The above failures are mainly seen on some Intel platforms where
the physical address width is larger than the Host's IOMMU
address width. In these cases, VFIO fails to map the MMIO regions
because the IOVAs would be larger than the IOMMU aperture regions.

Therefore, one way to solve this problem would be to ensure that
cpu->phys_bits = 
This can be done by parsing the IOMMU caps value from sysfs and
extracting the address width and using it to override the
phys_bits value as shown in this patch.

Previous attempt at solving this issue in OVMF:
https://edk2.groups.io/g/devel/topic/102359124

Cc: Gerd Hoffmann 
Cc: Philippe Mathieu-Daudé 
Cc: Alex Williamson 
Cc: Cédric Le Goater 
Cc: Laszlo Ersek 
Cc: Dongwon Kim 
Acked-by: Gerd Hoffmann 
Tested-by: Yanghang Liu 
Signed-off-by: Vivek Kasireddy 

---
v2:
- Replace the term passthrough with assigned (Laszlo)
- Update the commit message to note that both OVMF and Seabios
  guests are affected (Cédric)
- Update the subject to indicate what is done in the patch
---
 target/i386/host-cpu.c | 61 +-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c
index 92ecb7254b..5c9fcd7dc2 100644
--- a/target/i386/host-cpu.c
+++ b/target/i386/host-cpu.c
@@ -12,6 +12,8 @@
 #include "host-cpu.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
+#include "qemu/config-file.h"
+#include "qemu/option.h"
 #include "sysemu/sysemu.h"
 
 /* Note: Only safe for use on x86(-64) hosts */
@@ -51,11 +53,58 @@ static void host_cpu_enable_cpu_pm(X86CPU *cpu)
 env->features[FEAT_1_ECX] |= CPUID_EXT_MONITOR;
 }
 
+static int intel_iommu_check(void *opaque, QemuOpts *opts, Error **errp)
+{
+g_autofree char *dev_path = NULL, *iommu_path = NULL, *caps = NULL;
+const char *driver = qemu_opt_get(opts, "driver");
+const char *device = qemu_opt_get(opts, "host");
+uint32_t *iommu_phys_bits = opaque;
+struct stat st;
+uint64_t iommu_caps;
+
+/*
+ * Check if the user requested VFIO device assignment. We don't have
+ * to limit phys_bits if there are no valid assigned devices.
+ */
+if (g_strcmp0(driver, "vfio-pci") || !device) {
+return 0;
+}
+
+dev_path = g_strdup_printf("/sys/bus/pci/devices/%s", device);
+if (stat(dev_path, ) < 0) {
+return 0;
+}
+
+iommu_path = g_strdup_printf("%s/iommu/intel-iommu/cap", dev_path);
+if (stat(iommu_path, ) < 0) {
+return 0;
+}
+
+if (g_file_get_contents(iommu_path, , NULL, NULL)) {
+if (sscanf(caps, "%lx", _caps) != 1) {
+return 0;
+}
+*iommu_phys_bits = ((iommu_caps >> 16) & 0x3f) + 1;
+}
+
+return 0;
+}
+
+static uint32_t host_iommu_phys_bits(void)
+{
+uint32_t iommu_phys_bits = 0;
+
+qemu_opts_foreach(qemu_find_opts("device"),
+  intel_iommu_check, _phys_bits, NULL);
+return iommu_phys_bits;
+}
+
 static uint32_t host_cpu_adjust_phys_bits(X86CPU *cpu)
 {
 uint32_t host_phys_bits = host_cpu_phys_bits();
+uint32_t iommu_phys_bits = host_iommu_phys_bits();
 uint32_t phys_bits = cpu->phys_bits;
-static bool warned;
+static bool warned, warned2;
 
 /*
  * Print a warning if the user set it to a value that's not the
@@ -78,6 +127,16 @@ static uint32_t host_cpu_adjust_phys_bits(X86CPU *cpu)
 }
 }
 
+if (iommu_phys_bits && phys_bits > iommu_phys_bits) {
+phys_bits = iommu_phys_bits;
+if (!warned2) {
+warn_report("Using physical bits (%u)"
+" to prevent VFIO mapping failures",
+iommu_phys_bits);
+warned2 = true;
+}
+}
+
 return phys_bits;
 }
 
-- 
2.39.2

Re: [PATCH v2 0/3] s390x/pci: fix ISM reset

[Cédric Le Goater]

On 1/18/24 19:51, Matthew Rosato wrote:

Commit ef1535901a0 (re-)introduced an issue where passthrough ISM devices
on s390x would enter an error state after reboot.  This was previously fixed
by 03451953c79e, using device reset callbacks, however the change in
ef1535901a0 effectively triggers a cold reset of the pci bus before the
device reset callbacks are triggered.

To resolve this, this series proposes to remove the use of the reset callback
for ISM cleanup and instead trigger ISM reset from subsystem_reset before
triggering bus resets.  This has to happen before the bus resets because the
reset of s390-pcihost will trigger reset of the PCI bus followed by the
s390-pci bus, and the former will trigger vfio-pci reset / the aperture-wide
unmap that ISM gets upset about.
  
   /s390-pcihost (s390-pcihost)

 /pci.0 (PCI)
 /s390-pcibus.0 (s390-pcibus)
 
While fixing this, it was also noted that kernel warnings could be seen that

indicate a guest ISC reference count error.  That's because in some reset
cases we were not bothering to disable AIF, but would again re-enable it after
the reset (causing the reference count to grow erroneously).  This was a base
issue that went unnoticed because the kernel previously did not detect and
issue a warning for this scenario.


Changes for v2:
- Fold a typo fix from patch 2 into patch 1 where it belongs
- Add block comment re: timing of ISM reset
- Add review tags



Reviewed-by: Cédric Le Goater 

Thanks,

C.

Re: [PATCH v3] qemu-img: Fix Column Width and Improve Formatting in snapshot list

[Kevin Wolf]

Am 11.01.2024 um 18:25 hat Abhiram Tilak geschrieben:
> When running the command `qemu-img snapshot -l SNAPSHOT` the output of
> VM_CLOCK (measures the offset between host and VM clock) cannot to
> accommodate values in the order of thousands (4-digit).
> 
> This line [1] hints on the problem. Additionally, the column width for
> the VM_CLOCK field was reduced from 15 to 13 spaces in commit b39847a5
> in line [2], resulting in a shortage of space.
> 
> [1]: 
> https://gitlab.com/qemu-project/qemu/-/blob/master/block/qapi.c?ref_type=heads#L753
> [2]: 
> https://gitlab.com/qemu-project/qemu/-/blob/master/block/qapi.c?ref_type=heads#L763
> 
> This patch restores the column width to 15 spaces and makes adjustments
> to the affected iotests accordingly. Furthermore, addresses a potential source
> of confusion by removing whitespace in column headers. Example, VM CLOCK
> is modified to VM_CLOCK. Additionally a '--' symbol is introduced when
> ICOUNT returns no output for clarity.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2062
> Fixes: b39847a50553 (migration: introduce icount field for snapshots )
> Signed-off-by: Abhiram Tilak 

We can tweak the format a bit, but then we need to save those two
characters somewhere else so that the output still fits in 80
characters. We can probably reduce the size of the ID column.

Maybe what we should also do is decreasing the width of each field by
one and instead writing a space character into the format string. So if
we ever use up the space for one of the fields again, we would lose the
nice column alignment, but you could still recognise the individual
fields.

Kevin

Re: [PATCH 1/2] target/riscv: Add Zaamo and Zalrsc extensions

[Daniel Henrique Barboza]

On 1/15/24 13:25, Rob Bradford wrote:

These extensions represent the atomic operations from A (Zaamo) and the
Load-Reserved/Store-Conditional operations from A (Zalrsc)

Signed-off-by: Rob Bradford 
---


Reviewed-by: Daniel Henrique Barboza 


  target/riscv/cpu.c | 5 +
  target/riscv/cpu_cfg.h | 2 ++
  2 files changed, 7 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 8d3ec74a1c..604baf53c8 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -103,7 +103,9 @@ const RISCVIsaExtData isa_edata_arr[] = {
  ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause),
  ISA_EXT_DATA_ENTRY(zihpm, PRIV_VERSION_1_12_0, ext_zihpm),
  ISA_EXT_DATA_ENTRY(zmmul, PRIV_VERSION_1_12_0, ext_zmmul),
+ISA_EXT_DATA_ENTRY(zaamo, PRIV_VERSION_1_12_0, ext_zaamo),
  ISA_EXT_DATA_ENTRY(zacas, PRIV_VERSION_1_12_0, ext_zacas),
+ISA_EXT_DATA_ENTRY(zalrsc, PRIV_VERSION_1_12_0, ext_zalrsc),
  ISA_EXT_DATA_ENTRY(zawrs, PRIV_VERSION_1_12_0, ext_zawrs),
  ISA_EXT_DATA_ENTRY(zfa, PRIV_VERSION_1_12_0, ext_zfa),
  ISA_EXT_DATA_ENTRY(zfbfmin, PRIV_VERSION_1_12_0, ext_zfbfmin),
@@ -1491,6 +1493,9 @@ const RISCVCPUMultiExtConfig 
riscv_cpu_experimental_exts[] = {
  MULTI_EXT_CFG_BOOL("x-smaia", ext_smaia, false),
  MULTI_EXT_CFG_BOOL("x-ssaia", ext_ssaia, false),
  
+MULTI_EXT_CFG_BOOL("x-zaamo", ext_zaamo, false),

+MULTI_EXT_CFG_BOOL("x-zalrsc", ext_zalrsc, false),
+
  MULTI_EXT_CFG_BOOL("x-zvfh", ext_zvfh, false),
  MULTI_EXT_CFG_BOOL("x-zvfhmin", ext_zvfhmin, false),
  
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h

index fea14c275f..cc4c30244c 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -78,7 +78,9 @@ struct RISCVCPUConfig {
  bool ext_svnapot;
  bool ext_svpbmt;
  bool ext_zdinx;
+bool ext_zaamo;
  bool ext_zacas;
+bool ext_zalrsc;
  bool ext_zawrs;
  bool ext_zfa;
  bool ext_zfbfmin;

Re: [PATCH v2 3/3] s390x/pci: drive ISM reset from subsystem reset

[Eric Farman]

On Thu, 2024-01-18 at 13:51 -0500, Matthew Rosato wrote:
> ISM devices are sensitive to manipulation of the IOMMU, so the ISM
> device
> needs to be reset before the vfio-pci device is reset (triggering a
> full
> UNMAP).  In order to ensure this occurs, trigger ISM device resets
> from
> subsystem_reset before triggering the PCI bus reset (which will also
> trigger vfio-pci reset).  This only needs to be done for ISM devices
> which were enabled for use by the guest.
> Further, ensure that AIF is disabled as part of the reset event.
> 
> Fixes: ef1535901a ("s390x: do a subsystem reset before the unprotect
> on reboot")
> Fixes: 03451953c7 ("s390x/pci: reset ISM passthrough devices on
> shutdown and system reset")
> Reported-by: Cédric Le Goater 
> Signed-off-by: Matthew Rosato 
> ---
>  hw/s390x/s390-pci-bus.c | 26 +-
>  hw/s390x/s390-virtio-ccw.c  |  8 
>  include/hw/s390x/s390-pci-bus.h |  1 +
>  3 files changed, 26 insertions(+), 9 deletions(-)

Thanks for the reminder on ISM/interpretation in v1.

Reviewed-by: Eric Farman 

Re: [PATCH 19/24] configure: remove compiler sanity check

[Thomas Huth]

On 11/05/2023 11.50, Paolo Bonzini wrote:

The comment is not correct anymore, in that the usability test for
the compiler and linker are done after probing $cpu, and Meson will
redo them anyway.

Signed-off-by: Paolo Bonzini 
---
  configure | 14 --
  1 file changed, 14 deletions(-)

diff --git a/configure b/configure
index 6e5e91908a4b..85f82a626964 100755
--- a/configure
+++ b/configure
@@ -1090,20 +1090,6 @@ if test -z "$ninja"; then
  fi
  fi
  
-# Check that the C compiler works. Doing this here before testing

-# the host CPU ensures that we had a valid CC to autodetect the
-# $cpu var (and we should bail right here if that's not the case).
-# It also allows the help message to be printed without a CC.
-write_c_skeleton;
-if compile_object ; then
-  : C compiler works ok
-else
-error_exit "\"$cc\" either does not exist or does not work"
-fi
-if ! compile_prog ; then
-error_exit "\"$cc\" cannot build an executable (is your linker broken?)"
-fi
-
  # Consult white-list to determine whether to enable werror
  # by default.  Only enable by default for git builds
  if test -z "$werror" ; then


 Hi Paolo!

When running "configure" on a system without C compiler, the user is now 
presented with:


 ERROR: Unrecognized host OS (uname -s reports 'Linux')

That's completely unhelpful. Could we maybe revert this patch to get a 
better error message again?


 Thanks,
  Thomas

Re: [PATCH v2] block/blklogwrites: Protect mutable driver state with a mutex.

[Kevin Wolf]

Am 11.01.2024 um 17:32 hat Ari Sundholm geschrieben:
> During the review of a fix for a concurrency issue in blklogwrites,
> it was found that the driver needs an additional fix when enabling
> multiqueue, which is a new feature introduced in QEMU 9.0, as the
> driver state may be read and written by multiple threads at the same
> time, which was not the case when the driver was originally written.
> 
> Fix the multi-threaded scenario by introducing a mutex to protect the
> mutable fields in the driver state, and always having the mutex locked
> by the current thread when accessing them. Also use the mutex and a
> condition variable to ensure that the super block is not being written
> to by multiple threads concurrently.
> 
> Additionally, add the const qualifier to a few BDRVBlkLogWritesState
> pointer targets in contexts where the driver state is not written to.
> 
> Signed-off-by: Ari Sundholm 
> 
> v1->v2: Ensure that the super block is not written to concurrently.
> ---
>  block/blklogwrites.c | 77 +++-
>  1 file changed, 69 insertions(+), 8 deletions(-)
> 
> diff --git a/block/blklogwrites.c b/block/blklogwrites.c
> index ba717dab4d..f8bec7c863 100644
> --- a/block/blklogwrites.c
> +++ b/block/blklogwrites.c
> @@ -3,7 +3,7 @@
>   *
>   * Copyright (c) 2017 Tuomas Tynkkynen 
>   * Copyright (c) 2018 Aapo Vienamo 
> - * Copyright (c) 2018 Ari Sundholm 
> + * Copyright (c) 2018-2024 Ari Sundholm 
>   *
>   * This work is licensed under the terms of the GNU GPL, version 2 or later.
>   * See the COPYING file in the top-level directory.
> @@ -55,9 +55,34 @@ typedef struct {
>  BdrvChild *log_file;
>  uint32_t sectorsize;
>  uint32_t sectorbits;
> +uint64_t update_interval;
> +
> +/*
> + * The mutable state of the driver, consisting of the current log sector
> + * and the number of log entries.
> + *
> + * May be read and/or written from multiple threads, and the mutex must 
> be
> + * held when accessing these fields.
> + */
>  uint64_t cur_log_sector;
>  uint64_t nr_entries;
> -uint64_t update_interval;
> +QemuMutex mutex;
> +
> +/*
> + * The super block sequence number. Non-zero if a super block update is 
> in
> + * progress.
> + *
> + * The mutex must be held when accessing this field.
> + */
> +uint64_t super_update_seq;
> +
> +/*
> + * A condition variable to wait for and signal finished superblock 
> updates.
> + *
> + * Used with the mutex to ensure that only one thread be updating the 
> super
> + * block at a time.
> + */
> +QemuCond super_updated;
>  } BDRVBlkLogWritesState;
>  
>  static QemuOptsList runtime_opts = {
> @@ -169,6 +194,9 @@ static int blk_log_writes_open(BlockDriverState *bs, 
> QDict *options, int flags,
>  goto fail;
>  }
>  
> +qemu_mutex_init(>mutex);
> +qemu_cond_init(>super_updated);
> +
>  log_append = qemu_opt_get_bool(opts, "log-append", false);
>  
>  if (log_append) {
> @@ -231,6 +259,8 @@ static int blk_log_writes_open(BlockDriverState *bs, 
> QDict *options, int flags,
>  s->nr_entries = 0;
>  }
>  
> +s->super_update_seq = 0;
> +
>  if (!blk_log_writes_sector_size_valid(log_sector_size)) {
>  ret = -EINVAL;
>  error_setg(errp, "Invalid log sector size %"PRIu64, log_sector_size);
> @@ -255,6 +285,8 @@ fail_log:
>  bdrv_unref_child(bs, s->log_file);
>  bdrv_graph_wrunlock();
>  s->log_file = NULL;
> +qemu_cond_destroy(>super_updated);
> +qemu_mutex_destroy(>mutex);
>  }
>  fail:
>  qemu_opts_del(opts);
> @@ -269,6 +301,8 @@ static void blk_log_writes_close(BlockDriverState *bs)
>  bdrv_unref_child(bs, s->log_file);
>  s->log_file = NULL;
>  bdrv_graph_wrunlock();
> +qemu_cond_destroy(>super_updated);
> +qemu_mutex_destroy(>mutex);
>  }
>  
>  static int64_t coroutine_fn GRAPH_RDLOCK
> @@ -295,7 +329,7 @@ static void blk_log_writes_child_perm(BlockDriverState 
> *bs, BdrvChild *c,
>  
>  static void blk_log_writes_refresh_limits(BlockDriverState *bs, Error **errp)
>  {
> -BDRVBlkLogWritesState *s = bs->opaque;
> +const BDRVBlkLogWritesState *s = bs->opaque;
>  bs->bl.request_alignment = s->sectorsize;
>  }
>  
> @@ -338,15 +372,18 @@ blk_log_writes_co_do_log(BlkLogWritesLogReq *lr)
>   * driver may be modified by other driver operations while waiting for 
> the
>   * I/O to complete.
>   */
> +qemu_mutex_lock(>mutex);
>  const uint64_t entry_start_sector = s->cur_log_sector;
>  const uint64_t entry_offset = entry_start_sector << s->sectorbits;
>  const uint64_t qiov_aligned_size = ROUND_UP(lr->qiov->size, 
> s->sectorsize);
>  const uint64_t entry_aligned_size = qiov_aligned_size +
>  ROUND_UP(lr->zero_size, s->sectorsize);
>  const uint64_t entry_nr_sectors = entry_aligned_size >> s->sectorbits;
> +const uint64_t entry_seq 

[PATCH v2 2/3] s390x/pci: refresh fh before disabling aif

[Matthew Rosato]

Typically we refresh the host fh during CLP enable, however it's possible
that the device goes through multiple reset events before the guest
performs another CLP enable.  Let's handle this for now by refreshing the
host handle from vfio before disabling aif.

Fixes: 03451953c7 ("s390x/pci: reset ISM passthrough devices on shutdown and 
system reset")
Reported-by: Cédric Le Goater 
Reviewed-by: Eric Farman 
Signed-off-by: Matthew Rosato 
---
 hw/s390x/s390-pci-kvm.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/hw/s390x/s390-pci-kvm.c b/hw/s390x/s390-pci-kvm.c
index 1ee510436c..9eef4fc3ec 100644
--- a/hw/s390x/s390-pci-kvm.c
+++ b/hw/s390x/s390-pci-kvm.c
@@ -18,6 +18,7 @@
 #include "hw/s390x/s390-pci-bus.h"
 #include "hw/s390x/s390-pci-kvm.h"
 #include "hw/s390x/s390-pci-inst.h"
+#include "hw/s390x/s390-pci-vfio.h"
 #include "cpu_models.h"
 
 bool s390_pci_kvm_interp_allowed(void)
@@ -64,6 +65,14 @@ int s390_pci_kvm_aif_disable(S390PCIBusDevice *pbdev)
 return -EINVAL;
 }
 
+/*
+ * The device may have already been reset but we still want to relinquish
+ * the guest ISC, so always be sure to use an up-to-date host fh.
+ */
+if (!s390_pci_get_host_fh(pbdev, )) {
+return -EPERM;
+}
+
 rc = kvm_vm_ioctl(kvm_state, KVM_S390_ZPCI_OP, );
 if (rc == 0) {
 pbdev->aif = false;
-- 
2.43.0

[PATCH v2 3/3] s390x/pci: drive ISM reset from subsystem reset

[Matthew Rosato]

ISM devices are sensitive to manipulation of the IOMMU, so the ISM device
needs to be reset before the vfio-pci device is reset (triggering a full
UNMAP).  In order to ensure this occurs, trigger ISM device resets from
subsystem_reset before triggering the PCI bus reset (which will also
trigger vfio-pci reset).  This only needs to be done for ISM devices
which were enabled for use by the guest.
Further, ensure that AIF is disabled as part of the reset event.

Fixes: ef1535901a ("s390x: do a subsystem reset before the unprotect on reboot")
Fixes: 03451953c7 ("s390x/pci: reset ISM passthrough devices on shutdown and 
system reset")
Reported-by: Cédric Le Goater 
Signed-off-by: Matthew Rosato 
---
 hw/s390x/s390-pci-bus.c | 26 +-
 hw/s390x/s390-virtio-ccw.c  |  8 
 include/hw/s390x/s390-pci-bus.h |  1 +
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 347580ebac..3e57d5faca 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -151,20 +151,12 @@ static void s390_pci_shutdown_notifier(Notifier *n, void 
*opaque)
 pci_device_reset(pbdev->pdev);
 }
 
-static void s390_pci_reset_cb(void *opaque)
-{
-S390PCIBusDevice *pbdev = opaque;
-
-pci_device_reset(pbdev->pdev);
-}
-
 static void s390_pci_perform_unplug(S390PCIBusDevice *pbdev)
 {
 HotplugHandler *hotplug_ctrl;
 
 if (pbdev->pft == ZPCI_PFT_ISM) {
 notifier_remove(>shutdown_notifier);
-qemu_unregister_reset(s390_pci_reset_cb, pbdev);
 }
 
 /* Unplug the PCI device */
@@ -1132,7 +1124,6 @@ static void s390_pcihost_plug(HotplugHandler 
*hotplug_dev, DeviceState *dev,
 if (pbdev->pft == ZPCI_PFT_ISM) {
 pbdev->shutdown_notifier.notify = s390_pci_shutdown_notifier;
 qemu_register_shutdown_notifier(>shutdown_notifier);
-qemu_register_reset(s390_pci_reset_cb, pbdev);
 }
 } else {
 pbdev->fh |= FH_SHM_EMUL;
@@ -1279,6 +1270,23 @@ static void s390_pci_enumerate_bridge(PCIBus *bus, 
PCIDevice *pdev,
 pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, s->bus_no, 1);
 }
 
+void s390_pci_ism_reset(void)
+{
+S390pciState *s = s390_get_phb();
+
+S390PCIBusDevice *pbdev, *next;
+
+/* Trigger reset event for each passthrough ISM device currently in-use */
+QTAILQ_FOREACH_SAFE(pbdev, >zpci_devs, link, next) {
+if (pbdev->interp && pbdev->pft == ZPCI_PFT_ISM &&
+pbdev->fh & FH_MASK_ENABLE) {
+s390_pci_kvm_aif_disable(pbdev);
+
+pci_device_reset(pbdev->pdev);
+}
+}
+}
+
 static void s390_pcihost_reset(DeviceState *dev)
 {
 S390pciState *s = S390_PCI_HOST_BRIDGE(dev);
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index eaf61d3640..c99682b07d 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -118,6 +118,14 @@ static void subsystem_reset(void)
 DeviceState *dev;
 int i;
 
+/*
+ * ISM firmware is sensitive to unexpected changes to the IOMMU, which can
+ * occur during reset of the vfio-pci device (unmap of entire aperture).
+ * Ensure any passthrough ISM devices are reset now, while CPUs are paused
+ * but before vfio-pci cleanup occurs.
+ */
+s390_pci_ism_reset();
+
 for (i = 0; i < ARRAY_SIZE(reset_dev_types); i++) {
 dev = DEVICE(object_resolve_path_type("", reset_dev_types[i], NULL));
 if (dev) {
diff --git a/include/hw/s390x/s390-pci-bus.h b/include/hw/s390x/s390-pci-bus.h
index 435e788867..2c43ea123f 100644
--- a/include/hw/s390x/s390-pci-bus.h
+++ b/include/hw/s390x/s390-pci-bus.h
@@ -401,5 +401,6 @@ S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState 
*s,
   const char *target);
 S390PCIBusDevice *s390_pci_find_next_avail_dev(S390pciState *s,
S390PCIBusDevice *pbdev);
+void s390_pci_ism_reset(void);
 
 #endif
-- 
2.43.0

[PATCH v2 1/3] s390x/pci: avoid double enable/disable of aif

[Matthew Rosato]

Use a flag to keep track of whether AIF is currently enabled.  This can be
used to avoid enabling/disabling AIF multiple times as well as to determine
whether or not it should be disabled during reset processing.

Fixes: d0bc7091c2 ("s390x/pci: enable adapter event notification for 
interpreted devices")
Reported-by: Cédric Le Goater 
Reviewed-by: Eric Farman 
Signed-off-by: Matthew Rosato 
---
 hw/s390x/s390-pci-kvm.c | 25 +++--
 include/hw/s390x/s390-pci-bus.h |  1 +
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/s390-pci-kvm.c b/hw/s390x/s390-pci-kvm.c
index ff41e4106d..1ee510436c 100644
--- a/hw/s390x/s390-pci-kvm.c
+++ b/hw/s390x/s390-pci-kvm.c
@@ -27,6 +27,7 @@ bool s390_pci_kvm_interp_allowed(void)
 
 int s390_pci_kvm_aif_enable(S390PCIBusDevice *pbdev, ZpciFib *fib, bool assist)
 {
+int rc;
 struct kvm_s390_zpci_op args = {
 .fh = pbdev->fh,
 .op = KVM_S390_ZPCIOP_REG_AEN,
@@ -38,15 +39,35 @@ int s390_pci_kvm_aif_enable(S390PCIBusDevice *pbdev, 
ZpciFib *fib, bool assist)
 .u.reg_aen.flags = (assist) ? 0 : KVM_S390_ZPCIOP_REGAEN_HOST
 };
 
-return kvm_vm_ioctl(kvm_state, KVM_S390_ZPCI_OP, );
+if (pbdev->aif) {
+return -EINVAL;
+}
+
+rc = kvm_vm_ioctl(kvm_state, KVM_S390_ZPCI_OP, );
+if (rc == 0) {
+pbdev->aif = true;
+}
+
+return rc;
 }
 
 int s390_pci_kvm_aif_disable(S390PCIBusDevice *pbdev)
 {
+int rc;
+
 struct kvm_s390_zpci_op args = {
 .fh = pbdev->fh,
 .op = KVM_S390_ZPCIOP_DEREG_AEN
 };
 
-return kvm_vm_ioctl(kvm_state, KVM_S390_ZPCI_OP, );
+if (!pbdev->aif) {
+return -EINVAL;
+}
+
+rc = kvm_vm_ioctl(kvm_state, KVM_S390_ZPCI_OP, );
+if (rc == 0) {
+pbdev->aif = false;
+}
+
+return rc;
 }
diff --git a/include/hw/s390x/s390-pci-bus.h b/include/hw/s390x/s390-pci-bus.h
index b1bdbeaeb5..435e788867 100644
--- a/include/hw/s390x/s390-pci-bus.h
+++ b/include/hw/s390x/s390-pci-bus.h
@@ -361,6 +361,7 @@ struct S390PCIBusDevice {
 bool unplug_requested;
 bool interp;
 bool forwarding_assist;
+bool aif;
 QTAILQ_ENTRY(S390PCIBusDevice) link;
 };
 
-- 
2.43.0

[PATCH v2 0/3] s390x/pci: fix ISM reset

[Matthew Rosato]

Commit ef1535901a0 (re-)introduced an issue where passthrough ISM devices
on s390x would enter an error state after reboot.  This was previously fixed
by 03451953c79e, using device reset callbacks, however the change in
ef1535901a0 effectively triggers a cold reset of the pci bus before the
device reset callbacks are triggered.

To resolve this, this series proposes to remove the use of the reset callback
for ISM cleanup and instead trigger ISM reset from subsystem_reset before 
triggering bus resets.  This has to happen before the bus resets because the
reset of s390-pcihost will trigger reset of the PCI bus followed by the
s390-pci bus, and the former will trigger vfio-pci reset / the aperture-wide
unmap that ISM gets upset about.
 
  /s390-pcihost (s390-pcihost)
/pci.0 (PCI)
/s390-pcibus.0 (s390-pcibus)

While fixing this, it was also noted that kernel warnings could be seen that
indicate a guest ISC reference count error.  That's because in some reset
cases we were not bothering to disable AIF, but would again re-enable it after
the reset (causing the reference count to grow erroneously).  This was a base
issue that went unnoticed because the kernel previously did not detect and
issue a warning for this scenario.


Changes for v2:
- Fold a typo fix from patch 2 into patch 1 where it belongs
- Add block comment re: timing of ISM reset
- Add review tags


Matthew Rosato (3):
  s390x/pci: avoid double enable/disable of aif
  s390x/pci: refresh fh before disabling aif
  s390x/pci: drive ISM reset from subsystem reset

 hw/s390x/s390-pci-bus.c | 26 -
 hw/s390x/s390-pci-kvm.c | 34 +++--
 hw/s390x/s390-virtio-ccw.c  |  8 
 include/hw/s390x/s390-pci-bus.h |  2 ++
 4 files changed, 59 insertions(+), 11 deletions(-)

-- 
2.43.0

Re: [PATCH V2] Handle wrap around in limit calculation

[Shlomo Pongratz]

See Inline.

On 15/01/2024 18:47, Peter Maydell wrote:

On Mon, 15 Jan 2024 at 13:51, Shlomo Pongratz  wrote:

On 15/01/2024 12:37, Peter Maydell wrote:

For instance, the kernel code suggests that pre-460A
there's a 32 bit limit register, and post-460A there
is a 64-bit limit (with an "UPPER_LIMIT" register to
access the top 32 bits), plus a PCIE_ATU_INCREASE_REGION_SIZE
flag bit. That suggests we might either:
   (1) implement all that
   (2) say we're implementing a pre-460A version with a
   32 bit limit field
Presumably we're aiming for (2) here, but I find the
arithmetic you have in this patch completely confusing:
it doesn't look like something hardware would be doing
when it has a 64 bit base address register and a 32 bit limit
address register. It's also weird as C code, because you
add 0x1 when calculating tlimit, but this will
have no effect because of the AND with 0x in
the following line.

My guess at what the hardware is doing is "the actual
limit address takes its low 32 bits from the limit address
register and the high 32 bits from the high 32 bits of
the base address".

The original code which claims to be based on i.MX6 Applications Processor
actually fails for the example given there in page 4131 where the lower
is set to 0xd000
the upper to 0x800 and the limit to 0xd000 which gives us a size
of 0x8001,
which is a negative number. Therefore some fix need to be done.

Ah, I hadn't realised that this PCI controller was documented
in the i.MX6 applications processor reference manual. Looking
at that I see that the "upper base address register" is documented
as "Forms bits [63:32] of the start (and end) address of the address
region to be translated". That "(and end)" part confirms my
guess that the 64-bit limit address is supposed to be formed by
putting together the upper-base-address with the limit value.
Actually this is one implementation of one pre-460A and we don't even 
knows which.

And QEMU version is 0 which I assume should be some ideal implementation.
So as I suspect there is an implied HW limitation that a region should 
never cross

4G boundary e.g. [0x8000f000 0x8001efff] will be sent as
0x8000f000 and 0xefff resulting in[0x8000f000 
0x8000efff]

Your suggestion solve this issue and gives the correct address even if
the addresses are translated by for example by a multiple of 4G, e.g
0x2,
but it won't work if the range is translated in a way that is cross 4G
boundary (e.g. translate by 0x2000).

After reviewing the mathematics I've found a solution which to my
humiliation is more simple and it is likely that the HW
is doing it, and this is just to ignore the high 32 bits of the
calculation's result. That is:
const uint64_t size = ((uint64_t)viewport->limit - base + 1) & 0x;

That gives the wrong answer for the case where the limit register
is set to 0x_ (it will give you 0 rather than 0x1__).

This was a typo
The + 1 should have been added after masking.
That is the calculation is done in 32 bit.

    uint32_t tbase;
    uint32_t tsize;
    uint64_t size;
    uint64_t tlimit;

    // Use 32 bit calculation
    tbase = base; // Truncate
    tsize = limit - tbase;
    // Return to 64 bit
    size = tsize;
    size++;


I think my suggestion is:

uint64_t base = viewport->base;
uint64_t limit = viewport->limit;
uint64_t size;

/*
 * Versions 460A and above of this PCI controller have a
 * 64-bit limit register. We currently model the older type
 * where the limit register is 32 bits, and the upper 32 bits
 * of the end address are the same as the upper 32 bits of
 * the start address.
 */
limit = deposit64(limit, 32, 32, extract64(base, 32, 32));
size = limit - base + 1;

This will work the only thing is that it will not work if for example
The original range is [0x8000f000 0x8001efff]
Stuffing 0x8000f000 0x0efff to your solution yields
size 0x0001 and as result a limit of 0x8000efff
Note that the 32bit calculation above gives the original values.
Full simulation of all algorithms over some ranges can be found in
my gist 
https://gist.github.com/shlomopongartz/f82466b6044f3a1653890b43b046c443


Anyway I assume that in your opinion we should keep the HW limitation 
and not try to fix the HW

That's a fairly clear implementation of what the docs say the
behaviour is, and it gives us a nice easy place to add 64-bit
limit register support if we ever need to (by guarding the setting
of 'limit' with an "if 64-bit limit registers not enabled" check).

I'll be glad to see post 460A implementation.
Since until then it will be impossible to use the same device tree in a 
project
where the HW are working in a post-A460 controller and there range 
crosses 4G boundary.

thanks
-- PMM




*From:* Peter Maydell 

Re: [PATCH v3] backends/cryptodev: Do not ignore throttle/backends Errors

[Markus Armbruster]

Philippe Mathieu-Daudé  writes:

> Both cryptodev_backend_set_throttle() and CryptoDevBackendClass::init()
> can set their Error** argument. Do not ignore them, return early
> on failure. Without that, running into another failure trips
> error_setv()'s assertion. Use the ERRP_GUARD() macro as suggested
> in commit ae7c80a7bd ("error: New macro ERRP_GUARD()").
>
> Cc: qemu-sta...@nongnu.org
> Fixes: e7a775fd9f ("cryptodev: Account statistics")
> Fixes: 2580b452ff ("cryptodev: support QoS")
> Reviewed-by: zhenwei pi 
> Reviewed-by: Gonglei 
> Reviewed-by: Markus Armbruster 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  backends/cryptodev.c | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/backends/cryptodev.c b/backends/cryptodev.c
> index e5006bd215..fff89fd62a 100644
> --- a/backends/cryptodev.c
> +++ b/backends/cryptodev.c
> @@ -398,6 +398,7 @@ static void cryptodev_backend_set_ops(Object *obj, 
> Visitor *v,
>  static void
>  cryptodev_backend_complete(UserCreatable *uc, Error **errp)
>  {
> +ERRP_GUARD();
>  CryptoDevBackend *backend = CRYPTODEV_BACKEND(uc);
>  CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_GET_CLASS(uc);
>  uint32_t services;
> @@ -406,11 +407,20 @@ cryptodev_backend_complete(UserCreatable *uc, Error 
> **errp)
>  QTAILQ_INIT(>opinfos);
>  value = backend->tc.buckets[THROTTLE_OPS_TOTAL].avg;
>  cryptodev_backend_set_throttle(backend, THROTTLE_OPS_TOTAL, value, errp);
> +if (*errp) {
> +return;
> +}
>  value = backend->tc.buckets[THROTTLE_BPS_TOTAL].avg;
>  cryptodev_backend_set_throttle(backend, THROTTLE_BPS_TOTAL, value, errp);
> +if (*errp) {
> +return;
> +}
>  
>  if (bc->init) {
>  bc->init(backend, errp);
> +if (*errp) {
> +return;
> +}
>  }
>  
>  services = backend->conf.crypto_services;

Reviewed-by: Markus Armbruster 

[PATCH v3] backends/cryptodev: Do not ignore throttle/backends Errors

[Philippe Mathieu-Daudé]

Both cryptodev_backend_set_throttle() and CryptoDevBackendClass::init()
can set their Error** argument. Do not ignore them, return early
on failure. Without that, running into another failure trips
error_setv()'s assertion. Use the ERRP_GUARD() macro as suggested
in commit ae7c80a7bd ("error: New macro ERRP_GUARD()").

Cc: qemu-sta...@nongnu.org
Fixes: e7a775fd9f ("cryptodev: Account statistics")
Fixes: 2580b452ff ("cryptodev: support QoS")
Reviewed-by: zhenwei pi 
Reviewed-by: Gonglei 
Reviewed-by: Markus Armbruster 
Signed-off-by: Philippe Mathieu-Daudé 
---
 backends/cryptodev.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index e5006bd215..fff89fd62a 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -398,6 +398,7 @@ static void cryptodev_backend_set_ops(Object *obj, Visitor 
*v,
 static void
 cryptodev_backend_complete(UserCreatable *uc, Error **errp)
 {
+ERRP_GUARD();
 CryptoDevBackend *backend = CRYPTODEV_BACKEND(uc);
 CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_GET_CLASS(uc);
 uint32_t services;
@@ -406,11 +407,20 @@ cryptodev_backend_complete(UserCreatable *uc, Error 
**errp)
 QTAILQ_INIT(>opinfos);
 value = backend->tc.buckets[THROTTLE_OPS_TOTAL].avg;
 cryptodev_backend_set_throttle(backend, THROTTLE_OPS_TOTAL, value, errp);
+if (*errp) {
+return;
+}
 value = backend->tc.buckets[THROTTLE_BPS_TOTAL].avg;
 cryptodev_backend_set_throttle(backend, THROTTLE_BPS_TOTAL, value, errp);
+if (*errp) {
+return;
+}
 
 if (bc->init) {
 bc->init(backend, errp);
+if (*errp) {
+return;
+}
 }
 
 services = backend->conf.crypto_services;
-- 
2.41.0

Re: [PATCH v2 0/3] monitor: only run coroutine commands in qemu_aio_context

[Kevin Wolf]

Am 18.01.2024 um 15:48 hat Stefan Hajnoczi geschrieben:
> v2:
> - Filter image format in 141 test output [Kevin]
> - Fix pylint and mypy errors in 141 [Kevin]
> 
> Several bugs have been reported related to how QMP commands are rescheduled in
> qemu_aio_context:
> - https://gitlab.com/qemu-project/qemu/-/issues/1933
> - https://issues.redhat.com/browse/RHEL-17369
> - https://bugzilla.redhat.com/show_bug.cgi?id=2215192
> - https://bugzilla.redhat.com/show_bug.cgi?id=2214985
> 
> The first instance of the bug interacted with drain_call_rcu() temporarily
> dropping the BQL and resulted in vCPU threads entering device emulation code
> simultaneously (something that should never happen). I set out to make
> drain_call_rcu() safe to use in this environment, but Paolo and Kevin 
> discussed
> the possibility of avoiding rescheduling the monitor_qmp_dispatcher_co()
> coroutine for non-coroutine commands. This would prevent monitor commands from
> running during vCPU thread aio_poll() entirely and addresses the root cause.
> 
> This patch series implements this idea. qemu-iotests is sensitive to the exact
> order in which QMP events and responses are emitted. Running QMP handlers in
> the iohandler AioContext causes some QMP events to be ordered differently than
> before. It is therefore necessary to adjust the reference output in many test
> cases. The actual QMP code change is small and everything else is just to make
> qemu-iotests happy.
> 
> If you have bugs related to the same issue, please retest them with these
> patches. Thanks!

Thanks, applied to the block branch.

Kevin

[Stable-7.2.9 8/8] .gitlab-ci.d/buildtest.yml: Work around htags bug when environment is large

[Michael Tokarev]

From: Peter Maydell 

Sometimes the CI "pages" job fails with a message like this from
htags:

$ htags -anT --tree-view=filetree -m qemu_init -t "Welcome to the QEMU 
sourcecode"
htags: Negative exec line limit = -371

This is due to a bug in hflags where if the environment is too large it
falls over:
https://lists.gnu.org/archive/html/bug-global/2024-01/msg0.html

This happens to us because GitLab CI puts the commit message of the
commit under test into the CI_COMMIT_MESSAGE and/or CI_COMMIT_TAG_MESSAGE
environment variables, so the job will fail if the commit happens to
have a verbose commit message.

Work around the htags bug by unsetting these variables while running
htags.

Cc: qemu-sta...@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2080
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <2024025543.1573473-1-peter.mayd...@linaro.org>
Signed-off-by: Thomas Huth 
(cherry picked from commit 52a21689cd829c1cc931b59b5ee5bdb10dd578c1)
Signed-off-by: Michael Tokarev 

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 10886bb414..7243b8079b 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -637,7 +637,10 @@ pages:
 - mkdir -p public
 # HTML-ised source tree
 - make gtags
-- htags -anT --tree-view=filetree -m qemu_init
+# We unset variables to work around a bug in some htags versions
+# which causes it to fail when the environment is large
+- CI_COMMIT_MESSAGE= CI_COMMIT_TAG_MESSAGE= htags
+-anT --tree-view=filetree -m qemu_init
 -t "Welcome to the QEMU sourcecode"
 - mv HTML public/src
 # Project documentation
-- 
2.39.2

[Stable-7.2.9 6/8] hw/intc/arm_gicv3_cpuif: handle LPIs in in the list registers

[Michael Tokarev]

From: Peter Maydell 

The hypervisor can deliver (virtual) LPIs to a guest by setting up a
list register to have an intid which is an LPI.  The GIC has to treat
these a little differently to standard interrupt IDs, because LPIs
have no Active state, and so the guest will only EOI them, it will
not also deactivate them.  So icv_eoir_write() must do two things:

 * if the LPI ID is not in any list register, we drop the
   priority but do not increment the EOI count
 * if the LPI ID is in a list register, we immediately deactivate
   it, regardless of the split-drop-and-deactivate control

This can be seen in the VirtualWriteEOIR0() and VirtualWriteEOIR1()
pseudocode in the GICv3 architecture specification.

Without this fix, potentially a hypervisor guest might stall because
LPIs get stuck in a bogus Active+Pending state.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Tested-by: Miguel Luis 
(cherry picked from commit 82a65e3188abebb509510b391726711606aca642)
Signed-off-by: Michael Tokarev 

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index b17b29288c..f71b3b07d8 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -1432,16 +1432,25 @@ static void icv_eoir_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
 idx = icv_find_active(cs, irq);
 
 if (idx < 0) {
-/* No valid list register corresponding to EOI ID */
-icv_increment_eoicount(cs);
+/*
+ * No valid list register corresponding to EOI ID; if this is a vLPI
+ * not in the list regs then do nothing; otherwise increment EOI count
+ */
+if (irq < GICV3_LPI_INTID_START) {
+icv_increment_eoicount(cs);
+}
 } else {
 uint64_t lr = cs->ich_lr_el2[idx];
 int thisgrp = (lr & ICH_LR_EL2_GROUP) ? GICV3_G1NS : GICV3_G0;
 int lr_gprio = ich_lr_prio(lr) & icv_gprio_mask(cs, grp);
 
 if (thisgrp == grp && lr_gprio == dropprio) {
-if (!icv_eoi_split(env, cs)) {
-/* Priority drop and deactivate not split: deactivate irq now 
*/
+if (!icv_eoi_split(env, cs) || irq >= GICV3_LPI_INTID_START) {
+/*
+ * Priority drop and deactivate not split: deactivate irq now.
+ * LPIs always get their active state cleared immediately
+ * because no separate deactivate is expected.
+ */
 icv_deactivate_irq(cs, idx);
 }
 }
-- 
2.39.2

[Stable-7.2.9 2/8] vl: Improve error message for conflicting -incoming and -loadvm

[Michael Tokarev]

From: Kevin Wolf 

Currently, the conflict between -incoming and -loadvm is only detected
when loading the snapshot fails because the image is still inactive for
the incoming migration. This results in a suboptimal error message:

$ ./qemu-system-x86_64 -hda /tmp/test.qcow2 -loadvm foo -incoming defer
qemu-system-x86_64: Device 'ide0-hd0' is writable but does not support snapshots

Catch the situation already in qemu_validate_options() to improve the
message:

$ ./qemu-system-x86_64 -hda /tmp/test.qcow2 -loadvm foo -incoming defer
qemu-system-x86_64: 'incoming' and 'loadvm' options are mutually exclusive

Signed-off-by: Kevin Wolf 
Message-ID: <20231201142520.32255-3-kw...@redhat.com>
Signed-off-by: Kevin Wolf 
(cherry picked from commit 5a7f21efaf99c60614fe1967be1c0f9aa46c526e)
Signed-off-by: Michael Tokarev 

diff --git a/softmmu/vl.c b/softmmu/vl.c
index 5115221efe..ce88869618 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2321,6 +2321,10 @@ static void qemu_validate_options(const QDict 
*machine_opts)
   }
 }
 
+if (loadvm && incoming) {
+error_report("'incoming' and 'loadvm' options are mutually exclusive");
+exit(EXIT_FAILURE);
+}
 if (loadvm && preconfig_requested) {
 error_report("'preconfig' and 'loadvm' options are "
  "mutually exclusive");
-- 
2.39.2

[Stable-7.2.9 5/8] chardev/char.c: fix "abstract device type" error message

[Michael Tokarev]

Current error message:

 qemu-system-x86_64: -chardev spice,id=foo: Parameter 'driver' expects an 
abstract device type

while in fact the meaning is in reverse, -chardev expects
a non-abstract device type.

Fixes: 777357d758d9 ("chardev: qom-ify" 2016-12-07)
Signed-off-by: Michael Tokarev 
Reviewed-by: Zhao Liu 
(cherry picked from commit 4ad87cd4b2254197b7ac12e3da824854e6a90f8f)
Signed-off-by: Michael Tokarev 

diff --git a/chardev/char.c b/chardev/char.c
index b005df3ccf..193bbac054 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -519,7 +519,7 @@ static const ChardevClass *char_get_class(const char 
*driver, Error **errp)
 
 if (object_class_is_abstract(oc)) {
 error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "driver",
-   "an abstract device type");
+   "a non-abstract device type");
 return NULL;
 }
 
-- 
2.39.2

[Stable-7.2.9 3/8] iotests: Basic tests for internal snapshots

[Michael Tokarev]

From: Kevin Wolf 

We have a few test cases that include tests for corner case aspects of
internal snapshots, but nothing that tests that they actually function
as snapshots or that involves deleting a snapshot. Add a test for this
kind of basic internal snapshot functionality.

The error cases include a regression test for the crash we just fixed
with snapshot operations on inactive images.

Signed-off-by: Kevin Wolf 
Message-ID: <20231201142520.32255-4-kw...@redhat.com>
Signed-off-by: Kevin Wolf 
(cherry picked from commit bb6e2511eb48539b7dcbcb5f47772e156b9c45d1)
Signed-off-by: Michael Tokarev 

diff --git a/tests/qemu-iotests/tests/qcow2-internal-snapshots 
b/tests/qemu-iotests/tests/qcow2-internal-snapshots
new file mode 100755
index 00..36523aba06
--- /dev/null
+++ b/tests/qemu-iotests/tests/qcow2-internal-snapshots
@@ -0,0 +1,170 @@
+#!/usr/bin/env bash
+# group: rw quick
+#
+# Test case for internal snapshots in qcow2
+#
+# Copyright (C) 2023 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=kw...@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ../common.rc
+. ../common.filter
+
+# This tests qcow2-specific low-level functionality
+_supported_fmt qcow2
+_supported_proto generic
+# Internal snapshots are (currently) impossible with refcount_bits=1,
+# and generally impossible with external data files
+_unsupported_imgopts 'compat=0.10' 'refcount_bits=1[^0-9]' data_file
+
+IMG_SIZE=64M
+
+_qemu()
+{
+$QEMU -no-shutdown -nographic -monitor stdio -serial none \
+  -blockdev file,filename="$TEST_IMG",node-name=disk0-file \
+  -blockdev "$IMGFMT",file=disk0-file,node-name=disk0 \
+  -object iothread,id=iothread0 \
+  -device virtio-scsi,iothread=iothread0 \
+  -device scsi-hd,drive=disk0,share-rw=on \
+  "$@" 2>&1 |\
+_filter_qemu | _filter_hmp | _filter_qemu_io
+}
+
+_make_test_img $IMG_SIZE
+
+echo
+echo "=== Write some data, take a snapshot and overwrite part of it ==="
+echo
+
+{
+echo 'qemu-io disk0 "write -P0x11 0 1M"'
+# Give qemu some time to boot before saving the VM state
+sleep 0.5
+echo "savevm snap0"
+echo 'qemu-io disk0 "write -P0x22 0 512k"'
+echo "quit"
+} | _qemu
+
+echo
+$QEMU_IMG snapshot -l "$TEST_IMG" | _filter_date | _filter_vmstate_size
+_check_test_img
+
+echo
+echo "=== Verify that loading the snapshot reverts to the old content ==="
+echo
+
+{
+# -loadvm reverted the write from the previous QEMU instance
+echo 'qemu-io disk0 "read -P0x11 0 1M"'
+
+# Verify that it works without restarting QEMU, too
+echo 'qemu-io disk0 "write -P0x33 512k 512k"'
+echo "loadvm snap0"
+echo 'qemu-io disk0 "read -P0x11 0 1M"'
+
+# Verify COW by writing a partial cluster
+echo 'qemu-io disk0 "write -P0x33 63k 2k"'
+echo 'qemu-io disk0 "read -P0x11 0 63k"'
+echo 'qemu-io disk0 "read -P0x33 63k 2k"'
+echo 'qemu-io disk0 "read -P0x11 65k 63k"'
+
+# Take a second snapshot
+echo "savevm snap1"
+
+echo "quit"
+} | _qemu -loadvm snap0
+
+echo
+$QEMU_IMG snapshot -l "$TEST_IMG" | _filter_date | _filter_vmstate_size
+_check_test_img
+
+echo
+echo "=== qemu-img snapshot can revert to snapshots ==="
+echo
+
+$QEMU_IMG snapshot -a snap0 "$TEST_IMG"
+$QEMU_IO -c "read -P0x11 0 1M" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -a snap1 "$TEST_IMG"
+$QEMU_IO \
+-c "read -P0x11 0 63k" \
+-c "read -P0x33 63k 2k" \
+-c "read -P0x11 65k 63k" \
+"$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "=== Deleting snapshots ==="
+echo
+{
+# The active layer stays unaffected by deleting the snapshot
+echo "delvm snap1"
+echo 'qemu-io disk0 "read -P0x11 0 63k"'
+echo 'qemu-io disk0 "read -P0x33 63k 2k"'
+echo 'qemu-io disk0 "read -P0x11 65k 63k"'
+
+echo "quit"
+} | _qemu
+
+
+echo
+$QEMU_IMG snapshot -l "$TEST_IMG" | _filter_date | _filter_vmstate_size
+_check_test_img
+
+echo
+echo "=== Error cases ==="
+echo
+
+# snap1 should not exist any more
+_qemu -loadvm snap1
+
+echo
+{
+echo "loadvm snap1"
+echo "quit"
+} | _qemu
+
+# Snapshot operations and inactive images are incompatible
+echo
+_qemu -loadvm snap0 

[Stable-7.2.9 4/8] target/riscv: Fix mcycle/minstret increment behavior

[Michael Tokarev]

From: Xu Lu 

The mcycle/minstret counter's stop flag is mistakenly updated on a copy
on stack. Thus the counter increments even when the CY/IR bit in the
mcountinhibit register is set. This commit corrects its behavior.

Fixes: 3780e33732f88 (target/riscv: Support mcycle/minstret write operation)
Signed-off-by: Xu Lu 
Reviewed-by: Daniel Henrique Barboza 
Signed-off-by: Michael Tokarev 
(cherry picked from commit 5cb0e7abe1635cb82e0033260dac2b910d142f8c)
Signed-off-by: Michael Tokarev 

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 5c9a7ee287..15dba5f653 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -697,11 +697,11 @@ static int write_mhpmcounterh(CPURISCVState *env, int 
csrno, target_ulong val)
 static RISCVException riscv_pmu_read_ctr(CPURISCVState *env, target_ulong *val,
  bool upper_half, uint32_t ctr_idx)
 {
-PMUCTRState counter = env->pmu_ctrs[ctr_idx];
-target_ulong ctr_prev = upper_half ? counter.mhpmcounterh_prev :
- counter.mhpmcounter_prev;
-target_ulong ctr_val = upper_half ? counter.mhpmcounterh_val :
-counter.mhpmcounter_val;
+PMUCTRState *counter = >pmu_ctrs[ctr_idx];
+target_ulong ctr_prev = upper_half ? counter->mhpmcounterh_prev :
+ counter->mhpmcounter_prev;
+target_ulong ctr_val = upper_half ? counter->mhpmcounterh_val :
+counter->mhpmcounter_val;
 
 if (get_field(env->mcountinhibit, BIT(ctr_idx))) {
 /**
@@ -709,12 +709,12 @@ static RISCVException riscv_pmu_read_ctr(CPURISCVState 
*env, target_ulong *val,
  * stop the icount counting. Just return the counter value written by
  * the supervisor to indicate that counter was not incremented.
  */
-if (!counter.started) {
+if (!counter->started) {
 *val = ctr_val;
 return RISCV_EXCP_NONE;
 } else {
 /* Mark that the counter has been stopped */
-counter.started = false;
+counter->started = false;
 }
 }
 
-- 
2.39.2

[Stable-7.2.9 0/8] Patch Round-up for stable 7.2.9, freeze on 2024-01-27

[Michael Tokarev]

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

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

Patch freeze is 2024-01-27, and the release is planned for 2024-01-29:

  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 d3007d348ada Kevin Wolf:
   block: Fix crash when loading snapshot on inactive node
02 5a7f21efaf99 Kevin Wolf:
   vl: Improve error message for conflicting -incoming and -loadvm
03 bb6e2511eb48 Kevin Wolf:
   iotests: Basic tests for internal snapshots
04 5cb0e7abe163 Xu Lu:
   target/riscv: Fix mcycle/minstret increment behavior
05 4ad87cd4b225 Michael Tokarev:
   chardev/char.c: fix "abstract device type" error message
06 82a65e3188ab Peter Maydell:
   hw/intc/arm_gicv3_cpuif: handle LPIs in in the list registers
07 e358a25a97c7 Ilya Leoshkevich:
   target/s390x: Fix LAE setting a wrong access register
08 52a21689cd82 Peter Maydell:
   .gitlab-ci.d/buildtest.yml: Work around htags bug when environment is 
   large

[Stable-7.2.9 7/8] target/s390x: Fix LAE setting a wrong access register

[Michael Tokarev]

From: Ilya Leoshkevich 

LAE should set the access register corresponding to the first operand,
instead, it always modifies access register 1.

Co-developed-by: Ido Plat 
Cc: qemu-sta...@nongnu.org
Fixes: a1c7610a6879 ("target-s390x: implement LAY and LAEY instructions")
Reviewed-by: David Hildenbrand 
Signed-off-by: Ilya Leoshkevich 
Message-ID: <20240111092328.929421-2-...@linux.ibm.com>
Signed-off-by: Thomas Huth 
(cherry picked from commit e358a25a97c71c39e3513d9b869cdb82052e50b8)
Signed-off-by: Michael Tokarev 
(Mjt: target/s390x/tcg/translate.c: fixup for
 v8.1.0-1189-gad75a51e84  "tcg: Rename cpu_env to tcg_env" and
 v7.2.0-2636-g3ac6f91bca "target/s390x: Drop tcg_temp_free from translate.c")

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index b0173e968e..a257c06838 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -3394,6 +3394,7 @@ static DisasJumpType op_mov2e(DisasContext *s, DisasOps 
*o)
 {
 int b2 = get_field(s, b2);
 TCGv ar1 = tcg_temp_new_i64();
+int r1 = get_field(s, r1);
 
 o->out = o->in2;
 o->g_out = o->g_in2;
@@ -3419,7 +3420,7 @@ static DisasJumpType op_mov2e(DisasContext *s, DisasOps 
*o)
 break;
 }
 
-tcg_gen_st32_i64(ar1, cpu_env, offsetof(CPUS390XState, aregs[1]));
+tcg_gen_st32_i64(ar1, cpu_env, offsetof(CPUS390XState, aregs[r1]));
 tcg_temp_free_i64(ar1);
 
 return DISAS_NEXT;
-- 
2.39.2

[Stable-7.2.9 1/8] block: Fix crash when loading snapshot on inactive node

[Michael Tokarev]

From: Kevin Wolf 

bdrv_is_read_only() only checks if the node is configured to be
read-only eventually, but even if it returns false, writing to the node
may not be permitted at the moment (because it's inactive).

bdrv_is_writable() checks that the node can be written to right now, and
this is what the snapshot operations really need.

Change bdrv_can_snapshot() to use bdrv_is_writable() to fix crashes like
the following:

$ ./qemu-system-x86_64 -hda /tmp/test.qcow2 -loadvm foo -incoming defer
qemu-system-x86_64: ../block/io.c:1990: int bdrv_co_write_req_prepare(BdrvChild 
*, int64_t, int64_t, BdrvTrackedRequest *, int): Assertion `!(bs->open_flags & 
BDRV_O_INACTIVE)' failed.

The resulting error message after this patch isn't perfect yet, but at
least it doesn't crash any more:

$ ./qemu-system-x86_64 -hda /tmp/test.qcow2 -loadvm foo -incoming defer
qemu-system-x86_64: Device 'ide0-hd0' is writable but does not support snapshots

Signed-off-by: Kevin Wolf 
Message-ID: <20231201142520.32255-2-kw...@redhat.com>
Signed-off-by: Kevin Wolf 
(cherry picked from commit d3007d348adaaf04ee8b099a475282034a662414)
Signed-off-by: Michael Tokarev 

diff --git a/block/snapshot.c b/block/snapshot.c
index e22ac3eac6..86e29ca59f 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -190,8 +190,10 @@ static BlockDriverState 
*bdrv_snapshot_fallback(BlockDriverState *bs)
 int bdrv_can_snapshot(BlockDriverState *bs)
 {
 BlockDriver *drv = bs->drv;
+
 GLOBAL_STATE_CODE();
-if (!drv || !bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
+
+if (!drv || !bdrv_is_inserted(bs) || !bdrv_is_writable(bs)) {
 return 0;
 }
 
-- 
2.39.2

Re: [PATCH 0/2] target/s390x: Emulate CVDG

[Thomas Huth]

On 15/01/2024 21.21, Ilya Leoshkevich wrote:

Hi,

Ido reported that we are missing the CVDG emulation (which is very
similar to the existing CVD emulation). This series adds it along with
a test.


Just FYI, your patch made me curious which other instructions we still might 
be missing from chapter 7 in the Principles of Operation... with some shell 
scripting and manual fiddling, I ended up with this list:


0C  BRANCH AND SAVE AND SET MODEBASSM
0B  BRANCH AND SET MODE BSM
B21ACOMPARE AND FORM CODEWORD   CFC
B257COMPARE UNTIL SUBSTRING EQUAL   CUSE
B263COMPRESSION CALLCMPSC
4F  CONVERT TO BINARY (32)  CVB
E306CONVERT TO BINARY (32)  CVBY
E30ECONVERT TO BINARY (64)  CVBG
B24DCOPY ACCESS CPYA
EF  LOAD MULTIPLE DISJOINT  LMD
EE  PERFORM LOCKED OPERATIONPLO
B9BFTRANSLATE AND TEST EXTENDED TRTE
B9BDTRANSLATE AND TEST REVERSE EXTENDED TRTRE
0102UPDATE TREE UPT

There are some additional ones from the "Guarded-Storage Facility" and the 
"Transactional-Execution Facility", but these are optional AFAIK.


Some of these (like UPT) really look like sins from the CISC past, I guess 
we'll never need them for running Linux guests :-)


 Thomas

Re: [PATCH v4 3/6] target/riscv: Add helper functions to calculate current number of masked bits for pointer masking

[Deepak Gupta]

On Tue, Jan 9, 2024 at 2:31 AM Alexey Baturo  wrote:
>
> From: Alexey Baturo 
>
> Signed-off-by: Alexey Baturo 
> ---

> +
> +bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
> +{
> +bool virt_mem_en = false;
> +#ifndef CONFIG_USER_ONLY
> +int satp_mode = 0;
> +int priv_mode = cpu_address_mode(env);
> +/* Get current PMM field */
> +if (riscv_cpu_mxl(env) == MXL_RV32) {
> +satp_mode = get_field(env->satp, SATP32_MODE);
> +} else {
> +satp_mode = get_field(env->satp, SATP64_MODE);
> +}
> +virt_mem_en = ((satp_mode != VM_1_10_MBARE) && (priv_mode != PRV_M));
> +#endif
> +return virt_mem_en;

Obsessing a little bit on how to test PM enabled binaries with qemu-user.
If we return false above then we're not allowed to test binaries with
pointer masking enabled with qemu-user.
That use case is not required?

> +}
> +
> +int riscv_pm_get_pmlen(RISCVPmPmm pmm)
> +{
> +switch (pmm) {
> +case PMM_FIELD_DISABLED:
> +return 0;
> +case PMM_FIELD_PMLEN7:
> +return 7;
> +case PMM_FIELD_PMLEN16:
> +return 16;
> +default:
> +g_assert_not_reached();
> +}
> +return -1;
> +}
> +
>  #ifndef CONFIG_USER_ONLY
>
>  /*
> --
> 2.34.1
>
>

Updated invitation: QEMU developers fortnightly conference call @ Every 2 weeks from 15:00 to 16:00 on Tuesday from Tue 2024-01-09 to Tue 2024-01-23 (GMT+1) (qemu-devel@nongnu.org)

[Juan Quintela]

BEGIN:VCALENDAR
PRODID:-//Google Inc//Google Calendar 70.9054//EN
VERSION:2.0
CALSCALE:GREGORIAN
METHOD:REQUEST
BEGIN:VTIMEZONE
TZID:America/New_York
X-LIC-LOCATION:America/New_York
BEGIN:DAYLIGHT
TZOFFSETFROM:-0500
TZOFFSETTO:-0400
TZNAME:EDT
DTSTART:19700308T02
RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=2SU
END:DAYLIGHT
BEGIN:STANDARD
TZOFFSETFROM:-0400
TZOFFSETTO:-0500
TZNAME:EST
DTSTART:19701101T02
RRULE:FREQ=YEARLY;BYMONTH=11;BYDAY=1SU
END:STANDARD
END:VTIMEZONE
BEGIN:VEVENT
DTSTART;TZID=America/New_York:20240109T09
DTEND;TZID=America/New_York:20240109T10
RRULE:FREQ=WEEKLY;WKST=MO;UNTIL=20240123T045959Z;INTERVAL=2;BYDAY=TU
DTSTAMP:20240118T171539Z
ORGANIZER;CN=KVM call for qemu developers:mailto:eged7cki05lmu1tngvkl3thids
 @group.calendar.google.com
UID:5dt5ji87j5qrc00o63ktq7ghou_r20240109t140...@google.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=DECLINED;RSVP=TRUE
 ;CN=quint...@redhat.com;X-NUM-GUESTS=0:mailto:quint...@redhat.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=md...@redhat.com;X-NUM-GUESTS=0:mailto:md...@redhat.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=fel...@nutanix.com;X-NUM-GUESTS=0:mailto:fel...@nutanix.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=c...@f00f.org;X-NUM-GUESTS=0:mailto:c...@f00f.org
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=ebl...@redhat.com;X-NUM-GUESTS=0:mailto:ebl...@redhat.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=edgar.igles...@gmail.com;X-NUM-GUESTS=0:mailto:edgar.iglesias@gmail
 .com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=i...@theiggy.com;X-NUM-GUESTS=0:mailto:i...@theiggy.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=jidong.x...@gmail.com;X-NUM-GUESTS=0:mailto:jidong.x...@gmail.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=peter.mayd...@linaro.org;X-NUM-GUESTS=0:mailto:peter.maydell@linaro
 .org
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=richard.hender...@linaro.org;X-NUM-GUESTS=0:mailto:richard.henderso
 n...@linaro.org
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=stefa...@gmail.com;X-NUM-GUESTS=0:mailto:stefa...@gmail.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=TENTATIVE;RSVP=TRU
 E;CN=i...@bsdimp.com;X-NUM-GUESTS=0:mailto:i...@bsdimp.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=zwu.ker...@gmail.com;X-NUM-GUESTS=0:mailto:zwu.ker...@gmail.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=j...@nvidia.com;X-NUM-GUESTS=0:mailto:j...@nvidia.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=c...@nvidia.com;X-NUM-GUESTS=0:mailto:c...@nvidia.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=konrad.w...@oracle.com;X-NUM-GUESTS=0:mailto:konrad.w...@oracle.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=a...@rev.ng;X-NUM-GUESTS=0:mailto:a...@rev.ng
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=ACCEPTED;RSVP=TRUE
 ;CN=wei.w.w...@intel.com;X-NUM-GUESTS=0:mailto:wei.w.w...@intel.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=ACCEPTED;RSVP=TRUE
 ;CN=mbur...@qti.qualcomm.com;X-NUM-GUESTS=0:mailto:mbur...@qti.qualcomm.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;X-NUM-GUESTS=0:mailto:f4...@amsat.org
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=Joao Martins;X-NUM-GUESTS=0:mailto:joao.m.mart...@oracle.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=afaer...@suse.de;X-NUM-GUESTS=0:mailto:afaer...@suse.de
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=bbau...@redhat.com;X-NUM-GUESTS=0:mailto:bbau...@redhat.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=eric.au...@redhat.com;X-NUM-GUESTS=0:mailto:eric.au...@redhat.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=jan.kis...@web.de;X-NUM-GUESTS=0:mailto:jan.kis...@web.de
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=jjhe...@linux.vnet.ibm.com;X-NUM-GUESTS=0:mailto:jjhe...@linux.vnet
 .ibm.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=m...@linux.vnet.ibm.com;X-NUM-GUESTS=0:mailto:m...@linux.vnet.ibm.c
 om
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=z@139.com;X-NUM-GUESTS=0:mailto:z@139.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 

Updated invitation: QEMU developers fortnightly conference call @ Every 2 weeks from 15:00 to 16:00 on Tuesday from Tue 2024-01-23 to Wed 2023-02-22 (GMT+1) (qemu-devel@nongnu.org)

[Juan Quintela]

BEGIN:VCALENDAR
PRODID:-//Google Inc//Google Calendar 70.9054//EN
VERSION:2.0
CALSCALE:GREGORIAN
METHOD:REQUEST
BEGIN:VTIMEZONE
TZID:America/New_York
X-LIC-LOCATION:America/New_York
BEGIN:DAYLIGHT
TZOFFSETFROM:-0500
TZOFFSETTO:-0400
TZNAME:EDT
DTSTART:19700308T02
RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=2SU
END:DAYLIGHT
BEGIN:STANDARD
TZOFFSETFROM:-0400
TZOFFSETTO:-0500
TZNAME:EST
DTSTART:19701101T02
RRULE:FREQ=YEARLY;BYMONTH=11;BYDAY=1SU
END:STANDARD
END:VTIMEZONE
BEGIN:VEVENT
DTSTART;TZID=America/New_York:20240123T09
DTEND;TZID=America/New_York:20240123T10
RRULE:FREQ=WEEKLY;WKST=MO;UNTIL=20230222T045959Z;INTERVAL=2;BYDAY=TU
DTSTAMP:20240118T171540Z
ORGANIZER;CN=KVM call for qemu developers:mailto:eged7cki05lmu1tngvkl3thids
 @group.calendar.google.com
UID:nmiuq2m01pj3jnhkt1tgji0...@google.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=quint...@redhat.com;X-NUM-GUESTS=0:mailto:quint...@redhat.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=md...@redhat.com;X-NUM-GUESTS=0:mailto:md...@redhat.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=fel...@nutanix.com;X-NUM-GUESTS=0:mailto:fel...@nutanix.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=c...@f00f.org;X-NUM-GUESTS=0:mailto:c...@f00f.org
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=ebl...@redhat.com;X-NUM-GUESTS=0:mailto:ebl...@redhat.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=edgar.igles...@gmail.com;X-NUM-GUESTS=0:mailto:edgar.iglesias@gmail
 .com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=i...@theiggy.com;X-NUM-GUESTS=0:mailto:i...@theiggy.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=jidong.x...@gmail.com;X-NUM-GUESTS=0:mailto:jidong.x...@gmail.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=peter.mayd...@linaro.org;X-NUM-GUESTS=0:mailto:peter.maydell@linaro
 .org
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=richard.hender...@linaro.org;X-NUM-GUESTS=0:mailto:richard.henderso
 n...@linaro.org
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=stefa...@gmail.com;X-NUM-GUESTS=0:mailto:stefa...@gmail.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=i...@bsdimp.com;X-NUM-GUESTS=0:mailto:i...@bsdimp.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=zwu.ker...@gmail.com;X-NUM-GUESTS=0:mailto:zwu.ker...@gmail.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=j...@nvidia.com;X-NUM-GUESTS=0:mailto:j...@nvidia.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=c...@nvidia.com;X-NUM-GUESTS=0:mailto:c...@nvidia.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=konrad.w...@oracle.com;X-NUM-GUESTS=0:mailto:konrad.w...@oracle.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=a...@rev.ng;X-NUM-GUESTS=0:mailto:a...@rev.ng
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=wei.w.w...@intel.com;X-NUM-GUESTS=0:mailto:wei.w.w...@intel.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=mbur...@qti.qualcomm.com;X-NUM-GUESTS=0:mailto:mburton@qti.qualcomm
 .com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;X-NUM-GUESTS=0:mailto:f4...@amsat.org
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=Joao Martins;X-NUM-GUESTS=0:mailto:joao.m.mart...@oracle.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=afaer...@suse.de;X-NUM-GUESTS=0:mailto:afaer...@suse.de
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=bbau...@redhat.com;X-NUM-GUESTS=0:mailto:bbau...@redhat.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=eric.au...@redhat.com;X-NUM-GUESTS=0:mailto:eric.au...@redhat.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=jan.kis...@web.de;X-NUM-GUESTS=0:mailto:jan.kis...@web.de
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=jjhe...@linux.vnet.ibm.com;X-NUM-GUESTS=0:mailto:jjhe...@linux.vnet
 .ibm.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=m...@linux.vnet.ibm.com;X-NUM-GUESTS=0:mailto:m...@linux.vnet.ibm.c
 om
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=z@139.com;X-NUM-GUESTS=0:mailto:z@139.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 

Updated invitation: QEMU developers fortnightly conference call @ Every 2 weeks from 15:00 to 16:00 on Tuesday from Tue 2022-08-23 to Tue 2024-01-23 (GMT+2) (qemu-devel@nongnu.org)

[Juan Quintela]

BEGIN:VCALENDAR
PRODID:-//Google Inc//Google Calendar 70.9054//EN
VERSION:2.0
CALSCALE:GREGORIAN
METHOD:REQUEST
BEGIN:VTIMEZONE
TZID:America/New_York
X-LIC-LOCATION:America/New_York
BEGIN:DAYLIGHT
TZOFFSETFROM:-0500
TZOFFSETTO:-0400
TZNAME:EDT
DTSTART:19700308T02
RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=2SU
END:DAYLIGHT
BEGIN:STANDARD
TZOFFSETFROM:-0400
TZOFFSETTO:-0500
TZNAME:EST
DTSTART:19701101T02
RRULE:FREQ=YEARLY;BYMONTH=11;BYDAY=1SU
END:STANDARD
END:VTIMEZONE
BEGIN:VEVENT
DTSTART;TZID=America/New_York:20220823T09
DTEND;TZID=America/New_York:20220823T10
RRULE:FREQ=WEEKLY;WKST=MO;UNTIL=20240109T045959Z;INTERVAL=2;BYDAY=TU
DTSTAMP:20240118T171538Z
ORGANIZER;CN=KVM call for qemu developers:mailto:eged7cki05lmu1tngvkl3thids
 @group.calendar.google.com
UID:5dt5ji87j5qrc00o63ktq7g...@google.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;X-NUM-GUESTS=0:mailto:f4...@amsat.org
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=Joao Martins;X-NUM-GUESTS=0:mailto:joao.m.mart...@oracle.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=ACCEPTED;RSVP=TRUE
 ;CN=quint...@redhat.com;X-NUM-GUESTS=0:mailto:quint...@redhat.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=md...@redhat.com;X-NUM-GUESTS=0:mailto:md...@redhat.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=fel...@nutanix.com;X-NUM-GUESTS=0:mailto:fel...@nutanix.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=afaer...@suse.de;X-NUM-GUESTS=0:mailto:afaer...@suse.de
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=bbau...@redhat.com;X-NUM-GUESTS=0:mailto:bbau...@redhat.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=c...@f00f.org;X-NUM-GUESTS=0:mailto:c...@f00f.org
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=ebl...@redhat.com;X-NUM-GUESTS=0:mailto:ebl...@redhat.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=edgar.igles...@gmail.com;X-NUM-GUESTS=0:mailto:edgar.iglesias@gmail
 .com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=eric.au...@redhat.com;X-NUM-GUESTS=0:mailto:eric.au...@redhat.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=i...@theiggy.com;X-NUM-GUESTS=0:mailto:i...@theiggy.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=jan.kis...@web.de;X-NUM-GUESTS=0:mailto:jan.kis...@web.de
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=jidong.x...@gmail.com;X-NUM-GUESTS=0:mailto:jidong.x...@gmail.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=jjhe...@linux.vnet.ibm.com;X-NUM-GUESTS=0:mailto:jjhe...@linux.vnet
 .ibm.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=m...@linux.vnet.ibm.com;X-NUM-GUESTS=0:mailto:m...@linux.vnet.ibm.c
 om
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=peter.mayd...@linaro.org;X-NUM-GUESTS=0:mailto:peter.maydell@linaro
 .org
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=richard.hender...@linaro.org;X-NUM-GUESTS=0:mailto:richard.henderso
 n...@linaro.org
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=stefa...@gmail.com;X-NUM-GUESTS=0:mailto:stefa...@gmail.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=TENTATIVE;RSVP=TRU
 E;CN=i...@bsdimp.com;X-NUM-GUESTS=0:mailto:i...@bsdimp.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=z@139.com;X-NUM-GUESTS=0:mailto:z@139.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=zwu.ker...@gmail.com;X-NUM-GUESTS=0:mailto:zwu.ker...@gmail.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=j...@nvidia.com;X-NUM-GUESTS=0:mailto:j...@nvidia.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=c...@nvidia.com;X-NUM-GUESTS=0:mailto:c...@nvidia.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=Elena Ufimtseva;X-NUM-GUESTS=0:mailto:elena.ufimts...@oracle.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=konrad.w...@oracle.com;X-NUM-GUESTS=0:mailto:konrad.w...@oracle.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=a...@rev.ng;X-NUM-GUESTS=0:mailto:a...@rev.ng
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=a...@rev.ng;X-NUM-GUESTS=0:mailto:a...@rev.ng
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=Shameerali Kolothum 

Re: [PATCH] hw/arm/virt.c: Remove newline from error_report() string

[Philippe Mathieu-Daudé]

On 18/1/24 17:09, Peter Maydell wrote:

On Thu, 18 Jan 2024 at 15:30, Philippe Mathieu-Daudé  wrote:


On 18/1/24 16:17, Philippe Mathieu-Daudé wrote:

On 18/1/24 14:16, Peter Maydell wrote:

error_report() strings should not include trailing newlines; remove
the newline from the error we print when devices won't fit into the
address space of the CPU.

This commit also fixes the accidental hardcoded tabs that were in
this line, since we have to touch the line anyway.

Signed-off-by: Peter Maydell 
---
   hw/arm/virt.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 


BTW there is another case:

$ spatch \
--macro-file scripts/cocci-macro-file.h \
--sp-file scripts/coccinelle/err-bad-newline.cocci \
--keep-comments --use-gitgrep --dir .
./hw/arm/virt.c:1775:38:"Addressing limited to %d bits, but memory
exceeds it by %llu bytes\n"
./ui/gtk.c:1094:56:"gtk: unexpected touch event type\n"

We could run this some of these coccinelle scripts on CI,
as a new job in .gitlab-ci.d/static_checks.yml or extending
the check-patch job...


checkpatch catches it, if you read the output, which is
how I noticed this in the first place (I was originally
touching the line to fix the tab damage).


Ah, no need to waste more CI minutes then :)

Thanks!

[PATCH v4 2/3] ci: Add a migration compatibility test job

[Fabiano Rosas]

The migration tests have support for being passed two QEMU binaries to
test migration compatibility.

Add a CI job that builds the lastest release of QEMU and another job
that uses that version plus an already present build of the current
version and run the migration tests with the two, both as source and
destination. I.e.:

 old QEMU (n-1) -> current QEMU (development tree)
 current QEMU (development tree) -> old QEMU (n-1)

The purpose of this CI job is to ensure the code we're about to merge
will not cause a migration compatibility problem when migrating the
next release (which will contain that code) to/from the previous
release.

The version of migration-test used will be the one matching the older
QEMU. That way we can avoid special-casing new tests that wouldn't be
compatible with the older QEMU.

Note: for user forks, the version tags need to be pushed to gitlab
otherwise it won't be able to checkout a different version.

Signed-off-by: Fabiano Rosas 
---
 .gitlab-ci.d/buildtest.yml | 60 ++
 1 file changed, 60 insertions(+)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index e1c7801598..f0b0edc634 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -167,6 +167,66 @@ build-system-centos:
   x86_64-softmmu rx-softmmu sh4-softmmu nios2-softmmu
 MAKE_CHECK_ARGS: check-build
 
+# Previous QEMU release. Used for cross-version migration tests.
+build-previous-qemu:
+  extends: .native_build_job_template
+  artifacts:
+when: on_success
+expire_in: 2 days
+paths:
+  - build-previous
+exclude:
+  - build-previous/**/*.p
+  - build-previous/**/*.a.p
+  - build-previous/**/*.fa.p
+  - build-previous/**/*.c.o
+  - build-previous/**/*.c.o.d
+  - build-previous/**/*.fa
+  needs:
+job: amd64-opensuse-leap-container
+  variables:
+IMAGE: opensuse-leap
+TARGETS: x86_64-softmmu aarch64-softmmu
+  before_script:
+- export QEMU_PREV_VERSION="$(sed 's/\([0-9.]*\)\.[0-9]*/v\1.0/' VERSION)"
+- git checkout $QEMU_PREV_VERSION
+  after_script:
+- mv build build-previous
+
+.migration-compat-common:
+  extends: .common_test_job_template
+  needs:
+- job: build-previous-qemu
+- job: build-system-opensuse
+  # The old QEMU could have bugs unrelated to migration that are
+  # already fixed in the current development branch, so this test
+  # might fail.
+  allow_failure: true
+  variables:
+IMAGE: opensuse-leap
+MAKE_CHECK_ARGS: check-build
+  script:
+# Use the migration-tests from the older QEMU tree. This avoids
+# testing an old QEMU against new features/tests that it is not
+# compatible with.
+- cd build-previous
+# old to new
+- QTEST_QEMU_BINARY_SRC=./qemu-system-${TARGET}
+  QTEST_QEMU_BINARY=../build/qemu-system-${TARGET} 
./tests/qtest/migration-test
+# new to old
+- QTEST_QEMU_BINARY_DST=./qemu-system-${TARGET}
+  QTEST_QEMU_BINARY=../build/qemu-system-${TARGET} 
./tests/qtest/migration-test
+
+migration-compat-aarch64:
+  extends: .migration-compat-common
+  variables:
+TARGET: aarch64
+
+migration-compat-x86_64:
+  extends: .migration-compat-common
+  variables:
+TARGET: x86_64
+
 check-system-centos:
   extends: .native_test_job_template
   needs:
-- 
2.35.3

[PATCH v4 1/3] tests/qtest/migration: Don't use -cpu max for aarch64

[Fabiano Rosas]

The 'max' cpu is not expected to be stable in terms of features across
QEMU versions, so it should not be expected to migrate.

While the tests currently all pass with -cpu max, that is only because
we're not testing across QEMU versions, which is the more common
use-case for migration.

We've recently introduced compatibility tests that use two different
QEMU versions and the tests are now failing for aarch64. The next
patch adds those tests to CI, so we cannot use the 'max' cpu
anymore. Replace it with the 'neoverse-n1', which has a fixed set of
features.

Suggested-by: Peter Maydell 
Signed-off-by: Fabiano Rosas 
---
 tests/qtest/migration-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index d3066e119f..b2390656f0 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -820,7 +820,7 @@ static int test_migrate_start(QTestState **from, QTestState 
**to,
 memory_size = "150M";
 machine_alias = "virt";
 machine_opts = "gic-version=max";
-arch_opts = g_strdup_printf("-cpu max -kernel %s", bootpath);
+arch_opts = g_strdup_printf("-cpu neoverse-n1 -kernel %s", bootpath);
 start_address = ARM_TEST_MEM_START;
 end_address = ARM_TEST_MEM_END;
 } else {
-- 
2.35.3

[PATCH v4 0/3] migration & CI: Add a CI job for migration compat testing

[Fabiano Rosas]

Here's the second half of adding a migration compatibility test to CI.

We've already added support for running the full set of migration
tests with two QEMU binaries since commit 5050ad2a380
("tests/qtest/migration: Support more than one QEMU binary"), now
what's left is adding it to the CI.

changes since v3:

- Removed all the 'since' logic and started using the n-1 version of
  the tests.

- New patch to fix the aarch64 cpu. We shouldn't have been using
  'max'.

- New patch to disable aarch64 tests while the fix^ doesn't reach a
  released version.

v3:
https://lore.kernel.org/r/20240105180449.11562-1-faro...@suse.de
v2:
https://lore.kernel.org/r/20240104171857.20108-1-faro...@suse.de
v1:
https://lore.kernel.org/r/20231207155809.25673-1-faro...@suse.de

Fabiano Rosas (3):
  tests/qtest/migration: Don't use -cpu max for aarch64
  ci: Add a migration compatibility test job
  ci: Disable migration compatibility tests for aarch64

 .gitlab-ci.d/buildtest.yml   | 64 
 tests/qtest/migration-test.c |  2 +-
 2 files changed, 65 insertions(+), 1 deletion(-)

-- 
2.35.3

[PATCH v4 3/3] ci: Disable migration compatibility tests for aarch64

[Fabiano Rosas]

Until 9.0 is out, we need to keep the aarch64 job disabled because the
tests always use the n-1 version of migration-test. That happens to be
broken for aarch64 in 8.2. Once 9.0 is out, it will become the n-1
version and it will bring the fixed tests.

We can revert this patch when 9.0 releases.

Signed-off-by: Fabiano Rosas 
---
 .gitlab-ci.d/buildtest.yml | 4 
 1 file changed, 4 insertions(+)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index f0b0edc634..b344a4685f 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -217,10 +217,14 @@ build-previous-qemu:
 - QTEST_QEMU_BINARY_DST=./qemu-system-${TARGET}
   QTEST_QEMU_BINARY=../build/qemu-system-${TARGET} 
./tests/qtest/migration-test
 
+# This job is disabled until we release 9.0. The existing
+# migration-test in 8.2 is broken on aarch64. The fix was already
+# commited, but it will only take effect once 9.0 is out.
 migration-compat-aarch64:
   extends: .migration-compat-common
   variables:
 TARGET: aarch64
+QEMU_JOB_OPTIONAL: 1
 
 migration-compat-x86_64:
   extends: .migration-compat-common
-- 
2.35.3

[PATCH v2 2/4] Avoid conflicting types for 'copy_file_range'

[Manolo de Medici]

Compilation fails on systems where copy_file_range is already defined as a
stub.

The prototype of copy_file_range in glibc returns an ssize_t, not an off_t.

The function currently only exists on linux and freebsd, and in both cases
the return type is ssize_t

Signed-off-by: Manolo de Medici 
---
 block/file-posix.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 35684f7e21..f744b35642 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2000,12 +2000,13 @@ static int handle_aiocb_write_zeroes_unmap(void *opaque)
 }

 #ifndef HAVE_COPY_FILE_RANGE
-static off_t copy_file_range(int in_fd, off_t *in_off, int out_fd,
- off_t *out_off, size_t len, unsigned int flags)
+ssize_t copy_file_range (int infd, off_t *pinoff,
+ int outfd, off_t *poutoff,
+ size_t length, unsigned int flags)
 {
 #ifdef __NR_copy_file_range
-return syscall(__NR_copy_file_range, in_fd, in_off, out_fd,
-   out_off, len, flags);
+return (ssize_t)syscall(__NR_copy_file_range, infd, pinoff, outfd,
+poutoff, length, flags);
 #else
 errno = ENOSYS;
 return -1;
--
2.43.0

Re: [PATCH 2/4] reset: Allow multiple stages of system resets

[Peter Maydell]

On Thu, 18 Jan 2024 at 15:53, Philippe Mathieu-Daudé  wrote:
> Unfortunately despite DeviceReset being deprecated since 4 years
> in commit c11256aa6f ("hw/core: add Resettable support to BusClass
> and DeviceClass"), we keep adding code using this legacy API; for
> example in the last 4 months:
>
> - e029bb00a7 ("hw/pci-host: Add Astro system bus adapter found on
> PA-RISC machines")
> - 2880e676c0 ("Add virtio-sound device stub")
> - 4a58330343 ("hw/cxl: Add a switch mailbox CCI function")
> - 95e1019a4a ("vhost-user-scsi: free the inflight area when reset")
> - 6f9c3aaa34 ("fsl-imx: add simple RTC emulation for i.MX6 and i.MX7
> boards")

For plain old leaf device reset methods, I'm not too fussed
about this, because a reset method is exactly equivalent
to a single 'hold' phase method, and the relatively
small amount of code to convert from one to the other isn't
introducing a lot of extra complication.

The cleanup I really want to get back to is the bus reset
handling, because we only have a few buses that don't use
3-phase reset, and if we convert those then we can get rid
of the handling of transitional reset from BusClass
completely. Unfortunately I ran into a regression somewhere
in the PCI reset handling in the patchsets I was working on
which I never got back to to track down the problem.

thanks
-- PMM

[PATCH v2 3/4] Add the GNU/Hurd as a target host

[Manolo de Medici]

Signed-off-by: Manolo de Medici 
---
 configure | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure b/configure
index 21ab9a64e9..fb11ede5b2 100755
--- a/configure
+++ b/configure
@@ -353,6 +353,8 @@ elif check_define __NetBSD__; then
   host_os=netbsd
 elif check_define __APPLE__; then
   host_os=darwin
+elif check_define __GNU__; then
+  host_os=hurd
 else
   # This is a fatal error, but don't report it yet, because we
   # might be going to just print the --help text, or it might
--
2.43.0

Re: [PATCH] hw/arm/virt.c: Remove newline from error_report() string

[Peter Maydell]

On Thu, 18 Jan 2024 at 15:30, Philippe Mathieu-Daudé  wrote:
>
> On 18/1/24 16:17, Philippe Mathieu-Daudé wrote:
> > On 18/1/24 14:16, Peter Maydell wrote:
> >> error_report() strings should not include trailing newlines; remove
> >> the newline from the error we print when devices won't fit into the
> >> address space of the CPU.
> >>
> >> This commit also fixes the accidental hardcoded tabs that were in
> >> this line, since we have to touch the line anyway.
> >>
> >> Signed-off-by: Peter Maydell 
> >> ---
> >>   hw/arm/virt.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Reviewed-by: Philippe Mathieu-Daudé 
>
> BTW there is another case:
>
> $ spatch \
>--macro-file scripts/cocci-macro-file.h \
>--sp-file scripts/coccinelle/err-bad-newline.cocci \
>--keep-comments --use-gitgrep --dir .
> ./hw/arm/virt.c:1775:38:"Addressing limited to %d bits, but memory
> exceeds it by %llu bytes\n"
> ./ui/gtk.c:1094:56:"gtk: unexpected touch event type\n"
>
> We could run this some of these coccinelle scripts on CI,
> as a new job in .gitlab-ci.d/static_checks.yml or extending
> the check-patch job...

checkpatch catches it, if you read the output, which is
how I noticed this in the first place (I was originally
touching the line to fix the tab damage).

-- PMM

[PATCH v2 4/4] Exclude TPM ioctls definitions for the GNU/Hurd

[Manolo de Medici]

The Hurd currently doesn't have any TPM driver, compilation fails
for missing symbols unless these are left undefined.

Signed-off-by: Manolo de Medici 
---
 backends/tpm/tpm_ioctl.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/backends/tpm/tpm_ioctl.h b/backends/tpm/tpm_ioctl.h
index 1933ab6855..c721bf8847 100644
--- a/backends/tpm/tpm_ioctl.h
+++ b/backends/tpm/tpm_ioctl.h
@@ -274,7 +274,7 @@ typedef struct ptm_lockstorage ptm_lockstorage;
 #define PTM_CAP_SEND_COMMAND_HEADER (1 << 15)
 #define PTM_CAP_LOCK_STORAGE   (1 << 16)

-#ifndef _WIN32
+#if !defined(_WIN32) && !defined(__GNU__)
 enum {
 PTM_GET_CAPABILITY = _IOR('P', 0, ptm_cap),
 PTM_INIT   = _IOWR('P', 1, ptm_init),
--
2.43.0

[PATCH v2 1/4] Include new arbitrary limits if not already defined

[Manolo de Medici]

qemu uses the PATH_MAX and IOV_MAX constants extensively
in the code. Define these constants to sensible values ourselves
if the system doesn't define them already.

Signed-off-by: Manolo de Medici 
---
 include/qemu/osdep.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 9a405bed89..9fb6ac5c64 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -363,6 +363,14 @@ void QEMU_ERROR("code path is reachable")
 #define TIME_MAX TYPE_MAXIMUM(time_t)
 #endif

+#ifndef PATH_MAX
+#define PATH_MAX 1024
+#endif
+
+#ifndef IOV_MAX
+#define IOV_MAX 1024
+#endif
+
 /* Mac OSX has a  bug that incorrectly defines SIZE_MAX with
  * the wrong type. Our replacement isn't usable in preprocessor
  * expressions, but it is sufficient for our needs. */
--
2.43.0

[PATCH v2 0/4] Port qemu to GNU/Hurd

[Manolo de Medici]

Recently, a testsuite for gnumach, the GNU/Hurd microkernel, was developed
that uses qemu. Currently qemu cannot be compiled for the GNU/Hurd, as such,
this testsuite is available only for GNU/Linux users.

This patcheset allows qemu compilation in GNU/Hurd. With this patchset applied,
qemu can be compiled without any special configure options.

Please review, thanks,

Manolo de Medici (4):
  Include new arbitrary limits if not already defined
  Avoid conflicting types for 'copy_file_range'
  Add the GNU/Hurd as a target host
  Exclude TPM ioctls definitions for the GNU/Hurd

 backends/tpm/tpm_ioctl.h | 2 +-
 block/file-posix.c   | 9 +
 configure| 2 ++
 include/qemu/osdep.h | 8 
 4 files changed, 16 insertions(+), 5 deletions(-)

--
2.43.0

Re: [PATCH 2/4] reset: Allow multiple stages of system resets

[Philippe Mathieu-Daudé]

On 17/1/24 18:46, Peter Maydell wrote:

On Wed, 17 Jan 2024 at 09:16,  wrote:


From: Peter Xu 

QEMU resets do not have a way to order reset hooks.  Add one coarse grained
reset stage so that some devices can be reset later than some others.

Signed-off-by: Peter Xu 
---
  include/sysemu/reset.h |  5 
  hw/core/reset.c| 60 +++---
  2 files changed, 49 insertions(+), 16 deletions(-)

diff --git a/include/sysemu/reset.h b/include/sysemu/reset.h
index 609e4d50c2..0de697ce9f 100644
--- a/include/sysemu/reset.h
+++ b/include/sysemu/reset.h
@@ -5,9 +5,14 @@

  typedef void QEMUResetHandler(void *opaque);

+#define  QEMU_RESET_STAGES_N  2
+


Our reset handling APIs are already pretty complicated, and
raw qemu_register_reset() is kind of a legacy API that I would
prefer that we try to move away from, not add extra complexity to.

Our device reset design already has a multiple-phase system
(see docs/devel/reset.rst), part of the point of which is to
try to give us a way to deal with reset-ordering problems.
I feel like the right way to handle the issue you're trying to
address is to ensure that the thing that needs to happen last is
done in the 'exit' phase rather than the 'hold' phase (which is
where legacy reset methods get called).


I concur. Devices reset is hard, but bus reset is even harder.
Having a quick look, the issues tracked by Alex & Peter might
come from the PCI bridges using the legacy DeviceReset. In
particular functions like:

- hw/pci-bridge/pcie_root_port.c

 46 static void rp_reset_hold(Object *obj)
 47 {
 48 PCIDevice *d = PCI_DEVICE(obj);
 49 DeviceState *qdev = DEVICE(obj);
 50
 51 rp_aer_vector_update(d);
 52 pcie_cap_root_reset(d);
 53 pcie_cap_deverr_reset(d);
 54 pcie_cap_slot_reset(d);
 55 pcie_cap_arifwd_reset(d);
 56 pcie_acs_reset(d);
 57 pcie_aer_root_reset(d);
 58 pci_bridge_reset(qdev);
 59 pci_bridge_disable_base_limit(d);
 60 }

- hw/pci-bridge/pcie_pci_bridge.c

107 static void pcie_pci_bridge_reset(DeviceState *qdev)
108 {
109 PCIDevice *d = PCI_DEVICE(qdev);
110 pci_bridge_reset(qdev);
111 if (msi_present(d)) {
112 msi_reset(d);
113 }
114 shpc_reset(d);
115 }

- hw/pci-bridge/xio3130_downstream.c

 56 static void xio3130_downstream_reset(DeviceState *qdev)
 57 {
 58 PCIDevice *d = PCI_DEVICE(qdev);
 59
 60 pcie_cap_deverr_reset(d);
 61 pcie_cap_slot_reset(d);
 62 pcie_cap_arifwd_reset(d);
 63 pci_bridge_reset(qdev);
 64 }

should really be split and converted as ResettablePhases.

pci_bridge_reset() is likely a ResettableExitPhase one.


There are some annoying wrinkles here, notably that legacy
qemu_register_reset() doesn't support 3-phase reset and so
the phasing only happens for devices reset via the device/bus
tree hierarchy. But I think the way to go is to try to move
forward with that design (i.e. expand 3-phase reset to
qemu_register_reset() and/or move things using qemu_register_reset()
to device reset where that makes sense), not to ignore it and
put a completely different reset-ordering API in a different place.


Unfortunately despite DeviceReset being deprecated since 4 years
in commit c11256aa6f ("hw/core: add Resettable support to BusClass
and DeviceClass"), we keep adding code using this legacy API; for
example in the last 4 months:

- e029bb00a7 ("hw/pci-host: Add Astro system bus adapter found on 
PA-RISC machines")

- 2880e676c0 ("Add virtio-sound device stub")
- 4a58330343 ("hw/cxl: Add a switch mailbox CCI function")
- 95e1019a4a ("vhost-user-scsi: free the inflight area when reset")
- 6f9c3aaa34 ("fsl-imx: add simple RTC emulation for i.MX6 and i.MX7 
boards")


Regards,

Phil.

Re: [PATCH] ui/gtk: Strip trailing '\n' from error string arguments

[Markus Armbruster]

Philippe Mathieu-Daudé  writes:

> error_report() strings should not include trailing newlines.
>
> Noticed running:
>
>   $ spatch \
> --macro-file scripts/cocci-macro-file.h \
> --sp-file scripts/coccinelle/err-bad-newline.cocci \
> --keep-comments --use-gitgrep --dir .
>   ./ui/gtk.c:1094:56:"gtk: unexpected touch event type\n"
>
> Fixes: 5a4cb61ae1 ("ui/gtk: enable backend to send multi-touch events")
> Inspired-by: Peter Maydell 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  ui/gtk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 810d7fc796..c819c72a20 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -1091,7 +1091,7 @@ static gboolean gd_touch_event(GtkWidget *widget, 
> GdkEventTouch *touch,
>  type = INPUT_MULTI_TOUCH_TYPE_END;
>  break;
>  default:
> -warn_report("gtk: unexpected touch event type\n");
> +warn_report("gtk: unexpected touch event type");
>  return FALSE;
>  }

I don't like the error message (drop the "gtk: " prefix or work it into
the message), but that's a separate issue.

Reviewed-by: Markus Armbruster