Re: [PATCH v13 2/2] vhost-vdpa: add support for vIOMMU

[Cindy Lu]

On Mon, Mar 6, 2023 at 11:36 AM Jason Wang  wrote:
>
>
> 在 2023/2/8 10:57, Cindy Lu 写道:
> > 1.Add support for vIOMMU.
> > Add the new function to deal with iommu MR.
> > - during iommu_region_add register a specific IOMMU notifier,
> >and store all notifiers in a list.
> > - during iommu_region_del, compare and delete the IOMMU notifier from the 
> > list
> > - since the SVQ not support iommu yet, add the check for IOMMU
> >in vhost_vdpa_dev_start, if the SVQ and IOMMU enable at the same time
> >function will return fail.
> >
> > 2.Skip the check in vhost_vdpa_listener_skipped_section() while
> > MR is IOMMU, Move this check to  vhost_vdpa_iommu_map_notify()
>
>
> This need some tweak as well, it's better not repeat what is done in the
> code but why do you need this change. More could be found at:
>
> https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
>
sure, will change this
>
> >
> > Verified in vp_vdpa and vdpa_sim_net driver
> >
> > Signed-off-by: Cindy Lu 
> > ---
> >   hw/virtio/vhost-vdpa.c | 173 ++---
> >   include/hw/virtio/vhost-vdpa.h |  11 +++
> >   2 files changed, 173 insertions(+), 11 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > index 542e003101..46f676ab71 100644
> > --- a/hw/virtio/vhost-vdpa.c
> > +++ b/hw/virtio/vhost-vdpa.c
> > @@ -26,6 +26,7 @@
> >   #include "cpu.h"
> >   #include "trace.h"
> >   #include "qapi/error.h"
> > +#include "hw/virtio/virtio-access.h"
> >
> >   /*
> >* Return one past the end of the end of section. Be careful with uint64_t
> > @@ -60,15 +61,22 @@ static bool 
> > vhost_vdpa_listener_skipped_section(MemoryRegionSection *section,
> >iova_min, section->offset_within_address_space);
> >   return true;
> >   }
> > +/*
> > + * While using vIOMMU, Sometimes the section will be larger than 
> > iova_max
> > + * but the memory that  actually mapping is smaller, So skip the check
> > + * here. Will add the check-in vhost_vdpa_iommu_map_notify,
> > + *There is the real size that maps to the kernel
>
>
> Please tweak the comment, it has issues of whitespace, capitalization,
> punctuation marks.
>
sure will change this
>
> > + */
> >
> > -llend = vhost_vdpa_section_end(section);
> > -if (int128_gt(llend, int128_make64(iova_max))) {
> > -error_report("RAM section out of device range (max=0x%" PRIx64
> > - ", end addr=0x%" PRIx64 ")",
> > - iova_max, int128_get64(llend));
> > -return true;
> > +if (!memory_region_is_iommu(section->mr)) {
>
>
> Note related to this patch but should we exclude non ram region here as
> well?
>
Sure, will add this check

>
> > +llend = vhost_vdpa_section_end(section);
> > +if (int128_gt(llend, int128_make64(iova_max))) {
> > +error_report("RAM section out of device range (max=0x%" PRIx64
> > + ", end addr=0x%" PRIx64 ")",
> > + iova_max, int128_get64(llend));
> > +return true;
> > +}
> >   }
> > -
> >   return false;
> >   }
> >
> > @@ -185,6 +193,118 @@ static void vhost_vdpa_listener_commit(MemoryListener 
> > *listener)
> >   v->iotlb_batch_begin_sent = false;
> >   }
> >
> > +static void vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry 
> > *iotlb)
> > +{
> > +struct vdpa_iommu *iommu = container_of(n, struct vdpa_iommu, n);
> > +
> > +hwaddr iova = iotlb->iova + iommu->iommu_offset;
> > +struct vhost_vdpa *v = iommu->dev;
> > +void *vaddr;
> > +int ret;
> > +Int128 llend;
> > +
> > +if (iotlb->target_as != _space_memory) {
> > +error_report("Wrong target AS \"%s\", only system memory is 
> > allowed",
> > + iotlb->target_as->name ? iotlb->target_as->name : 
> > "none");
> > +return;
> > +}
> > +RCU_READ_LOCK_GUARD();
> > +/* check if RAM section out of device range */
> > +llend = int128_add(int128_makes64(iotlb->addr_mask), 
> > int128_makes64(iova));
> > +if (int128_gt(llend, int128_make64(v->iova_range.last))) {
> > +error_report("RAM section out of device range (max=0x%" PRIx64
> > + ", end addr=0x%" PRIx64 ")",
> > + v->iova_range.last, int128_get64(llend));
> > +return;
>
>
> Can you meet this condition? If yes, should we crop instead of fail here?
>
Based on my test, we didn't meet this condition. so just put an error
report here.

>
> > +}
> > +
> > +vhost_vdpa_iotlb_batch_begin_once(v);
>
>
> Where do we send the VHOST_IOTLB_BATCH_END message, or do we even need
> any batching here?
>
the VHOST_IOTLB_BATCH_END message was send by
vhost_vdpa_listener_commit, because we only use
one vhost_vdpa_memory_listener and no-iommu mode will also need to use
this listener, So we still need to add the batch begin here, based on
my 

Re: [PATCH for-8.1 v2 25/26] target/riscv: rework write_misa()

[liweiwei]

On 2023/3/15 00:49, Daniel Henrique Barboza wrote:

write_misa() must use as much common logic as possible. We want to open
code just the bits that are exclusive to the CSR write operation and TCG
internals.

Rewrite write_misa() to work as follows:

- supress RVC right after verifying that we're not updating RVG;

- mask the write using misa_ext_mask to avoid enabling unsupported
   extensions;

- emulate the steps done by realize(): validate the candidate misa_ext
   val, then validate the configuration with the candidate misa_ext val,
   and finally commit the changes to cpu->cfg.

If any of the validation steps fails simply ignore the write operation.

Let's keep write_misa() as experimental for now until this logic gains
enough mileage.

Signed-off-by: Daniel Henrique Barboza 
---
  target/riscv/cpu.c | 12 +---
  target/riscv/cpu.h |  6 ++
  target/riscv/csr.c | 47 +-
  3 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 5bd92e1cda..4789a7b70d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1027,9 +1027,8 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU 
*cpu)
  }
  
  
-static void riscv_cpu_validate_misa_ext(CPURISCVState *env,

-uint32_t misa_ext,
-Error **errp)
+void riscv_cpu_validate_misa_ext(CPURISCVState *env, uint32_t misa_ext,
+ Error **errp)
  {
  Error *local_err = NULL;
  
@@ -1134,9 +1133,8 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)

   * candidate misa_ext value. No changes in env->misa_ext
   * are made.
   */
-static void riscv_cpu_validate_extensions(RISCVCPU *cpu,
-  uint32_t misa_ext,
-  Error **errp)
+void riscv_cpu_validate_extensions(RISCVCPU *cpu, uint32_t misa_ext,
+   Error **errp)
  {
  if (cpu->cfg.epmp && !cpu->cfg.pmp) {
  /*
@@ -1227,7 +1225,7 @@ static void riscv_cpu_validate_extensions(RISCVCPU *cpu,
  }
  }
  
-static void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu)

+void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu)
  {
  if (cpu->cfg.ext_zk) {
  cpu->cfg.ext_zkn = true;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index dbb4df9df0..ca2ba6a647 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -593,6 +593,12 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
  char *riscv_isa_string(RISCVCPU *cpu);
  void riscv_cpu_list(void);
  
+void riscv_cpu_validate_misa_ext(CPURISCVState *env, uint32_t misa_ext,

+ Error **errp);
+void riscv_cpu_validate_extensions(RISCVCPU *cpu, uint32_t misa_ext,
+   Error **errp);
+void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu);
+
  #define cpu_list riscv_cpu_list
  #define cpu_mmu_index riscv_cpu_mmu_index
  
diff --git a/target/riscv/csr.c b/target/riscv/csr.c

index 918d442ebd..6f26e7dbcd 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1343,6 +1343,9 @@ static RISCVException read_misa(CPURISCVState *env, int 
csrno,
  static RISCVException write_misa(CPURISCVState *env, int csrno,
   target_ulong val)
  {
+RISCVCPU *cpu = env_archcpu(env);
+Error *local_err = NULL;
+
  if (!riscv_cpu_cfg(env)->misa_w) {
  /* drop write to misa */
  return RISCV_EXCP_NONE;
@@ -1353,47 +1356,39 @@ static RISCVException write_misa(CPURISCVState *env, 
int csrno,
  return RISCV_EXCP_NONE;
  }
  
-/* 'I' or 'E' must be present */

-if (!(val & (RVI | RVE))) {
-/* It is not, drop write to misa */
-return RISCV_EXCP_NONE;
-}
-
-/* 'E' excludes all other extensions */
-if (val & RVE) {
-/*
- * when we support 'E' we can do "val = RVE;" however
- * for now we just drop writes if 'E' is present.
- */
-return RISCV_EXCP_NONE;
-}
-
  /*
- * misa.MXL writes are not supported by QEMU.
- * Drop writes to those bits.
+ * Suppress 'C' if next instruction is not aligned
+ * TODO: this should check next_pc
   */
+if ((val & RVC) && (GETPC() & ~3) != 0) {
+val &= ~RVC;
+}
  
  /* Mask extensions that are not supported by this hart */

  val &= env->misa_ext_mask;
  
-/* 'D' depends on 'F', so clear 'D' if 'F' is not present */

-if ((val & RVD) && !(val & RVF)) {
-val &= ~RVD;
+/* If nothing changed, do nothing. */
+if (val == env->misa_ext) {
+return RISCV_EXCP_NONE;
  }
  
  /*

- * Suppress 'C' if next instruction is not aligned
- * TODO: this should check next_pc
+ * This flow is similar to what riscv_cpu_realize() does,
+ * with the difference that we will update env->misa_ext
+ * value if 

Re: [PATCH V3 0/2] qemu: vhost-user: Support Xen memory mapping quirks

[Viresh Kumar]

On 09-03-23, 14:20, Viresh Kumar wrote:
> Hello,
> 
> This patchset tries to update the vhost-user protocol to make it support 
> special
> memory mapping required in case of Xen hypervisor.
> 
> The first patch is mostly cleanup and second one introduces a new xen specific
> feature.
> 
> V2->V3:
> - Remove the extra message and instead update the memory regions to carry
>   additional data.
> 
> - Drop the one region one mmap relationship and allow back-end to map only 
> parts
>   of a region at once, required for Xen grant mappings.
> 
> - Additional cleanup patch 1/2.

Stefan,

Does this version look better ?

-- 
viresh

[PATCH v3 2/2] pci: introduce slot_reserved_auto_mask and slot_reserved_manual_mask

[Chuck Zmudzinski]

Commit 4f67543bb8c5 ("xen/pt: reserve PCI slot 2 for Intel igd-passthru")
uses slot_reserved_mask to reserve slot 2 for the Intel igd for the
xenfv machine when the guest is configured for igd-passthru.

Prior to that commit, a single 32-bit mask was sufficient to meet the
needs of the only machine that used the 32-bit slot_reserved_mask, the
sun4u machine. However, the requirements of the xenfv machine with
igd-passthru is somewhat different from the requirements of the sun4u
machine.

First, the sun4u machine reserves slots in such a way that no device
can be assigned to a reserved slot, but the xenfv machine needs to
reserve a single slot that is reserved for a particular device, the
Intel igd. The necessary logic to ensure that the reserved slot is used
by the Intel igd was recently added by the aforementioned commit.

Second, it is useful to limit slot reservation in the case of the xenfv
machine with the Intel igd to devices configured for automatic slot
assignment so an administrator can assign a device to the reserved slot
by manually specifying the reserved slot address on the command line,
but the sun4u machine requires slot reservation for all devices, whether
or not the device is configured for automatic slot assignment or
configured manually by specifying a slot address on the command line. In
other words, for the sun4u machine, the required behavior is that an
attempt to assign a reserved slot to a device must always result in an
error, but it is useful to allow manual assignment of a reserved slot to
succeed for the xenfv machine with the Intel igd.

The necessary logic to implement the desired behavior of reserving one
or more slots only for the case of automatic slot allocation has not yet
been implemented, and that is the purpose of this patch.

The implementation is simple: the xenfv machine only sets
slot_reserved_auto_mask and the sun4u machine sets both
slot_reserved_manual_mask and slot_reserved_auto_mask. A single
"set" accessor function allows xenfv and sun4u machines to set the
value of the two masks appropriately for each use case.

Since the xenfv machine needs to implement additional logic to detect
the Intel igd and clear the bit in the mask to allow a particular device
to use the reserved slot, there is a need for a "get" and "clear" accessor
function for slot_reserved_auto_mask, but these accessor functions are
not needed for slot_reserved_manual_mask because the sun4u machine has
no need to get the value of the mask or clear bits in the mask.

Link: 
https://lore.kernel.org/qemu-devel/20230106064838-mutt-send-email-...@kernel.org/
Signed-off-by: Chuck Zmudzinski 
---
Changelog

v3: Change Subject of patch from
"pci: allow slot_reserved_mask to be ignored with manual slot assignment" To
"pci: introduce slot_reserved_auto_mask and slot_reserved_manual_mask"

Substantially reword the commit message to clearly explain the reasons
this patch is needed

Apply changes in response to comments on v2:
   - slot_reserved_mask -> slot_reserved_auto_mask
   - remove enforce_slot_reserved_mask_manual
   - remove pci_bus_ignore_slot_reserved_mask_manual
   - add slot_reserved_manual_mask
   - pci_bus_devfn_reserved -> pci_bus_devfn_reserved_auto
   - change code in pci_bus_devfn_reserved_manual appropriately
   - pci_bus_set_slot_reserved_mask -> pci_bus_set_slot_reserved_masks
   - use renamed "set" function to set value of both masks for sun4u 
and xenfv cases
   - pci_bus_get_slot_reserved_mask -> pci_bus_get_slot_reserved_auto_mask
   - pci_bus_clear_slot_reserved_mask -> 
pci_bus_clear_slot_reserved_auto_mask

v2: Change Subject of patch from
"pci: add enforce_slot_reserved_mask_manual property" To
"pci: allow slot_reserved_mask to be ignored with manual slot assignment"

Add pci_bus_ignore_slot_reserved_mask_manual function

Call pci_bus_ignore_slot_reserved_mask_manual at appropriate place
in hw/pci-host/i440fx.c

 hw/pci/pci.c | 29 ++---
 hw/sparc64/sun4u.c   |  6 +++---
 hw/xen/xen_pt.c  |  6 +++---
 include/hw/pci/pci.h |  6 +++---
 include/hw/pci/pci_bus.h |  3 ++-
 5 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 8a87ccc8b0..58a530a651 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -500,7 +500,8 @@ static void pci_root_bus_internal_init(PCIBus *bus, 
DeviceState *parent,
 {
 assert(PCI_FUNC(devfn_min) == 0);
 bus->devfn_min = devfn_min;
-bus->slot_reserved_mask = 0x0;
+bus->slot_reserved_auto_mask = 0x0;
+bus->slot_reserved_manual_mask = 0x0;
 bus->address_space_mem = address_space_mem;
 bus->address_space_io = address_space_io;
 bus->flags |= PCI_BUS_IS_ROOT;
@@ -,24 +1112,30 @@ static bool pci_bus_devfn_available(PCIBus *bus, int 
devfn)
 return !(bus->devices[devfn]);
 }
 
-static bool pci_bus_devfn_reserved(PCIBus *bus, int devfn)
+static bool 

[PATCH v3 1/2] pci: avoid accessing slot_reserved_mask directly outside of pci.c

[Chuck Zmudzinski]

This patch provides accessor functions as replacements for direct
access to slot_reserved_mask according to the comment at the top
of include/hw/pci/pci_bus.h which advises that data structures for
PCIBus should not be directly accessed but instead be accessed using
accessor functions in pci.h.

Three accessor functions can conveniently replace all direct accesses
of slot_reserved_mask. With this patch, the new accessor functions are
used in hw/sparc64/sun4u.c and hw/xen/xen_pt.c and pci_bus.h is removed
from the included header files of the same two files.

No functional change intended.

Signed-off-by: Chuck Zmudzinski 
---
Changelog

v3: This patch is unchanged since v2.

v2: This is the first version of this patch, it did not exist in v1.

 hw/pci/pci.c | 15 +++
 hw/sparc64/sun4u.c   |  7 +++
 hw/xen/xen_pt.c  |  7 +++
 include/hw/pci/pci.h |  3 +++
 4 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index def5000e7b..8a87ccc8b0 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1116,6 +1116,21 @@ static bool pci_bus_devfn_reserved(PCIBus *bus, int 
devfn)
 return bus->slot_reserved_mask & (1UL << PCI_SLOT(devfn));
 }
 
+uint32_t pci_bus_get_slot_reserved_mask(PCIBus *bus)
+{
+return bus->slot_reserved_mask;
+}
+
+void pci_bus_set_slot_reserved_mask(PCIBus *bus, uint32_t mask)
+{
+bus->slot_reserved_mask |= mask;
+}
+
+void pci_bus_clear_slot_reserved_mask(PCIBus *bus, uint32_t mask)
+{
+bus->slot_reserved_mask &= ~mask;
+}
+
 /* -1 for devfn means auto assign */
 static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
  const char *name, int devfn,
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index a25e951f9d..eae7589462 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -31,7 +31,6 @@
 #include "hw/irq.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_bridge.h"
-#include "hw/pci/pci_bus.h"
 #include "hw/pci/pci_host.h"
 #include "hw/qdev-properties.h"
 #include "hw/pci-host/sabre.h"
@@ -608,9 +607,9 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
 /* Only in-built Simba APBs can exist on the root bus, slot 0 on busA is
reserved (leaving no slots free after on-board devices) however slots
0-3 are free on busB */
-pci_bus->slot_reserved_mask = 0xfffc;
-pci_busA->slot_reserved_mask = 0xfff1;
-pci_busB->slot_reserved_mask = 0xfff0;
+pci_bus_set_slot_reserved_mask(pci_bus, 0xfffc);
+pci_bus_set_slot_reserved_mask(pci_busA, 0xfff1);
+pci_bus_set_slot_reserved_mask(pci_busB, 0xfff0);
 
 ebus = pci_new_multifunction(PCI_DEVFN(1, 0), true, TYPE_EBUS);
 qdev_prop_set_uint64(DEVICE(ebus), "console-serial-base",
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index 2d33d178ad..a540149639 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -57,7 +57,6 @@
 #include 
 
 #include "hw/pci/pci.h"
-#include "hw/pci/pci_bus.h"
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
 #include "xen_pt.h"
@@ -951,7 +950,7 @@ void xen_igd_reserve_slot(PCIBus *pci_bus)
 }
 
 XEN_PT_LOG(0, "Reserving PCI slot 2 for IGD\n");
-pci_bus->slot_reserved_mask |= XEN_PCI_IGD_SLOT_MASK;
+pci_bus_set_slot_reserved_mask(pci_bus, XEN_PCI_IGD_SLOT_MASK);
 }
 
 static void xen_igd_clear_slot(DeviceState *qdev, Error **errp)
@@ -971,7 +970,7 @@ static void xen_igd_clear_slot(DeviceState *qdev, Error 
**errp)
 return;
 }
 
-if (!(pci_bus->slot_reserved_mask & XEN_PCI_IGD_SLOT_MASK)) {
+if (!(pci_bus_get_slot_reserved_mask(pci_bus) & XEN_PCI_IGD_SLOT_MASK)) {
 xpdc->pci_qdev_realize(qdev, errp);
 return;
 }
@@ -982,7 +981,7 @@ static void xen_igd_clear_slot(DeviceState *qdev, Error 
**errp)
 s->real_device.dev == XEN_PCI_IGD_DEV &&
 s->real_device.func == XEN_PCI_IGD_FN &&
 s->real_device.vendor_id == PCI_VENDOR_ID_INTEL) {
-pci_bus->slot_reserved_mask &= ~XEN_PCI_IGD_SLOT_MASK;
+pci_bus_clear_slot_reserved_mask(pci_bus, XEN_PCI_IGD_SLOT_MASK);
 XEN_PT_LOG(pci_dev, "Intel IGD found, using slot 2\n");
 }
 xpdc->pci_qdev_realize(qdev, errp);
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index d5a40cd058..935b4b91b4 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -287,6 +287,9 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
 void pci_bus_map_irqs(PCIBus *bus, pci_map_irq_fn map_irq);
 void pci_bus_irqs_cleanup(PCIBus *bus);
 int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
+uint32_t pci_bus_get_slot_reserved_mask(PCIBus *bus);
+void pci_bus_set_slot_reserved_mask(PCIBus *bus, uint32_t mask);
+void pci_bus_clear_slot_reserved_mask(PCIBus *bus, uint32_t mask);
 /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
 static inline int pci_swizzle(int slot, int pin)
 {
-- 
2.39.2

[PATCH v3 0/2] pci: slot_reserved_mask improvements

[Chuck Zmudzinski]

This patch series consists of two patches. The first provides accessor
functions in pci.h to avoid direct access of slot_reserved_mask
according to the comment at the top of include/hw/pci/pci_bus.h. No
functional change is intended with this patch.

The second patch replaces slot_reserved_mask with two new masks,
slot_reserved_auto_mask and slot_reserved_manual_mask so the current
behavior of reserving slot 2 for the Intel IGD for the xenfv machine
will be ignored if an administrator manually configures a device to use
the reserved slot.

The current behavior of always reserving slots in the sun4u machine is
preserved by this patch series; the patch series only changes how
slot_reserved_mask works in the xenfv machine. Although the patch
series can affect xenfv machines configured for igd-passthru if an
administrator assigns some of the pci slot addresses manually, it
does not affect the libxl default configuration for igd-passthru because
libxl uses automatic slot assignment by default.

Testing:
   - Tested xenfv/igd with both manual and auto slot allocation - behaves as 
expected
   - Verified that qemu-system-sparc64 still compiles with the patches to 
sun4u.c
   - xen4u machine not tested -- Mark, can you do this?

Link: 
https://lore.kernel.org/qemu-devel/20230106064838-mutt-send-email-...@kernel.org/

Chuck Zmudzinski (2):
  pci: avoid accessing slot_reserved_mask directly outside of pci.c
  pci: introduce slot_reserved_auto_mask and slot_reserved_manual_mask

Changelog

v3: Re-work second patch in response to comments/discussion of v2

v2: Add first patch and cover letter to make this a 2-patch series
Make changes to the second patch (see second patch for changelog)

 hw/pci/pci.c | 32 +++-
 hw/sparc64/sun4u.c   |  7 +++
 hw/xen/xen_pt.c  |  7 +++
 include/hw/pci/pci.h |  3 +++
 include/hw/pci/pci_bus.h |  3 ++-
 5 files changed, 38 insertions(+), 14 deletions(-)

-- 
2.39.2

Re: [PATCH for-8.1 v2 22/26] target/riscv: error out on priv failure for RVH

[liweiwei]

On 2023/3/15 00:49, Daniel Henrique Barboza wrote:

We have one last case where we're changing env->misa_ext* during
validation. riscv_cpu_disable_priv_spec_isa_exts(), at the end of
riscv_cpu_validate_set_extensions(), will disable cpu->cfg.ext_h and
cpu->cfg.ext_v if priv_ver check fails.

This check can be done in riscv_cpu_validate_misa_ext(). The difference
here is that we're not silently disable it: we'll error out. Silently
disabling a MISA extension after all the validation is completed can
can cause inconsistencies that we don't have to deal with. Verify ealier
and fail faster.

Note that we're ignoring RVV priv_ver validation since its minimal priv
is also the minimal value we support. RVH will error out if enabled
under priv_ver under 1_12_0.

As a bonus, we're guaranteeing that all env->misa_ext* changes will
happen up until riscv_set_G_virt_ext(). We don't have to worry about
keeping env->misa_ext in sync with cpu->cfg.

Signed-off-by: Daniel Henrique Barboza 
---
  target/riscv/cpu.c | 28 +++-
  1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f8f416d6dd..1f72e1b8ce 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1067,6 +1067,20 @@ static void riscv_cpu_validate_misa_ext(CPURISCVState 
*env,
  return;
  }
  
+/*

+ * Check for priv spec version. RVH is 1_12_0, RVV is 1_10_0.
+ * We don't support anything under 1_10_0 so skip checking
+ * priv for RVV.
+ *
+ * We're hardcoding it here to avoid looping into the
+ * 50+ entries of isa_edata_arr[] just to check the RVH
+ * entry.
+ */
+if (misa_ext & RVH && env->priv_ver < PRIV_VERSION_1_12_0) {
+error_setg(errp, "H extension requires priv spec 1.12.0");
+return;
+}
+
  if (misa_ext & RVV) {
  /*
   * V depends on Zve64d, which requires D. It also
@@ -1117,14 +1131,10 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, 
Error **errp)
  
  /*

   * Check consistency between chosen extensions while setting
- * cpu->cfg accordingly, setting env->misa_ext and
- * misa_ext_mask in the end.
+ * cpu->cfg accordingly.
   */
  static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
  {
-CPURISCVState *env = >env;
-uint32_t ext = 0;
-
  if (cpu->cfg.epmp && !cpu->cfg.pmp) {
  /*
   * Enhanced PMP should only be available
@@ -1241,10 +1251,6 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU 
*cpu, Error **errp)
   * validated and set everything we need.
   */
  riscv_cpu_disable_priv_spec_isa_exts(cpu);
-
-ext = riscv_get_misa_ext_with_cpucfg(>cfg);
-
-env->misa_ext_mask = env->misa_ext = ext;


This can be removed in the patch 17.

Regards,

Weiwei Li


  }
  
  #ifndef CONFIG_USER_ONLY

@@ -1355,6 +1361,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
  return;
  }
  
+/*

+ * This is the last point where env->misa_ext* can
+ * be changed.
+ */
  if (cpu->cfg.ext_g) {
  riscv_set_G_virt_ext(cpu);
  }

Re: [PATCH v5 0/2] target/riscv: refactor Zicond and reuse in XVentanaCondOps

[Alistair Francis]

On Wed, Mar 8, 2023 at 4:09 AM Philipp Tomsich  wrote:
>
>
> After the original Zicond support was stuck/fell through the cracks on
> the mailing list at v3 (and a different implementation was merged in
> the meanwhile), we now refactor Zicond and then reuse it in
> XVentanaCondOps.
>
>
> Philipp Tomsich (2):
>   target/riscv: refactor Zicond support
>   target/riscv: redirect XVentanaCondOps to use the Zicond functions

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  MAINTAINERS   |  2 +-
>  target/riscv/insn_trans/trans_rvzicond.c.inc  | 36 +++
>  .../insn_trans/trans_xventanacondops.c.inc| 18 ++
>  3 files changed, 25 insertions(+), 31 deletions(-)
>
> --
> 2.34.1
>
>

Re: [PATCH 0/4] target/riscv: Some CPURISCVState related cleanup and simplification

[Alistair Francis]

On Thu, Mar 9, 2023 at 5:15 PM Weiwei Li  wrote:
>
> The patchset tries to:
>
> - Use riscv_cpu_cfg(env) instead of env_archcpu().cfg.
> - Use env_archcpu() to get RISCVCPU pointer from env directly
> - Use CPURISCVState as argument directly in riscv_cpu_update_mip and 
> riscv_timer_write_timecmp to simplify type conversion
> - Remove RISCVCPU argument of riscv_csrrw_check, and get cfg infomation from 
> CPURISCVState directly
>
> The port is available here:
> https://github.com/plctlab/plct-qemu/tree/plct-cleanup-upstream
>
> Weiwei Li (4):
>   target/riscv: Avoid env_archcpu() when reading RISCVCPUConfig
>   target/riscv: Simplify getting RISCVCPU pointer from env
>   target/riscv: Simplify type conversion for CPURISCVState
>   target/riscv: Simplify arguments for riscv_csrrw_check

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  target/riscv/cpu.c |  6 +--
>  target/riscv/cpu.h |  3 +-
>  target/riscv/cpu_helper.c  | 17 
>  target/riscv/csr.c | 87 --
>  target/riscv/gdbstub.c |  4 +-
>  target/riscv/pmu.c | 14 +++---
>  target/riscv/time_helper.c | 15 +++
>  target/riscv/time_helper.h |  2 +-
>  8 files changed, 57 insertions(+), 91 deletions(-)
>
> --
> 2.25.1
>
>

Re: [PATCH for-8.1 v2 19/26] target/riscv/cpu:c add misa_ext V-> D & F dependency

[liweiwei]

On 2023/3/15 00:49, Daniel Henrique Barboza wrote:

We have a chained dependency in riscv_cpu_validate_set_extensions()
related to RVV. If RVV is set, we enable other extensions such as
Zve64d, Zve64f and Zve32f, and these depends on misa bits RVD and RVF.
Thus, we're making RVV depend on RVD and RVF.

Let's add this dependency in riscv_cpu_validate_misa_ext() to fail
earlier.

Signed-off-by: Daniel Henrique Barboza 
---
  target/riscv/cpu.c | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 83b1b874ee..fa1954a850 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1060,6 +1060,20 @@ static void riscv_cpu_validate_misa_ext(RISCVCPU *cpu, 
Error **errp)
  error_setg(errp, "D extension requires F extension");
  return;
  }
+
+if (cpu->cfg.ext_v) {
+/*
+ * V depends on Zve64d, which requires D. It also
+ * depends on Zve64f, which depends on Zve32f,
+ * which requires F.
+ *
+ * This means that V depends on both D and F.
+ */
+if (!(cpu->cfg.ext_d && cpu->cfg.ext_f)) {
+error_setg(errp, "V extension requires D and F extensions");
+return;
+}
+}
  }


It seems not necessary to add this check here, since "zve64d requires D" 
will be checked later.


By the way,  "D requires  F" is checked before, so  check on F is 
redundant here.


Regards,

Weiwei Li

  
  static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)

Re: [PATCH for-8.1 v2 16/26] target/riscv/cpu.c: split RVG code from validate_set_extensions()

[liweiwei]

On 2023/3/15 00:49, Daniel Henrique Barboza wrote:

We can set all RVG related extensions during realize time, before
validate_set_extensions() itself. It will also avoid re-enabling
RVG via write_misa() when the CSR start to using the same validation
code realize() does.

Note that we're setting both cfg->ext_N and env->misa_ext bits, instead
of just setting cfg->ext_N. The intention here is to start syncing all
misa_ext operations with its cpu->cfg flags, in preparation to allow for
the validate function to operate using a misa_ext. This doesn't make any
difference for the current code state, but will be a requirement for
write_misa() later on.

Signed-off-by: Daniel Henrique Barboza 
---
  target/riscv/cpu.c | 55 +-
  1 file changed, 40 insertions(+), 15 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 48ad7372b9..133807e39f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -281,6 +281,42 @@ static uint32_t 
riscv_get_misa_ext_with_cpucfg(RISCVCPUConfig *cfg)
  return ext;
  }
  
+static void riscv_set_G_virt_ext(RISCVCPU *cpu)

+{
+CPURISCVState *env = >env;
+RISCVCPUConfig *cfg = >cfg;
+
+if (!(cfg->ext_i && cfg->ext_m && cfg->ext_a &&
+  cfg->ext_f && cfg->ext_d &&
+  cfg->ext_icsr && cfg->ext_ifencei)) {
+
+warn_report("Setting G will also set IMAFD_Zicsr_Zifencei");
+cfg->ext_i = true;
+env->misa_ext |= RVI;
+
+cfg->ext_m = true;
+env->misa_ext |= RVM;
+
+cfg->ext_a = true;
+env->misa_ext |= RVA;
+
+cfg->ext_f = true;
+env->misa_ext |= RVF;
+
+cfg->ext_d = true;
+env->misa_ext |= RVD;
+
+cfg->ext_icsr = true;
+cfg->ext_ifencei = true;
+
+/*
+ * Update misa_ext_mask since this is called
+ * only during riscv_cpu_realize().
+ */
+env->misa_ext_mask = env->misa_ext;
+}


Another two question:

- whether we should set 'G' when all these extensions are supported?

- whether 'G'should be disabled if some of the extensions are disabled 
by write_misa?


Regards,

Weiwei Li


+}
+
  static void riscv_set_cpucfg_with_misa(RISCVCPUConfig *cfg,
 uint32_t misa_ext)
  {
@@ -1036,21 +1072,6 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU 
*cpu, Error **errp)
  return;
  }
  
-/* Do some ISA extension error checking */

-if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
-cpu->cfg.ext_a && cpu->cfg.ext_f &&
-cpu->cfg.ext_d &&
-cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) {
-warn_report("Setting G will also set IMAFD_Zicsr_Zifencei");
-cpu->cfg.ext_i = true;
-cpu->cfg.ext_m = true;
-cpu->cfg.ext_a = true;
-cpu->cfg.ext_f = true;
-cpu->cfg.ext_d = true;
-cpu->cfg.ext_icsr = true;
-cpu->cfg.ext_ifencei = true;
-}
-
  if (cpu->cfg.ext_i && cpu->cfg.ext_e) {
  error_setg(errp,
 "I and E extensions are incompatible");
@@ -1313,6 +1334,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
  return;
  }
  
+if (cpu->cfg.ext_g) {

+riscv_set_G_virt_ext(cpu);
+}
+
  riscv_cpu_validate_set_extensions(cpu, _err);
  if (local_err != NULL) {
  error_propagate(errp, local_err);

Re: [PATCH for-8.1 v2 15/26] target/riscv: do not allow RVG in write_misa()

[liweiwei]

On 2023/3/15 00:49, Daniel Henrique Barboza wrote:

We're getting ready to use riscv_cpu_validate_set_extensions() to unify
the handling of write_misa() with the rest of the code base. But first
we need to deal with RVG.

The 'G' virtual extension enables a set of extensions in the CPU. At
this moment, this is done at the start of our validation step in
riscv_cpu_validate_set_extensions(). This means that enabling G will
enable other extensions in the CPU before resuming the validation.

This also means that, in case a write_misa() validation fails, we're
going to set cpu->cfg attributes that are unrelated to misa_ext bits
(icsr and ifencei). These would be 2 extra states that we would need to
store to fallback from a validation failure.

Since write_misa() is still on experimental state let's make our lives
easier for now and disable RVG updates.

Signed-off-by: Daniel Henrique Barboza 
---
  target/riscv/csr.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d522efc0b6..918d442ebd 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1348,6 +1348,11 @@ static RISCVException write_misa(CPURISCVState *env, int 
csrno,
  return RISCV_EXCP_NONE;
  }
  
+/* Changing 'G' state is unsupported */

+if (val & RVG) {
+return RISCV_EXCP_NONE;
+}
+


'val & G' is not equal  "Changing 'G'" .

Regards,

Weiwei Li


  /* 'I' or 'E' must be present */
  if (!(val & (RVI | RVE))) {
  /* It is not, drop write to misa */

[ANNOUNCE] QEMU 8.0.0-rc0 is now available

[Michael Roth]

Hello,

On behalf of the QEMU Team, I'd like to announce the availability of the
first release candidate for the QEMU 8.0 release. This release is meant
for testing purposes and should not be used in a production environment.

  http://download.qemu.org/qemu-8.0.0-rc0.tar.xz
  http://download.qemu.org/qemu-8.0.0-rc0.tar.xz.sig

You can help improve the quality of the QEMU 8.0 release by testing this
release and reporting bugs using our GitLab issue tracker:

  https://gitlab.com/qemu-project/qemu/-/milestones/8#tab-issues

The release plan, as well a documented known issues for release
candidates, are available at:

  http://wiki.qemu.org/Planning/8.0

Please add entries to the ChangeLog for the 8.0 release below:

  http://wiki.qemu.org/ChangeLog/8.0

Thank you to everyone involved!

[PATCH] tests/tcg/xtensa: allow testing big-endian cores

[Max Filippov]

Don't disable all big-endian tests, instead check whether $(CORE) is
supported by the configured $(QEMU) and enable tests if it is.

Signed-off-by: Max Filippov 
Reviewed-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS| 1 +
 tests/tcg/xtensa/Makefile.softmmu-target   | 4 ++--
 tests/tcg/xtensaeb/Makefile.softmmu-target | 5 +
 3 files changed, 8 insertions(+), 2 deletions(-)
 create mode 100644 tests/tcg/xtensaeb/Makefile.softmmu-target

diff --git a/MAINTAINERS b/MAINTAINERS
index d51ddee0b94b..94faa804610e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -371,6 +371,7 @@ S: Maintained
 F: target/xtensa/
 F: hw/xtensa/
 F: tests/tcg/xtensa/
+F: tests/tcg/xtensaeb/
 F: disas/xtensa.c
 F: include/hw/xtensa/xtensa-isa.h
 F: configs/devices/xtensa*/default.mak
diff --git a/tests/tcg/xtensa/Makefile.softmmu-target 
b/tests/tcg/xtensa/Makefile.softmmu-target
index 948c0e6506bd..ba6cd9fde3fe 100644
--- a/tests/tcg/xtensa/Makefile.softmmu-target
+++ b/tests/tcg/xtensa/Makefile.softmmu-target
@@ -2,7 +2,8 @@
 # Xtensa softmmu tests
 #
 
-ifneq ($(TARGET_BIG_ENDIAN),y)
+CORE=dc232b
+ifneq ($(shell $(QEMU) -cpu help | grep -w $(CORE)),)
 
 XTENSA_SRC = $(SRC_PATH)/tests/tcg/xtensa
 XTENSA_ALL = $(filter-out $(XTENSA_SRC)/linker.ld.S,$(wildcard 
$(XTENSA_SRC)/*.S))
@@ -15,7 +16,6 @@ XTENSA_USABLE_TESTS = $(filter-out $(XTENSA_BROKEN_TESTS), 
$(XTENSA_TESTS))
 TESTS += $(XTENSA_USABLE_TESTS)
 VPATH += $(XTENSA_SRC)
 
-CORE=dc232b
 QEMU_OPTS+=-M sim -cpu $(CORE) -nographic -semihosting -icount 6 $(EXTFLAGS) 
-kernel
 
 INCLUDE_DIRS = $(SRC_PATH)/target/xtensa/core-$(CORE)
diff --git a/tests/tcg/xtensaeb/Makefile.softmmu-target 
b/tests/tcg/xtensaeb/Makefile.softmmu-target
new file mode 100644
index ..4204a96d53c0
--- /dev/null
+++ b/tests/tcg/xtensaeb/Makefile.softmmu-target
@@ -0,0 +1,5 @@
+#
+# Xtensa softmmu tests
+#
+
+include $(SRC_PATH)/tests/tcg/xtensa/Makefile.softmmu-target
-- 
2.30.2

[PATCH v2 0/3] target/s390x: Implement Early Exception Recognition

[Ilya Leoshkevich]

v1: https://lists.gnu.org/archive/html/qemu-devel/2023-03/msg04372.html
v1 -> v2: Fix SSM and STOSM (Nina).
  Fix LPSW (Nina).
  Check bits 12 and 24 (Nina).
  Improve the commit message (Nina).
  Improve naming (David).

Hi,

Currently loading bad PSW flags does not lead to an exception, which is
not correct. This series fixes this by implementing what PoP calls
"Early Exception Recognition". Since it applies to both loading PSW with
LPSW/LPSWE and to interrupt handling, s390_cpu_set_psw() looks like the
right place for it to be in. SSM and STOSM need special handling, which
is implemented inline.

Patch 1 fixes the LPSW instruction (which is related), patch 2 
implements Early Exception Recognition, patch 3 adds a number of tests.

Best regards,
Ilya

Ilya Leoshkevich (3):
  target/s390x: Fix LPSW
  target/s390x: Implement Early Exception Recognition
  tests/tcg/s390x: Add PSW modification tests

 target/s390x/cpu.c  | 26 +++
 target/s390x/cpu.h  |  1 +
 target/s390x/tcg/excp_helper.c  |  3 +-
 target/s390x/tcg/translate.c| 38 --
 tests/tcg/s390x/Makefile.softmmu-target |  5 +++
 tests/tcg/s390x/exrl-ssm-early.S| 43 +
 tests/tcg/s390x/lpsw.S  | 36 +
 tests/tcg/s390x/lpswe-early.S   | 38 ++
 tests/tcg/s390x/ssm-early.S | 41 +++
 tests/tcg/s390x/stosm-early.S   | 41 +++
 10 files changed, 261 insertions(+), 11 deletions(-)
 create mode 100644 tests/tcg/s390x/exrl-ssm-early.S
 create mode 100644 tests/tcg/s390x/lpsw.S
 create mode 100644 tests/tcg/s390x/lpswe-early.S
 create mode 100644 tests/tcg/s390x/ssm-early.S
 create mode 100644 tests/tcg/s390x/stosm-early.S

-- 
2.39.2

[PATCH v2 2/3] target/s390x: Implement Early Exception Recognition

[Ilya Leoshkevich]

Generate a specification exception if a reserved bit is set in the PSW
mask or if the PSW address is out of bounds dictated by the addressing
mode.

Reported-by: Nina Schoetterl-Glausch 
Signed-off-by: Ilya Leoshkevich 
---
 target/s390x/cpu.c | 26 ++
 target/s390x/cpu.h |  1 +
 target/s390x/tcg/excp_helper.c |  3 ++-
 target/s390x/tcg/translate.c   | 16 
 4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index b10a8541ff8..40fdeaa9056 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -41,6 +41,26 @@
 #define CR0_RESET   0xE0UL
 #define CR14_RESET  0xC200UL;
 
+#ifndef CONFIG_USER_ONLY
+static bool is_early_exception_psw(uint64_t mask, uint64_t addr)
+{
+if (mask & PSW_MASK_RESERVED) {
+return true;
+}
+
+switch (mask & (PSW_MASK_32 | PSW_MASK_64)) {
+case 0:
+return addr & ~0xffULL;
+case PSW_MASK_32:
+return addr & ~0x7fffULL;
+case PSW_MASK_32 | PSW_MASK_64:
+return false;
+default: /* PSW_MASK_64 */
+return true;
+}
+}
+#endif
+
 void s390_cpu_set_psw(CPUS390XState *env, uint64_t mask, uint64_t addr)
 {
 #ifndef CONFIG_USER_ONLY
@@ -57,6 +77,12 @@ void s390_cpu_set_psw(CPUS390XState *env, uint64_t mask, 
uint64_t addr)
 env->cc_op = (mask >> 44) & 3;
 
 #ifndef CONFIG_USER_ONLY
+if (is_early_exception_psw(mask, addr)) {
+env->int_pgm_ilen = 0;
+trigger_pgm_exception(env, PGM_SPECIFICATION);
+return;
+}
+
 if ((old_mask ^ mask) & PSW_MASK_PER) {
 s390_cpu_recompute_watchpoints(env_cpu(env));
 }
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 7d6d01325b2..16f63547513 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -292,6 +292,7 @@ extern const VMStateDescription vmstate_s390_cpu;
 #define PSW_MASK_32 0x8000ULL
 #define PSW_MASK_SHORT_ADDR 0x7fffULL
 #define PSW_MASK_SHORT_CTRL 0x8000ULL
+#define PSW_MASK_RESERVED   0xb80800fe7fffULL
 
 #undef PSW_ASC_PRIMARY
 #undef PSW_ASC_ACCREG
diff --git a/target/s390x/tcg/excp_helper.c b/target/s390x/tcg/excp_helper.c
index bc767f04438..a7829b1e494 100644
--- a/target/s390x/tcg/excp_helper.c
+++ b/target/s390x/tcg/excp_helper.c
@@ -212,7 +212,8 @@ static void do_program_interrupt(CPUS390XState *env)
 LowCore *lowcore;
 int ilen = env->int_pgm_ilen;
 
-assert(ilen == 2 || ilen == 4 || ilen == 6);
+assert((env->int_pgm_code == PGM_SPECIFICATION && ilen == 0) ||
+   ilen == 2 || ilen == 4 || ilen == 6);
 
 switch (env->int_pgm_code) {
 case PGM_PER:
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 2e1e7e046a6..7832cf02a68 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -4068,9 +4068,23 @@ static DisasJumpType op_sske(DisasContext *s, DisasOps 
*o)
 return DISAS_NEXT;
 }
 
+static void gen_check_psw_mask(DisasContext *s)
+{
+TCGv_i64 reserved = tcg_temp_new_i64();
+TCGLabel *ok = gen_new_label();
+
+tcg_gen_andi_i64(reserved, psw_mask, PSW_MASK_RESERVED);
+tcg_gen_brcondi_i64(TCG_COND_EQ, reserved, 0, ok);
+gen_program_exception(s, PGM_SPECIFICATION);
+gen_set_label(ok);
+}
+
 static DisasJumpType op_ssm(DisasContext *s, DisasOps *o)
 {
 tcg_gen_deposit_i64(psw_mask, psw_mask, o->in2, 56, 8);
+
+gen_check_psw_mask(s);
+
 /* Exit to main loop to reevaluate s390_cpu_exec_interrupt.  */
 s->exit_to_mainloop = true;
 return DISAS_TOO_MANY;
@@ -4331,6 +4345,8 @@ static DisasJumpType op_stnosm(DisasContext *s, DisasOps 
*o)
 tcg_gen_ori_i64(psw_mask, psw_mask, i2 << 56);
 }
 
+gen_check_psw_mask(s);
+
 /* Exit to main loop to reevaluate s390_cpu_exec_interrupt.  */
 s->exit_to_mainloop = true;
 return DISAS_TOO_MANY;
-- 
2.39.2

[PATCH v2 3/3] tests/tcg/s390x: Add PSW modification tests

[Ilya Leoshkevich]

Add several small tests that check the PSW modification instructions:

* lpsw.S checks whether LPSW works correctly in the "happy" case.

* lpswe-early.S checks whether early exceptions are recognized and
  whether the correct ILC and old PSW are stored when they happen.

* ssm-early.S, stosm-early.S and exrl-ssm-early.S check the special
  handling of SSM and STOSM with respect to early exceptions.

Signed-off-by: Ilya Leoshkevich 
---
 tests/tcg/s390x/Makefile.softmmu-target |  5 +++
 tests/tcg/s390x/exrl-ssm-early.S| 43 +
 tests/tcg/s390x/lpsw.S  | 36 +
 tests/tcg/s390x/lpswe-early.S   | 38 ++
 tests/tcg/s390x/ssm-early.S | 41 +++
 tests/tcg/s390x/stosm-early.S   | 41 +++
 6 files changed, 204 insertions(+)
 create mode 100644 tests/tcg/s390x/exrl-ssm-early.S
 create mode 100644 tests/tcg/s390x/lpsw.S
 create mode 100644 tests/tcg/s390x/lpswe-early.S
 create mode 100644 tests/tcg/s390x/ssm-early.S
 create mode 100644 tests/tcg/s390x/stosm-early.S

diff --git a/tests/tcg/s390x/Makefile.softmmu-target 
b/tests/tcg/s390x/Makefile.softmmu-target
index 725b6c598db..607f6ba21a4 100644
--- a/tests/tcg/s390x/Makefile.softmmu-target
+++ b/tests/tcg/s390x/Makefile.softmmu-target
@@ -9,3 +9,8 @@ QEMU_OPTS=-action panic=exit-failure -kernel
 TESTS += unaligned-lowcore
 TESTS += bal
 TESTS += sam
+TESTS += lpsw
+TESTS += lpswe-early
+TESTS += ssm-early
+TESTS += stosm-early
+TESTS += exrl-ssm-early
diff --git a/tests/tcg/s390x/exrl-ssm-early.S b/tests/tcg/s390x/exrl-ssm-early.S
new file mode 100644
index 000..68fbd87b3a5
--- /dev/null
+++ b/tests/tcg/s390x/exrl-ssm-early.S
@@ -0,0 +1,43 @@
+/*
+ * Test early exception recognition using EXRL + SSM.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+.org 0x8d
+ilc:
+.org 0x8e
+program_interruption_code:
+.org 0x150
+program_old_psw:
+.org 0x1D0 /* program new PSW */
+.quad 0,pgm
+.org 0x200 /* lowcore padding */
+
+.globl _start
+_start:
+exrl %r0,ssm
+expected_pswa:
+j failure
+ssm:
+ssm ssm_op
+
+pgm:
+chhsi program_interruption_code,0x6  /* specification exception? */
+jne failure
+cli ilc,6/* ilc for EXRL? */
+jne failure
+clc program_old_psw(16),expected_old_psw /* correct old PSW? */
+jne failure
+lpswe success_psw
+failure:
+lpswe failure_psw
+
+ssm_op:
+.byte 0x08   /* bit 4 set */
+.align 8
+expected_old_psw:
+.quad 0x08018000,expected_pswa   /* bit 2 set */
+success_psw:
+.quad 0x2,0xfff/* see is_special_wait_psw() */
+failure_psw:
+.quad 0x2,0/* disabled wait */
diff --git a/tests/tcg/s390x/lpsw.S b/tests/tcg/s390x/lpsw.S
new file mode 100644
index 000..b37dec59b73
--- /dev/null
+++ b/tests/tcg/s390x/lpsw.S
@@ -0,0 +1,36 @@
+/*
+ * Test the LPSW instruction.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+.org 0x140
+svc_old_psw:
+.org 0x1c0 /* supervisor call new PSW */
+.quad 0x8000,svc   /* 31-bit mode */
+.org 0x200 /* lowcore padding */
+
+.globl _start
+_start:
+lpsw short_psw
+lpsw_target:
+svc 0
+expected_pswa:
+j failure
+
+svc:
+clc svc_old_psw(16),expected_psw   /* correct full PSW? */
+jne failure
+lpswe success_psw
+failure:
+lpswe failure_psw
+
+.align 8
+short_psw:
+.long 0x90001,0x8000+lpsw_target /* problem state,
+64-bit mode */
+expected_psw:
+.quad 0x100018000,expected_pswa  /* corresponds to short_psw */
+success_psw:
+.quad 0x2,0xfff/* see is_special_wait_psw() */
+failure_psw:
+.quad 0x2,0/* disabled wait */
diff --git a/tests/tcg/s390x/lpswe-early.S b/tests/tcg/s390x/lpswe-early.S
new file mode 100644
index 000..90a7f213dfb
--- /dev/null
+++ b/tests/tcg/s390x/lpswe-early.S
@@ -0,0 +1,38 @@
+/*
+ * Test early exception recognition using LPSWE.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+.org 0x8d
+ilc:
+.org 0x8e
+program_interruption_code:
+.org 0x150
+program_old_psw:
+.org 0x1D0 /* program new PSW */
+.quad 0,pgm
+.org 0x200 /* lowcore padding */
+
+.globl _start
+_start:
+lpswe bad_psw
+j failure
+
+pgm:
+chhsi program_interruption_code,0x6  /* specification exception? */
+jne failure
+cli ilc,0/* ilc zero? */
+jne failure
+clc program_old_psw(16),bad_psw  /* correct old PSW? */
+jne failure
+lpswe success_psw
+failure:
+lpswe failure_psw
+
+ 

[PATCH v2 1/3] target/s390x: Fix LPSW

[Ilya Leoshkevich]

Currently LPSW does not invert the mask bit 12 and incorrectly copies
the BA bit into the address.

Fix by generating code similar to what s390_cpu_load_normal() does.

Reported-by: Nina Schoetterl-Glausch 
Co-developed-by: Nina Schoetterl-Glausch 
Signed-off-by: Ilya Leoshkevich 
---
 target/s390x/tcg/translate.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 14c3896d529..2e1e7e046a6 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -2910,19 +2910,21 @@ static DisasJumpType op_lpp(DisasContext *s, DisasOps 
*o)
 
 static DisasJumpType op_lpsw(DisasContext *s, DisasOps *o)
 {
-TCGv_i64 t1, t2;
+TCGv_i64 mask, addr;
 
 per_breaking_event(s);
 
-t1 = tcg_temp_new_i64();
-t2 = tcg_temp_new_i64();
-tcg_gen_qemu_ld_i64(t1, o->in2, get_mem_index(s),
-MO_TEUL | MO_ALIGN_8);
-tcg_gen_addi_i64(o->in2, o->in2, 4);
-tcg_gen_qemu_ld32u(t2, o->in2, get_mem_index(s));
-/* Convert the 32-bit PSW_MASK into the 64-bit PSW_MASK.  */
-tcg_gen_shli_i64(t1, t1, 32);
-gen_helper_load_psw(cpu_env, t1, t2);
+/*
+ * Convert the short PSW into the normal PSW, similar to what
+ * s390_cpu_load_normal() does.
+ */
+mask = tcg_temp_new_i64();
+addr = tcg_temp_new_i64();
+tcg_gen_qemu_ld_i64(mask, o->in2, get_mem_index(s), MO_TEUQ | MO_ALIGN_8);
+tcg_gen_andi_i64(addr, mask, PSW_MASK_SHORT_ADDR);
+tcg_gen_andi_i64(mask, mask, PSW_MASK_SHORT_CTRL);
+tcg_gen_xori_i64(mask, mask, PSW_MASK_SHORTPSW);
+gen_helper_load_psw(cpu_env, mask, addr);
 return DISAS_NORETURN;
 }
 
-- 
2.39.2

Re: [PATCH] migration/rdma: Fix return-path case

[lizhij...@fujitsu.com]

On 15/03/2023 01:15, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> The RDMA code has return-path handling code, but it's only enabled
> if postcopy is enabled; if the 'return-path' migration capability
> is enabled, the return path is NOT setup but the core migration
> code still tries to use it and breaks.
> 
> Enable the RDMA return path if either postcopy or the return-path
> capability is enabled.
> 
> bz: https://bugzilla.redhat.com/show_bug.cgi?id=2063615
> 
> Signed-off-by: Dr. David Alan Gilbert 

LGTM.

Reviewed-by: Li Zhijian 



> ---
>   migration/rdma.c | 8 +---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 288eadc2d2..9d70e9885b 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3373,7 +3373,8 @@ static int qemu_rdma_accept(RDMAContext *rdma)
>* initialize the RDMAContext for return path for postcopy after first
>* connection request reached.
>*/
> -if (migrate_postcopy() && !rdma->is_return_path) {
> +if ((migrate_postcopy() || migrate_use_return_path())
> +&& !rdma->is_return_path) {
>   rdma_return_path = qemu_rdma_data_init(rdma->host_port, NULL);
>   if (rdma_return_path == NULL) {
>   rdma_ack_cm_event(cm_event);
> @@ -3455,7 +3456,8 @@ static int qemu_rdma_accept(RDMAContext *rdma)
>   }
>   
>   /* Accept the second connection request for return path */
> -if (migrate_postcopy() && !rdma->is_return_path) {
> +if ((migrate_postcopy() || migrate_use_return_path())
> +&& !rdma->is_return_path) {
>   qemu_set_fd_handler(rdma->channel->fd, 
> rdma_accept_incoming_migration,
>   NULL,
>   (void *)(intptr_t)rdma->return_path);
> @@ -4192,7 +4194,7 @@ void rdma_start_outgoing_migration(void *opaque,
>   }
>   
>   /* RDMA postcopy need a separate queue pair for return path */
> -if (migrate_postcopy()) {
> +if (migrate_postcopy() || migrate_use_return_path()) {
>   rdma_return_path = qemu_rdma_data_init(host_port, errp);
>   
>   if (rdma_return_path == NULL) {

Re: [PATCH] tests/tcg/xtensa: add linker.ld to CLEANFILES

[Wilfred Mallawa]

On Tue, 2023-03-14 at 17:08 -0700, Max Filippov wrote:
> On Tue, Mar 14, 2023 at 4:41 PM Wilfred Mallawa
>  wrote:
> > 
> > On Tue, 2023-03-14 at 15:08 -0700, Max Filippov wrote:
> > > Linker script for xtensa tests must be preprocessed for a
> > > specific
> > > target, remove it as a part of make clean.
> > > 
> > > Signed-off-by: Max Filippov 
> > > ---
> > >  tests/tcg/xtensa/Makefile.softmmu-target | 1 +
> > >  1 file changed, 1 insertion(+)
> 
> > Wilfred Mallawa 
> 
> The tag is missing, I assume you meant Reviewed-by.

Ah crap, sorry! yes I did.

Wilfred
> 

[PATCH v2 0/2] Fix EXECUTE of relative long instructions

[Ilya Leoshkevich]

v1: https://lists.gnu.org/archive/html/qemu-devel/2023-03/msg04316.html
v1 -> v2: Address the middle of an array in the test (Richard).
  Rebase - not 100% trivial, so not carrying Reviewed-bys.

Hi,

This series fixes EXECUTE of instructions like LARL, LGLR, etc.
Currently the address calculation uses EXECUTE's address as a base,
while it should be using that of the target instruction.
Patch 1 fixes the issue, patch 2 adds a test.

Best regards,
Ilya

Ilya Leoshkevich (2):
  target/s390x: Fix EXECUTE of relative long instructions
  tests/tcg/s390x: Add ex-relative-long.c

 target/s390x/cpu.h |   1 +
 target/s390x/tcg/mem_helper.c  |   1 +
 target/s390x/tcg/translate.c   |  13 ++-
 tests/tcg/s390x/Makefile.target|   1 +
 tests/tcg/s390x/ex-relative-long.c | 159 +
 5 files changed, 174 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/s390x/ex-relative-long.c

-- 
2.39.2

[PATCH v2 2/2] tests/tcg/s390x: Add ex-relative-long.c

[Ilya Leoshkevich]

Test EXECUTE and EXECUTE RELATIVE LONG with relative long instructions
as targets.

Signed-off-by: Ilya Leoshkevich 
---
 tests/tcg/s390x/Makefile.target|   1 +
 tests/tcg/s390x/ex-relative-long.c | 159 +
 2 files changed, 160 insertions(+)
 create mode 100644 tests/tcg/s390x/ex-relative-long.c

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index cf93b966862..90bc48227db 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -29,6 +29,7 @@ TESTS+=clst
 TESTS+=long-double
 TESTS+=cdsg
 TESTS+=chrl
+TESTS+=ex-relative-long
 
 cdsg: CFLAGS+=-pthread
 cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/ex-relative-long.c 
b/tests/tcg/s390x/ex-relative-long.c
new file mode 100644
index 000..4caa8c1b962
--- /dev/null
+++ b/tests/tcg/s390x/ex-relative-long.c
@@ -0,0 +1,159 @@
+/* Check EXECUTE with relative long instructions as targets. */
+#include 
+#include 
+
+struct test {
+const char *name;
+long (*func)(long reg, long *cc);
+long exp_reg;
+long exp_mem;
+long exp_cc;
+};
+
+/*
+ * Each test sets the MEM_IDXth element of the mem array to MEM and uses a
+ * single relative long instruction on it. The other elements remain zero.
+ * This is in order to prevent stumbling upon MEM in random memory in case
+ * there is an off-by-a-small-value bug.
+ *
+ * Note that while gcc supports the ZL constraint for relative long operands,
+ * clang doesn't, so the assembly code accesses mem[MEM_IDX] using MEM_ASM.
+ */
+long mem[0x1000];
+#define MEM_IDX 0x800
+#define MEM_ASM "mem+0x800*8"
+
+/* Initial %r2 value. */
+#define REG 0x1234567887654321
+
+/* Initial mem[MEM_IDX] value. */
+#define MEM 0xfedcba9889abcdef
+
+/* Initial cc value. */
+#define CC 0
+
+/* Relative long instructions and their expected effects. */
+#define FOR_EACH_INSN(F)   
\
+F(cgfrl,  REG, MEM,2)  
\
+F(cghrl,  REG, MEM,2)  
\
+F(cgrl,   REG, MEM,2)  
\
+F(chrl,   REG, MEM,1)  
\
+F(clgfrl, REG, MEM,2)  
\
+F(clghrl, REG, MEM,2)  
\
+F(clgrl,  REG, MEM,1)  
\
+F(clhrl,  REG, MEM,2)  
\
+F(clrl,   REG, MEM,1)  
\
+F(crl,REG, MEM,1)  
\
+F(larl,   (long)[MEM_IDX], MEM,CC) 
\
+F(lgfrl,  0xfedcba98,  MEM,CC) 
\
+F(lghrl,  0xfedc,  MEM,CC) 
\
+F(lgrl,   MEM, MEM,CC) 
\
+F(lhrl,   0x12345678fedc,  MEM,CC) 
\
+F(llghrl, 0xfedc,  MEM,CC) 
\
+F(llhrl,  0x12345678fedc,  MEM,CC) 
\
+F(lrl,0x12345678fedcba98,  MEM,CC) 
\
+F(stgrl,  REG, REG,CC) 
\
+F(sthrl,  REG, 0x4321ba9889abcdef, CC) 
\
+F(strl,   REG, 0x8765432189abcdef, CC)
+
+/* Test functions. */
+#define DEFINE_EX_TEST(insn, exp_reg, exp_mem, exp_cc) 
\
+static long test_ex_ ## insn(long reg, long *cc)   
\
+{  
\
+register long reg_val asm("r2");   
\
+long cc_val, mask, target; 
\
+   
\
+reg_val = reg; 
\
+asm("xgr %[cc_val],%[cc_val]\n"  /* initial cc */  
\
+"lghi %[mask],0x20\n"/* make target use %r2 */ 
\
+"larl %[target],0f\n"  
\
+"ex %[mask],0(%[target])\n"
\
+"jg 1f\n"  
\
+"0: " #insn " %%r0," MEM_ASM "\n"  
\
+"1: ipm %[cc_val]\n"   
\
+: [cc_val] "=" (cc_val)  
\
+, [mask] "=" (mask) 

[PATCH v2 1/2] target/s390x: Fix EXECUTE of relative long instructions

[Ilya Leoshkevich]

The code uses the wrong base for relative addressing: it should use the
target instruction address and not the EXECUTE's address.

Fix by storing the target instruction address in the new CPUS390XState
member and loading it from the code generated by gen_ri2().

Reported-by: Nina Schoetterl-Glausch 
Signed-off-by: Ilya Leoshkevich 
---
 target/s390x/cpu.h|  1 +
 target/s390x/tcg/mem_helper.c |  1 +
 target/s390x/tcg/translate.c  | 13 -
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 7d6d01325b2..8aaf8dd5a3b 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -87,6 +87,7 @@ struct CPUArchState {
 uint64_t cc_vr;
 
 uint64_t ex_value;
+uint64_t ex_target;
 
 uint64_t __excp_addr;
 uint64_t psa;
diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index 6835c26dda4..00afae2b640 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -2530,6 +2530,7 @@ void HELPER(ex)(CPUS390XState *env, uint32_t ilen, 
uint64_t r1, uint64_t addr)
that ex_value is non-zero, which flags that we are in a state
that requires such execution.  */
 env->ex_value = insn | ilen;
+env->ex_target = addr;
 }
 
 uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 14c3896d529..e938d8538f8 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -5747,7 +5747,18 @@ static void in2_a2(DisasContext *s, DisasOps *o)
 
 static TCGv gen_ri2(DisasContext *s)
 {
-return tcg_constant_i64(s->base.pc_next + (int64_t)get_field(s, i2) * 2);
+int64_t delta = (int64_t)get_field(s, i2) * 2;
+TCGv ri2;
+
+if (unlikely(s->ex_value)) {
+ri2 = tcg_temp_new_i64();
+tcg_gen_ld_i64(ri2, cpu_env, offsetof(CPUS390XState, ex_target));
+tcg_gen_addi_i64(ri2, ri2, delta);
+} else {
+ri2 = tcg_constant_i64(s->base.pc_next + delta);
+}
+
+return ri2;
 }
 
 static void in2_ri2(DisasContext *s, DisasOps *o)
-- 
2.39.2

Re: [PATCH] tests/tcg/xtensa: add linker.ld to CLEANFILES

[Max Filippov]

On Tue, Mar 14, 2023 at 4:41 PM Wilfred Mallawa  wrote:
>
> On Tue, 2023-03-14 at 15:08 -0700, Max Filippov wrote:
> > Linker script for xtensa tests must be preprocessed for a specific
> > target, remove it as a part of make clean.
> >
> > Signed-off-by: Max Filippov 
> > ---
> >  tests/tcg/xtensa/Makefile.softmmu-target | 1 +
> >  1 file changed, 1 insertion(+)

> Wilfred Mallawa 

The tag is missing, I assume you meant Reviewed-by.

-- 
Thanks.
-- Max

Re: [PATCH for-8.0] hw/char/cadence_uart: Fix guards on invalid BRGR/BDIV settings

[Alistair Francis]

On Wed, Mar 15, 2023 at 3:09 AM Peter Maydell  wrote:
>
> The cadence UART attempts to avoid allowing the guset to set invalid
> baud rate register values in the uart_write() function.  However it
> does the "mask to the size of the register field" and "check for
> invalid values" in the wrong order, which means that a malicious
> guest can get a bogus value into the register by setting also some
> high bits in the value, and cause QEMU to crash by division-by-zero.
>
> Do the mask before the bounds check instead of afterwards.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1493
> Signed-off-by: Peter Maydell 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/char/cadence_uart.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index c069a30842e..807e3985419 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -450,13 +450,15 @@ static MemTxResult uart_write(void *opaque, hwaddr 
> offset,
>  }
>  break;
>  case R_BRGR: /* Baud rate generator */
> +value &= 0x;
>  if (value >= 0x01) {
> -s->r[offset] = value & 0x;
> +s->r[offset] = value;
>  }
>  break;
>  case R_BDIV:/* Baud rate divider */
> +value &= 0xff;
>  if (value >= 0x04) {
> -s->r[offset] = value & 0xFF;
> +s->r[offset] = value;
>  }
>  break;
>  default:
> --
> 2.34.1
>
>

Re: [PATCH] tests/tcg/xtensa: add linker.ld to CLEANFILES

[Wilfred Mallawa]

On Tue, 2023-03-14 at 15:08 -0700, Max Filippov wrote:
> Linker script for xtensa tests must be preprocessed for a specific
> target, remove it as a part of make clean.
> 
> Signed-off-by: Max Filippov 
> ---
>  tests/tcg/xtensa/Makefile.softmmu-target | 1 +
>  1 file changed, 1 insertion(+)
Wilfred Mallawa 
> 
> diff --git a/tests/tcg/xtensa/Makefile.softmmu-target
> b/tests/tcg/xtensa/Makefile.softmmu-target
> index 973e55298ee4..948c0e6506bd 100644
> --- a/tests/tcg/xtensa/Makefile.softmmu-target
> +++ b/tests/tcg/xtensa/Makefile.softmmu-target
> @@ -26,6 +26,7 @@ ASFLAGS = -Wa,--no-absolute-literals
>  LDFLAGS = -Tlinker.ld -nostartfiles -nostdlib
>  
>  CRT    = crt.o vectors.o
> +CLEANFILES += linker.ld
>  
>  linker.ld: linker.ld.S
> $(CC) $(XTENSA_INC) -E -P $< -o $@

Re: [PATCH for-8.0] hw/char/cadence_uart: Fix guards on invalid BRGR/BDIV settings

[Wilfred Mallawa]

On Tue, 2023-03-14 at 17:08 +, Peter Maydell wrote:
> The cadence UART attempts to avoid allowing the guset to set invalid
> baud rate register values in the uart_write() function.  However it
> does the "mask to the size of the register field" and "check for
> invalid values" in the wrong order, which means that a malicious
> guest can get a bogus value into the register by setting also some
> high bits in the value, and cause QEMU to crash by division-by-zero.
> 
> Do the mask before the bounds check instead of afterwards.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1493
> Signed-off-by: Peter Maydell 
> ---
>  hw/char/cadence_uart.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
Reviewed-by: Wilfred Mallawa 
> 
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index c069a30842e..807e3985419 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -450,13 +450,15 @@ static MemTxResult uart_write(void *opaque,
> hwaddr offset,
>  }
>  break;
>  case R_BRGR: /* Baud rate generator */
> +    value &= 0x;
>  if (value >= 0x01) {
> -    s->r[offset] = value & 0x;
> +    s->r[offset] = value;
>  }
>  break;
>  case R_BDIV:    /* Baud rate divider */
> +    value &= 0xff;
>  if (value >= 0x04) {
> -    s->r[offset] = value & 0xFF;
> +    s->r[offset] = value;
>  }
>  break;
>  default:

[PATCH 2/2] tests/tcg/s390x: Add rxsbg.c

[Ilya Leoshkevich]

Add a small test for RXSBG with T=1 to prevent regressions.

Signed-off-by: Ilya Leoshkevich 
---
 tests/tcg/s390x/Makefile.target |  1 +
 tests/tcg/s390x/rxsbg.c | 25 +
 2 files changed, 26 insertions(+)
 create mode 100644 tests/tcg/s390x/rxsbg.c

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index cf93b966862..b4d0d704534 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -29,6 +29,7 @@ TESTS+=clst
 TESTS+=long-double
 TESTS+=cdsg
 TESTS+=chrl
+TESTS+=rxsbg
 
 cdsg: CFLAGS+=-pthread
 cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/rxsbg.c b/tests/tcg/s390x/rxsbg.c
new file mode 100644
index 000..b7f35411899
--- /dev/null
+++ b/tests/tcg/s390x/rxsbg.c
@@ -0,0 +1,25 @@
+/*
+ * Smoke test RXSBG instruction with T=1.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include 
+#include 
+
+int main(void)
+{
+unsigned long r1, r2, cc;
+
+r1 = 0xc8dc86a225a77bb4;
+r2 = 0xd6aff24fa3e7320;
+cc = 0;
+asm("rxsbg %[r1],%[r2],177,43,228\n"
+"ipm %[cc]"
+: [cc] "+r" (cc)
+: [r1] "r" (r1)
+, [r2] "r" (r2)
+: "cc");
+cc = (cc >> 28) & 1;
+assert(cc == 1);
+
+return EXIT_SUCCESS;
+}
-- 
2.39.2

[PATCH 0/2] target/s390x: Fix R[NOX]SBG with T=1

[Ilya Leoshkevich]

Hi,

This series fixes ROTATE THEN  SELECTED BITS when
test-results control is on. The problem is the incorrect translation,
which confuses the register allocator.

Patch 1 is the fix, patch 2 adds a test.

Best regards,
Ilya

Ilya Leoshkevich (2):
  target/s390x: Fix R[NOX]SBG with T=1
  tests/tcg/s390x: Add rxsbg.c

 target/s390x/tcg/translate.c|  3 +++
 tests/tcg/s390x/Makefile.target |  1 +
 tests/tcg/s390x/rxsbg.c | 25 +
 3 files changed, 29 insertions(+)
 create mode 100644 tests/tcg/s390x/rxsbg.c

-- 
2.39.2

[PATCH 1/2] target/s390x: Fix R[NOX]SBG with T=1

[Ilya Leoshkevich]

RXSBG usage in the "filetests" test from the wasmtime testsuite makes
tcg_reg_alloc_op() attempt to temp_load() a TEMP_VAL_DEAD temporary,
causing an assertion failure:

0x01000a70:  ec14 b040 3057  rxsbg%r1, %r4, 0xb0, 0x40, 0x30

OP after optimization and liveness analysis:
  01000a70 0004 0006
 rotl_i64 tmp2,r4,$0x30   dead: 1 2  pref=0x
 and_i64 tmp2,tmp2,$0x8000dead: 1  pref=0x
[xor_i64 tmp3,tmp3,tmp2   dead: 1 2  pref=0x]
 and_i64 cc_dst,tmp3,$0x8000  sync: 0  dead: 0 1 2  pref=0x
 mov_i64 psw_addr,$0x1000a76  sync: 0  dead: 0 1  pref=0x
 mov_i32 cc_op,$0x6   sync: 0  dead: 0 1  pref=0x
 call lookup_tb_ptr,$0x6,$1,tmp8,env  dead: 1  pref=none
 goto_ptr tmp8dead: 0
 set_label $L0
 exit_tb $0x7fffe809d183

../tcg/tcg.c:3865: tcg fatal error

The reason is that tmp3 does not have an initial value, which confuses
the register allocator. This also affects the correctness of the
results.

Fix by assigning R1 to it.

Fixes: d6c6372e186e ("target-s390: Implement R[NOX]SBG")
Signed-off-by: Ilya Leoshkevich 
---
 target/s390x/tcg/translate.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 14c3896d529..6dd2f41ad08 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -3696,10 +3696,13 @@ static DisasJumpType op_rosbg(DisasContext *s, DisasOps 
*o)
 int i4 = get_field(s, i4);
 int i5 = get_field(s, i5);
 uint64_t mask;
+TCGv_i64 tmp;
 
 /* If this is a test-only form, arrange to discard the result.  */
 if (i3 & 0x80) {
+tmp = o->out;
 o->out = tcg_temp_new_i64();
+tcg_gen_mov_i64(o->out, tmp);
 }
 
 i3 &= 63;
-- 
2.39.2

[PATCH] tests/tcg/xtensa: add linker.ld to CLEANFILES

[Max Filippov]

Linker script for xtensa tests must be preprocessed for a specific
target, remove it as a part of make clean.

Signed-off-by: Max Filippov 
---
 tests/tcg/xtensa/Makefile.softmmu-target | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/tcg/xtensa/Makefile.softmmu-target 
b/tests/tcg/xtensa/Makefile.softmmu-target
index 973e55298ee4..948c0e6506bd 100644
--- a/tests/tcg/xtensa/Makefile.softmmu-target
+++ b/tests/tcg/xtensa/Makefile.softmmu-target
@@ -26,6 +26,7 @@ ASFLAGS = -Wa,--no-absolute-literals
 LDFLAGS = -Tlinker.ld -nostartfiles -nostdlib
 
 CRT= crt.o vectors.o
+CLEANFILES += linker.ld
 
 linker.ld: linker.ld.S
$(CC) $(XTENSA_INC) -E -P $< -o $@
-- 
2.30.2

Re: [PATCH 3/4] hw/arm: Add WDT to Allwinner-H3 and Orangepi-PC

[Niek Linnenbank]

On Sat, Mar 11, 2023 at 3:42 PM Strahinja Jankovic <
strahinjapjanko...@gmail.com> wrote:

> This patch adds WDT to Allwinner-H3 and Orangepi-PC.
> WDT is added as an overlay to the Timer module memory area.
>
> Signed-off-by: Strahinja Jankovic 
>

Reviewed-by: Niek Linnenbank 


> ---
>  docs/system/arm/orangepi.rst  | 1 +
>  hw/arm/Kconfig| 1 +
>  hw/arm/allwinner-h3.c | 8 
>  include/hw/arm/allwinner-h3.h | 5 -
>  4 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/docs/system/arm/orangepi.rst b/docs/system/arm/orangepi.rst
> index e5973600a1..9afa54213b 100644
> --- a/docs/system/arm/orangepi.rst
> +++ b/docs/system/arm/orangepi.rst
> @@ -26,6 +26,7 @@ The Orange Pi PC machine supports the following devices:
>   * System Control module
>   * Security Identifier device
>   * TWI (I2C)
> + * Watchdog timer
>
>  Limitations
>  """
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index ec15248536..7d916f5450 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -337,6 +337,7 @@ config ALLWINNER_H3
>  select ALLWINNER_A10_PIT
>  select ALLWINNER_SUN8I_EMAC
>  select ALLWINNER_I2C
> +select ALLWINNER_WDT
>  select SERIAL
>  select ARM_TIMER
>  select ARM_GIC
> diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
> index 69d0ad6f50..f05afddf7e 100644
> --- a/hw/arm/allwinner-h3.c
> +++ b/hw/arm/allwinner-h3.c
> @@ -49,6 +49,7 @@ const hwaddr allwinner_h3_memmap[] = {
>  [AW_H3_DEV_OHCI3]  = 0x01c1d400,
>  [AW_H3_DEV_CCU]= 0x01c2,
>  [AW_H3_DEV_PIT]= 0x01c20c00,
> +[AW_H3_DEV_WDT]= 0x01c20ca0,
>  [AW_H3_DEV_UART0]  = 0x01c28000,
>  [AW_H3_DEV_UART1]  = 0x01c28400,
>  [AW_H3_DEV_UART2]  = 0x01c28800,
> @@ -234,6 +235,8 @@ static void allwinner_h3_init(Object *obj)
>  object_initialize_child(obj, "twi1",  >i2c1,  TYPE_AW_I2C_SUN6I);
>  object_initialize_child(obj, "twi2",  >i2c2,  TYPE_AW_I2C_SUN6I);
>  object_initialize_child(obj, "r_twi", >r_twi, TYPE_AW_I2C_SUN6I);
> +
> +object_initialize_child(obj, "wdt", >wdt, TYPE_AW_WDT_SUN6I);
>  }
>
>  static void allwinner_h3_realize(DeviceState *dev, Error **errp)
> @@ -453,6 +456,11 @@ static void allwinner_h3_realize(DeviceState *dev,
> Error **errp)
>  sysbus_connect_irq(SYS_BUS_DEVICE(>r_twi), 0,
> qdev_get_gpio_in(DEVICE(>gic),
> AW_H3_GIC_SPI_R_TWI));
>
> +/* WDT */
> +sysbus_realize(SYS_BUS_DEVICE(>wdt), _fatal);
> +sysbus_mmio_map_overlap(SYS_BUS_DEVICE(>wdt), 0,
> +s->memmap[AW_H3_DEV_WDT], 1);
> +
>  /* Unimplemented devices */
>  for (i = 0; i < ARRAY_SIZE(unimplemented); i++) {
>  create_unimplemented_device(unimplemented[i].device_name,
> diff --git a/include/hw/arm/allwinner-h3.h b/include/hw/arm/allwinner-h3.h
> index 59e0f822d2..f15d6d7cc7 100644
> --- a/include/hw/arm/allwinner-h3.h
> +++ b/include/hw/arm/allwinner-h3.h
> @@ -48,6 +48,7 @@
>  #include "hw/net/allwinner-sun8i-emac.h"
>  #include "hw/rtc/allwinner-rtc.h"
>  #include "hw/i2c/allwinner-i2c.h"
> +#include "hw/watchdog/allwinner-wdt.h"
>  #include "target/arm/cpu.h"
>  #include "sysemu/block-backend.h"
>
> @@ -96,7 +97,8 @@ enum {
>  AW_H3_DEV_RTC,
>  AW_H3_DEV_CPUCFG,
>  AW_H3_DEV_R_TWI,
> -AW_H3_DEV_SDRAM
> +AW_H3_DEV_SDRAM,
> +AW_H3_DEV_WDT
>  };
>
>  /** Total number of CPU cores in the H3 SoC */
> @@ -141,6 +143,7 @@ struct AwH3State {
>  AWI2CState r_twi;
>  AwSun8iEmacState emac;
>  AwRtcState rtc;
> +AwWdtState wdt;
>  GICState gic;
>  MemoryRegion sram_a1;
>  MemoryRegion sram_a2;
> --
> 2.30.2
>
>

-- 
Niek Linnenbank

[PATCH v2] Use f-strings in python scripts

[Marco Liebel]

Replace python 2 format string with f-strings

Signed-off-by: Marco Liebel 
---
 target/hexagon/gen_analyze_funcs.py | 115 -
 target/hexagon/gen_helper_funcs.py  |  54 ++--
 target/hexagon/gen_helper_protos.py |  10 +-
 target/hexagon/gen_idef_parser_funcs.py |   8 +-
 target/hexagon/gen_op_attribs.py|   4 +-
 target/hexagon/gen_op_regs.py   |  10 +-
 target/hexagon/gen_opcodes_def.py   |   2 +-
 target/hexagon/gen_printinsn.py |  14 +-
 target/hexagon/gen_shortcode.py |   2 +-
 target/hexagon/gen_tcg_func_table.py|   2 +-
 target/hexagon/gen_tcg_funcs.py | 317 +++-
 target/hexagon/hex_common.py|   4 +-
 12 files changed, 243 insertions(+), 299 deletions(-)

diff --git a/target/hexagon/gen_analyze_funcs.py 
b/target/hexagon/gen_analyze_funcs.py
index ebd3e7afb9..1e246209e8 100755
--- a/target/hexagon/gen_analyze_funcs.py
+++ b/target/hexagon/gen_analyze_funcs.py
@@ -29,57 +29,49 @@ def is_predicated(tag):
 return 'A_CONDEXEC' in hex_common.attribdict[tag]
 
 def analyze_opn_old(f, tag, regtype, regid, regno):
-regN = "%s%sN" % (regtype, regid)
+regN = f"{regtype}{regid}N"
 predicated = "true" if is_predicated(tag) else "false"
 if (regtype == "R"):
 if (regid in {"ss", "tt"}):
-f.write("//const int %s = insn->regno[%d];\n" % \
-(regN, regno))
+f.write(f"//const int {regN} = insn->regno[{regno}];\n")
 elif (regid in {"dd", "ee", "xx", "yy"}):
-f.write("const int %s = insn->regno[%d];\n" % (regN, regno))
-f.write("ctx_log_reg_write_pair(ctx, %s, %s);\n" % \
-(regN, predicated))
+f.write(f"const int {regN} = insn->regno[{regno}];\n")
+f.write(f"ctx_log_reg_write_pair(ctx, {regN}, 
{predicated});\n")
 elif (regid in {"s", "t", "u", "v"}):
-f.write("//const int %s = insn->regno[%d];\n" % \
-(regN, regno))
+f.write(f"//const int {regN} = insn->regno[{regno}];\n")
 elif (regid in {"d", "e", "x", "y"}):
-f.write("const int %s = insn->regno[%d];\n" % (regN, regno))
-f.write("ctx_log_reg_write(ctx, %s, %s);\n" % \
-(regN, predicated))
+f.write(f"const int {regN} = insn->regno[{regno}];\n")
+f.write(f"ctx_log_reg_write(ctx, {regN}, {predicated});\n")
 else:
 print("Bad register parse: ", regtype, regid)
 elif (regtype == "P"):
 if (regid in {"s", "t", "u", "v"}):
-f.write("//const int %s = insn->regno[%d];\n" % \
-(regN, regno))
+f.write(f"//const int {regN} = insn->regno[{regno}];\n")
 elif (regid in {"d", "e", "x"}):
-f.write("const int %s = insn->regno[%d];\n" % (regN, regno))
-f.write("ctx_log_pred_write(ctx, %s);\n" % (regN))
+f.write(f"const int {regN} = insn->regno[{regno}];\n")
+f.write(f"ctx_log_pred_write(ctx, {regN});\n")
 else:
 print("Bad register parse: ", regtype, regid)
 elif (regtype == "C"):
 if (regid == "ss"):
-f.write("//const int %s = insn->regno[%d] + HEX_REG_SA0;\n" % \
-(regN, regno))
+f.write(f"//const int {regN} = insn->regno[{regno}] "
+ "+ HEX_REG_SA0;\n")
 elif (regid == "dd"):
-f.write("const int %s = insn->regno[%d] + HEX_REG_SA0;\n" % \
-(regN, regno))
-f.write("ctx_log_reg_write_pair(ctx, %s, %s);\n" % \
-(regN, predicated))
+f.write(f"const int {regN} = insn->regno[{regno}] "
+ "+ HEX_REG_SA0;\n")
+f.write(f"ctx_log_reg_write_pair(ctx, {regN}, 
{predicated});\n")
 elif (regid == "s"):
-f.write("//const int %s = insn->regno[%d] + HEX_REG_SA0;\n" % \
-(regN, regno))
+f.write(f"//const int {regN} = insn->regno[{regno}] "
+ "+ HEX_REG_SA0;\n")
 elif (regid == "d"):
-f.write("const int %s = insn->regno[%d] + HEX_REG_SA0;\n" % \
-(regN, regno))
-f.write("ctx_log_reg_write(ctx, %s, %s);\n" % \
-(regN, predicated))
+f.write(f"const int {regN} = insn->regno[{regno}] "
+ "+ HEX_REG_SA0;\n")
+f.write(f"ctx_log_reg_write(ctx, {regN}, {predicated});\n")
 else:
 print("Bad register parse: ", regtype, regid)
 elif (regtype == "M"):
 if (regid == "u"):
-f.write("//const int %s = insn->regno[%d];\n"% \
-(regN, regno))
+f.write(f"//const int {regN} = insn->regno[{regno}];\n")
 else:
 print("Bad register parse: ", regtype, regid)
 

Re: [PATCH 2/4] hw/arm: Add WDT to Allwinner-A10 and Cubieboard

[Niek Linnenbank]

Hi Strahinja,


On Sat, Mar 11, 2023 at 3:42 PM Strahinja Jankovic <
strahinjapjanko...@gmail.com> wrote:

> This patch adds WDT to Allwinner-A10 and Cubieboard.
> WDT is added as an overlay to the Timer module memory map.
>
> Signed-off-by: Strahinja Jankovic 
> ---
>  docs/system/arm/cubieboard.rst | 1 +
>  hw/arm/Kconfig | 1 +
>  hw/arm/allwinner-a10.c | 7 +++
>  include/hw/arm/allwinner-a10.h | 2 ++
>  4 files changed, 11 insertions(+)
>
> diff --git a/docs/system/arm/cubieboard.rst
> b/docs/system/arm/cubieboard.rst
> index 8d485f5435..58c4a2d3ea 100644
> --- a/docs/system/arm/cubieboard.rst
> +++ b/docs/system/arm/cubieboard.rst
> @@ -15,3 +15,4 @@ Emulated devices:
>  - USB controller
>  - SATA controller
>  - TWI (I2C) controller
> +- Watchdog timer
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index b5aed4aff5..ec15248536 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -325,6 +325,7 @@ config ALLWINNER_A10
>  select ALLWINNER_A10_PIC
>  select ALLWINNER_A10_CCM
>  select ALLWINNER_A10_DRAMC
> +select ALLWINNER_WDT
>  select ALLWINNER_EMAC
>  select ALLWINNER_I2C
>  select AXP209_PMU
> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
> index b7ca795c71..b0ea3f7f66 100644
> --- a/hw/arm/allwinner-a10.c
> +++ b/hw/arm/allwinner-a10.c
> @@ -38,6 +38,7 @@
>  #define AW_A10_EHCI_BASE0x01c14000
>  #define AW_A10_OHCI_BASE0x01c14400
>  #define AW_A10_SATA_BASE0x01c18000
> +#define AW_A10_WDT_BASE 0x01c20c90
>

Unfortunately I couldn't find any details about the watchdog in the
Allwinner A10 datasheet "A10_Datasheet.pdf", except for a very brief
summary in chapter 9.1 in the Timer Controller. But I did find that linux
is using this same base address and registers with the shared driver code
in drivers/watchdog/sunxi_wdt.c.

Looks good to me.

Reviewed-by: Niek Linnenbank 


>  #define AW_A10_RTC_BASE 0x01c20d00
>  #define AW_A10_I2C0_BASE0x01c2ac00
>
> @@ -92,6 +93,8 @@ static void aw_a10_init(Object *obj)
>  object_initialize_child(obj, "mmc0", >mmc0, TYPE_AW_SDHOST_SUN4I);
>
>  object_initialize_child(obj, "rtc", >rtc, TYPE_AW_RTC_SUN4I);
> +
> +object_initialize_child(obj, "wdt", >wdt, TYPE_AW_WDT_SUN4I);
>  }
>
>  static void aw_a10_realize(DeviceState *dev, Error **errp)
> @@ -203,6 +206,10 @@ static void aw_a10_realize(DeviceState *dev, Error
> **errp)
>  sysbus_realize(SYS_BUS_DEVICE(>i2c0), _fatal);
>  sysbus_mmio_map(SYS_BUS_DEVICE(>i2c0), 0, AW_A10_I2C0_BASE);
>  sysbus_connect_irq(SYS_BUS_DEVICE(>i2c0), 0, qdev_get_gpio_in(dev,
> 7));
> +
> +/* WDT */
> +sysbus_realize(SYS_BUS_DEVICE(>wdt), _fatal);
> +sysbus_mmio_map_overlap(SYS_BUS_DEVICE(>wdt), 0, AW_A10_WDT_BASE,
> 1);
>  }
>
>  static void aw_a10_class_init(ObjectClass *oc, void *data)
> diff --git a/include/hw/arm/allwinner-a10.h
> b/include/hw/arm/allwinner-a10.h
> index 095afb225d..cd1465c613 100644
> --- a/include/hw/arm/allwinner-a10.h
> +++ b/include/hw/arm/allwinner-a10.h
> @@ -13,6 +13,7 @@
>  #include "hw/misc/allwinner-a10-ccm.h"
>  #include "hw/misc/allwinner-a10-dramc.h"
>  #include "hw/i2c/allwinner-i2c.h"
> +#include "hw/watchdog/allwinner-wdt.h"
>  #include "sysemu/block-backend.h"
>
>  #include "target/arm/cpu.h"
> @@ -41,6 +42,7 @@ struct AwA10State {
>  AwSdHostState mmc0;
>  AWI2CState i2c0;
>  AwRtcState rtc;
> +AwWdtState wdt;
>  MemoryRegion sram_a;
>  EHCISysBusState ehci[AW_A10_NUM_USB];
>  OHCISysBusState ohci[AW_A10_NUM_USB];
> --
> 2.30.2
>
>

-- 
Niek Linnenbank

Re: [PATCH 1/4] hw/watchdog: Allwinner WDT emulation for system reset

[Niek Linnenbank]

Hi Strahinja,


On Sat, Mar 11, 2023 at 3:41 PM Strahinja Jankovic <
strahinjapjanko...@gmail.com> wrote:

> This patch adds basic support for Allwinner WDT.
> Both sun4i and sun6i variants are supported.
> However, interrupt generation is not supported, so WDT can be used only to
> trigger system reset.
>
> Signed-off-by: Strahinja Jankovic 
> ---
>  hw/watchdog/Kconfig |   4 +
>  hw/watchdog/allwinner-wdt.c | 428 
>  hw/watchdog/meson.build |   1 +
>  hw/watchdog/trace-events|   7 +
>  include/hw/watchdog/allwinner-wdt.h | 123 
>  5 files changed, 563 insertions(+)
>  create mode 100644 hw/watchdog/allwinner-wdt.c
>  create mode 100644 include/hw/watchdog/allwinner-wdt.h
>
> diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
> index 66e1d029e3..861fd00334 100644
> --- a/hw/watchdog/Kconfig
> +++ b/hw/watchdog/Kconfig
> @@ -20,3 +20,7 @@ config WDT_IMX2
>
>  config WDT_SBSA
>  bool
> +
> +config ALLWINNER_WDT
> +bool
> +select PTIMER
> diff --git a/hw/watchdog/allwinner-wdt.c b/hw/watchdog/allwinner-wdt.c
> new file mode 100644
> index 00..cf16ec7a56
> --- /dev/null
> +++ b/hw/watchdog/allwinner-wdt.c
> @@ -0,0 +1,428 @@
> +/*
> + * Allwinner Watchdog emulation
> + *
> + * Copyright (C) 2023 Strahinja Jankovic 
> + *
> + *  This file is derived from Allwinner RTC,
> + *  by Niek Linnenbank.
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qemu/units.h"
> +#include "qemu/module.h"
> +#include "trace.h"
> +#include "hw/sysbus.h"
> +#include "hw/registerfields.h"
> +#include "hw/watchdog/allwinner-wdt.h"
> +#include "sysemu/watchdog.h"
> +#include "migration/vmstate.h"
> +
> +/* WDT registers */
> +enum {
> +REG_IRQ_EN = 0, /* Watchdog interrupt enable */
>

Since we are doing a check "if (!c->regmap[offset])" below, should the enum
values begin with 1 instead?


> +REG_IRQ_STA,/* Watchdog interrupt status */
> +REG_CTRL,   /* Watchdog control register */
> +REG_CFG,/* Watchdog configuration register */
> +REG_MODE,   /* Watchdog mode register */
> +};
> +
> +/* Universal WDT register flags */
> +#define WDT_RESTART_MASK(1 << 0)
> +#define WDT_EN_MASK (1 << 0)
> +
> +/* sun4i specific WDT register flags */
> +#define RST_EN_SUN4I_MASK   (1 << 1)
> +#define INTV_VALUE_SUN4I_SHIFT  (3)
> +#define INTV_VALUE_SUN4I_MASK   (0xfu << INTV_VALUE_SUN4I_SHIFT)
> +
> +/* sun6i specific WDT register flags */
> +#define RST_EN_SUN6I_MASK   (1 << 0)
> +#define KEY_FIELD_SUN6I_SHIFT   (1)
> +#define KEY_FIELD_SUN6I_MASK(0xfffu << KEY_FIELD_SUN6I_SHIFT)
> +#define KEY_FIELD_SUN6I (0xA57u)
> +#define INTV_VALUE_SUN6I_SHIFT  (4)
> +#define INTV_VALUE_SUN6I_MASK   (0xfu << INTV_VALUE_SUN6I_SHIFT)
> +
> +/* Map of INTV_VALUE to 0.5s units. */
> +static const uint8_t allwinner_wdt_count_map[] = {
> +1,
> +2,
> +4,
> +6,
> +8,
> +10,
> +12,
> +16,
> +20,
> +24,
> +28,
> +32
> +};
> +
> +/* WDT sun4i register map (offset to name) */
> +const uint8_t allwinner_wdt_sun4i_regmap[] = {
> +[0x] = REG_CTRL,
> +[0x0004] = REG_MODE,
> +};
> +
> +/* WDT sun6i register map (offset to name) */
> +const uint8_t allwinner_wdt_sun6i_regmap[] = {
> +[0x] = REG_IRQ_EN,
> +[0x0004] = REG_IRQ_STA,
> +[0x0010] = REG_CTRL,
> +[0x0014] = REG_CFG,
> +[0x0018] = REG_MODE,
> +};
> +
> +static bool allwinner_wdt_sun4i_read(AwWdtState *s, uint32_t offset)
> +{
> +/* no sun4i specific registers currently implemented */
> +return false;
> +}
> +
> +static bool allwinner_wdt_sun4i_write(AwWdtState *s, uint32_t offset,
> +  uint32_t data)
> +{
> +/* no sun4i specific registers currently implemented */
> +return false;
> +}
> +
> +static bool allwinner_wdt_sun4i_can_reset_system(AwWdtState *s)
> +{
> +if (s->regs[REG_MODE] & RST_EN_SUN6I_MASK) {
> +return true;
> +} else {
> +return false;
> +}
> +}
> +
> +static bool allwinner_wdt_sun4i_is_key_valid(AwWdtState *s, uint32_t val)
> +{
> +/* sun4i has no key */
> +return true;
> +}
> +
> +static uint8_t 

[PATCH] migration: Wait on preempt channel in preempt thread

[Peter Xu]

QEMU main thread will wait until dest preempt channel established during
processing the LISTEN command (within the whole postcopy PACKAGED data), by
waiting on the semaphore postcopy_qemufile_dst_done.

That's racy, because it's possible that the dest QEMU main thread hasn't
yet accept()ed the new connection when processing the LISTEN event.  The
sem_wait() will yield the main thread without being able to run anything
else including the accept() of the new socket, which can cause deadlock
within the main thread.

To avoid the race, move the "wait channel" from main thread to the preempt
thread right at the start.

Reported-by: Peter Maydell 
Fixes: 5655aab079 ("migration: Postpone postcopy preempt channel to be after 
main")
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Peter Xu 
---

PS: This patch is supposed to fix the test breakage reported here:

https://lore.kernel.org/r/ZBBIaX+cZD5Ud2wQ@work-vm
---
 migration/postcopy-ram.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index f54f44d899..41c0713650 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1197,11 +1197,6 @@ int postcopy_ram_incoming_setup(MigrationIncomingState 
*mis)
 }
 
 if (migrate_postcopy_preempt()) {
-/*
- * The preempt channel is established in asynchronous way.  Wait
- * for its completion.
- */
-qemu_sem_wait(>postcopy_qemufile_dst_done);
 /*
  * This thread needs to be created after the temp pages because
  * it'll fetch RAM_CHANNEL_POSTCOPY PostcopyTmpPage immediately.
@@ -1668,6 +1663,12 @@ void *postcopy_preempt_thread(void *opaque)
 
 qemu_sem_post(>thread_sync_sem);
 
+/*
+ * The preempt channel is established in asynchronous way.  Wait
+ * for its completion.
+ */
+qemu_sem_wait(>postcopy_qemufile_dst_done);
+
 /* Sending RAM_SAVE_FLAG_EOS to terminate this thread */
 qemu_mutex_lock(>postcopy_prio_thread_mutex);
 while (1) {
-- 
2.39.1

Re: [PATCH] tests/qtest/migration-test: Disable migration/multifd/tcp/plain/cancel

[Peter Xu]

On Tue, Mar 14, 2023 at 07:31:38PM +, Peter Maydell wrote:
> On Tue, 14 Mar 2023 at 16:46, Peter Xu  wrote:
> > I've attached that simple fix.  Peter, is it easy to verify it?  I'm not
> > sure the reproducability, fine by me too if it's easier to just disable
> > preempt tests for 8.0 release.
> 
> If you all are happy that the fix is correct, I think the easiest thing
> is just to get it into master (via the usual migration tree, or I can
> apply it directly if you like), and we'll see if it helps.

I'll leave that to you maintainers to decide.  In all case, let me post a
formal patch then.  Thanks.

-- 
Peter Xu

RE: [PATCH] Use f-strings in python scripts

[Marco Liebel]

> -Original Message-
> From: Taylor Simpson 
> Sent: Dienstag, 14. März 2023 18:54
> To: Marco Liebel (QUIC) ; qemu-
> de...@nongnu.org
> Subject: RE: [PATCH] Use f-strings in python scripts
> 
> 
> 
> > -Original Message-
> > From: Marco Liebel (QUIC) 
> > Sent: Monday, March 13, 2023 11:26 AM
> > To: qemu-devel@nongnu.org
> > Cc: Taylor Simpson ; Marco Liebel (QUIC)
> > 
> > Subject: [PATCH] Use f-strings in python scripts
> >
> > Replace python 2 format string with f-strings
> >
> > Signed-off-by: Marco Liebel 
> > ---
> >  target/hexagon/gen_helper_funcs.py  |  54 ++--
> >  target/hexagon/gen_helper_protos.py |  10 +-
> >  target/hexagon/gen_idef_parser_funcs.py |   8 +-
> >  target/hexagon/gen_op_attribs.py|   4 +-
> >  target/hexagon/gen_op_regs.py   |  10 +-
> >  target/hexagon/gen_opcodes_def.py   |   2 +-
> >  target/hexagon/gen_printinsn.py |  14 +-
> >  target/hexagon/gen_shortcode.py |   2 +-
> >  target/hexagon/gen_tcg_func_table.py|   2 +-
> >  target/hexagon/gen_tcg_funcs.py | 317 +++-
> >  target/hexagon/hex_common.py|   4 +-
> >  11 files changed, 198 insertions(+), 229 deletions(-)
> 
> Tested-by: Taylor Simpson 
> Reviewed-by: Taylor Simpson 
> 
> Could you also create a patch to do the same thing to
> target/hexagon/gen_analyze_funcs.py?

Of course. I'll add it to this one and send v2 later.

Marco

> 
> Thanks,
> Taylor
> 

Re: [PATCH v5 0/4] AioContext removal: LinuxAioState and ThreadPool

[Kevin Wolf]

Am 03.02.2023 um 14:17 hat Emanuele Giuseppe Esposito geschrieben:
> Just remove some AioContext lock in LinuxAioState and ThreadPool.
> Not related to anything specific, so I decided to send it as
> a separate patch.
> 
> These patches are taken from Paolo's old draft series.

Thanks, applied to the block-next branch.

Kevin

Re: [PATCH v2] include/block: fixup typos

[Kevin Wolf]

Am 13.03.2023 um 01:37 hat Wilfred Mallawa geschrieben:
> From: Wilfred Mallawa 
> 
> Fixup a few minor typos
> 
> Signed-off-by: Wilfred Mallawa 

Thanks, applied to the block-next branch.

Kevin

Re: [PATCH 4/4] tests/avocado: Add reboot tests to Cubieboard

[Niek Linnenbank]

Hi Strahinja,

Looks good! I re-ran the tests on my machine, and they work fine:

ARMBIAN_ARTIFACTS_CACHED=yes AVOCADO_ALLOW_LARGE_STORAGE=yes
./build/tests/venv/bin/avocado --show=app,console run -t
machine:orangepi-pc -t machine:cubieboard
tests/avocado/boot_linux_console.py
...
|console: Tue Mar 14 19:56:37 UTC 2023
\console: Starting root file system check:
PASS (22.45 s)
RESULTS: PASS 8 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 |
CANCEL 0
JOB TIME   : 189.23 s

Reviewed-by: Niek Linnenbank 
Tested-by: Niek Linnenbank 

On Sat, Mar 11, 2023 at 3:42 PM Strahinja Jankovic <
strahinjapjanko...@gmail.com> wrote:

> Cubieboard tests end with comment "reboot not functioning; omit test".
> Fix this so reboot is done at the end of each test.
>
> Signed-off-by: Strahinja Jankovic 
> ---
>  tests/avocado/boot_linux_console.py | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/tests/avocado/boot_linux_console.py
> b/tests/avocado/boot_linux_console.py
> index 574609bf43..c0675809e6 100644
> --- a/tests/avocado/boot_linux_console.py
> +++ b/tests/avocado/boot_linux_console.py
> @@ -581,7 +581,10 @@ def test_arm_cubieboard_initrd(self):
>  'Allwinner sun4i/sun5i')
>  exec_command_and_wait_for_pattern(self, 'cat /proc/iomem',
>  'system-control@1c0')
> -# cubieboard's reboot is not functioning; omit reboot test.
> +exec_command_and_wait_for_pattern(self, 'reboot',
> +'reboot: Restarting
> system')
> +# Wait for VM to shut down gracefully
> +self.vm.wait()
>
>  def test_arm_cubieboard_sata(self):
>  """
> @@ -625,7 +628,10 @@ def test_arm_cubieboard_sata(self):
>  'Allwinner sun4i/sun5i')
>  exec_command_and_wait_for_pattern(self, 'cat /proc/partitions',
>  'sda')
> -# cubieboard's reboot is not functioning; omit reboot test.
> +exec_command_and_wait_for_pattern(self, 'reboot',
> +'reboot: Restarting
> system')
> +# Wait for VM to shut down gracefully
> +self.vm.wait()
>
>  @skipUnless(os.getenv('AVOCADO_ALLOW_LARGE_STORAGE'), 'storage
> limited')
>  def test_arm_cubieboard_openwrt_22_03_2(self):
> @@ -672,7 +678,10 @@ def test_arm_cubieboard_openwrt_22_03_2(self):
>
>  exec_command_and_wait_for_pattern(self, 'cat /proc/cpuinfo',
>  'Allwinner sun4i/sun5i')
> -# cubieboard's reboot is not functioning; omit reboot test.
> +exec_command_and_wait_for_pattern(self, 'reboot',
> +'reboot: Restarting
> system')
> +# Wait for VM to shut down gracefully
> +self.vm.wait()
>
>  @skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might
> timeout')
>  def test_arm_quanta_gsj(self):
> --
> 2.30.2
>
>

-- 
Niek Linnenbank

Re: [PATCH] tests/qtest/migration-test: Disable migration/multifd/tcp/plain/cancel

[Peter Maydell]

On Tue, 14 Mar 2023 at 16:46, Peter Xu  wrote:
> I've attached that simple fix.  Peter, is it easy to verify it?  I'm not
> sure the reproducability, fine by me too if it's easier to just disable
> preempt tests for 8.0 release.

If you all are happy that the fix is correct, I think the easiest thing
is just to get it into master (via the usual migration tree, or I can
apply it directly if you like), and we'll see if it helps.

thanks
-- PMM

Re: [PATCH] DO-NOT-MERGE: pipewire sample code

[Volker Rümelin]

Am 14.03.23 um 12:50 schrieb Dorinda Bassey:

Hi Volker,

Thank you for the clarification. I see the problem now.
So is it safe to say that:

@@ -104,8 +104,9 @@ playback_on_process(void *data)
     /* calculate the total no of bytes to read data from buffer */
     req = b->requested * v->frame_size;
     if (req == 0) {
-        req = 4096 * v->frame_size;
+        req = v->g->dev->timer_period * v->info.rate * v->frame_size 
* 1 / 2 / 100;

     }

I used 50% of the timer-period and the frame_size which would give a 
close margin to what the downstream audio device writes to the DAC.


Hi,

50% is fine by me. But you should rearrange the term. The multiplication 
by v->frame_size should come last, otherwise it's not guaranteed that 
req is a multiple of v->frame_size. I would cast timer_period to 
uint64_t. Then timer_period * info.rate has a 32bit * 32bit => 64bit 
result. 2 us * 256000 frames/s already has more than 32 bits.


+        req = (uint64_t)v->g->dev->timer_period * v->info.rate * 1 / 2 
/ 100 * v->frame_size;


With best regards,
Volker



Thanks,
Dorinda.

On Mon, Mar 13, 2023 at 9:06 PM Volker Rümelin  
wrote:


Am 13.03.23 um 13:28 schrieb Dorinda Bassey:
> Hi Volker,
>
> Thanks for the patch, I've tested the patch and it works. I
don't hear
> the choppy audio with this option "qemu-system-x86_64 -device
> ich9-intel-hda -device hda-duplex,audiodev=audio0 -audiodev
> pipewire,id=audio0,out.frequency=96000,in.frequency=96000 "
>
>     I don't understand how the req == 0 case can work at all.
>
> how this works is that  b->requested could be zero when no
suggestion
> is provided. For playback streams, this field contains the
suggested
> amount of data to provide. hence the reason for this check.

Hi Dorinda,

there has to be a control mechanism that ensures that our write
rate on
average is exactly the frame rate that the down stream audio device
writes to the DAC. My question was how can this work if we always
write
4096 frames.

The answer is, that after a 4096 frames write, the callback is
delayed
by 4096 frames / 44100 frames/s = 93ms. This ensures that our
write rate
is exactly 44100 frames/s.

This means a fixed 4096 frames write is wrong for the req == 0
case. We
have to write 75% of timer-period frames.

If you want to test this yourself, just ignore req and assume it's 0.

With best regards,
Volker

>
>     I suggest to use the same option names as the pulseaudio
backend.
>     out.latency is the effective Pipewire buffer size.
>
> Ack.
>
> Thanks,
> Dorinda.
>
>
> On Sat, Mar 11, 2023 at 5:19 PM Volker Rümelin

> wrote:
>
>     > Based-on:<20230306171020.381116-1-dbas...@redhat.com>
>     > ([PATCH v7] audio/pwaudio.c: Add Pipewire audio backend
for QEMU)
>     >
>     > This is sample code for the review of the pipewire backed. The
>     > code actually works.
>     >
>     > An email with explanations for the changes will follow.
>     >
>     > Signed-off-by: Volker Rümelin
>     > ---
>     >   audio/pwaudio.c | 67
>     +
>     >   qapi/audio.json | 10 +++-
>     >   2 files changed, 49 insertions(+), 28 deletions(-)
>     >
>     > diff --git a/audio/pwaudio.c b/audio/pwaudio.c
>     > index d357761152..8e2a38938f 100644
>     > --- a/audio/pwaudio.c
>     > +++ b/audio/pwaudio.c
>     > @@ -23,7 +23,6 @@
>     >   #define AUDIO_CAP "pipewire"
>     >   #define RINGBUFFER_SIZE    (1u << 22)
>     >   #define RINGBUFFER_MASK    (RINGBUFFER_SIZE - 1)
>     > -#define BUFFER_SAMPLES    512
>     >
>     >   #include "audio_int.h"
>     >
>     > @@ -48,6 +47,7 @@ typedef struct PWVoice {
>     >       struct pw_stream *stream;
>     >       struct spa_hook stream_listener;
>     >       struct spa_audio_info_raw info;
>     > +    uint32_t highwater_mark;
>     >       uint32_t frame_size;
>     >       struct spa_ringbuffer ring;
>     >       uint8_t buffer[RINGBUFFER_SIZE];
>     > @@ -82,7 +82,7 @@ playback_on_process(void *data)
>     >       void *p;
>     >       struct pw_buffer *b;
>     >       struct spa_buffer *buf;
>     > -    uint32_t n_frames, req, index, n_bytes;
>     > +    uint32_t req, index, n_bytes;
>     >       int32_t avail;
>     >
>     >       if (!v->stream) {
>     > @@ -105,8 +105,7 @@ playback_on_process(void *data)
>     >       if (req == 0) {
>     >           req = 4096 * v->frame_size;
>     >       }
>
>     I don't understand how the req == 0 case can work at all. The
>     downstream
>     audio device is the thinnest point in the playback stream.
We can't
>  

Re: [PULL 0/3] Trivial branch for 8.0 patches

[Peter Maydell]

On Tue, 14 Mar 2023 at 13:53, Laurent Vivier  wrote:
>
> The following changes since commit 284c52eec2d0a1b9c47f06c3eee46762c5fc0915:
>
>   Merge tag 'win-socket-pull-request' of 
> https://gitlab.com/marcandre.lureau/qemu into staging (2023-03-13 13:44:17 
> +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/laurent_vivier/qemu.git 
> tags/trivial-branch-for-8.0-pull-request
>
> for you to fetch changes up to fcc8f37ca3eca968932e5da716ec5e7fc05fdcf4:
>
>   MAINTAINERS: Remove CXL maintainer Ben Widawsky (2023-03-14 14:46:38 +0100)
>
> 
> trivial branch pull request 20230314
>
> Update MAINTAINER file
> Fix typo in qemu-options.hx
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/8.0
for any user-visible changes.

-- PMM

Re: [PULL v2 00/18] Display patches

[Peter Maydell]

On Mon, 13 Mar 2023 at 20:02,  wrote:
>
> From: Marc-André Lureau 
>
> The following changes since commit 284c52eec2d0a1b9c47f06c3eee46762c5fc0915:
>
>   Merge tag 'win-socket-pull-request' of 
> https://gitlab.com/marcandre.lureau/qemu into staging (2023-03-13 13:44:17 
> +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/marcandre.lureau/qemu.git tags/display-pull-request
>
> for you to fetch changes up to 4814d3cbf9f921b6f60a384b4aa3fc3151fdd3a7:
>
>   ui/dbus: restrict opengl to gbm-enabled config (2023-03-13 23:48:45 +0400)
>
> 
> ui: dbus & misc fixes
>
> v2:
> - fix crash spotted by avocado VNC test
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/8.0
for any user-visible changes.

-- PMM

Re: [PULL 0/2] riscv-to-apply queue

[Peter Maydell]

On Tue, 14 Mar 2023 at 06:39, Alistair Francis
 wrote:
>
> From: Alistair Francis 
>
> The following changes since commit 284c52eec2d0a1b9c47f06c3eee46762c5fc0915:
>
>   Merge tag 'win-socket-pull-request' of 
> https://gitlab.com/marcandre.lureau/qemu into staging (2023-03-13 13:44:17 
> +)
>
> are available in the Git repository at:
>
>   https://github.com/alistair23/qemu.git tags/pull-riscv-to-apply-20230314
>
> for you to fetch changes up to 0d581506de803204c5a321100afa270573382932:
>
>   Fix incorrect register name in disassembler for fmv,fabs,fneg instructions 
> (2023-03-14 16:36:43 +1000)
>
> 
> Seventh RISC-V PR for 8.0
>
> * Fix slli_uw decoding
> * Fix incorrect register name in disassembler for fmv,fabs,fneg instructions
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/8.0
for any user-visible changes.

-- PMM

Re: [PATCH for-8.0] hw/char/cadence_uart: Fix guards on invalid BRGR/BDIV settings

[Edgar E. Iglesias]

On Tue, Mar 14, 2023 at 6:08 PM Peter Maydell 
wrote:

> The cadence UART attempts to avoid allowing the guset to set invalid
> baud rate register values in the uart_write() function.  However it
> does the "mask to the size of the register field" and "check for
> invalid values" in the wrong order, which means that a malicious
> guest can get a bogus value into the register by setting also some
> high bits in the value, and cause QEMU to crash by division-by-zero.
>
> Do the mask before the bounds check instead of afterwards.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1493
> Signed-off-by: Peter Maydell 
>

Reviewed-by: Edgar E. Iglesias 



> ---
>  hw/char/cadence_uart.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index c069a30842e..807e3985419 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -450,13 +450,15 @@ static MemTxResult uart_write(void *opaque, hwaddr
> offset,
>  }
>  break;
>  case R_BRGR: /* Baud rate generator */
> +value &= 0x;
>  if (value >= 0x01) {
> -s->r[offset] = value & 0x;
> +s->r[offset] = value;
>  }
>  break;
>  case R_BDIV:/* Baud rate divider */
> +value &= 0xff;
>  if (value >= 0x04) {
> -s->r[offset] = value & 0xFF;
> +s->r[offset] = value;
>  }
>  break;
>  default:
> --
> 2.34.1
>
>

Re: [qemu PATCH] hw/cxl/cxl_device: Replace magic number in CXLError definition

[Dave Jiang]

On 3/14/23 9:53 AM, Fan Ni wrote:

Replace the magic number 32 with CXL_RAS_ERR_HEADER_NUM for better code
readability and maintainability.

Signed-off-by: Fan Ni 

Reviewed-by: Dave Jiang 


---
  include/hw/cxl/cxl_device.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index d589f78202..34fde62eac 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -235,7 +235,7 @@ REG64(CXL_MEM_DEV_STS, 0)
  typedef struct CXLError {
  QTAILQ_ENTRY(CXLError) node;
  int type; /* Error code as per FE definition */
-uint32_t header[32];
+uint32_t header[CXL_RAS_ERR_HEADER_NUM];
  } CXLError;
  
  typedef QTAILQ_HEAD(, CXLError) CXLErrorList;

Re: [PATCH v2 01/28] accel/tcg: Introduce translator_use_goto_tb

[Richard Henderson]

On 3/14/23 06:47, Wu, Fei wrote:

On 3/13/2023 11:00 PM, Richard Henderson wrote:

On 3/13/23 07:13, Wu, Fei2 wrote:

Hi Richard,

Sorry for disturbing you. I'm doing some perf profiling on qemu-riscv64,
I see 10%+ faster to build stress-ng without the following patch. I know
it's incorrect to just skip this patch, I'm wondering if we can do
something on intercepting mmap/mprotect (very rare), e.g. even
invalidating all the TBs, but keep the cross-page block chaining.


It also affects breakpoints.

I have no good ideas for how to keep cross-page block chaining without
breaking either of these use cases.  If you come up with a good idea,
please post on qemu-devel for discussion.


Thank you for reply. I am new to qemu/tcg, lots of details and
backgrounds need to catch up.

If we only want to address user-mode qemu, and assume this cross-page
chain, first page -> second page:

* breakpoints. If a new bp is added to second page, the chain is hard to
maintain, but it looks acceptable to flush all TBs and fall back to
current non-cross-page implementation during debugging? I think It's
different from the full system situation here:
https://gitlab.com/qemu-project/qemu/-/issues/404

* mprotect. If the 2nd page remains 'X' permission after mprotect, the
chain is still valid, if it's changed to non-X, then the syscall
interceptor will change the permission of corresponding host page to
non-X, it will be segfault as expected?

* mmap. I cannot figure out the situation. Is there any unit test for
this, or could you please shed some light?

Also munmap, but handled via the same path through page_set_flags, see

if (inval_tb) {
tb_invalidate_phys_range(start, end);
}

There is no unit test for mmap over an existing code page.
I believe we do have one for mprotect.

You could plausibly add a global variable choosing between link-all-pages and 
link-one-page modes; it would be protected by mmap_lock.  For link-all-pages mode, the 
above tb_invalidate_phys_range becomes tb_flush.  We probably want to start in 
link-one-page mode if gdbstub is active, which is the only way to set breakpoints in 
user-only mode.


I expect mprotect/mmap over existing executable pages to be extremely rare.  I expect 
munmap of existing executable pages to be rare-ish, with dlclose() being the most common 
case.  You might wish to change from link-all-pages mode to link-one-page mode after one 
or more instances.


And as I said, this discussion should happen on qemu-devel.


r~

Re: [PATCH] migration/rdma: Fix return-path case

[Dr. David Alan Gilbert]

* Peter Xu (pet...@redhat.com) wrote:
> On Tue, Mar 14, 2023 at 05:15:58PM +, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > The RDMA code has return-path handling code, but it's only enabled
> > if postcopy is enabled; if the 'return-path' migration capability
> > is enabled, the return path is NOT setup but the core migration
> > code still tries to use it and breaks.
> > 
> > Enable the RDMA return path if either postcopy or the return-path
> > capability is enabled.
> > 
> > bz: https://bugzilla.redhat.com/show_bug.cgi?id=2063615
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> 
> Acked-by: Peter Xu 
> 
> > @@ -3373,7 +3373,8 @@ static int qemu_rdma_accept(RDMAContext *rdma)
> >   * initialize the RDMAContext for return path for postcopy after first
> >   * connection request reached.
> >   */
> > -if (migrate_postcopy() && !rdma->is_return_path) {
> > +if ((migrate_postcopy() || migrate_use_return_path())
> > +&& !rdma->is_return_path) {
> >  rdma_return_path = qemu_rdma_data_init(rdma->host_port, NULL);
> >  if (rdma_return_path == NULL) {
> >  rdma_ack_cm_event(cm_event);
> 
> It's not extremely clear to me yet on when we should use migrate_postcopy()
> and when to use migrate_postcopy_ram().  I think it's because I don't know
> enough on the dirty-bitmaps capability.  Do we have some good documentation
> somewhere?

Hmm that's probably a good point.

> Not much I get from the qapi doc..
> 
> # @dirty-bitmaps: If enabled, QEMU will migrate named dirty bitmaps.
> # (since 2.12)

I don't know of any good docs; I think this is a blocks mechanism; I'm
not even sure if it needs the return path.

Dave

> Thanks,
> 
> -- 
> Peter Xu
> 
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH 1/2] target/s390x: Implement Early Exception Recognition

[Nina Schoetterl-Glausch]

On Tue, 2023-03-14 at 12:00 +0100, Ilya Leoshkevich wrote:
> Generate specification exception if a reserved bit is set in the PSW

Generate a ...
> mask or if the PSW address is out of bounds dictated by the addresing

addresSing

> mode.

Does this approach also work with SET SYSTEM MASK and STORE THEN OR SYSTEM
MASK, i.e. do these call s390_cpu_set_psw via some code path I'm not seeing?

In any case, you don't seem to implement their special requirements with regards
to the ilen, so you should at least mention that.

> 
> Reported-by: Nina Schoetterl-Glausch 
> Signed-off-by: Ilya Leoshkevich 
> ---
>  target/s390x/cpu.c | 26 ++
>  target/s390x/cpu.h |  1 +
>  target/s390x/tcg/excp_helper.c |  3 ++-
>  3 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index b10a8541ff8..8e6e46aa3d8 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -41,6 +41,26 @@
>  #define CR0_RESET   0xE0UL
>  #define CR14_RESET  0xC200UL;
>  
> +#ifndef CONFIG_USER_ONLY
> +static bool is_early_exception_recognized(uint64_t mask, uint64_t addr)
> +{
> +if (mask & PSW_MASK_RESERVED) {
> +return true;
> +}
> +
> +switch (mask & (PSW_MASK_32 | PSW_MASK_64)) {
> +case 0:
> +return addr & ~0xffULL;
> +case PSW_MASK_32:
> +return addr & ~0x7fffULL;
> +case PSW_MASK_32 | PSW_MASK_64:
> +return false;
> +default: /* PSW_MASK_64 */
> +return true;
> +}
> +}
> +#endif
> +
>  void s390_cpu_set_psw(CPUS390XState *env, uint64_t mask, uint64_t addr)
>  {
>  #ifndef CONFIG_USER_ONLY
> @@ -57,6 +77,12 @@ void s390_cpu_set_psw(CPUS390XState *env, uint64_t mask, 
> uint64_t addr)
>  env->cc_op = (mask >> 44) & 3;
>  
>  #ifndef CONFIG_USER_ONLY
> +if (is_early_exception_recognized(mask, addr)) {
> +env->int_pgm_ilen = 0;
> +trigger_pgm_exception(env, PGM_SPECIFICATION);
> +return;
> +}
> +
>  if ((old_mask ^ mask) & PSW_MASK_PER) {
>  s390_cpu_recompute_watchpoints(env_cpu(env));
>  }
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 7d6d01325b2..b4de6945936 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -292,6 +292,7 @@ extern const VMStateDescription vmstate_s390_cpu;
>  #define PSW_MASK_32 0x8000ULL
>  #define PSW_MASK_SHORT_ADDR 0x7fffULL
>  #define PSW_MASK_SHORT_CTRL 0x8000ULL
> +#define PSW_MASK_RESERVED   0xb87e7fffULL

What about bit 12?

With bit 12 added:
I haven't checked, but I don't think that would be sufficient for short PSWs 
loaded
with lpsw. I'm not seeing op_lpsw flipping the 12th bit.
op_lpsw looks pretty broken to me overall, putting the BA bit into the address.
I think it should look something like this (didn't actually try it):

static DisasJumpType op_lpsw(DisasContext *s, DisasOps *o)
{
TCGv_i64 t1, t2;

per_breaking_event(s);

t1 = tcg_temp_new_i64();
t2 = tcg_temp_new_i64();
tcg_gen_qemu_ld_i64(t1, o->in2, get_mem_index(s),
MO_TEUQ | MO_ALIGN);//load 8byte instead of 
4
tcg_gen_addi_i64(o->in2, o->in2, 4);
tcg_gen_qemu_ld32u(t2, o->in2, get_mem_index(s));
tcg_gen_andi_i64(t2, t2, PSW_MASK_SHORT_ADDR);
/* Convert the 32-bit PSW_MASK into the 64-bit PSW_MASK.  */
tcg_gen_andi_i64(t1, t1, PSW_MASK_SHORT_CTRL);
tcg_gen_xori_i64(t1, t1, PSW_MASK_SHORTPSW);
gen_helper_load_psw(cpu_env, t1, t2);
return DISAS_NORETURN;
}
>  
>  #undef PSW_ASC_PRIMARY
>  #undef PSW_ASC_ACCREG
> diff --git a/target/s390x/tcg/excp_helper.c b/target/s390x/tcg/excp_helper.c
> index bc767f04438..a7829b1e494 100644
> --- a/target/s390x/tcg/excp_helper.c
> +++ b/target/s390x/tcg/excp_helper.c
> @@ -212,7 +212,8 @@ static void do_program_interrupt(CPUS390XState *env)
>  LowCore *lowcore;
>  int ilen = env->int_pgm_ilen;
>  
> -assert(ilen == 2 || ilen == 4 || ilen == 6);
> +assert((env->int_pgm_code == PGM_SPECIFICATION && ilen == 0) ||
> +   ilen == 2 || ilen == 4 || ilen == 6);
>  
>  switch (env->int_pgm_code) {
>  case PGM_PER:

Re: [qemu PATCH] hw/cxl/cxl_device: Replace magic number in CXLError definition

[Davidlohr Bueso]

On Tue, 14 Mar 2023, Fan Ni wrote:


Replace the magic number 32 with CXL_RAS_ERR_HEADER_NUM for better code
readability and maintainability.



Reviewed-by: Davidlohr Bueso 


Signed-off-by: Fan Ni 
---
include/hw/cxl/cxl_device.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index d589f78202..34fde62eac 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -235,7 +235,7 @@ REG64(CXL_MEM_DEV_STS, 0)
typedef struct CXLError {
QTAILQ_ENTRY(CXLError) node;
int type; /* Error code as per FE definition */
-uint32_t header[32];
+uint32_t header[CXL_RAS_ERR_HEADER_NUM];
} CXLError;

typedef QTAILQ_HEAD(, CXLError) CXLErrorList;
--
2.25.1

Re: [PATCH] migration/rdma: Fix return-path case

[Peter Xu]

On Tue, Mar 14, 2023 at 05:15:58PM +, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> The RDMA code has return-path handling code, but it's only enabled
> if postcopy is enabled; if the 'return-path' migration capability
> is enabled, the return path is NOT setup but the core migration
> code still tries to use it and breaks.
> 
> Enable the RDMA return path if either postcopy or the return-path
> capability is enabled.
> 
> bz: https://bugzilla.redhat.com/show_bug.cgi?id=2063615
> 
> Signed-off-by: Dr. David Alan Gilbert 

Acked-by: Peter Xu 

> @@ -3373,7 +3373,8 @@ static int qemu_rdma_accept(RDMAContext *rdma)
>   * initialize the RDMAContext for return path for postcopy after first
>   * connection request reached.
>   */
> -if (migrate_postcopy() && !rdma->is_return_path) {
> +if ((migrate_postcopy() || migrate_use_return_path())
> +&& !rdma->is_return_path) {
>  rdma_return_path = qemu_rdma_data_init(rdma->host_port, NULL);
>  if (rdma_return_path == NULL) {
>  rdma_ack_cm_event(cm_event);

It's not extremely clear to me yet on when we should use migrate_postcopy()
and when to use migrate_postcopy_ram().  I think it's because I don't know
enough on the dirty-bitmaps capability.  Do we have some good documentation
somewhere?

Not much I get from the qapi doc..

# @dirty-bitmaps: If enabled, QEMU will migrate named dirty bitmaps.
# (since 2.12)

Thanks,

-- 
Peter Xu

RE: [PATCH] Use f-strings in python scripts

[Taylor Simpson]

> -Original Message-
> From: Marco Liebel (QUIC) 
> Sent: Monday, March 13, 2023 11:26 AM
> To: qemu-devel@nongnu.org
> Cc: Taylor Simpson ; Marco Liebel (QUIC)
> 
> Subject: [PATCH] Use f-strings in python scripts
> 
> Replace python 2 format string with f-strings
> 
> Signed-off-by: Marco Liebel 
> ---
>  target/hexagon/gen_helper_funcs.py  |  54 ++--
>  target/hexagon/gen_helper_protos.py |  10 +-
>  target/hexagon/gen_idef_parser_funcs.py |   8 +-
>  target/hexagon/gen_op_attribs.py|   4 +-
>  target/hexagon/gen_op_regs.py   |  10 +-
>  target/hexagon/gen_opcodes_def.py   |   2 +-
>  target/hexagon/gen_printinsn.py |  14 +-
>  target/hexagon/gen_shortcode.py |   2 +-
>  target/hexagon/gen_tcg_func_table.py|   2 +-
>  target/hexagon/gen_tcg_funcs.py | 317 +++-
>  target/hexagon/hex_common.py|   4 +-
>  11 files changed, 198 insertions(+), 229 deletions(-)

Tested-by: Taylor Simpson 
Reviewed-by: Taylor Simpson 

Could you also create a patch to do the same thing to 
target/hexagon/gen_analyze_funcs.py?

Thanks,
Taylor

Re: [PATCH] tests/qtest/migration-test: Disable migration/multifd/tcp/plain/cancel

[Daniel P . Berrangé]

On Tue, Mar 14, 2023 at 12:46:34PM -0400, Peter Xu wrote:
> On Tue, Mar 14, 2023 at 10:11:53AM +, Dr. David Alan Gilbert wrote:
> > OK, I think I kind of see what's happening here, one for Peter Xu.
> > If I'm right it's a race something like:
> >   a) The test harness tells the source it wants to enter postcopy
> >   b) The harness then waits for the source to stop
> >   c) ... and the dest to start 
> > 
> >   It's blocked on one of b but can't tell which
> > 
> >   d) The main thread in the dest is waiting for the postcopy recovery fd
> > to be opened
> >   e) But I think the source is still trying to send normal precopy RAM
> > and perhaps hasn't got around yet to opening that socket yet
> >   f) But I think the dest isn't reading from the main channel at that
> > point because of (d)
> 
> I think this analysis is spot on.  Thanks Dave!
> 
> Src qemu does this with below order:
> 
> 1. setup preempt channel
> 1.1. connect()  --> this is done in another thread
> 1.2. sem_wait(postcopy_qemufile_src_sem) --> make sure it's created
> 2. prepare postcopy package (LISTEN, non-iterable states, ping-3, RUN)
> 3. send the package
> 
> So logically the sequence is guaranteed so that when LISTEN cmd is
> processed, we should have connect()ed already.
> 
> But I think there's one thing missing on dest.. since the accept() on the
> dest node should be run in the main thread, meanwhile the LISTEN cmd is
> also processed on the main thread, even if the listening socket is trying
> to kick the main thread to do the accept() (so the connection has
> established) it won't be able to kick the final accept() as main thread is
> waiting in the semaphore.  That caused a deadlock.
> 
> A simple fix I can think of is moving the wait channel operation outside
> the main thread, e.g. to the preempt thread.
> 
> I've attached that simple fix.  Peter, is it easy to verify it?  I'm not
> sure the reproducability, fine by me too if it's easier to just disable
> preempt tests for 8.0 release.
> 
> Thanks,
> 
> -- 
> Peter Xu

> From 92f2f90d2eb270ca158479bfd9a5a855ec7ddf4d Mon Sep 17 00:00:00 2001
> From: Peter Xu 
> Date: Tue, 14 Mar 2023 12:24:02 -0400
> Subject: [PATCH] migration: Wait on preempt channel in preempt thread
> 
> QEMU main thread will wait until dest preempt channel established during
> processing the LISTEN command (within the whole postcopy PACKAGED data), by
> waiting on the semaphore postcopy_qemufile_dst_done.
> 
> That's racy, because it's possible that the dest QEMU main thread hasn't
> yet accept()ed the new connection when processing the LISTEN event.  The
> sem_wait() will yield the main thread without being able to run anything
> else including the accept() of the new socket, which can cause deadlock
> within the main thread.
> 
> To avoid the race, move the "wait channel" from main thread to the preempt
> thread right at the start.
> 
> Reported-by: Peter Maydell 
> Fixes: 5655aab079 ("migration: Postpone postcopy preempt channel to be after 
> main")
> Signed-off-by: Peter Xu 
> ---
>  migration/postcopy-ram.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)

Reviewed-by: Daniel P. Berrangé 


This description of the bug & proposed fixed matches what I could
infer as the flaw from the stack trace's Peter provided.

> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index f54f44d899..41c0713650 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -1197,11 +1197,6 @@ int postcopy_ram_incoming_setup(MigrationIncomingState 
> *mis)
>  }
>  
>  if (migrate_postcopy_preempt()) {
> -/*
> - * The preempt channel is established in asynchronous way.  Wait
> - * for its completion.
> - */
> -qemu_sem_wait(>postcopy_qemufile_dst_done);
>  /*
>   * This thread needs to be created after the temp pages because
>   * it'll fetch RAM_CHANNEL_POSTCOPY PostcopyTmpPage immediately.
> @@ -1668,6 +1663,12 @@ void *postcopy_preempt_thread(void *opaque)
>  
>  qemu_sem_post(>thread_sync_sem);
>  
> +/*
> + * The preempt channel is established in asynchronous way.  Wait
> + * for its completion.
> + */
> +qemu_sem_wait(>postcopy_qemufile_dst_done);
> +
>  /* Sending RAM_SAVE_FLAG_EOS to terminate this thread */
>  qemu_mutex_lock(>postcopy_prio_thread_mutex);
>  while (1) {
> -- 
> 2.39.1
> 


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

Re: [PATCH] tests/qtest/migration-test: Disable postcopy/preempt tests

[Thomas Huth]

On 14/03/2023 15.17, Peter Maydell wrote:

On Tue, 14 Mar 2023 at 14:14, Thomas Huth  wrote:


On 14/03/2023 15.08, Peter Maydell wrote:

On Tue, 14 Mar 2023 at 14:01, Thomas Huth  wrote:


On 14/03/2023 14.33, Peter Maydell wrote:

The postcopy/preempt tests seem to have a race which makes them hang
on the s390x CI runner.  Disable them for the moment, while we
investigate.  As with the other disabled subtest, you can opt back in
by setting QEMU_TEST_FLAKY_TESTS=1 in your environment.

Suggested-by: Dr. David Alan Gilbert 
Signed-off-by: Peter Maydell 
---
tests/qtest/migration-test.c | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index d4ab3934ed2..4643f7f49dc 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2464,6 +2464,11 @@ int main(int argc, char **argv)
const char *arch = qtest_get_arch();
g_autoptr(GError) err = NULL;
int ret;
+/*
+ * Race condition suspected in the postcopy/preempt tests: see
+ * 
https://lore.kernel.org/qemu-devel/CAFEAcA-q1UwPePdHTzXNSX4i6Urh3j6h51kymy6=7szdafu...@mail.gmail.com/
+ */
+bool skip_postcopy_preempt = getenv("QEMU_TEST_FLAKY_TESTS");


You could maybe also call the variale skip_flaky_tests and use it in the
other spot where you recently added a getenv() already.


That would make it a bit harder to do a simple revert of the
commits when we figure out the cause of the problem, though.


Ok, fair.

So with the "!" added before the getenv:
Reviewed-by: Thomas Huth 

Re: [PATCH for-8.0] hw/char/cadence_uart: Fix guards on invalid BRGR/BDIV settings

[Thomas Huth]

On 14/03/2023 18.08, Peter Maydell wrote:

The cadence UART attempts to avoid allowing the guset to set invalid
baud rate register values in the uart_write() function.  However it
does the "mask to the size of the register field" and "check for
invalid values" in the wrong order, which means that a malicious
guest can get a bogus value into the register by setting also some
high bits in the value, and cause QEMU to crash by division-by-zero.

Do the mask before the bounds check instead of afterwards.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1493
Signed-off-by: Peter Maydell 
---
  hw/char/cadence_uart.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index c069a30842e..807e3985419 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -450,13 +450,15 @@ static MemTxResult uart_write(void *opaque, hwaddr offset,
  }
  break;
  case R_BRGR: /* Baud rate generator */
+value &= 0x;
  if (value >= 0x01) {
-s->r[offset] = value & 0x;
+s->r[offset] = value;
  }
  break;
  case R_BDIV:/* Baud rate divider */
+value &= 0xff;
  if (value >= 0x04) {
-s->r[offset] = value & 0xFF;
+s->r[offset] = value;
  }
  break;
  default:


Reviewed-by: Thomas Huth 

Re: [PATCH for-8.1 v2 04/26] target/riscv: add PRIV_VERSION_LATEST

[Richard Henderson]

On 3/14/23 09:49, Daniel Henrique Barboza wrote:

All these generic CPUs are using the latest priv available, at this
moment PRIV_VERSION_1_12_0:

- riscv_any_cpu_init()
- rv32_base_cpu_init()
- rv64_base_cpu_init()
- rv128_base_cpu_init()

Create a new PRIV_VERSION_LATEST enum and use it in those cases. I'll
make it easier to update everything at once when a new priv version is
available.

Signed-off-by: Daniel Henrique Barboza
---
  target/riscv/cpu.c | 8 
  target/riscv/cpu.h | 2 ++
  2 files changed, 6 insertions(+), 4 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-8.1] hw: Add compat machines for 8.1

[Cédric Le Goater]

On 3/14/23 18:30, Cornelia Huck wrote:

Add 8.1 machine types for arm/i440fx/m68k/q35/s390x/spapr.

Signed-off-by: Cornelia Huck 


For ppc,

Reviewed-by: Cédric Le Goater 

Thanks,

C.



---
  hw/arm/virt.c  |  9 -
  hw/core/machine.c  |  3 +++
  hw/i386/pc.c   |  3 +++
  hw/i386/pc_piix.c  | 16 +---
  hw/i386/pc_q35.c   | 14 --
  hw/m68k/virt.c |  9 -
  hw/ppc/spapr.c | 15 +--
  hw/s390x/s390-virtio-ccw.c | 14 +-
  include/hw/boards.h|  3 +++
  include/hw/i386/pc.h   |  3 +++
  10 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ac626b3bef74..267fe56fae76 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -3232,10 +3232,17 @@ static void machvirt_machine_init(void)
  }
  type_init(machvirt_machine_init);
  
+static void virt_machine_8_1_options(MachineClass *mc)

+{
+}
+DEFINE_VIRT_MACHINE_AS_LATEST(8, 1)
+
  static void virt_machine_8_0_options(MachineClass *mc)
  {
+virt_machine_8_1_options(mc);
+compat_props_add(mc->compat_props, hw_compat_8_0, hw_compat_8_0_len);
  }
-DEFINE_VIRT_MACHINE_AS_LATEST(8, 0)
+DEFINE_VIRT_MACHINE(8, 0)
  
  static void virt_machine_7_2_options(MachineClass *mc)

  {
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 45e3d24fdc89..5bda87fc7d91 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -39,6 +39,9 @@
  #include "hw/virtio/virtio.h"
  #include "hw/virtio/virtio-pci.h"
  
+GlobalProperty hw_compat_8_0[] = {};

+const size_t hw_compat_8_0_len = G_N_ELEMENTS(hw_compat_8_0);
+
  GlobalProperty hw_compat_7_2[] = {
  { "e1000e", "migrate-timadj", "off" },
  { "virtio-mem", "x-early-migration", "false" },
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 1489abf010a6..615e1d3d06ad 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -116,6 +116,9 @@
  { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
  { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },
  
+GlobalProperty pc_compat_8_0[] = {};

+const size_t pc_compat_8_0_len = G_N_ELEMENTS(pc_compat_8_0);
+
  GlobalProperty pc_compat_7_2[] = {
  { "ICH9-LPC", "noreboot", "true" },
  };
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 30eedd62a3a0..21591dad8d92 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -454,21 +454,31 @@ static void pc_i440fx_machine_options(MachineClass *m)
  machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE);
  }
  
-static void pc_i440fx_8_0_machine_options(MachineClass *m)

+static void pc_i440fx_8_1_machine_options(MachineClass *m)
  {
  pc_i440fx_machine_options(m);
  m->alias = "pc";
  m->is_default = true;
  }
  
+DEFINE_I440FX_MACHINE(v8_1, "pc-i440fx-8.1", NULL,

+  pc_i440fx_8_1_machine_options);
+
+static void pc_i440fx_8_0_machine_options(MachineClass *m)
+{
+pc_i440fx_8_1_machine_options(m);
+m->alias = NULL;
+m->is_default = false;
+compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len);
+compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len);
+}
+
  DEFINE_I440FX_MACHINE(v8_0, "pc-i440fx-8.0", NULL,
pc_i440fx_8_0_machine_options);
  
  static void pc_i440fx_7_2_machine_options(MachineClass *m)

  {
  pc_i440fx_8_0_machine_options(m);
-m->alias = NULL;
-m->is_default = false;
  compat_props_add(m->compat_props, hw_compat_7_2, hw_compat_7_2_len);
  compat_props_add(m->compat_props, pc_compat_7_2, pc_compat_7_2_len);
  }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 797ba347fd05..f02919d92c46 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -373,19 +373,29 @@ static void pc_q35_machine_options(MachineClass *m)
  m->max_cpus = 288;
  }
  
-static void pc_q35_8_0_machine_options(MachineClass *m)

+static void pc_q35_8_1_machine_options(MachineClass *m)
  {
  pc_q35_machine_options(m);
  m->alias = "q35";
  }
  
+DEFINE_Q35_MACHINE(v8_1, "pc-q35-8.1", NULL,

+   pc_q35_8_1_machine_options);
+
+static void pc_q35_8_0_machine_options(MachineClass *m)
+{
+pc_q35_8_1_machine_options(m);
+m->alias = NULL;
+compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len);
+compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len);
+}
+
  DEFINE_Q35_MACHINE(v8_0, "pc-q35-8.0", NULL,
 pc_q35_8_0_machine_options);
  
  static void pc_q35_7_2_machine_options(MachineClass *m)

  {
  pc_q35_8_0_machine_options(m);
-m->alias = NULL;
  compat_props_add(m->compat_props, hw_compat_7_2, hw_compat_7_2_len);
  compat_props_add(m->compat_props, pc_compat_7_2, pc_compat_7_2_len);
  }
diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c
index 4cb5beef1a0c..f5ca2ca9054c 100644
--- a/hw/m68k/virt.c
+++ b/hw/m68k/virt.c
@@ -346,10 +346,17 @@ type_init(virt_machine_register_types)
  } \
  

[PATCH for-8.1] hw: Add compat machines for 8.1

[Cornelia Huck]

Add 8.1 machine types for arm/i440fx/m68k/q35/s390x/spapr.

Signed-off-by: Cornelia Huck 
---
 hw/arm/virt.c  |  9 -
 hw/core/machine.c  |  3 +++
 hw/i386/pc.c   |  3 +++
 hw/i386/pc_piix.c  | 16 +---
 hw/i386/pc_q35.c   | 14 --
 hw/m68k/virt.c |  9 -
 hw/ppc/spapr.c | 15 +--
 hw/s390x/s390-virtio-ccw.c | 14 +-
 include/hw/boards.h|  3 +++
 include/hw/i386/pc.h   |  3 +++
 10 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ac626b3bef74..267fe56fae76 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -3232,10 +3232,17 @@ static void machvirt_machine_init(void)
 }
 type_init(machvirt_machine_init);
 
+static void virt_machine_8_1_options(MachineClass *mc)
+{
+}
+DEFINE_VIRT_MACHINE_AS_LATEST(8, 1)
+
 static void virt_machine_8_0_options(MachineClass *mc)
 {
+virt_machine_8_1_options(mc);
+compat_props_add(mc->compat_props, hw_compat_8_0, hw_compat_8_0_len);
 }
-DEFINE_VIRT_MACHINE_AS_LATEST(8, 0)
+DEFINE_VIRT_MACHINE(8, 0)
 
 static void virt_machine_7_2_options(MachineClass *mc)
 {
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 45e3d24fdc89..5bda87fc7d91 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -39,6 +39,9 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-pci.h"
 
+GlobalProperty hw_compat_8_0[] = {};
+const size_t hw_compat_8_0_len = G_N_ELEMENTS(hw_compat_8_0);
+
 GlobalProperty hw_compat_7_2[] = {
 { "e1000e", "migrate-timadj", "off" },
 { "virtio-mem", "x-early-migration", "false" },
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 1489abf010a6..615e1d3d06ad 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -116,6 +116,9 @@
 { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
 { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },
 
+GlobalProperty pc_compat_8_0[] = {};
+const size_t pc_compat_8_0_len = G_N_ELEMENTS(pc_compat_8_0);
+
 GlobalProperty pc_compat_7_2[] = {
 { "ICH9-LPC", "noreboot", "true" },
 };
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 30eedd62a3a0..21591dad8d92 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -454,21 +454,31 @@ static void pc_i440fx_machine_options(MachineClass *m)
 machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE);
 }
 
-static void pc_i440fx_8_0_machine_options(MachineClass *m)
+static void pc_i440fx_8_1_machine_options(MachineClass *m)
 {
 pc_i440fx_machine_options(m);
 m->alias = "pc";
 m->is_default = true;
 }
 
+DEFINE_I440FX_MACHINE(v8_1, "pc-i440fx-8.1", NULL,
+  pc_i440fx_8_1_machine_options);
+
+static void pc_i440fx_8_0_machine_options(MachineClass *m)
+{
+pc_i440fx_8_1_machine_options(m);
+m->alias = NULL;
+m->is_default = false;
+compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len);
+compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len);
+}
+
 DEFINE_I440FX_MACHINE(v8_0, "pc-i440fx-8.0", NULL,
   pc_i440fx_8_0_machine_options);
 
 static void pc_i440fx_7_2_machine_options(MachineClass *m)
 {
 pc_i440fx_8_0_machine_options(m);
-m->alias = NULL;
-m->is_default = false;
 compat_props_add(m->compat_props, hw_compat_7_2, hw_compat_7_2_len);
 compat_props_add(m->compat_props, pc_compat_7_2, pc_compat_7_2_len);
 }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 797ba347fd05..f02919d92c46 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -373,19 +373,29 @@ static void pc_q35_machine_options(MachineClass *m)
 m->max_cpus = 288;
 }
 
-static void pc_q35_8_0_machine_options(MachineClass *m)
+static void pc_q35_8_1_machine_options(MachineClass *m)
 {
 pc_q35_machine_options(m);
 m->alias = "q35";
 }
 
+DEFINE_Q35_MACHINE(v8_1, "pc-q35-8.1", NULL,
+   pc_q35_8_1_machine_options);
+
+static void pc_q35_8_0_machine_options(MachineClass *m)
+{
+pc_q35_8_1_machine_options(m);
+m->alias = NULL;
+compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len);
+compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len);
+}
+
 DEFINE_Q35_MACHINE(v8_0, "pc-q35-8.0", NULL,
pc_q35_8_0_machine_options);
 
 static void pc_q35_7_2_machine_options(MachineClass *m)
 {
 pc_q35_8_0_machine_options(m);
-m->alias = NULL;
 compat_props_add(m->compat_props, hw_compat_7_2, hw_compat_7_2_len);
 compat_props_add(m->compat_props, pc_compat_7_2, pc_compat_7_2_len);
 }
diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c
index 4cb5beef1a0c..f5ca2ca9054c 100644
--- a/hw/m68k/virt.c
+++ b/hw/m68k/virt.c
@@ -346,10 +346,17 @@ type_init(virt_machine_register_types)
 } \
 type_init(machvirt_machine_##major##_##minor##_init);
 
+static void virt_machine_8_1_options(MachineClass *mc)
+{
+}
+DEFINE_VIRT_MACHINE(8, 1, true)
+
 static void 

[PATCH] migration/rdma: Fix return-path case

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

The RDMA code has return-path handling code, but it's only enabled
if postcopy is enabled; if the 'return-path' migration capability
is enabled, the return path is NOT setup but the core migration
code still tries to use it and breaks.

Enable the RDMA return path if either postcopy or the return-path
capability is enabled.

bz: https://bugzilla.redhat.com/show_bug.cgi?id=2063615

Signed-off-by: Dr. David Alan Gilbert 
---
 migration/rdma.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 288eadc2d2..9d70e9885b 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3373,7 +3373,8 @@ static int qemu_rdma_accept(RDMAContext *rdma)
  * initialize the RDMAContext for return path for postcopy after first
  * connection request reached.
  */
-if (migrate_postcopy() && !rdma->is_return_path) {
+if ((migrate_postcopy() || migrate_use_return_path())
+&& !rdma->is_return_path) {
 rdma_return_path = qemu_rdma_data_init(rdma->host_port, NULL);
 if (rdma_return_path == NULL) {
 rdma_ack_cm_event(cm_event);
@@ -3455,7 +3456,8 @@ static int qemu_rdma_accept(RDMAContext *rdma)
 }
 
 /* Accept the second connection request for return path */
-if (migrate_postcopy() && !rdma->is_return_path) {
+if ((migrate_postcopy() || migrate_use_return_path())
+&& !rdma->is_return_path) {
 qemu_set_fd_handler(rdma->channel->fd, rdma_accept_incoming_migration,
 NULL,
 (void *)(intptr_t)rdma->return_path);
@@ -4192,7 +4194,7 @@ void rdma_start_outgoing_migration(void *opaque,
 }
 
 /* RDMA postcopy need a separate queue pair for return path */
-if (migrate_postcopy()) {
+if (migrate_postcopy() || migrate_use_return_path()) {
 rdma_return_path = qemu_rdma_data_init(host_port, errp);
 
 if (rdma_return_path == NULL) {
-- 
2.39.2

Re: [PATCH v2 0/6] block: switch to AIO_WAIT_WHILE_UNLOCKED() where possible

[Kevin Wolf]

Am 09.03.2023 um 20:08 hat Stefan Hajnoczi geschrieben:
> v2:
> - Clarify NULL ctx argument in Patch 1 commit description [Kevin]
> 
> AIO_WAIT_WHILE_UNLOCKED() is the future replacement for AIO_WAIT_WHILE(). Most
> callers haven't been converted yet because they rely on the AioContext lock. I
> looked through the code and found the easy cases that can be converted today.

Thanks, applied to block-next.

Kevin

[PATCH for-8.0] hw/char/cadence_uart: Fix guards on invalid BRGR/BDIV settings

[Peter Maydell]

The cadence UART attempts to avoid allowing the guset to set invalid
baud rate register values in the uart_write() function.  However it
does the "mask to the size of the register field" and "check for
invalid values" in the wrong order, which means that a malicious
guest can get a bogus value into the register by setting also some
high bits in the value, and cause QEMU to crash by division-by-zero.

Do the mask before the bounds check instead of afterwards.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1493
Signed-off-by: Peter Maydell 
---
 hw/char/cadence_uart.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index c069a30842e..807e3985419 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -450,13 +450,15 @@ static MemTxResult uart_write(void *opaque, hwaddr offset,
 }
 break;
 case R_BRGR: /* Baud rate generator */
+value &= 0x;
 if (value >= 0x01) {
-s->r[offset] = value & 0x;
+s->r[offset] = value;
 }
 break;
 case R_BDIV:/* Baud rate divider */
+value &= 0xff;
 if (value >= 0x04) {
-s->r[offset] = value & 0xFF;
+s->r[offset] = value;
 }
 break;
 default:
-- 
2.34.1

Re: [PATCH 2/2] tests/tcg/s390x: Add ex-relative-long.c

[Richard Henderson]

On 3/13/23 16:38, Ilya Leoshkevich wrote:

Test EXECUTE and EXECUTE RELATIVE LONG with relative long instructions
as targets.

Signed-off-by: Ilya Leoshkevich 
---
  tests/tcg/s390x/Makefile.target|   1 +
  tests/tcg/s390x/ex-relative-long.c | 149 +
  2 files changed, 150 insertions(+)
  create mode 100644 tests/tcg/s390x/ex-relative-long.c

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 72ad309b273..ed2709ee2c3 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -28,6 +28,7 @@ TESTS+=div
  TESTS+=clst
  TESTS+=long-double
  TESTS+=cdsg
+TESTS+=ex-relative-long
  
  cdsg: CFLAGS+=-pthread

  cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/ex-relative-long.c 
b/tests/tcg/s390x/ex-relative-long.c
new file mode 100644
index 000..e47dac7e2c3
--- /dev/null
+++ b/tests/tcg/s390x/ex-relative-long.c
@@ -0,0 +1,149 @@
+/* Check EXECUTE with relative long instructions as targets. */
+#include 
+#include 
+
+struct test {
+const char *name;
+long (*func)(long reg, long *cc);
+long exp_reg;
+long exp_mem;
+long exp_cc;
+};
+
+/* Variable targeted by relative long instructions. */
+long mem;


I guess you're assuming that the adjacent memory, which the buggy qemu would address, 
contains something other than




+/* Initial "mem" value. */
+#define MEM 0xfedcba9889abcdef


this?  Perhaps better to use an array, and address the middle of it?


r~

Re: [PATCH v2 0/4] hw/arm/virt: Support dirty ring

[Peter Maydell]

On Mon, 13 Mar 2023 at 07:13, Gavin Shan  wrote:
>
> On 2/27/23 12:26 PM, Gavin Shan wrote:
> > This series intends to support dirty ring for live migration for arm64. The
> > dirty ring use discrete buffer to track dirty pages. For arm64, the 
> > speciality
> > is to use backup bitmap to track dirty pages when there is no-running-vcpu
> > context. It's known that the backup bitmap needs to be synchronized when
> > KVM device "kvm-arm-gicv3" or "arm-its-kvm" has been enabled. The backup
> > bitmap is collected in the last stage of migration. The policy here is to
> > always enable the backup bitmap extension. The overhead to synchronize the
> > backup bitmap in the last stage of migration, when those two devices aren't
> > used, is introduced. However, the overhead should be very small and 
> > acceptable.
> > The benefit is to support future cases where those two devices are used 
> > without
> > modifying the code.
> >
> > PATCH[1] add migration last stage indicator
> > PATCH[2] synchronize the backup bitmap in the last stage of migration
> > PATCH[3] add helper kvm_dirty_ring_init() to enable dirty ring
> > PATCH[4] enable dirty ring for arm64
> >
> > v1: https://lists.nongnu.org/archive/html/qemu-arm/2023-02/msg00434.html
> > RFCv1: https://lists.nongnu.org/archive/html/qemu-arm/2023-02/msg00171.html
> >
> > Testing
> > ===
> > (1) kvm-unit-tests/its-pending-migration and kvm-unit-tests/its-migration 
> > with
> >  dirty ring or normal dirty page tracking mechanism. All test cases 
> > passed.
> >
> >  QEMU=./qemu.main/build/qemu-system-aarch64 ACCEL=kvm \
> >  ./its-pending-migration
> >
> >  QEMU=./qemu.main/build/qemu-system-aarch64 ACCEL=kvm \
> >  ./its-migration
> >
> >  QEMU=./qemu.main/build/qemu-system-aarch64 
> > ACCEL=kvm,dirty-ring-size=65536 \
> >  ./its-pending-migration
> >
> >  QEMU=./qemu.main/build/qemu-system-aarch64 
> > ACCEL=kvm,dirty-ring-size=65536 \
> >  ./its-migration
> >
> > (2) Combinations of migration, post-copy migration, e1000e and virtio-net
> >  devices. All test cases passed.
> >
> >  -netdev tap,id=net0,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown  
> > \
> >  -device e1000e,bus=pcie.5,netdev=net0,mac=52:54:00:f1:26:a0
> >
> >  -netdev tap,id=vnet0,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown 
> > \
> >  -device virtio-net-pci,bus=pcie.6,netdev=vnet0,mac=52:54:00:f1:26:b0
> >
> > Changelog
> > =
> > v2:
> >* Drop PATCH[v1 1/6] to synchronize linux-headers
> > (Gavin)
> >* More restrictive comments about struct MemoryListener::log_sync_global 
> > (PeterX)
> >* Always enable the backup bitmap extension  
> > (PeterM)
> > v1:
> >* Combine two patches into one PATCH[v1 2/6] for the last stage 
> > indicator(PeterX)
> >* Drop the secondary bitmap and use the original one directly
> > (Juan)
> >* Avoid "goto out" in helper kvm_dirty_ring_init()   
> > (Juan)
> >
>
> Ping, Paolo and Peter Maydell. Please take a look to see if it can be
> merged, thanks!

No objections here; I'm assuming that since it's only touching
KVM core code that it would go via Paolo. However, as a new
feature it has missed softfreeze for 8.0, so it'll have to
wait until the tree re-opens for 8.1.

thanks
-- PMM

Re: [PATCH] hw/ide: Remove unuseful IDE_DMA__COUNT definition

[Philippe Mathieu-Daudé]

ping?

On 24/2/23 16:34, Philippe Mathieu-Daudé wrote:

First, IDE_DMA__COUNT represents the number of enumerated
values, but is incorrectly listed as part of the enum.

Second, IDE_DMA_CMD_str() is internal to core.c and only
takes sane enums from ide_dma_cmd. So no need to check the
'enval' argument is within the enum range. Only checks
IDE_DMA_CMD_lookup[] entry is not NULL.

Both combined, IDE_DMA__COUNT can go.

Reduce IDE_DMA_CMD_lookup[] scope which is only used locally.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/ide/core.c | 10 +-
  include/hw/ide/internal.h |  3 ---
  2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 5d1039378f..8bf61e7267 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -63,19 +63,19 @@ static const int smart_attributes[][12] = {
  { 190,  0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00, 0x00, 0x32},
  };
  
-const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT] = {

+static const char *IDE_DMA_CMD_lookup[] = {
  [IDE_DMA_READ] = "DMA READ",
  [IDE_DMA_WRITE] = "DMA WRITE",
  [IDE_DMA_TRIM] = "DMA TRIM",
-[IDE_DMA_ATAPI] = "DMA ATAPI"
+[IDE_DMA_ATAPI] = "DMA ATAPI",
  };
  
  static const char *IDE_DMA_CMD_str(enum ide_dma_cmd enval)

  {
-if ((unsigned)enval < IDE_DMA__COUNT) {
-return IDE_DMA_CMD_lookup[enval];
+if (!IDE_DMA_CMD_lookup[enval]) {
+return "DMA UNKNOWN CMD";
  }
-return "DMA UNKNOWN CMD";
+return IDE_DMA_CMD_lookup[enval];
  }
  
  static void ide_dummy_transfer_stop(IDEState *s);

diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index fc0aa81a88..e864fe8caf 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -352,11 +352,8 @@ enum ide_dma_cmd {
  IDE_DMA_WRITE,
  IDE_DMA_TRIM,
  IDE_DMA_ATAPI,
-IDE_DMA__COUNT
  };
  
-extern const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT];

-
  #define ide_cmd_is_read(s) \
  ((s)->dma_cmd == IDE_DMA_READ)
  

Re: [PATCH] scripts/git.orderfile: Display QAPI script changes before schema ones

[Philippe Mathieu-Daudé]

Cc'ing qemu-trivial@

On 24/2/23 22:41, Philippe Mathieu-Daudé wrote:

When modifying QAPI scripts and modifying C files along,
it makes sense to display QAPI changes first.

Signed-off-by: Philippe Mathieu-Daudé 
---
Failed example:
https://lore.kernel.org/qemu-devel/20230224155451.20211-3-phi...@linaro.org/
---
  scripts/git.orderfile | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/scripts/git.orderfile b/scripts/git.orderfile
index 8edac0380b..70adc1a74a 100644
--- a/scripts/git.orderfile
+++ b/scripts/git.orderfile
@@ -22,6 +22,8 @@ Makefile*
  *.mak
  meson.build
  
+# qapi scripts

+scripts/qapi*
  # qapi schema
  qapi/*.json
  qga/*.json

Re: [PATCH v2 2/2] pci: allow slot_reserved_mask to be ignored with manual slot assignment

[Chuck Zmudzinski]

On 3/14/2023 10:39 AM, Mark Cave-Ayland wrote:
> On 14/03/2023 14:21, Chuck Zmudzinski wrote:
>
> > On 3/14/2023 9:41 AM, Mark Cave-Ayland wrote:
> >> On 14/03/2023 13:26, Chuck Zmudzinski wrote:
> >>
> >>> On 3/14/2023 9:17 AM, Michael S. Tsirkin wrote:
>  On Tue, Mar 14, 2023 at 12:43:12PM +, Mark Cave-Ayland wrote:
> > On 14/03/2023 06:33, Michael S. Tsirkin wrote:
> >
> >> On Tue, Mar 14, 2023 at 12:01:09AM -0400, Chuck Zmudzinski wrote:
> >>> Commit 4f67543bb8c5 ("xen/pt: reserve PCI slot 2 for Intel 
> >>> igd-passthru")
> >>> uses slot_reserved_mask to reserve slot 2 for the Intel IGD for the
> >>> xenfv machine when the guest is configured for igd-passthru.
> >>>
> >>> A desired extension to that commit is to allow use of the reserved 
> >>> slot
> >>> if the administrator manually configures a device to use the reserved
> >>> slot. Currently, slot_reserved_mask is enforced unconditionally. With
> >>> this patch, the pci bus can be configured so the slot is only reserved
> >>> if the pci device to be added to the bus is configured for automatic
> >>> slot assignment.
> >>>
> >>> To enable the desired behavior of slot_reserved_mask machine, add a
> >>> boolean member enforce_slot_reserved_mask_manual to struct PCIBus and
> >>> add a function pci_bus_ignore_slot_reserved_mask_manual which can be
> >>> called to change the default behavior of always enforcing
> >>> slot_reserved_mask so, in that case, slot_reserved_mask is only 
> >>> enforced
> >>> when the pci device being added is configured for automatic slot
> >>> assignment.
> >>>
> >>> Call the new pci_bus_ignore_slot_reserved_mask_manual function after
> >>> creating the pci bus for the pc/i440fx/xenfv machine type to implement
> >>> the desired behavior of causing slot_reserved_mask to only apply when
> >>> the pci device to be added to a pc/i440fx/xenfv machine is configured
> >>> for automatic slot assignment.
> >>>
> >>> Link: 
> >>> https://lore.kernel.org/qemu-devel/20230106064838-mutt-send-email-...@kernel.org/
> >>> Signed-off-by: Chuck Zmudzinski 
> >>
> >> I really dislike this.
> >> It seems that xen should not have used slot_reserved_mask,
> >> and instead needs something new like slot_manual_mask.
> >> No?
> >
> > My suggestion was to move the validation logic to a separate callback
> > function in PCIBus (see
> > https://lists.gnu.org/archive/html/qemu-devel/2023-03/msg03988.html) but
> > perhaps I wasn't clear enough in pointing out that I was thinking this 
> > could
> > *replace* the existing slot_reserved_mask mechanism, rather than 
> > providing a
> > hook to allow it to be manipulated.
> >
> > Here's a very rough patch put together over lunch that attempts this for
> > pci_bus_devfn_reserved(): the idea is that sun4u and Xen would call
> > pci_bus_set_slot_reserved_fn() with a suitable pci_slot_reserved_fn
> > implementation, and slot_reserved_mask gets removed completely i.e.:
> >
> >
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index def5000e7b..30b856499a 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -493,6 +493,13 @@ bool pci_bus_bypass_iommu(PCIBus *bus)
> >return host_bridge->bypass_iommu;
> >}
> >
> > +static bool pci_bus_default_slot_reserved(PCISlotReservationType 
> > restype,
> > +  int devfn)
> > +{
> > +/* All slots accessible by default */
> > +return false;
> > +}
> > +
> >static void pci_root_bus_internal_init(PCIBus *bus, DeviceState 
> > *parent,
> >   MemoryRegion 
> > *address_space_mem,
> >   MemoryRegion 
> > *address_space_io,
> > @@ -500,7 +507,7 @@ static void pci_root_bus_internal_init(PCIBus *bus,
> > DeviceState *parent,
> >{
> >assert(PCI_FUNC(devfn_min) == 0);
> >bus->devfn_min = devfn_min;
> > -bus->slot_reserved_mask = 0x0;
> > +bus->slot_reserved_fn = pci_bus_default_slot_reserved;
> >bus->address_space_mem = address_space_mem;
> >bus->address_space_io = address_space_io;
> >bus->flags |= PCI_BUS_IS_ROOT;
> > @@ -,9 +1118,15 @@ static bool pci_bus_devfn_available(PCIBus *bus, 
> > int devfn)
> >return !(bus->devices[devfn]);
> >}
> >
> > -static bool pci_bus_devfn_reserved(PCIBus *bus, int devfn)
> > +static bool pci_bus_devfn_reserved(PCIBus *bus, int devfn,
> > +   PCISlotReservationType restype)
> > +{
> > +return bus->slot_reserved_fn(restype, devfn);
> > +}
> > +
> > +void pci_bus_set_slot_reserved_fn(PCIBus *bus, 

Re: [PATCH 1/2] target/s390x: Implement Early Exception Recognition

[David Hildenbrand]

On 14.03.23 12:00, Ilya Leoshkevich wrote:

Generate specification exception if a reserved bit is set in the PSW
mask or if the PSW address is out of bounds dictated by the addresing
mode.

Reported-by: Nina Schoetterl-Glausch 


Unofficially known to be broken (and ignored) for a long time :D


Signed-off-by: Ilya Leoshkevich 
---
  target/s390x/cpu.c | 26 ++
  target/s390x/cpu.h |  1 +
  target/s390x/tcg/excp_helper.c |  3 ++-
  3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index b10a8541ff8..8e6e46aa3d8 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -41,6 +41,26 @@
  #define CR0_RESET   0xE0UL
  #define CR14_RESET  0xC200UL;
  
+#ifndef CONFIG_USER_ONLY

+static bool is_early_exception_recognized(uint64_t mask, uint64_t addr)


Nit: I'd call this is_early_exception_psw() or sth like that.

Thanks!

Reviewed-by: David Hildenbrand 

--
Thanks,

David / dhildenb

[qemu PATCH] hw/cxl/cxl_device: Replace magic number in CXLError definition

[Fan Ni]

Replace the magic number 32 with CXL_RAS_ERR_HEADER_NUM for better code
readability and maintainability.

Signed-off-by: Fan Ni 
---
 include/hw/cxl/cxl_device.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index d589f78202..34fde62eac 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -235,7 +235,7 @@ REG64(CXL_MEM_DEV_STS, 0)
 typedef struct CXLError {
 QTAILQ_ENTRY(CXLError) node;
 int type; /* Error code as per FE definition */
-uint32_t header[32];
+uint32_t header[CXL_RAS_ERR_HEADER_NUM];
 } CXLError;
 
 typedef QTAILQ_HEAD(, CXLError) CXLErrorList;
-- 
2.25.1

[PATCH for-8.1 v2 18/26] target/risc/cpu.c: add riscv_cpu_validate_misa_ext()

[Daniel Henrique Barboza]

Even after taking RVG off from riscv_cpu_validate_set_extensions(), the
function is still doing too much. It is validating misa bits, then
validating named extensions, and if the validation succeeds it's doing
more changes in both cpu->cfg and MISA bits.

It works for the support we have today, since we'll error out during
realize() time. This is not enough to support write_misa() though - we
don't want to error out if userspace writes something odd in the CSR.

This patch starts splitting riscv_cpu_validate_set_extensions() into a
three step process: validate misa_ext, validate cpu->cfg, then commit
the configuration. This separation will allow us to use these functions
in write_misa() without having to worry about saving CPU state during
runtime because the function changed state but failed to validate.

riscv_cpu_validate_misa_ext() will host all validations related to misa
bits only. Validations using misa bits + name extensions will remain in
validate_set_extensions().

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 77 ++
 1 file changed, 43 insertions(+), 34 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index af5a1e6a43..83b1b874ee 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1025,6 +1025,43 @@ static void 
riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
 }
 }
 
+static void riscv_cpu_validate_misa_ext(RISCVCPU *cpu, Error **errp)
+{
+if (cpu->cfg.ext_i && cpu->cfg.ext_e) {
+error_setg(errp,
+   "I and E extensions are incompatible");
+return;
+}
+
+if (!cpu->cfg.ext_i && !cpu->cfg.ext_e) {
+error_setg(errp,
+   "Either I or E extension must be set");
+return;
+}
+
+if (cpu->cfg.ext_s && !cpu->cfg.ext_u) {
+error_setg(errp,
+   "Setting S extension without U extension is illegal");
+return;
+}
+
+if (cpu->cfg.ext_h && !cpu->cfg.ext_i) {
+error_setg(errp,
+   "H depends on an I base integer ISA with 32 x registers");
+return;
+}
+
+if (cpu->cfg.ext_h && !cpu->cfg.ext_s) {
+error_setg(errp, "H extension implicitly requires S-mode");
+return;
+}
+
+if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
+error_setg(errp, "D extension requires F extension");
+return;
+}
+}
+
 static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
 {
 RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
@@ -1072,35 +1109,6 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU 
*cpu, Error **errp)
 return;
 }
 
-if (cpu->cfg.ext_i && cpu->cfg.ext_e) {
-error_setg(errp,
-   "I and E extensions are incompatible");
-return;
-}
-
-if (!cpu->cfg.ext_i && !cpu->cfg.ext_e) {
-error_setg(errp,
-   "Either I or E extension must be set");
-return;
-}
-
-if (cpu->cfg.ext_s && !cpu->cfg.ext_u) {
-error_setg(errp,
-   "Setting S extension without U extension is illegal");
-return;
-}
-
-if (cpu->cfg.ext_h && !cpu->cfg.ext_i) {
-error_setg(errp,
-   "H depends on an I base integer ISA with 32 x registers");
-return;
-}
-
-if (cpu->cfg.ext_h && !cpu->cfg.ext_s) {
-error_setg(errp, "H extension implicitly requires S-mode");
-return;
-}
-
 if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
 error_setg(errp, "F extension requires Zicsr");
 return;
@@ -1120,11 +1128,6 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU 
*cpu, Error **errp)
 return;
 }
 
-if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
-error_setg(errp, "D extension requires F extension");
-return;
-}
-
 /* The V vector extension depends on the Zve64d extension */
 if (cpu->cfg.ext_v) {
 cpu->cfg.ext_zve64d = true;
@@ -1338,6 +1341,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 riscv_set_G_virt_ext(cpu);
 }
 
+riscv_cpu_validate_misa_ext(cpu, _err);
+if (local_err != NULL) {
+error_propagate(errp, local_err);
+return;
+}
+
 riscv_cpu_validate_set_extensions(cpu, _err);
 if (local_err != NULL) {
 error_propagate(errp, local_err);
-- 
2.39.2

[PATCH for-8.1 v2 12/26] target/riscv/cpu.c: redesign register_cpu_props()

[Daniel Henrique Barboza]

Now that the function is a no-op if 'env.misa_ext != 0', and no one that
are setting misa_ext != 0 is calling it because set_misa() is setting
the cpu cfg accordingly, remove the now deprecated code and rename the
function to register_generic_cpu_props().

This function is now doing exactly what the name says: it is creating
user-facing properties to allow changes in the CPU cfg via the QEMU
command line, setting default values if no user input is provided.

Note that there's the possibility of a CPU to set a certain misa value
and, at the same, also want user-facing flags and defaults from this
function. This is not the case since commit 26b2bc58599c ("target/riscv:
Don't expose the CPU properties on names CPUs"), but given that this is
also a possibility, clarify in the function that using this function
will overwrite existing values in cpu->cfg.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 48 ++
 1 file changed, 10 insertions(+), 38 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 7841676473..6b5096d25e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -221,7 +221,7 @@ static const char * const riscv_intr_names[] = {
 "reserved"
 };
 
-static void register_cpu_props(Object *obj);
+static void register_generic_cpu_props(Object *obj);
 
 const char *riscv_cpu_get_trap_name(target_ulong cause, bool async)
 {
@@ -386,7 +386,7 @@ static void rv64_base_cpu_init(Object *obj)
 CPURISCVState *env = _CPU(obj)->env;
 /* We set this in the realise function */
 set_misa(env, MXL_RV64, 0);
-register_cpu_props(obj);
+register_generic_cpu_props(obj);
 /* Set latest version of privileged specification */
 env->priv_ver = PRIV_VERSION_LATEST;
 #ifndef CONFIG_USER_ONLY
@@ -475,7 +475,7 @@ static void rv128_base_cpu_init(Object *obj)
 CPURISCVState *env = _CPU(obj)->env;
 /* We set this in the realise function */
 set_misa(env, MXL_RV128, 0);
-register_cpu_props(obj);
+register_generic_cpu_props(obj);
 /* Set latest version of privileged specification */
 env->priv_ver = PRIV_VERSION_LATEST;
 #ifndef CONFIG_USER_ONLY
@@ -488,7 +488,7 @@ static void rv32_base_cpu_init(Object *obj)
 CPURISCVState *env = _CPU(obj)->env;
 /* We set this in the realise function */
 set_misa(env, MXL_RV32, 0);
-register_cpu_props(obj);
+register_generic_cpu_props(obj);
 /* Set latest version of privileged specification */
 env->priv_ver = PRIV_VERSION_LATEST;
 #ifndef CONFIG_USER_ONLY
@@ -575,7 +575,7 @@ static void riscv_host_cpu_init(Object *obj)
 #elif defined(TARGET_RISCV64)
 set_misa(env, MXL_RV64, 0);
 #endif
-register_cpu_props(obj);
+register_generic_cpu_props(obj);
 }
 #endif
 
@@ -1557,44 +1557,16 @@ static Property riscv_cpu_extensions[] = {
 };
 
 /*
- * Register CPU props based on env.misa_ext. If a non-zero
- * value was set, register only the required cpu->cfg.ext_*
- * properties and leave. env.misa_ext = 0 means that we want
- * all the default properties to be registered.
+ * Register generic CPU props with user-facing flags declared
+ * in riscv_cpu_extensions[].
+ *
+ * Note that this will overwrite existing values in cpu->cfg.
  */
-static void register_cpu_props(Object *obj)
+static void register_generic_cpu_props(Object *obj)
 {
-RISCVCPU *cpu = RISCV_CPU(obj);
-uint32_t misa_ext = cpu->env.misa_ext;
 Property *prop;
 DeviceState *dev = DEVICE(obj);
 
-/*
- * If misa_ext is not zero, set cfg properties now to
- * allow them to be read during riscv_cpu_realize()
- * later on.
- */
-if (cpu->env.misa_ext != 0) {
-cpu->cfg.ext_i = misa_ext & RVI;
-cpu->cfg.ext_e = misa_ext & RVE;
-cpu->cfg.ext_m = misa_ext & RVM;
-cpu->cfg.ext_a = misa_ext & RVA;
-cpu->cfg.ext_f = misa_ext & RVF;
-cpu->cfg.ext_d = misa_ext & RVD;
-cpu->cfg.ext_v = misa_ext & RVV;
-cpu->cfg.ext_c = misa_ext & RVC;
-cpu->cfg.ext_s = misa_ext & RVS;
-cpu->cfg.ext_u = misa_ext & RVU;
-cpu->cfg.ext_h = misa_ext & RVH;
-cpu->cfg.ext_j = misa_ext & RVJ;
-
-/*
- * We don't want to set the default riscv_cpu_extensions
- * in this case.
- */
-return;
-}
-
 for (prop = riscv_cpu_extensions; prop && prop->name; prop++) {
 qdev_property_add_static(dev, prop);
 }
-- 
2.39.2

[PATCH for-8.1 v2 11/26] target/riscv/cpu.c: set cpu config in set_misa()

[Daniel Henrique Barboza]

set_misa() is setting all 'misa' related env states and nothing else.
But other functions, namely riscv_cpu_validate_set_extensions(), uses
the config object to do its job.

This creates a need to set the single letter extensions in the cfg
object to keep both in sync. At this moment this is being done by
register_cpu_props(), forcing every CPU to do a call to this function.

Let's beef up set_misa() and make the function do the sync for us. This
will relieve named CPUs to having to call register_cpu_props(), which
will then be redesigned to a more specialized role next.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 40 
 target/riscv/cpu.h |  4 ++--
 2 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 36c55abda0..7841676473 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -236,8 +236,40 @@ const char *riscv_cpu_get_trap_name(target_ulong cause, 
bool async)
 
 static void set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext)
 {
+RISCVCPU *cpu;
+
 env->misa_mxl_max = env->misa_mxl = mxl;
 env->misa_ext_mask = env->misa_ext = ext;
+
+/*
+ * ext = 0 will only be a thing during cpu_init() functions
+ * as a way of setting an extension-agnostic CPU. We do
+ * not support clearing misa_ext* and the ext_N flags in
+ * RISCVCPUConfig in regular circunstances.
+ */
+if (ext == 0) {
+return;
+}
+
+/*
+ * We can't use riscv_cpu_cfg() in this case because it is
+ * a read-only inline and we're going to change the values
+ * of cpu->cfg.
+ */
+cpu = env_archcpu(env);
+
+cpu->cfg.ext_i = ext & RVI;
+cpu->cfg.ext_e = ext & RVE;
+cpu->cfg.ext_m = ext & RVM;
+cpu->cfg.ext_a = ext & RVA;
+cpu->cfg.ext_f = ext & RVF;
+cpu->cfg.ext_d = ext & RVD;
+cpu->cfg.ext_v = ext & RVV;
+cpu->cfg.ext_c = ext & RVC;
+cpu->cfg.ext_s = ext & RVS;
+cpu->cfg.ext_u = ext & RVU;
+cpu->cfg.ext_h = ext & RVH;
+cpu->cfg.ext_j = ext & RVJ;
 }
 
 #ifndef CONFIG_USER_ONLY
@@ -340,7 +372,6 @@ static void riscv_any_cpu_init(Object *obj)
 #endif
 
 env->priv_ver = PRIV_VERSION_LATEST;
-register_cpu_props(obj);
 
 /* inherited from parent obj via riscv_cpu_init() */
 cpu->cfg.ext_ifencei = true;
@@ -368,7 +399,6 @@ static void rv64_sifive_u_cpu_init(Object *obj)
 RISCVCPU *cpu = RISCV_CPU(obj);
 CPURISCVState *env = >env;
 set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
-register_cpu_props(obj);
 env->priv_ver = PRIV_VERSION_1_10_0;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV39);
@@ -387,7 +417,6 @@ static void rv64_sifive_e_cpu_init(Object *obj)
 RISCVCPU *cpu = RISCV_CPU(obj);
 
 set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
-register_cpu_props(obj);
 env->priv_ver = PRIV_VERSION_1_10_0;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
@@ -472,8 +501,6 @@ static void rv32_sifive_u_cpu_init(Object *obj)
 RISCVCPU *cpu = RISCV_CPU(obj);
 CPURISCVState *env = >env;
 set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
-
-register_cpu_props(obj);
 env->priv_ver = PRIV_VERSION_1_10_0;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV32);
@@ -492,7 +519,6 @@ static void rv32_sifive_e_cpu_init(Object *obj)
 RISCVCPU *cpu = RISCV_CPU(obj);
 
 set_misa(env, MXL_RV32, RVI | RVM | RVA | RVC | RVU);
-register_cpu_props(obj);
 env->priv_ver = PRIV_VERSION_1_10_0;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
@@ -510,7 +536,6 @@ static void rv32_ibex_cpu_init(Object *obj)
 RISCVCPU *cpu = RISCV_CPU(obj);
 
 set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU);
-register_cpu_props(obj);
 env->priv_ver = PRIV_VERSION_1_11_0;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
@@ -529,7 +554,6 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
 RISCVCPU *cpu = RISCV_CPU(obj);
 
 set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU);
-register_cpu_props(obj);
 env->priv_ver = PRIV_VERSION_1_10_0;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 76f81c6b68..ebe0fff668 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -66,8 +66,8 @@
 #define RV(x) ((target_ulong)1 << (x - 'A'))
 
 /*
- * Consider updating register_cpu_props() when adding
- * new MISA bits here.
+ * Consider updating set_misa() when adding new
+ * MISA bits here.
  */
 #define RVI RV('I')
 #define RVE RV('E') /* E and I are mutually exclusive */
-- 
2.39.2

[PATCH for-8.1 v2 15/26] target/riscv: do not allow RVG in write_misa()

[Daniel Henrique Barboza]

We're getting ready to use riscv_cpu_validate_set_extensions() to unify
the handling of write_misa() with the rest of the code base. But first
we need to deal with RVG.

The 'G' virtual extension enables a set of extensions in the CPU. At
this moment, this is done at the start of our validation step in
riscv_cpu_validate_set_extensions(). This means that enabling G will
enable other extensions in the CPU before resuming the validation.

This also means that, in case a write_misa() validation fails, we're
going to set cpu->cfg attributes that are unrelated to misa_ext bits
(icsr and ifencei). These would be 2 extra states that we would need to
store to fallback from a validation failure.

Since write_misa() is still on experimental state let's make our lives
easier for now and disable RVG updates.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/csr.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d522efc0b6..918d442ebd 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1348,6 +1348,11 @@ static RISCVException write_misa(CPURISCVState *env, int 
csrno,
 return RISCV_EXCP_NONE;
 }
 
+/* Changing 'G' state is unsupported */
+if (val & RVG) {
+return RISCV_EXCP_NONE;
+}
+
 /* 'I' or 'E' must be present */
 if (!(val & (RVI | RVE))) {
 /* It is not, drop write to misa */
-- 
2.39.2

[PATCH for-8.1 v2 22/26] target/riscv: error out on priv failure for RVH

[Daniel Henrique Barboza]

We have one last case where we're changing env->misa_ext* during
validation. riscv_cpu_disable_priv_spec_isa_exts(), at the end of
riscv_cpu_validate_set_extensions(), will disable cpu->cfg.ext_h and
cpu->cfg.ext_v if priv_ver check fails.

This check can be done in riscv_cpu_validate_misa_ext(). The difference
here is that we're not silently disable it: we'll error out. Silently
disabling a MISA extension after all the validation is completed can
can cause inconsistencies that we don't have to deal with. Verify ealier
and fail faster.

Note that we're ignoring RVV priv_ver validation since its minimal priv
is also the minimal value we support. RVH will error out if enabled
under priv_ver under 1_12_0.

As a bonus, we're guaranteeing that all env->misa_ext* changes will
happen up until riscv_set_G_virt_ext(). We don't have to worry about
keeping env->misa_ext in sync with cpu->cfg.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 28 +++-
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f8f416d6dd..1f72e1b8ce 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1067,6 +1067,20 @@ static void riscv_cpu_validate_misa_ext(CPURISCVState 
*env,
 return;
 }
 
+/*
+ * Check for priv spec version. RVH is 1_12_0, RVV is 1_10_0.
+ * We don't support anything under 1_10_0 so skip checking
+ * priv for RVV.
+ *
+ * We're hardcoding it here to avoid looping into the
+ * 50+ entries of isa_edata_arr[] just to check the RVH
+ * entry.
+ */
+if (misa_ext & RVH && env->priv_ver < PRIV_VERSION_1_12_0) {
+error_setg(errp, "H extension requires priv spec 1.12.0");
+return;
+}
+
 if (misa_ext & RVV) {
 /*
  * V depends on Zve64d, which requires D. It also
@@ -1117,14 +1131,10 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, 
Error **errp)
 
 /*
  * Check consistency between chosen extensions while setting
- * cpu->cfg accordingly, setting env->misa_ext and
- * misa_ext_mask in the end.
+ * cpu->cfg accordingly.
  */
 static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
 {
-CPURISCVState *env = >env;
-uint32_t ext = 0;
-
 if (cpu->cfg.epmp && !cpu->cfg.pmp) {
 /*
  * Enhanced PMP should only be available
@@ -1241,10 +1251,6 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU 
*cpu, Error **errp)
  * validated and set everything we need.
  */
 riscv_cpu_disable_priv_spec_isa_exts(cpu);
-
-ext = riscv_get_misa_ext_with_cpucfg(>cfg);
-
-env->misa_ext_mask = env->misa_ext = ext;
 }
 
 #ifndef CONFIG_USER_ONLY
@@ -1355,6 +1361,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+/*
+ * This is the last point where env->misa_ext* can
+ * be changed.
+ */
 if (cpu->cfg.ext_g) {
 riscv_set_G_virt_ext(cpu);
 }
-- 
2.39.2

[PATCH for-8.1 v2 02/26] target/riscv/cpu.c: remove set_vext_version()

[Daniel Henrique Barboza]

This setter is doing nothing else but setting env->vext_ver. Assign the
value directly.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 18591aa53a..2752efe1eb 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -245,11 +245,6 @@ static void set_priv_version(CPURISCVState *env, int 
priv_ver)
 env->priv_ver = priv_ver;
 }
 
-static void set_vext_version(CPURISCVState *env, int vext_ver)
-{
-env->vext_ver = vext_ver;
-}
-
 #ifndef CONFIG_USER_ONLY
 static uint8_t satp_mode_from_str(const char *satp_mode_str)
 {
@@ -839,7 +834,7 @@ static void riscv_cpu_validate_v(CPURISCVState *env, 
RISCVCPUConfig *cfg,
 qemu_log("vector version is not specified, "
  "use the default value v1.0\n");
 }
-set_vext_version(env, vext_version);
+env->vext_ver = vext_version;
 }
 
 /*
-- 
2.39.2

[PATCH for-8.1 v2 25/26] target/riscv: rework write_misa()

[Daniel Henrique Barboza]

write_misa() must use as much common logic as possible. We want to open
code just the bits that are exclusive to the CSR write operation and TCG
internals.

Rewrite write_misa() to work as follows:

- supress RVC right after verifying that we're not updating RVG;

- mask the write using misa_ext_mask to avoid enabling unsupported
  extensions;

- emulate the steps done by realize(): validate the candidate misa_ext
  val, then validate the configuration with the candidate misa_ext val,
  and finally commit the changes to cpu->cfg.

If any of the validation steps fails simply ignore the write operation.

Let's keep write_misa() as experimental for now until this logic gains
enough mileage.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 12 +---
 target/riscv/cpu.h |  6 ++
 target/riscv/csr.c | 47 +-
 3 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 5bd92e1cda..4789a7b70d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1027,9 +1027,8 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU 
*cpu)
 }
 
 
-static void riscv_cpu_validate_misa_ext(CPURISCVState *env,
-uint32_t misa_ext,
-Error **errp)
+void riscv_cpu_validate_misa_ext(CPURISCVState *env, uint32_t misa_ext,
+ Error **errp)
 {
 Error *local_err = NULL;
 
@@ -1134,9 +1133,8 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, 
Error **errp)
  * candidate misa_ext value. No changes in env->misa_ext
  * are made.
  */
-static void riscv_cpu_validate_extensions(RISCVCPU *cpu,
-  uint32_t misa_ext,
-  Error **errp)
+void riscv_cpu_validate_extensions(RISCVCPU *cpu, uint32_t misa_ext,
+   Error **errp)
 {
 if (cpu->cfg.epmp && !cpu->cfg.pmp) {
 /*
@@ -1227,7 +1225,7 @@ static void riscv_cpu_validate_extensions(RISCVCPU *cpu,
 }
 }
 
-static void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu)
+void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu)
 {
 if (cpu->cfg.ext_zk) {
 cpu->cfg.ext_zkn = true;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index dbb4df9df0..ca2ba6a647 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -593,6 +593,12 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 char *riscv_isa_string(RISCVCPU *cpu);
 void riscv_cpu_list(void);
 
+void riscv_cpu_validate_misa_ext(CPURISCVState *env, uint32_t misa_ext,
+ Error **errp);
+void riscv_cpu_validate_extensions(RISCVCPU *cpu, uint32_t misa_ext,
+   Error **errp);
+void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu);
+
 #define cpu_list riscv_cpu_list
 #define cpu_mmu_index riscv_cpu_mmu_index
 
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 918d442ebd..6f26e7dbcd 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1343,6 +1343,9 @@ static RISCVException read_misa(CPURISCVState *env, int 
csrno,
 static RISCVException write_misa(CPURISCVState *env, int csrno,
  target_ulong val)
 {
+RISCVCPU *cpu = env_archcpu(env);
+Error *local_err = NULL;
+
 if (!riscv_cpu_cfg(env)->misa_w) {
 /* drop write to misa */
 return RISCV_EXCP_NONE;
@@ -1353,47 +1356,39 @@ static RISCVException write_misa(CPURISCVState *env, 
int csrno,
 return RISCV_EXCP_NONE;
 }
 
-/* 'I' or 'E' must be present */
-if (!(val & (RVI | RVE))) {
-/* It is not, drop write to misa */
-return RISCV_EXCP_NONE;
-}
-
-/* 'E' excludes all other extensions */
-if (val & RVE) {
-/*
- * when we support 'E' we can do "val = RVE;" however
- * for now we just drop writes if 'E' is present.
- */
-return RISCV_EXCP_NONE;
-}
-
 /*
- * misa.MXL writes are not supported by QEMU.
- * Drop writes to those bits.
+ * Suppress 'C' if next instruction is not aligned
+ * TODO: this should check next_pc
  */
+if ((val & RVC) && (GETPC() & ~3) != 0) {
+val &= ~RVC;
+}
 
 /* Mask extensions that are not supported by this hart */
 val &= env->misa_ext_mask;
 
-/* 'D' depends on 'F', so clear 'D' if 'F' is not present */
-if ((val & RVD) && !(val & RVF)) {
-val &= ~RVD;
+/* If nothing changed, do nothing. */
+if (val == env->misa_ext) {
+return RISCV_EXCP_NONE;
 }
 
 /*
- * Suppress 'C' if next instruction is not aligned
- * TODO: this should check next_pc
+ * This flow is similar to what riscv_cpu_realize() does,
+ * with the difference that we will update env->misa_ext
+ * value if everything is ok.
  */
-if ((val & RVC) && (GETPC() & ~3) != 0) {
-val &= ~RVC;
+

[PATCH for-8.1 v2 23/26] target/riscv: split riscv_cpu_validate_set_extensions()

[Daniel Henrique Barboza]

We're now ready to split riscv_cpu_validate_set_extensions() in two.
None of these steps are going to touch env->misa_ext*.

riscv_cpu_validate_extensions() will take care of all validations based
on cpu->cfg values. cpu->cfg changes that are required for the
validation are being tolerated here. This is the case of extensions such
as ext_zfh enabling ext_zfhmin.

The RVV chain enablement (ext_zve64d, ext_zve64f and ext_zve32f) is also
being tolerated because the risk of failure is being mitigated by the
RVV -> RVD && RVF dependency in validate_misa_ext() done prior.

In an ideal world we would have all these extensions declared as object
properties, with getters and setters, and we would be able to, e.g.,
enable ext_zfhmin as soon as ext_zfh is enabled. This would avoid
cpu->cfg changes during riscv_cpu_validate_extensions(). Easier said
than done, not just because of the hundreds of lines involved in it, but
also because we want these properties to be available just for generic
CPUs (named CPUs don't want these properties exposed for users). For now
we'll work with that we have.

riscv_cpu_commit_cpu_cfg() is the last step of the validation where more
cpu->cfg properties are set and disabling of extensions due to priv spec
happens. We're already validated everything we wanted, so any cpu->cfg
change made here is valid.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1f72e1b8ce..e423d3e2d2 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1130,10 +1130,10 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, 
Error **errp)
 }
 
 /*
- * Check consistency between chosen extensions while setting
- * cpu->cfg accordingly.
+ * Check consistency between chosen extensions. No changes
+ * in env->misa_ext are made.
  */
-static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
+static void riscv_cpu_validate_extensions(RISCVCPU *cpu, Error **errp)
 {
 if (cpu->cfg.epmp && !cpu->cfg.pmp) {
 /*
@@ -1222,7 +1222,10 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU 
*cpu, Error **errp)
 return;
 }
 }
+}
 
+static void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu)
+{
 if (cpu->cfg.ext_zk) {
 cpu->cfg.ext_zkn = true;
 cpu->cfg.ext_zkr = true;
@@ -1375,12 +1378,14 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
-riscv_cpu_validate_set_extensions(cpu, _err);
+riscv_cpu_validate_extensions(cpu, _err);
 if (local_err != NULL) {
 error_propagate(errp, local_err);
 return;
 }
 
+riscv_cpu_commit_cpu_cfg(cpu);
+
 #ifndef CONFIG_USER_ONLY
 if (cpu->cfg.ext_sstc) {
 riscv_timer_init(cpu);
-- 
2.39.2

[PATCH for-8.1 v2 14/26] target/riscv: add RVG

[Daniel Henrique Barboza]

The 'G' bit in misa_ext is a virtual extension that enables a set of
extensions (i, m, a, f, d, icsr and ifencei). We'll want to avoid
setting it for write_misa(). Add it so we can gate write_misa() properly
against it.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 4 
 target/riscv/cpu.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 28d4c5f768..48ad7372b9 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -274,6 +274,9 @@ static uint32_t 
riscv_get_misa_ext_with_cpucfg(RISCVCPUConfig *cfg)
 if (cfg->ext_j) {
 ext |= RVJ;
 }
+if (cfg->ext_g) {
+ext |= RVG;
+}
 
 return ext;
 }
@@ -293,6 +296,7 @@ static void riscv_set_cpucfg_with_misa(RISCVCPUConfig *cfg,
 cfg->ext_u = misa_ext & RVU;
 cfg->ext_h = misa_ext & RVH;
 cfg->ext_j = misa_ext & RVJ;
+cfg->ext_g = misa_ext & RVG;
 }
 
 static void set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 2263629332..dbb4df9df0 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -82,6 +82,7 @@
 #define RVU RV('U')
 #define RVH RV('H')
 #define RVJ RV('J')
+#define RVG RV('G')
 
 
 /* Privileged specification version */
-- 
2.39.2

[PATCH for-8.1 v2 26/26] target/riscv: update cpu->cfg misa bits in commit_cpu_cfg()

[Daniel Henrique Barboza]

write_misa() is able to use the same validation workflow
riscv_cpu_realize() uses. But it's still not capable of updating
cpu->cfg misa props yet.

We have no way of blocking future (and current) code from checking
env->misa_ext (via riscv_has_ext()) or reading cpu->cfg directly, so our
best alternative is to keep everything in sync.

riscv_cpu_commit_cpu_cfg() now receives an extra 'misa_ext' parameter.
If this val is different from the existing env->misa_ext, update
env->misa and cpu->cfg with the new value. riscv_cpu_realize() will
ignore this code since env->misa_ext isn't touched during validation,
but write_misa() will use it to keep cpu->cfg in sync with the new
env->misa_ext value.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 16 ++--
 target/riscv/cpu.h |  2 +-
 target/riscv/csr.c |  3 +--
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 4789a7b70d..059931daea 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1225,8 +1225,20 @@ void riscv_cpu_validate_extensions(RISCVCPU *cpu, 
uint32_t misa_ext,
 }
 }
 
-void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu)
+void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu, uint32_t misa_ext)
 {
+CPURISCVState *env = >env;
+
+/*
+ * write_misa() needs to update cpu->cfg with the new
+ * MISA bits. This is a no-op for the riscv_cpu_realize()
+ * path.
+ */
+if (env->misa_ext != misa_ext) {
+env->misa_ext = misa_ext;
+riscv_set_cpucfg_with_misa(>cfg, misa_ext);
+}
+
 if (cpu->cfg.ext_zk) {
 cpu->cfg.ext_zkn = true;
 cpu->cfg.ext_zkr = true;
@@ -1385,7 +1397,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
-riscv_cpu_commit_cpu_cfg(cpu);
+riscv_cpu_commit_cpu_cfg(cpu, env->misa_ext);
 
 #ifndef CONFIG_USER_ONLY
 if (cpu->cfg.ext_sstc) {
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index ca2ba6a647..befc3b8fff 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -597,7 +597,7 @@ void riscv_cpu_validate_misa_ext(CPURISCVState *env, 
uint32_t misa_ext,
  Error **errp);
 void riscv_cpu_validate_extensions(RISCVCPU *cpu, uint32_t misa_ext,
Error **errp);
-void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu);
+void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu, uint32_t misa_ext);
 
 #define cpu_list riscv_cpu_list
 #define cpu_mmu_index riscv_cpu_mmu_index
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6f26e7dbcd..0da0ffdaed 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1387,7 +1387,7 @@ static RISCVException write_misa(CPURISCVState *env, int 
csrno,
 return RISCV_EXCP_NONE;
 }
 
-riscv_cpu_commit_cpu_cfg(cpu);
+riscv_cpu_commit_cpu_cfg(cpu, val);
 
 if (!(val & RVF)) {
 env->mstatus &= ~MSTATUS_FS;
@@ -1395,7 +1395,6 @@ static RISCVException write_misa(CPURISCVState *env, int 
csrno,
 
 /* flush translation cache */
 tb_flush(env_cpu(env));
-env->misa_ext = val;
 env->xl = riscv_cpu_mxl(env);
 return RISCV_EXCP_NONE;
 }
-- 
2.39.2

[PATCH for-8.1 v2 04/26] target/riscv: add PRIV_VERSION_LATEST

[Daniel Henrique Barboza]

All these generic CPUs are using the latest priv available, at this
moment PRIV_VERSION_1_12_0:

- riscv_any_cpu_init()
- rv32_base_cpu_init()
- rv64_base_cpu_init()
- rv128_base_cpu_init()

Create a new PRIV_VERSION_LATEST enum and use it in those cases. I'll
make it easier to update everything at once when a new priv version is
available.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 8 
 target/riscv/cpu.h | 2 ++
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 18032dfd4e..1ee322001b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -338,7 +338,7 @@ static void riscv_any_cpu_init(Object *obj)
 VM_1_10_SV32 : VM_1_10_SV57);
 #endif
 
-env->priv_ver = PRIV_VERSION_1_12_0;
+env->priv_ver = PRIV_VERSION_LATEST;
 register_cpu_props(obj);
 }
 
@@ -350,7 +350,7 @@ static void rv64_base_cpu_init(Object *obj)
 set_misa(env, MXL_RV64, 0);
 register_cpu_props(obj);
 /* Set latest version of privileged specification */
-env->priv_ver = PRIV_VERSION_1_12_0;
+env->priv_ver = PRIV_VERSION_LATEST;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV57);
 #endif
@@ -426,7 +426,7 @@ static void rv128_base_cpu_init(Object *obj)
 set_misa(env, MXL_RV128, 0);
 register_cpu_props(obj);
 /* Set latest version of privileged specification */
-env->priv_ver = PRIV_VERSION_1_12_0;
+env->priv_ver = PRIV_VERSION_LATEST;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV57);
 #endif
@@ -439,7 +439,7 @@ static void rv32_base_cpu_init(Object *obj)
 set_misa(env, MXL_RV32, 0);
 register_cpu_props(obj);
 /* Set latest version of privileged specification */
-env->priv_ver = PRIV_VERSION_1_12_0;
+env->priv_ver = PRIV_VERSION_LATEST;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV32);
 #endif
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 638e47c75a..76f81c6b68 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -88,6 +88,8 @@ enum {
 PRIV_VERSION_1_10_0 = 0,
 PRIV_VERSION_1_11_0,
 PRIV_VERSION_1_12_0,
+
+PRIV_VERSION_LATEST = PRIV_VERSION_1_12_0,
 };
 
 #define VEXT_VERSION_1_00_0 0x0001
-- 
2.39.2

[PATCH for-8.1 v2 24/26] target/riscv: use misa_ext val in riscv_cpu_validate_extensions()

[Daniel Henrique Barboza]

Similar to what we did with riscv_cpu_validate_misa_ext(), let's read
all MISA bits from a misa_ext val instead of reading from the cpu->cfg
object.

This will allow write_misa() to use riscv_cpu_validate_extensions().

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index e423d3e2d2..5bd92e1cda 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1130,10 +1130,13 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, 
Error **errp)
 }
 
 /*
- * Check consistency between chosen extensions. No changes
- * in env->misa_ext are made.
+ * Check consistency between cpu->cfg extensions and a
+ * candidate misa_ext value. No changes in env->misa_ext
+ * are made.
  */
-static void riscv_cpu_validate_extensions(RISCVCPU *cpu, Error **errp)
+static void riscv_cpu_validate_extensions(RISCVCPU *cpu,
+  uint32_t misa_ext,
+  Error **errp)
 {
 if (cpu->cfg.epmp && !cpu->cfg.pmp) {
 /*
@@ -1144,12 +1147,12 @@ static void riscv_cpu_validate_extensions(RISCVCPU 
*cpu, Error **errp)
 return;
 }
 
-if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
+if (misa_ext & RVF && !cpu->cfg.ext_icsr) {
 error_setg(errp, "F extension requires Zicsr");
 return;
 }
 
-if ((cpu->cfg.ext_zawrs) && !cpu->cfg.ext_a) {
+if ((cpu->cfg.ext_zawrs) && !(misa_ext & RVA)) {
 error_setg(errp, "Zawrs extension requires A extension");
 return;
 }
@@ -1158,13 +1161,13 @@ static void riscv_cpu_validate_extensions(RISCVCPU 
*cpu, Error **errp)
 cpu->cfg.ext_zfhmin = true;
 }
 
-if (cpu->cfg.ext_zfhmin && !cpu->cfg.ext_f) {
+if (cpu->cfg.ext_zfhmin && !(misa_ext & RVF)) {
 error_setg(errp, "Zfh/Zfhmin extensions require F extension");
 return;
 }
 
 /* The V vector extension depends on the Zve64d extension */
-if (cpu->cfg.ext_v) {
+if (misa_ext & RVV) {
 cpu->cfg.ext_zve64d = true;
 }
 
@@ -1178,12 +1181,12 @@ static void riscv_cpu_validate_extensions(RISCVCPU 
*cpu, Error **errp)
 cpu->cfg.ext_zve32f = true;
 }
 
-if (cpu->cfg.ext_zve64d && !cpu->cfg.ext_d) {
+if (cpu->cfg.ext_zve64d && !(misa_ext & RVD)) {
 error_setg(errp, "Zve64d/V extensions require D extension");
 return;
 }
 
-if (cpu->cfg.ext_zve32f && !cpu->cfg.ext_f) {
+if (cpu->cfg.ext_zve32f && !(misa_ext & RVF)) {
 error_setg(errp, "Zve32f/Zve64f extensions require F extension");
 return;
 }
@@ -1216,7 +1219,7 @@ static void riscv_cpu_validate_extensions(RISCVCPU *cpu, 
Error **errp)
 error_setg(errp, "Zfinx extension requires Zicsr");
 return;
 }
-if (cpu->cfg.ext_f) {
+if (misa_ext & RVF) {
 error_setg(errp,
"Zfinx cannot be supported together with F extension");
 return;
@@ -1378,7 +1381,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
-riscv_cpu_validate_extensions(cpu, _err);
+riscv_cpu_validate_extensions(cpu, env->misa_ext, _err);
 if (local_err != NULL) {
 error_propagate(errp, local_err);
 return;
-- 
2.39.2

[PATCH for-8.1 v2 10/26] target/riscv/cpu.c: avoid set_misa() in validate_set_extensions()

[Daniel Henrique Barboza]

set_misa() will be tuned up to do more than it's already doing and it
will be redundant to what riscv_cpu_validate_set_extensions() does.

Note that we don't ever change env->misa_mlx in this function, so
set_misa() can be replaced by just assigning env->misa_ext and
env->misa_ext_mask to 'ext'.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index c7b6e7b84b..36c55abda0 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -949,7 +949,8 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, 
Error **errp)
 
 /*
  * Check consistency between chosen extensions while setting
- * cpu->cfg accordingly, doing a set_misa() in the end.
+ * cpu->cfg accordingly, setting env->misa_ext and
+ * misa_ext_mask in the end.
  */
 static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
 {
@@ -1168,7 +1169,7 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU 
*cpu, Error **errp)
 ext |= RVJ;
 }
 
-set_misa(env, env->misa_mxl, ext);
+env->misa_ext_mask = env->misa_ext = ext;
 }
 
 #ifndef CONFIG_USER_ONLY
-- 
2.39.2

[PATCH for-8.1 v2 19/26] target/riscv/cpu:c add misa_ext V-> D & F dependency

[Daniel Henrique Barboza]

We have a chained dependency in riscv_cpu_validate_set_extensions()
related to RVV. If RVV is set, we enable other extensions such as
Zve64d, Zve64f and Zve32f, and these depends on misa bits RVD and RVF.
Thus, we're making RVV depend on RVD and RVF.

Let's add this dependency in riscv_cpu_validate_misa_ext() to fail
earlier.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 83b1b874ee..fa1954a850 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1060,6 +1060,20 @@ static void riscv_cpu_validate_misa_ext(RISCVCPU *cpu, 
Error **errp)
 error_setg(errp, "D extension requires F extension");
 return;
 }
+
+if (cpu->cfg.ext_v) {
+/*
+ * V depends on Zve64d, which requires D. It also
+ * depends on Zve64f, which depends on Zve32f,
+ * which requires F.
+ *
+ * This means that V depends on both D and F.
+ */
+if (!(cpu->cfg.ext_d && cpu->cfg.ext_f)) {
+error_setg(errp, "V extension requires D and F extensions");
+return;
+}
+}
 }
 
 static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
-- 
2.39.2

[PATCH for-8.1 v2 16/26] target/riscv/cpu.c: split RVG code from validate_set_extensions()

[Daniel Henrique Barboza]

We can set all RVG related extensions during realize time, before
validate_set_extensions() itself. It will also avoid re-enabling
RVG via write_misa() when the CSR start to using the same validation
code realize() does.

Note that we're setting both cfg->ext_N and env->misa_ext bits, instead
of just setting cfg->ext_N. The intention here is to start syncing all
misa_ext operations with its cpu->cfg flags, in preparation to allow for
the validate function to operate using a misa_ext. This doesn't make any
difference for the current code state, but will be a requirement for
write_misa() later on.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 55 +-
 1 file changed, 40 insertions(+), 15 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 48ad7372b9..133807e39f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -281,6 +281,42 @@ static uint32_t 
riscv_get_misa_ext_with_cpucfg(RISCVCPUConfig *cfg)
 return ext;
 }
 
+static void riscv_set_G_virt_ext(RISCVCPU *cpu)
+{
+CPURISCVState *env = >env;
+RISCVCPUConfig *cfg = >cfg;
+
+if (!(cfg->ext_i && cfg->ext_m && cfg->ext_a &&
+  cfg->ext_f && cfg->ext_d &&
+  cfg->ext_icsr && cfg->ext_ifencei)) {
+
+warn_report("Setting G will also set IMAFD_Zicsr_Zifencei");
+cfg->ext_i = true;
+env->misa_ext |= RVI;
+
+cfg->ext_m = true;
+env->misa_ext |= RVM;
+
+cfg->ext_a = true;
+env->misa_ext |= RVA;
+
+cfg->ext_f = true;
+env->misa_ext |= RVF;
+
+cfg->ext_d = true;
+env->misa_ext |= RVD;
+
+cfg->ext_icsr = true;
+cfg->ext_ifencei = true;
+
+/*
+ * Update misa_ext_mask since this is called
+ * only during riscv_cpu_realize().
+ */
+env->misa_ext_mask = env->misa_ext;
+}
+}
+
 static void riscv_set_cpucfg_with_misa(RISCVCPUConfig *cfg,
uint32_t misa_ext)
 {
@@ -1036,21 +1072,6 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU 
*cpu, Error **errp)
 return;
 }
 
-/* Do some ISA extension error checking */
-if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
-cpu->cfg.ext_a && cpu->cfg.ext_f &&
-cpu->cfg.ext_d &&
-cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) {
-warn_report("Setting G will also set IMAFD_Zicsr_Zifencei");
-cpu->cfg.ext_i = true;
-cpu->cfg.ext_m = true;
-cpu->cfg.ext_a = true;
-cpu->cfg.ext_f = true;
-cpu->cfg.ext_d = true;
-cpu->cfg.ext_icsr = true;
-cpu->cfg.ext_ifencei = true;
-}
-
 if (cpu->cfg.ext_i && cpu->cfg.ext_e) {
 error_setg(errp,
"I and E extensions are incompatible");
@@ -1313,6 +1334,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+if (cpu->cfg.ext_g) {
+riscv_set_G_virt_ext(cpu);
+}
+
 riscv_cpu_validate_set_extensions(cpu, _err);
 if (local_err != NULL) {
 error_propagate(errp, local_err);
-- 
2.39.2

[PATCH for-8.1 v2 03/26] target/riscv/cpu.c: remove set_priv_version()

[Daniel Henrique Barboza]

The setter is doing nothing special. Just set env->priv_ver directly.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 30 +-
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 2752efe1eb..18032dfd4e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -240,11 +240,6 @@ static void set_misa(CPURISCVState *env, RISCVMXL mxl, 
uint32_t ext)
 env->misa_ext_mask = env->misa_ext = ext;
 }
 
-static void set_priv_version(CPURISCVState *env, int priv_ver)
-{
-env->priv_ver = priv_ver;
-}
-
 #ifndef CONFIG_USER_ONLY
 static uint8_t satp_mode_from_str(const char *satp_mode_str)
 {
@@ -343,7 +338,7 @@ static void riscv_any_cpu_init(Object *obj)
 VM_1_10_SV32 : VM_1_10_SV57);
 #endif
 
-set_priv_version(env, PRIV_VERSION_1_12_0);
+env->priv_ver = PRIV_VERSION_1_12_0;
 register_cpu_props(obj);
 }
 
@@ -355,7 +350,7 @@ static void rv64_base_cpu_init(Object *obj)
 set_misa(env, MXL_RV64, 0);
 register_cpu_props(obj);
 /* Set latest version of privileged specification */
-set_priv_version(env, PRIV_VERSION_1_12_0);
+env->priv_ver = PRIV_VERSION_1_12_0;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV57);
 #endif
@@ -366,7 +361,7 @@ static void rv64_sifive_u_cpu_init(Object *obj)
 CPURISCVState *env = _CPU(obj)->env;
 set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
 register_cpu_props(obj);
-set_priv_version(env, PRIV_VERSION_1_10_0);
+env->priv_ver = PRIV_VERSION_1_10_0;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV39);
 #endif
@@ -379,7 +374,7 @@ static void rv64_sifive_e_cpu_init(Object *obj)
 
 set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
 register_cpu_props(obj);
-set_priv_version(env, PRIV_VERSION_1_10_0);
+env->priv_ver = PRIV_VERSION_1_10_0;
 cpu->cfg.mmu = false;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
@@ -392,7 +387,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
 RISCVCPU *cpu = RISCV_CPU(obj);
 
 set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
-set_priv_version(env, PRIV_VERSION_1_11_0);
+env->priv_ver = PRIV_VERSION_1_11_0;
 
 cpu->cfg.ext_g = true;
 cpu->cfg.ext_c = true;
@@ -431,7 +426,7 @@ static void rv128_base_cpu_init(Object *obj)
 set_misa(env, MXL_RV128, 0);
 register_cpu_props(obj);
 /* Set latest version of privileged specification */
-set_priv_version(env, PRIV_VERSION_1_12_0);
+env->priv_ver = PRIV_VERSION_1_12_0;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV57);
 #endif
@@ -444,7 +439,7 @@ static void rv32_base_cpu_init(Object *obj)
 set_misa(env, MXL_RV32, 0);
 register_cpu_props(obj);
 /* Set latest version of privileged specification */
-set_priv_version(env, PRIV_VERSION_1_12_0);
+env->priv_ver = PRIV_VERSION_1_12_0;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV32);
 #endif
@@ -454,8 +449,9 @@ static void rv32_sifive_u_cpu_init(Object *obj)
 {
 CPURISCVState *env = _CPU(obj)->env;
 set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
+
 register_cpu_props(obj);
-set_priv_version(env, PRIV_VERSION_1_10_0);
+env->priv_ver = PRIV_VERSION_1_10_0;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV32);
 #endif
@@ -468,7 +464,7 @@ static void rv32_sifive_e_cpu_init(Object *obj)
 
 set_misa(env, MXL_RV32, RVI | RVM | RVA | RVC | RVU);
 register_cpu_props(obj);
-set_priv_version(env, PRIV_VERSION_1_10_0);
+env->priv_ver = PRIV_VERSION_1_10_0;
 cpu->cfg.mmu = false;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
@@ -482,7 +478,7 @@ static void rv32_ibex_cpu_init(Object *obj)
 
 set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU);
 register_cpu_props(obj);
-set_priv_version(env, PRIV_VERSION_1_11_0);
+env->priv_ver = PRIV_VERSION_1_11_0;
 cpu->cfg.mmu = false;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
@@ -497,7 +493,7 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
 
 set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU);
 register_cpu_props(obj);
-set_priv_version(env, PRIV_VERSION_1_10_0);
+env->priv_ver = PRIV_VERSION_1_10_0;
 cpu->cfg.mmu = false;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
@@ -1160,7 +1156,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 }
 
 if (priv_version >= PRIV_VERSION_1_10_0) {
-set_priv_version(env, priv_version);
+env->priv_ver = priv_version;
 }
 
 /* Force disable extensions if priv spec version does not match */
-- 
2.39.2

[PATCH for-8.1 v2 07/26] target/riscv: move pmp and epmp validations to validate_set_extensions()

[Daniel Henrique Barboza]

In the near future, write_misa() will use a variation of what we have
now as riscv_cpu_validate_set_extensions(). The pmp and epmp validation
will be required in write_misa() and it's already required here in
riscv_cpu_realize(), so move it to riscv_cpu_validate_set_extensions().

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1a298e5e55..7458845fec 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -916,6 +916,15 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU 
*cpu, Error **errp)
 Error *local_err = NULL;
 uint32_t ext = 0;
 
+if (cpu->cfg.epmp && !cpu->cfg.pmp) {
+/*
+ * Enhanced PMP should only be available
+ * on harts with PMP support
+ */
+error_setg(errp, "Invalid configuration: EPMP requires PMP support");
+return;
+}
+
 /* Do some ISA extension error checking */
 if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
 cpu->cfg.ext_a && cpu->cfg.ext_f &&
@@ -1228,16 +1237,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
-if (cpu->cfg.epmp && !cpu->cfg.pmp) {
-/*
- * Enhanced PMP should only be available
- * on harts with PMP support
- */
-error_setg(errp, "Invalid configuration: EPMP requires PMP support");
-return;
-}
-
-
 #ifndef CONFIG_USER_ONLY
 if (cpu->cfg.ext_sstc) {
 riscv_timer_init(cpu);
-- 
2.39.2

[PATCH for-8.1 v2 06/26] target/riscv/cpu.c: add riscv_cpu_validate_misa_mxl()

[Daniel Henrique Barboza]

Let's remove more code that is open coded in riscv_cpu_realize() and put
it into a helper. Let's also add an error message instead of just
asserting out if env->misa_mxl_max != env->misa_mlx.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 51 ++
 1 file changed, 33 insertions(+), 18 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 17b301967c..1a298e5e55 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -879,6 +879,33 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU 
*cpu)
 }
 }
 
+static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
+{
+RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
+CPUClass *cc = CPU_CLASS(mcc);
+CPURISCVState *env = >env;
+
+/* Validate that MISA_MXL is set properly. */
+switch (env->misa_mxl_max) {
+#ifdef TARGET_RISCV64
+case MXL_RV64:
+case MXL_RV128:
+cc->gdb_core_xml_file = "riscv-64bit-cpu.xml";
+break;
+#endif
+case MXL_RV32:
+cc->gdb_core_xml_file = "riscv-32bit-cpu.xml";
+break;
+default:
+g_assert_not_reached();
+}
+
+if (env->misa_mxl_max != env->misa_mxl) {
+error_setg(errp, "misa_mxl_max must be equal to misa_mxl");
+return;
+}
+}
+
 /*
  * Check consistency between chosen extensions while setting
  * cpu->cfg accordingly, doing a set_misa() in the end.
@@ -1180,9 +1207,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 {
 CPUState *cs = CPU(dev);
 RISCVCPU *cpu = RISCV_CPU(dev);
-CPURISCVState *env = >env;
 RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
-CPUClass *cc = CPU_CLASS(mcc);
 Error *local_err = NULL;
 
 cpu_exec_realizefn(cs, _err);
@@ -1197,6 +1222,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+riscv_cpu_validate_misa_mxl(cpu, _err);
+if (local_err != NULL) {
+error_propagate(errp, local_err);
+return;
+}
+
 if (cpu->cfg.epmp && !cpu->cfg.pmp) {
 /*
  * Enhanced PMP should only be available
@@ -1213,22 +1244,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 }
 #endif /* CONFIG_USER_ONLY */
 
-/* Validate that MISA_MXL is set properly. */
-switch (env->misa_mxl_max) {
-#ifdef TARGET_RISCV64
-case MXL_RV64:
-case MXL_RV128:
-cc->gdb_core_xml_file = "riscv-64bit-cpu.xml";
-break;
-#endif
-case MXL_RV32:
-cc->gdb_core_xml_file = "riscv-32bit-cpu.xml";
-break;
-default:
-g_assert_not_reached();
-}
-assert(env->misa_mxl_max == env->misa_mxl);
-
 riscv_cpu_validate_set_extensions(cpu, _err);
 if (local_err != NULL) {
 error_propagate(errp, local_err);
-- 
2.39.2

[PATCH for-8.1 v2 13/26] target/riscv: put env->misa_ext <-> cpu->cfg code into helpers

[Daniel Henrique Barboza]

The extremely tedious code that sets cpu->cfg based on misa_ext, and
vice-versa, is scattered around riscv_cpu_validate_set_extensions() and
set_misa().

Introduce helpers to do this work, cleaning up the logic of both
functions a bit. While we're at it, add a note in cpu.h informing that
any future change in MISA RV* bits should also be reflected in the
helpers as well.

We'll want to keep env->misa_ext changes in sync with cpu->cfg during
realize() in the next patches, and both helpers will have a role to play
in that.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 120 -
 target/riscv/cpu.h |   3 +-
 2 files changed, 65 insertions(+), 58 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 6b5096d25e..28d4c5f768 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -234,10 +234,69 @@ const char *riscv_cpu_get_trap_name(target_ulong cause, 
bool async)
 }
 }
 
-static void set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext)
+static uint32_t riscv_get_misa_ext_with_cpucfg(RISCVCPUConfig *cfg)
 {
-RISCVCPU *cpu;
+uint32_t ext = 0;
 
+if (cfg->ext_i) {
+ext |= RVI;
+}
+if (cfg->ext_e) {
+ext |= RVE;
+}
+if (cfg->ext_m) {
+ext |= RVM;
+}
+if (cfg->ext_a) {
+ext |= RVA;
+}
+if (cfg->ext_f) {
+ext |= RVF;
+}
+if (cfg->ext_d) {
+ext |= RVD;
+}
+if (cfg->ext_c) {
+ext |= RVC;
+}
+if (cfg->ext_s) {
+ext |= RVS;
+}
+if (cfg->ext_u) {
+ext |= RVU;
+}
+if (cfg->ext_h) {
+ext |= RVH;
+}
+if (cfg->ext_v) {
+ext |= RVV;
+}
+if (cfg->ext_j) {
+ext |= RVJ;
+}
+
+return ext;
+}
+
+static void riscv_set_cpucfg_with_misa(RISCVCPUConfig *cfg,
+   uint32_t misa_ext)
+{
+cfg->ext_i = misa_ext & RVI;
+cfg->ext_e = misa_ext & RVE;
+cfg->ext_m = misa_ext & RVM;
+cfg->ext_a = misa_ext & RVA;
+cfg->ext_f = misa_ext & RVF;
+cfg->ext_d = misa_ext & RVD;
+cfg->ext_v = misa_ext & RVV;
+cfg->ext_c = misa_ext & RVC;
+cfg->ext_s = misa_ext & RVS;
+cfg->ext_u = misa_ext & RVU;
+cfg->ext_h = misa_ext & RVH;
+cfg->ext_j = misa_ext & RVJ;
+}
+
+static void set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext)
+{
 env->misa_mxl_max = env->misa_mxl = mxl;
 env->misa_ext_mask = env->misa_ext = ext;
 
@@ -251,25 +310,7 @@ static void set_misa(CPURISCVState *env, RISCVMXL mxl, 
uint32_t ext)
 return;
 }
 
-/*
- * We can't use riscv_cpu_cfg() in this case because it is
- * a read-only inline and we're going to change the values
- * of cpu->cfg.
- */
-cpu = env_archcpu(env);
-
-cpu->cfg.ext_i = ext & RVI;
-cpu->cfg.ext_e = ext & RVE;
-cpu->cfg.ext_m = ext & RVM;
-cpu->cfg.ext_a = ext & RVA;
-cpu->cfg.ext_f = ext & RVF;
-cpu->cfg.ext_d = ext & RVD;
-cpu->cfg.ext_v = ext & RVV;
-cpu->cfg.ext_c = ext & RVC;
-cpu->cfg.ext_s = ext & RVS;
-cpu->cfg.ext_u = ext & RVU;
-cpu->cfg.ext_h = ext & RVH;
-cpu->cfg.ext_j = ext & RVJ;
+riscv_set_cpucfg_with_misa(_archcpu(env)->cfg, ext);
 }
 
 #ifndef CONFIG_USER_ONLY
@@ -1156,42 +1197,7 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU 
*cpu, Error **errp)
  */
 riscv_cpu_disable_priv_spec_isa_exts(cpu);
 
-if (cpu->cfg.ext_i) {
-ext |= RVI;
-}
-if (cpu->cfg.ext_e) {
-ext |= RVE;
-}
-if (cpu->cfg.ext_m) {
-ext |= RVM;
-}
-if (cpu->cfg.ext_a) {
-ext |= RVA;
-}
-if (cpu->cfg.ext_f) {
-ext |= RVF;
-}
-if (cpu->cfg.ext_d) {
-ext |= RVD;
-}
-if (cpu->cfg.ext_c) {
-ext |= RVC;
-}
-if (cpu->cfg.ext_s) {
-ext |= RVS;
-}
-if (cpu->cfg.ext_u) {
-ext |= RVU;
-}
-if (cpu->cfg.ext_h) {
-ext |= RVH;
-}
-if (cpu->cfg.ext_v) {
-ext |= RVV;
-}
-if (cpu->cfg.ext_j) {
-ext |= RVJ;
-}
+ext = riscv_get_misa_ext_with_cpucfg(>cfg);
 
 env->misa_ext_mask = env->misa_ext = ext;
 }
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index ebe0fff668..2263629332 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -66,7 +66,8 @@
 #define RV(x) ((target_ulong)1 << (x - 'A'))
 
 /*
- * Consider updating set_misa() when adding new
+ * Consider updating riscv_get_misa_ext_with_cpucfg()
+ * and riscv_set_cpucfg_with_misa() when adding new
  * MISA bits here.
  */
 #define RVI RV('I')
-- 
2.39.2

[PATCH for-8.1 v2 20/26] target/riscv: move riscv_cpu_validate_v() to validate_misa_ext()

[Daniel Henrique Barboza]

riscv_cpu_validate_v() consists of checking RVV related attributes, such
as vlen and elen, and setting env->vext_spec.

This can be done during riscv_cpu_validate_misa_ext() time, allowing us
to fail earlier if RVV constrains are not met.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index fa1954a850..0d8524d0d9 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1027,6 +1027,9 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU 
*cpu)
 
 static void riscv_cpu_validate_misa_ext(RISCVCPU *cpu, Error **errp)
 {
+CPURISCVState *env = >env;
+Error *local_err = NULL;
+
 if (cpu->cfg.ext_i && cpu->cfg.ext_e) {
 error_setg(errp,
"I and E extensions are incompatible");
@@ -1073,6 +1076,12 @@ static void riscv_cpu_validate_misa_ext(RISCVCPU *cpu, 
Error **errp)
 error_setg(errp, "V extension requires D and F extensions");
 return;
 }
+
+riscv_cpu_validate_v(env, >cfg, _err);
+if (local_err != NULL) {
+error_propagate(errp, local_err);
+return;
+}
 }
 }
 
@@ -,7 +1120,6 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, 
Error **errp)
 static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
 {
 CPURISCVState *env = >env;
-Error *local_err = NULL;
 uint32_t ext = 0;
 
 if (cpu->cfg.epmp && !cpu->cfg.pmp) {
@@ -1202,14 +1210,6 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU 
*cpu, Error **errp)
 }
 }
 
-if (cpu->cfg.ext_v) {
-riscv_cpu_validate_v(env, >cfg, _err);
-if (local_err != NULL) {
-error_propagate(errp, local_err);
-return;
-}
-}
-
 if (cpu->cfg.ext_zk) {
 cpu->cfg.ext_zkn = true;
 cpu->cfg.ext_zkr = true;
-- 
2.39.2

[PATCH for-8.1 v2 05/26] target/riscv/cpu.c: add priv_spec validate/disable_exts helpers

[Daniel Henrique Barboza]

We're doing env->priv_spec validation and assignment at the start of
riscv_cpu_realize(), which is fine, but then we're doing a force disable
on extensions that aren't compatible with the priv version.

This second step is being done too early. The disabled extensions might be
re-enabled again in riscv_cpu_validate_set_extensions() by accident. A
better place to put this code is at the end of
riscv_cpu_validate_set_extensions() after all the validations are
completed.

Add a new helper, riscv_cpu_disable_priv_spec_isa_exts(), to disable the
extesions after the validation is done. While we're at it, create a
riscv_cpu_validate_priv_spec() helper to host all env->priv_spec related
validation to unclog riscv_cpu_realize a bit.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 91 --
 1 file changed, 56 insertions(+), 35 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1ee322001b..17b301967c 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -833,6 +833,52 @@ static void riscv_cpu_validate_v(CPURISCVState *env, 
RISCVCPUConfig *cfg,
 env->vext_ver = vext_version;
 }
 
+static void riscv_cpu_validate_priv_spec(RISCVCPU *cpu, Error **errp)
+{
+CPURISCVState *env = >env;
+int priv_version = -1;
+
+if (cpu->cfg.priv_spec) {
+if (!g_strcmp0(cpu->cfg.priv_spec, "v1.12.0")) {
+priv_version = PRIV_VERSION_1_12_0;
+} else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
+priv_version = PRIV_VERSION_1_11_0;
+} else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
+priv_version = PRIV_VERSION_1_10_0;
+} else {
+error_setg(errp,
+   "Unsupported privilege spec version '%s'",
+   cpu->cfg.priv_spec);
+return;
+}
+
+env->priv_ver = priv_version;
+}
+}
+
+static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
+{
+CPURISCVState *env = >env;
+int i;
+
+/* Force disable extensions if priv spec version does not match */
+for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
+if (isa_ext_is_enabled(cpu, _edata_arr[i]) &&
+(env->priv_ver < isa_edata_arr[i].min_version)) {
+isa_ext_update_enabled(cpu, _edata_arr[i], false);
+#ifndef CONFIG_USER_ONLY
+warn_report("disabling %s extension for hart 0x" TARGET_FMT_lx
+" because privilege spec version does not match",
+isa_edata_arr[i].name, env->mhartid);
+#else
+warn_report("disabling %s extension because "
+"privilege spec version does not match",
+isa_edata_arr[i].name);
+#endif
+}
+}
+}
+
 /*
  * Check consistency between chosen extensions while setting
  * cpu->cfg accordingly, doing a set_misa() in the end.
@@ -1002,6 +1048,12 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU 
*cpu, Error **errp)
 cpu->cfg.ext_zksh = true;
 }
 
+/*
+ * Disable isa extensions based on priv spec after we
+ * validated and set everything we need.
+ */
+riscv_cpu_disable_priv_spec_isa_exts(cpu);
+
 if (cpu->cfg.ext_i) {
 ext |= RVI;
 }
@@ -1131,7 +1183,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 CPURISCVState *env = >env;
 RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
 CPUClass *cc = CPU_CLASS(mcc);
-int i, priv_version = -1;
 Error *local_err = NULL;
 
 cpu_exec_realizefn(cs, _err);
@@ -1140,40 +1191,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
-if (cpu->cfg.priv_spec) {
-if (!g_strcmp0(cpu->cfg.priv_spec, "v1.12.0")) {
-priv_version = PRIV_VERSION_1_12_0;
-} else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
-priv_version = PRIV_VERSION_1_11_0;
-} else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
-priv_version = PRIV_VERSION_1_10_0;
-} else {
-error_setg(errp,
-   "Unsupported privilege spec version '%s'",
-   cpu->cfg.priv_spec);
-return;
-}
-}
-
-if (priv_version >= PRIV_VERSION_1_10_0) {
-env->priv_ver = priv_version;
-}
-
-/* Force disable extensions if priv spec version does not match */
-for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
-if (isa_ext_is_enabled(cpu, _edata_arr[i]) &&
-(env->priv_ver < isa_edata_arr[i].min_version)) {
-isa_ext_update_enabled(cpu, _edata_arr[i], false);
-#ifndef CONFIG_USER_ONLY
-warn_report("disabling %s extension for hart 0x" TARGET_FMT_lx
-" because privilege spec version does not match",
-isa_edata_arr[i].name, env->mhartid);
-#else
-warn_report("disabling %s extension because "
-   

[PATCH for-8.1 v2 00/26] target/riscv: rework CPU extensions validation

[Daniel Henrique Barboza]

Hello,

In this v2 the most notable changes were done after Liu Zhiwei review in
[1], in particular the comments made in patch 17. To allow for
write_misa() validation, without the need to store and restore cpu->cfg
state, more design changes were required in the existing validation
logic.

The validation code was split in three stages: validate misa_ext,
validate cpu config and commit cpu config. riscv_cpu_validate_misa_ext()
handles all validations related exclusively to env->misa_ext bits.
riscv_cpu_validate_extensions() does the remaining validations with the
named extensions we have. riscv_cpu_commit_cpu_cfg() is the last step,
only executed after all the previous validations were ok.

All validations are done using a tentative misa_ext val, instead of
env->misa_ext or cpu->cfg.ext_N props. write_misa() is then able to
validate a misa_ext without having to change cpu->cfg needlesly. 

Another change is that now we're forcing a sync between env->misa_ext
and cpu->cfg. This was needed to allow for the validation split
mentioned above. It'll also give more consistency throughout the code,
granting that we're always getting the same information whether we're
using cpu->cfg or an API such as riscv_has_ext().

All other premises from v1 are kept. All code changes suggested in v1
were implemented.

Patches are based on Alistair's riscv-to-apply.next.


Changes from v1:
- patch 14 ("target/riscv/cpu.c: do not allow RVE to be set"): dropped 
- patch 4:
  - PRIV_VERSION_LATEST is now an enum value instead of a macro
- patch 5:
  - merged env->priv_ver cond assignment to the previous if clause
- a handful of patches added to allow for validate_set_extensions() to
  be split in three functions
- validation in write_misa() does not require commit changes to cpu->cfg
  beforehand
- v1 link: https://lists.gnu.org/archive/html/qemu-devel/2023-03/msg03219.html

[1] https://lists.gnu.org/archive/html/qemu-devel/2023-03/msg03219.html

Daniel Henrique Barboza (26):
  target/riscv/cpu.c: add riscv_cpu_validate_v()
  target/riscv/cpu.c: remove set_vext_version()
  target/riscv/cpu.c: remove set_priv_version()
  target/riscv: add PRIV_VERSION_LATEST
  target/riscv/cpu.c: add priv_spec validate/disable_exts helpers
  target/riscv/cpu.c: add riscv_cpu_validate_misa_mxl()
  target/riscv: move pmp and epmp validations to
validate_set_extensions()
  target/riscv/cpu.c: validate extensions before riscv_timer_init()
  target/riscv/cpu.c: remove cfg setup from riscv_cpu_init()
  target/riscv/cpu.c: avoid set_misa() in validate_set_extensions()
  target/riscv/cpu.c: set cpu config in set_misa()
  target/riscv/cpu.c: redesign register_cpu_props()
  target/riscv: put env->misa_ext <-> cpu->cfg code into helpers
  target/riscv: add RVG
  target/riscv: do not allow RVG in write_misa()
  target/riscv/cpu.c: split RVG code from validate_set_extensions()
  target/riscv: write env->misa_ext* in register_generic_cpu_props()
  target/risc/cpu.c: add riscv_cpu_validate_misa_ext()
  target/riscv/cpu:c add misa_ext V-> D & F dependency
  target/riscv: move riscv_cpu_validate_v() to validate_misa_ext()
  target/riscv: validate_misa_ext() now validates a misa_ext val
  target/riscv: error out on priv failure for RVH
  target/riscv: split riscv_cpu_validate_set_extensions()
  target/riscv: use misa_ext val in riscv_cpu_validate_extensions()
  target/riscv: rework write_misa()
  target/riscv: update cpu->cfg misa bits in commit_cpu_cfg()

 target/riscv/cpu.c | 661 -
 target/riscv/cpu.h |  14 +-
 target/riscv/csr.c |  47 ++--
 3 files changed, 448 insertions(+), 274 deletions(-)

-- 
2.39.2

[PATCH for-8.1 v2 21/26] target/riscv: validate_misa_ext() now validates a misa_ext val

[Daniel Henrique Barboza]

We have all MISA specific validations in riscv_cpu_validate_misa_ext(),
and we have a guarantee that env->misa_ext will always be in sync with
cpu->cfg at this point during realize time, so let's convert it to use a
'misa_ext' parameter instead of reading cpu->cfg.

This will prepare the function to be used in write_misa() where we won't
have an updated cpu->cfg object to work with. riscv_cpu_validate_v() is
changed to receive a const pointer to the cpu->cfg object via
riscv_cpu_cfg().

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 30 +-
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 0d8524d0d9..f8f416d6dd 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -939,7 +939,8 @@ static void riscv_cpu_disas_set_info(CPUState *s, 
disassemble_info *info)
 }
 }
 
-static void riscv_cpu_validate_v(CPURISCVState *env, RISCVCPUConfig *cfg,
+static void riscv_cpu_validate_v(CPURISCVState *env,
+ const RISCVCPUConfig *cfg,
  Error **errp)
 {
 int vext_version = VEXT_VERSION_1_00_0;
@@ -1025,46 +1026,48 @@ static void 
riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
 }
 }
 
-static void riscv_cpu_validate_misa_ext(RISCVCPU *cpu, Error **errp)
+
+static void riscv_cpu_validate_misa_ext(CPURISCVState *env,
+uint32_t misa_ext,
+Error **errp)
 {
-CPURISCVState *env = >env;
 Error *local_err = NULL;
 
-if (cpu->cfg.ext_i && cpu->cfg.ext_e) {
+if (misa_ext & RVI && misa_ext & RVE) {
 error_setg(errp,
"I and E extensions are incompatible");
 return;
 }
 
-if (!cpu->cfg.ext_i && !cpu->cfg.ext_e) {
+if (!(misa_ext & RVI) && !(misa_ext & RVE)) {
 error_setg(errp,
"Either I or E extension must be set");
 return;
 }
 
-if (cpu->cfg.ext_s && !cpu->cfg.ext_u) {
+if (misa_ext & RVS && !(misa_ext & RVU)) {
 error_setg(errp,
"Setting S extension without U extension is illegal");
 return;
 }
 
-if (cpu->cfg.ext_h && !cpu->cfg.ext_i) {
+if (misa_ext & RVH && !(misa_ext & RVI)) {
 error_setg(errp,
"H depends on an I base integer ISA with 32 x registers");
 return;
 }
 
-if (cpu->cfg.ext_h && !cpu->cfg.ext_s) {
+if (misa_ext & RVH && !(misa_ext & RVS)) {
 error_setg(errp, "H extension implicitly requires S-mode");
 return;
 }
 
-if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
+if (misa_ext & RVD && !(misa_ext & RVF)) {
 error_setg(errp, "D extension requires F extension");
 return;
 }
 
-if (cpu->cfg.ext_v) {
+if (misa_ext & RVV) {
 /*
  * V depends on Zve64d, which requires D. It also
  * depends on Zve64f, which depends on Zve32f,
@@ -1072,12 +1075,12 @@ static void riscv_cpu_validate_misa_ext(RISCVCPU *cpu, 
Error **errp)
  *
  * This means that V depends on both D and F.
  */
-if (!(cpu->cfg.ext_d && cpu->cfg.ext_f)) {
+if (!(misa_ext & RVD && misa_ext & RVF)) {
 error_setg(errp, "V extension requires D and F extensions");
 return;
 }
 
-riscv_cpu_validate_v(env, >cfg, _err);
+riscv_cpu_validate_v(env, riscv_cpu_cfg(env), _err);
 if (local_err != NULL) {
 error_propagate(errp, local_err);
 return;
@@ -1331,6 +1334,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 CPUState *cs = CPU(dev);
 RISCVCPU *cpu = RISCV_CPU(dev);
 RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
+CPURISCVState *env = >env;
 Error *local_err = NULL;
 
 cpu_exec_realizefn(cs, _err);
@@ -1355,7 +1359,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 riscv_set_G_virt_ext(cpu);
 }
 
-riscv_cpu_validate_misa_ext(cpu, _err);
+riscv_cpu_validate_misa_ext(env, env->misa_ext, _err);
 if (local_err != NULL) {
 error_propagate(errp, local_err);
 return;
-- 
2.39.2

[PATCH for-8.1 v2 09/26] target/riscv/cpu.c: remove cfg setup from riscv_cpu_init()

[Daniel Henrique Barboza]

We have 4 config settings being done in riscv_cpu_init(): ext_ifencei,
ext_icsr, mmu and pmp. This is also the constructor of the "riscv-cpu"
device, which happens to be the parent device of every RISC-V cpu.

The result is that these 4 configs are being set every time, and every
other CPU should always account for them. CPUs such as sifive_e need to
disable settings that aren't enabled simply because the parent class
happens to be enabling it.

Moving all configurations from the parent class to each CPU will
centralize the config of each CPU into its own init(), which is clearer
than having to account to whatever happens to be set in the parent
device. These settings are also being set in register_cpu_props() when
no 'misa_ext' is set, so for these CPUs we don't need changes. Named
CPUs will receive all cfgs that the parent were setting into their
init().

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 60 --
 1 file changed, 48 insertions(+), 12 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index fef55d7d79..c7b6e7b84b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -325,7 +325,8 @@ static void set_satp_mode_default_map(RISCVCPU *cpu)
 
 static void riscv_any_cpu_init(Object *obj)
 {
-CPURISCVState *env = _CPU(obj)->env;
+RISCVCPU *cpu = RISCV_CPU(obj);
+CPURISCVState *env = >env;
 #if defined(TARGET_RISCV32)
 set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
 #elif defined(TARGET_RISCV64)
@@ -340,6 +341,12 @@ static void riscv_any_cpu_init(Object *obj)
 
 env->priv_ver = PRIV_VERSION_LATEST;
 register_cpu_props(obj);
+
+/* inherited from parent obj via riscv_cpu_init() */
+cpu->cfg.ext_ifencei = true;
+cpu->cfg.ext_icsr = true;
+cpu->cfg.mmu = true;
+cpu->cfg.pmp = true;
 }
 
 #if defined(TARGET_RISCV64)
@@ -358,13 +365,20 @@ static void rv64_base_cpu_init(Object *obj)
 
 static void rv64_sifive_u_cpu_init(Object *obj)
 {
-CPURISCVState *env = _CPU(obj)->env;
+RISCVCPU *cpu = RISCV_CPU(obj);
+CPURISCVState *env = >env;
 set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
 register_cpu_props(obj);
 env->priv_ver = PRIV_VERSION_1_10_0;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV39);
 #endif
+
+/* inherited from parent obj via riscv_cpu_init() */
+cpu->cfg.ext_ifencei = true;
+cpu->cfg.ext_icsr = true;
+cpu->cfg.mmu = true;
+cpu->cfg.pmp = true;
 }
 
 static void rv64_sifive_e_cpu_init(Object *obj)
@@ -375,10 +389,14 @@ static void rv64_sifive_e_cpu_init(Object *obj)
 set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
 register_cpu_props(obj);
 env->priv_ver = PRIV_VERSION_1_10_0;
-cpu->cfg.mmu = false;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
 #endif
+
+/* inherited from parent obj via riscv_cpu_init() */
+cpu->cfg.ext_ifencei = true;
+cpu->cfg.ext_icsr = true;
+cpu->cfg.pmp = true;
 }
 
 static void rv64_thead_c906_cpu_init(Object *obj)
@@ -411,6 +429,10 @@ static void rv64_thead_c906_cpu_init(Object *obj)
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(cpu, VM_1_10_SV39);
 #endif
+
+/* inherited from parent obj via riscv_cpu_init() */
+cpu->cfg.ext_ifencei = true;
+cpu->cfg.pmp = true;
 }
 
 static void rv128_base_cpu_init(Object *obj)
@@ -447,7 +469,8 @@ static void rv32_base_cpu_init(Object *obj)
 
 static void rv32_sifive_u_cpu_init(Object *obj)
 {
-CPURISCVState *env = _CPU(obj)->env;
+RISCVCPU *cpu = RISCV_CPU(obj);
+CPURISCVState *env = >env;
 set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
 
 register_cpu_props(obj);
@@ -455,6 +478,12 @@ static void rv32_sifive_u_cpu_init(Object *obj)
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV32);
 #endif
+
+/* inherited from parent obj via riscv_cpu_init() */
+cpu->cfg.ext_ifencei = true;
+cpu->cfg.ext_icsr = true;
+cpu->cfg.mmu = true;
+cpu->cfg.pmp = true;
 }
 
 static void rv32_sifive_e_cpu_init(Object *obj)
@@ -465,10 +494,14 @@ static void rv32_sifive_e_cpu_init(Object *obj)
 set_misa(env, MXL_RV32, RVI | RVM | RVA | RVC | RVU);
 register_cpu_props(obj);
 env->priv_ver = PRIV_VERSION_1_10_0;
-cpu->cfg.mmu = false;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
 #endif
+
+/* inherited from parent obj via riscv_cpu_init() */
+cpu->cfg.ext_ifencei = true;
+cpu->cfg.ext_icsr = true;
+cpu->cfg.pmp = true;
 }
 
 static void rv32_ibex_cpu_init(Object *obj)
@@ -479,11 +512,15 @@ static void rv32_ibex_cpu_init(Object *obj)
 set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU);
 register_cpu_props(obj);
 env->priv_ver = PRIV_VERSION_1_11_0;
-cpu->cfg.mmu = false;
 #ifndef CONFIG_USER_ONLY
 set_satp_mode_max_supported(cpu, 

[PATCH for-8.1 v2 17/26] target/riscv: write env->misa_ext* in register_generic_cpu_props()

[Daniel Henrique Barboza]

In the process of creating the user-facing flags in
register_generic_cpu_props() we're also setting default values for the
cpu->cfg flags that represents MISA bits.

Leaving it as is will cause a discrepancy between users of this function
(at this moment the non-named CPUs) and named CPUs. Named CPUs are using
set_misa() with a non-zero 'ext' value, writing cpu->cfg in the process.
They'll reach riscv_cpu_realize() in a state where env->misa_ext will
reflect cpu->cfg, allowing functions to choose whether to use
env->misa_ext or cpu->cfg to validate MISA bits.

If we guarantee that env->misa_ext will always reflect cpu->cfg at the
start of riscv_cpu_realize(), functions will be able to no longer rely
on cpu->cfg for MISA validation. This happens to be one blocker we have to
properly support write_misa().

Sync env->misa_ext* in register_generic_cpu_props(). This will leave
only a single place where there's a cpu->cfg change that needs to be
converted back to env->misa_ext*: right after disabling priv spec
extensions, at the end of riscv_cpu_validate_set_extensions(). We'll
deal with it shortly.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 133807e39f..af5a1e6a43 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1595,10 +1595,12 @@ static Property riscv_cpu_extensions[] = {
  * Register generic CPU props with user-facing flags declared
  * in riscv_cpu_extensions[].
  *
- * Note that this will overwrite existing values in cpu->cfg.
+ * Note that this will overwrite existing values in cpu->cfg
+ * and MISA.
  */
 static void register_generic_cpu_props(Object *obj)
 {
+RISCVCPU *cpu = RISCV_CPU(obj);
 Property *prop;
 DeviceState *dev = DEVICE(obj);
 
@@ -1609,6 +1611,10 @@ static void register_generic_cpu_props(Object *obj)
 #ifndef CONFIG_USER_ONLY
 riscv_add_satp_mode_properties(obj);
 #endif
+
+/* Keep env->misa_ext and misa_ext_mask updated */
+cpu->env.misa_ext = riscv_get_misa_ext_with_cpucfg(>cfg);
+cpu->env.misa_ext_mask = cpu->env.misa_ext;
 }
 
 static Property riscv_cpu_properties[] = {
-- 
2.39.2

[PATCH for-8.1 v2 08/26] target/riscv/cpu.c: validate extensions before riscv_timer_init()

[Daniel Henrique Barboza]

There is no need to init timers if we're not even sure that our
extensions are valid. Execute riscv_cpu_validate_set_extensions() before
riscv_timer_init().

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 7458845fec..fef55d7d79 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1237,12 +1237,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
-#ifndef CONFIG_USER_ONLY
-if (cpu->cfg.ext_sstc) {
-riscv_timer_init(cpu);
-}
-#endif /* CONFIG_USER_ONLY */
-
 riscv_cpu_validate_set_extensions(cpu, _err);
 if (local_err != NULL) {
 error_propagate(errp, local_err);
@@ -1250,6 +1244,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 }
 
 #ifndef CONFIG_USER_ONLY
+if (cpu->cfg.ext_sstc) {
+riscv_timer_init(cpu);
+}
+
 if (cpu->cfg.pmu_num) {
 if (!riscv_pmu_init(cpu, cpu->cfg.pmu_num) && cpu->cfg.ext_sscofpmf) {
 cpu->pmu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
-- 
2.39.2

[PATCH for-8.1 v2 01/26] target/riscv/cpu.c: add riscv_cpu_validate_v()

[Daniel Henrique Barboza]

The RVV verification will error out if fails and it's being done at the
end of riscv_cpu_validate_set_extensions(). Let's put it in its own
function and do it earlier.

We'll move it out of riscv_cpu_validate_set_extensions() in the near future,
but for now this is enough to clean the code a bit.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/cpu.c | 86 ++
 1 file changed, 49 insertions(+), 37 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1e97473af2..18591aa53a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -802,6 +802,46 @@ static void riscv_cpu_disas_set_info(CPUState *s, 
disassemble_info *info)
 }
 }
 
+static void riscv_cpu_validate_v(CPURISCVState *env, RISCVCPUConfig *cfg,
+ Error **errp)
+{
+int vext_version = VEXT_VERSION_1_00_0;
+
+if (!is_power_of_2(cfg->vlen)) {
+error_setg(errp, "Vector extension VLEN must be power of 2");
+return;
+}
+if (cfg->vlen > RV_VLEN_MAX || cfg->vlen < 128) {
+error_setg(errp,
+   "Vector extension implementation only supports VLEN "
+   "in the range [128, %d]", RV_VLEN_MAX);
+return;
+}
+if (!is_power_of_2(cfg->elen)) {
+error_setg(errp, "Vector extension ELEN must be power of 2");
+return;
+}
+if (cfg->elen > 64 || cfg->elen < 8) {
+error_setg(errp,
+   "Vector extension implementation only supports ELEN "
+   "in the range [8, 64]");
+return;
+}
+if (cfg->vext_spec) {
+if (!g_strcmp0(cfg->vext_spec, "v1.0")) {
+vext_version = VEXT_VERSION_1_00_0;
+} else {
+error_setg(errp, "Unsupported vector spec version '%s'",
+   cfg->vext_spec);
+return;
+}
+} else {
+qemu_log("vector version is not specified, "
+ "use the default value v1.0\n");
+}
+set_vext_version(env, vext_version);
+}
+
 /*
  * Check consistency between chosen extensions while setting
  * cpu->cfg accordingly, doing a set_misa() in the end.
@@ -809,6 +849,7 @@ static void riscv_cpu_disas_set_info(CPUState *s, 
disassemble_info *info)
 static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
 {
 CPURISCVState *env = >env;
+Error *local_err = NULL;
 uint32_t ext = 0;
 
 /* Do some ISA extension error checking */
@@ -939,6 +980,14 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU 
*cpu, Error **errp)
 }
 }
 
+if (cpu->cfg.ext_v) {
+riscv_cpu_validate_v(env, >cfg, _err);
+if (local_err != NULL) {
+error_propagate(errp, local_err);
+return;
+}
+}
+
 if (cpu->cfg.ext_zk) {
 cpu->cfg.ext_zkn = true;
 cpu->cfg.ext_zkr = true;
@@ -993,44 +1042,7 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU 
*cpu, Error **errp)
 ext |= RVH;
 }
 if (cpu->cfg.ext_v) {
-int vext_version = VEXT_VERSION_1_00_0;
 ext |= RVV;
-if (!is_power_of_2(cpu->cfg.vlen)) {
-error_setg(errp,
-   "Vector extension VLEN must be power of 2");
-return;
-}
-if (cpu->cfg.vlen > RV_VLEN_MAX || cpu->cfg.vlen < 128) {
-error_setg(errp,
-   "Vector extension implementation only supports VLEN "
-   "in the range [128, %d]", RV_VLEN_MAX);
-return;
-}
-if (!is_power_of_2(cpu->cfg.elen)) {
-error_setg(errp,
-   "Vector extension ELEN must be power of 2");
-return;
-}
-if (cpu->cfg.elen > 64 || cpu->cfg.elen < 8) {
-error_setg(errp,
-   "Vector extension implementation only supports ELEN "
-   "in the range [8, 64]");
-return;
-}
-if (cpu->cfg.vext_spec) {
-if (!g_strcmp0(cpu->cfg.vext_spec, "v1.0")) {
-vext_version = VEXT_VERSION_1_00_0;
-} else {
-error_setg(errp,
-   "Unsupported vector spec version '%s'",
-   cpu->cfg.vext_spec);
-return;
-}
-} else {
-qemu_log("vector version is not specified, "
- "use the default value v1.0\n");
-}
-set_vext_version(env, vext_version);
 }
 if (cpu->cfg.ext_j) {
 ext |= RVJ;
-- 
2.39.2

Re: [PATCH] tests/qtest/migration-test: Disable migration/multifd/tcp/plain/cancel

[Peter Xu]

On Tue, Mar 14, 2023 at 10:11:53AM +, Dr. David Alan Gilbert wrote:
> OK, I think I kind of see what's happening here, one for Peter Xu.
> If I'm right it's a race something like:
>   a) The test harness tells the source it wants to enter postcopy
>   b) The harness then waits for the source to stop
>   c) ... and the dest to start 
> 
>   It's blocked on one of b but can't tell which
> 
>   d) The main thread in the dest is waiting for the postcopy recovery fd
> to be opened
>   e) But I think the source is still trying to send normal precopy RAM
> and perhaps hasn't got around yet to opening that socket yet
>   f) But I think the dest isn't reading from the main channel at that
> point because of (d)

I think this analysis is spot on.  Thanks Dave!

Src qemu does this with below order:

1. setup preempt channel
1.1. connect()  --> this is done in another thread
1.2. sem_wait(postcopy_qemufile_src_sem) --> make sure it's created
2. prepare postcopy package (LISTEN, non-iterable states, ping-3, RUN)
3. send the package

So logically the sequence is guaranteed so that when LISTEN cmd is
processed, we should have connect()ed already.

But I think there's one thing missing on dest.. since the accept() on the
dest node should be run in the main thread, meanwhile the LISTEN cmd is
also processed on the main thread, even if the listening socket is trying
to kick the main thread to do the accept() (so the connection has
established) it won't be able to kick the final accept() as main thread is
waiting in the semaphore.  That caused a deadlock.

A simple fix I can think of is moving the wait channel operation outside
the main thread, e.g. to the preempt thread.

I've attached that simple fix.  Peter, is it easy to verify it?  I'm not
sure the reproducability, fine by me too if it's easier to just disable
preempt tests for 8.0 release.

Thanks,

-- 
Peter Xu
>From 92f2f90d2eb270ca158479bfd9a5a855ec7ddf4d Mon Sep 17 00:00:00 2001
From: Peter Xu 
Date: Tue, 14 Mar 2023 12:24:02 -0400
Subject: [PATCH] migration: Wait on preempt channel in preempt thread

QEMU main thread will wait until dest preempt channel established during
processing the LISTEN command (within the whole postcopy PACKAGED data), by
waiting on the semaphore postcopy_qemufile_dst_done.

That's racy, because it's possible that the dest QEMU main thread hasn't
yet accept()ed the new connection when processing the LISTEN event.  The
sem_wait() will yield the main thread without being able to run anything
else including the accept() of the new socket, which can cause deadlock
within the main thread.

To avoid the race, move the "wait channel" from main thread to the preempt
thread right at the start.

Reported-by: Peter Maydell 
Fixes: 5655aab079 ("migration: Postpone postcopy preempt channel to be after 
main")
Signed-off-by: Peter Xu 
---
 migration/postcopy-ram.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index f54f44d899..41c0713650 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1197,11 +1197,6 @@ int postcopy_ram_incoming_setup(MigrationIncomingState 
*mis)
 }
 
 if (migrate_postcopy_preempt()) {
-/*
- * The preempt channel is established in asynchronous way.  Wait
- * for its completion.
- */
-qemu_sem_wait(>postcopy_qemufile_dst_done);
 /*
  * This thread needs to be created after the temp pages because
  * it'll fetch RAM_CHANNEL_POSTCOPY PostcopyTmpPage immediately.
@@ -1668,6 +1663,12 @@ void *postcopy_preempt_thread(void *opaque)
 
 qemu_sem_post(>thread_sync_sem);
 
+/*
+ * The preempt channel is established in asynchronous way.  Wait
+ * for its completion.
+ */
+qemu_sem_wait(>postcopy_qemufile_dst_done);
+
 /* Sending RAM_SAVE_FLAG_EOS to terminate this thread */
 qemu_mutex_lock(>postcopy_prio_thread_mutex);
 while (1) {
-- 
2.39.1

Re: [PATCH 2/2] tests/tcg/s390x: Add early-exception-recognition.S

[Richard Henderson]

On 3/14/23 04:00, Ilya Leoshkevich wrote:

Add a small test that checks whether early exceptions are recognized
and whether the correct ILC and old PSW are stored when they happen.

Signed-off-by: Ilya Leoshkevich
---
  tests/tcg/s390x/Makefile.softmmu-target   |  1 +
  tests/tcg/s390x/early-exception-recognition.S | 38 +++
  2 files changed, 39 insertions(+)
  create mode 100644 tests/tcg/s390x/early-exception-recognition.S


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 1/2] target/s390x: Implement Early Exception Recognition

[Richard Henderson]

On 3/14/23 04:00, Ilya Leoshkevich wrote:

Generate specification exception if a reserved bit is set in the PSW
mask or if the PSW address is out of bounds dictated by the addresing
mode.

Reported-by: Nina Schoetterl-Glausch
Signed-off-by: Ilya Leoshkevich
---
  target/s390x/cpu.c | 26 ++
  target/s390x/cpu.h |  1 +
  target/s390x/tcg/excp_helper.c |  3 ++-
  3 files changed, 29 insertions(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~