Re: [PATCH v4 05/24] Revert "replay: stop us hanging in rr_wait_io_event"
On Wed Mar 13, 2024 at 7:03 AM AEST, Alex Bennée wrote: > "Nicholas Piggin" writes: > > > On Tue Mar 12, 2024 at 11:33 PM AEST, Alex Bennée wrote: > >> Nicholas Piggin writes: > >> > >> > This reverts commit 1f881ea4a444ef36a8b6907b0b82be4b3af253a2. > >> > > >> > That commit causes reverse_debugging.py test failures, and does > >> > not seem to solve the root cause of the problem x86-64 still > >> > hangs in record/replay tests. > >> > >> I'm still finding the reverse debugging tests failing with this series. > > > > :( > > > > In gitlab CI or your own testing? What are you running exactly? > > My own - my mistake I didn't get a clean build because of the format > bug. However I'm seeing new failures: > > env QEMU_TEST_FLAKY_TESTS=1 AVOCADO_TIMEOUT_EXPECTED=1 ./pyvenv/bin/avocado > run ./tests/avocado/reverse_debugging.py > Fetching asset from > ./tests/avocado/reverse_debugging.py:ReverseDebugging_AArch64.test_aarch64_virt > JOB ID : bd4b29f7afaa24dc6e32933ea9bc5e46bbc3a5a4 > JOB LOG: > /home/alex/avocado/job-results/job-2024-03-12T20.58-bd4b29f/job.log >(1/5) > ./tests/avocado/reverse_debugging.py:ReverseDebugging_X86_64.test_x86_64_pc: > PASS (4.49 s) >(2/5) > ./tests/avocado/reverse_debugging.py:ReverseDebugging_X86_64.test_x86_64_q35: > PASS (4.50 s) >(3/5) > ./tests/avocado/reverse_debugging.py:ReverseDebugging_AArch64.test_aarch64_virt: > FAIL: Invalid PC (read 2d941e4d7f28 instead of 2d941e4d7f2c) (3.06 s) Okay, this is the new test I added. It runs for 1 second then reverse-steps from the end of the trace. aarch64 is flaky -- pc is at a different place at the same icount after the reverse-step (which is basically the second replay). This indicates some non-determinism in execution, or something in machine reset or migration is not restoring the state exactly. aarch64 ran okay few times including gitlab CI before I posted the series, but turns out it does break quite often too. x86 has a problem with this too so I disabled it there. I'll disable it for aarch64 too for now. x86 and aarch64 can run the replay_linux.py test quite well (after this series), which is much longer and more complicated. The difference there is that it is only a single replay, it never resets the machine or loads the initial snapshot for reverse-debugging. So to me that indicates that execution is probably deterministic, but its the reset reload that has the problem. Thanks, Nick
Re: [PATCH v2 03/10] ppc/spapr|pnv: Remove SAO from pa-features
On Thu Mar 14, 2024 at 12:34 PM AEST, David Gibson wrote:
> On Tue, Mar 12, 2024 at 11:14:12PM +1000, Nicholas Piggin wrote:
> > SAO is a page table attribute that strengthens the memory ordering of
> > accesses. QEMU with MTTCG does not implement this, so clear it in
> > ibm,pa-features. This is an obscure feature that has been removed from
> > POWER10 ISA v3.1, there isn't much concern with removing it.
> >
> > Reviewed-by: Harsh Prateek Bora
> > Signed-off-by: Nicholas Piggin
>
> Usually altering a user visible feature like this without versioning
> would be a no-no. However, I think it's probably ok here: AFAICT the
> feature was basically never used, it didn't work in some cases anyway,
> and it's now gone away.
Thanks David, I appreciate you keeping an eye on these kinds of
compatibility issues from time to time.
Yeah, we established that it doesn't really matter for Linux code out
there, but you thought it's ugly to change this based on the host
configuration for pseries machines.
And if this change does cause problems, it's quite possible that
configuration was broken anyway, so that's arguably preferable to
continuing to advertise a broken or at least non-migratable feature.
Thanks,
Nick
>
> > ---
> > hw/ppc/pnv.c | 2 +-
> > hw/ppc/spapr.c | 14 ++
> > 2 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 0b47b92baa..aa9786e970 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -150,7 +150,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc,
> > void *fdt)
> > uint32_t page_sizes_prop[64];
> > size_t page_sizes_prop_size;
> > const uint8_t pa_features[] = { 24, 0,
> > -0xf6, 0x3f, 0xc7, 0xc0, 0x80, 0xf0,
> > +0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0,
> > 0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
> > 0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
> > 0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 55263f0815..3108d7c532 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -233,17 +233,23 @@ static void spapr_dt_pa_features(SpaprMachineState
> > *spapr,
> > PowerPCCPU *cpu,
> > void *fdt, int offset)
> > {
> > +/*
> > + * SSO (SAO) ordering is supported on KVM and thread=single hosts,
> > + * but not MTTCG, so disable it. To advertise it, a cap would have
> > + * to be added, or support implemented for MTTCG.
> > + */
> > +
> > uint8_t pa_features_206[] = { 6, 0,
> > -0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 };
> > +0xf6, 0x1f, 0xc7, 0x00, 0x00, 0xc0 };
> > uint8_t pa_features_207[] = { 24, 0,
> > -0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0,
> > +0xf6, 0x1f, 0xc7, 0xc0, 0x00, 0xf0,
> > 0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
> > 0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
> > 0x80, 0x00, 0x80, 0x00, 0x00, 0x00 };
> > uint8_t pa_features_300[] = { 66, 0,
> > /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: fri[nzpm]|DABRX|SPRG3|SLB0|PP110
> > */
> > -/* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, SSO, 5:
> > LE|CFAR|EB|LSQ */
> > -0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0, /* 0 - 5 */
> > +/* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
> > +0xf6, 0x1f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
> > /* 6: DS207 */
> > 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
> > /* 16: Vector */
Re: [PATCH-for-9.0? 05/12] target/ppc: Rename init_excp_4xx_softmmu() -> init_excp_4xx()
On Thu Mar 14, 2024 at 7:33 AM AEST, Philippe Mathieu-Daudé wrote:
> Unify with other init_excp_FOO() in the same file.
>
> Signed-off-by: Philippe Mathieu-Daudé
Acked-by: Nicholas Piggin
> ---
> target/ppc/cpu_init.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 7e65f08147..b208bd91a0 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -1642,7 +1642,7 @@ static void register_8xx_sprs(CPUPPCState *env)
>
>
> /*/
> /* Exception vectors models
> */
> -static void init_excp_4xx_softmmu(CPUPPCState *env)
> +static void init_excp_4xx(CPUPPCState *env)
> {
> #if !defined(CONFIG_USER_ONLY)
> env->excp_vectors[POWERPC_EXCP_CRITICAL] = 0x0100;
> @@ -2120,7 +2120,7 @@ static void init_proc_405(CPUPPCState *env)
> env->id_tlbs = 0;
> env->tlb_type = TLB_EMB;
> #endif
> -init_excp_4xx_softmmu(env);
> +init_excp_4xx(env);
> env->dcache_line_size = 32;
> env->icache_line_size = 32;
> /* Allocate hardware IRQ controller */
RE: [PATCH v1 3/6] intel_iommu: Add a framework to check and sync host IOMMU cap/ecap
>-Original Message-
>From: Michael S. Tsirkin
>Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
>sync host IOMMU cap/ecap
>
>On Wed, Mar 13, 2024 at 07:54:11AM +, Duan, Zhenzhong wrote:
>>
>>
>> >-Original Message-
>> >From: Michael S. Tsirkin
>> >Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
>> >sync host IOMMU cap/ecap
>> >
>> >On Wed, Mar 13, 2024 at 02:52:39AM +, Duan, Zhenzhong wrote:
>> >> Hi Michael,
>> >>
>> >> >-Original Message-
>> >> >From: Michael S. Tsirkin
>> >> >Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check
>and
>> >> >sync host IOMMU cap/ecap
>> >> >
>> >> >On Wed, Feb 28, 2024 at 05:44:29PM +0800, Zhenzhong Duan wrote:
>> >> >> From: Yi Liu
>> >> >>
>> >> >> Add a framework to check and synchronize host IOMMU cap/ecap
>with
>> >> >> vIOMMU cap/ecap.
>> >> >>
>> >> >> The sequence will be:
>> >> >>
>> >> >> vtd_cap_init() initializes iommu->cap/ecap.
>> >> >> vtd_check_hdev() update iommu->cap/ecap based on host cap/ecap.
>> >> >> iommu->cap_frozen set when machine create done, iommu-
>>cap/ecap
>> >> >become readonly.
>> >> >>
>> >> >> Implementation details for different backends will be in following
>> >patches.
>> >> >>
>> >> >> Signed-off-by: Yi Liu
>> >> >> Signed-off-by: Yi Sun
>> >> >> Signed-off-by: Zhenzhong Duan
>> >> >> ---
>> >> >> include/hw/i386/intel_iommu.h | 1 +
>> >> >> hw/i386/intel_iommu.c | 50
>> >> >++-
>> >> >> 2 files changed, 50 insertions(+), 1 deletion(-)
>> >> >>
>> >> >> diff --git a/include/hw/i386/intel_iommu.h
>> >> >b/include/hw/i386/intel_iommu.h
>> >> >> index bbc7b96add..c71a133820 100644
>> >> >> --- a/include/hw/i386/intel_iommu.h
>> >> >> +++ b/include/hw/i386/intel_iommu.h
>> >> >> @@ -283,6 +283,7 @@ struct IntelIOMMUState {
>> >> >>
>> >> >> uint64_t cap; /* The value of capability reg */
>> >> >> uint64_t ecap; /* The value of extended
>> >> >> capability reg
>*/
>> >> >> +bool cap_frozen;/* cap/ecap become read-only after
>> >frozen */
>> >> >>
>> >> >> uint32_t context_cache_gen; /* Should be in [1,MAX] */
>> >> >> GHashTable *iotlb; /* IOTLB */
>> >> >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> >> >> index ffa1ad6429..a9f9dfd6a7 100644
>> >> >> --- a/hw/i386/intel_iommu.c
>> >> >> +++ b/hw/i386/intel_iommu.c
>> >> >> @@ -35,6 +35,8 @@
>> >> >> #include "sysemu/kvm.h"
>> >> >> #include "sysemu/dma.h"
>> >> >> #include "sysemu/sysemu.h"
>> >> >> +#include "hw/vfio/vfio-common.h"
>> >> >> +#include "sysemu/iommufd.h"
>> >> >> #include "hw/i386/apic_internal.h"
>> >> >> #include "kvm/kvm_i386.h"
>> >> >> #include "migration/vmstate.h"
>> >> >> @@ -3819,6 +3821,38 @@ VTDAddressSpace
>> >> >*vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
>> >> >> return vtd_dev_as;
>> >> >> }
>> >> >>
>> >> >> +static int vtd_check_legacy_hdev(IntelIOMMUState *s,
>> >> >> + IOMMULegacyDevice *ldev,
>> >> >> + Error **errp)
>> >> >> +{
>> >> >> +return 0;
>> >> >> +}
>> >> >> +
>> >> >> +static int vtd_check_iommufd_hdev(IntelIOMMUState *s,
>> >> >> + IOMMUFDDevice *idev,
>> >> >> + Error **errp)
>> >> >> +{
>> >> >> +return 0;
>> >> >> +}
>> >> >> +
>> >> >> +static int vtd_check_hdev(IntelIOMMUState *s,
>> >VTDHostIOMMUDevice
>> >> >*vtd_hdev,
>> >> >> + Error **errp)
>> >> >> +{
>> >> >> +HostIOMMUDevice *base_dev = vtd_hdev->dev;
>> >> >> +IOMMUFDDevice *idev;
>> >> >> +
>> >> >> +if (base_dev->type == HID_LEGACY) {
>> >> >> +IOMMULegacyDevice *ldev = container_of(base_dev,
>> >> >> + IOMMULegacyDevice,
>> >> >> base);
>> >> >> +
>> >> >> +return vtd_check_legacy_hdev(s, ldev, errp);
>> >> >> +}
>> >> >> +
>> >> >> +idev = container_of(base_dev, IOMMUFDDevice, base);
>> >> >> +
>> >> >> +return vtd_check_iommufd_hdev(s, idev, errp);
>> >> >> +}
>> >> >> +
>> >> >> static int vtd_dev_set_iommu_device(PCIBus *bus, void *opaque,
>int
>> >> >devfn,
>> >> >> HostIOMMUDevice *base_dev, Error
>> >> >> **errp)
>> >> >> {
>> >> >> @@ -3829,6 +3863,7 @@ static int
>> >vtd_dev_set_iommu_device(PCIBus
>> >> >*bus, void *opaque, int devfn,
>> >> >> .devfn = devfn,
>> >> >> };
>> >> >> struct vtd_as_key *new_key;
>> >> >> +int ret;
>> >> >>
>> >> >> assert(base_dev);
>> >> >>
>> >> >> @@ -3848,6 +3883,13 @@ static int
>> >vtd_dev_set_iommu_device(PCIBus
>> >> >*bus, void *opaque, int devfn,
>> >> >> vtd_hdev->iommu_state = s;
>> >> >> vtd_hdev->dev = base_dev;
>> >> >>
>> >> >> +ret = vtd_check_hdev(s, vtd_hdev, errp);
>> >> >> +if (ret) {
>> >> >> +g_free(vtd_hdev);
Re: [PATCH for-9.0 v14 4/8] target/riscv/vector_helpers: do early exit when vstart >= vl
Hi Daniel,
According the v spec section 15.2 & 15.3.
"The vcpop.m instruction writes x[rd] even if vl=0 (with the value 0,
since no mask elements are active).
Traps on vcpop.m are always reported with a vstart of 0. The vcpop.m
instruction will raise an illegal instruction exception if vstart is
non-zero."
"The vfirst.m instruction writes x[rd] even if vl=0 (with the value -1,
since no mask elements are active).
Traps on vfirst are always reported with a vstart of 0. The vfirst
instruction will raise an illegal instruction exception if vstart is
non-zero."
Both the vcpop.m and vfirst.m instructions will raise illegal
instruction exception with non-zero vstart.
And currently both the trans_vcpop_m and trans_vfirst_m translate
functions check the vstart_eq_zero flag.
So I think the early exit checking in the vcpop.m and vfirstm helper
functions may be redundant.
@@ -4585,6 +4641,11 @@ target_ulong HELPER(vcpop_m)(void *v0, void *vs2,
CPURISCVState *env,
uint32_t vl = env->vl;
int i;
+if (env->vstart >= env->vl) {
+env->vstart = 0;
+return 0;
+}
+
for (i = env->vstart; i < vl; i++) {
if (vm || vext_elem_mask(v0, i)) {
if (vext_elem_mask(vs2, i)) {
According v spec section 15.3
""The vfirst.m instruction writes x[rd] even if vl=0 (with the value -1,
since no mask elements are active)."
If both the vstart and vl are 0 here, the early exit checking will
return the wrong value 0 (the return value should be -1) here.
@@ -4604,6 +4665,11 @@ target_ulong HELPER(vfirst_m)(void *v0, void *vs2,
CPURISCVState *env,
uint32_t vl = env->vl;
int i;
+if (env->vstart >= env->vl) {
+env->vstart = 0;
+return 0;
+}
+
for (i = env->vstart; i < vl; i++) {
if (vm || vext_elem_mask(v0, i)) {
if (vext_elem_mask(vs2, i)) {
RE: [PATCH] vfio/iommufd: Fix memory leak
>-Original Message-
>From: Cédric Le Goater
>Sent: Thursday, March 14, 2024 5:06 AM
>To: qemu-devel@nongnu.org
>Cc: Alex Williamson ; Cédric Le Goater
>; Eric Auger ; Liu, Yi L
>; Duan, Zhenzhong
>Subject: [PATCH] vfio/iommufd: Fix memory leak
>
>Make sure variable contents is freed if scanf fails.
>
>Cc: Eric Auger
>Cc: Yi Liu
>Cc: Zhenzhong Duan
>Fixes: CID 1540007
>Fixes: 5ee3dc7af785 ("vfio/iommufd: Implement the iommufd backend")
>Signed-off-by: Cédric Le Goater
Reviewed-by: Zhenzhong Duan
Unrelated to this patch, I see there are four g_free calls, not clear if it's
deserved
to cleanup with g_autofree.
Thanks
Zhenzhong
>---
> hw/vfio/iommufd.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>index
>a75a785e90c64cdcc4d10c88d217801b3f536cdb..cd549e0ee8573e75772c5
>1cc96153762a6bc8550 100644
>--- a/hw/vfio/iommufd.c
>+++ b/hw/vfio/iommufd.c
>@@ -152,9 +152,8 @@ static int iommufd_cdev_getfd(const char
>*sysfs_path, Error **errp)
>
> if (sscanf(contents, "%d:%d", , ) != 2) {
> error_setg(errp, "failed to get major:minor for \"%s\"",
> vfio_dev_path);
>-goto out_free_dev_path;
>+goto out_free_contents;
> }
>-g_free(contents);
> vfio_devt = makedev(major, minor);
>
> vfio_path = g_strdup_printf("/dev/vfio/devices/%s", dent->d_name);
>@@ -166,6 +165,8 @@ static int iommufd_cdev_getfd(const char
>*sysfs_path, Error **errp)
> trace_iommufd_cdev_getfd(vfio_path, ret);
> g_free(vfio_path);
>
>+out_free_contents:
>+g_free(contents);
> out_free_dev_path:
> g_free(vfio_dev_path);
> out_close_dir:
>--
>2.44.0
Re: [PATCH] hw/virtio: Add support for VDPA network simulation devices
On Thu, Mar 14, 2024 at 3:52 AM Michael S. Tsirkin wrote:
>
> On Wed, Mar 13, 2024 at 07:51:08PM +0100, Thomas Weißschuh wrote:
> > On 2024-02-21 15:38:02+0800, Hao Chen wrote:
> > > This patch adds support for VDPA network simulation devices.
> > > The device is developed based on virtio-net and tap backend,
> > > and supports hardware live migration function.
> > >
> > > For more details, please refer to "docs/system/devices/vdpa-net.rst"
> > >
> > > Signed-off-by: Hao Chen
> > > ---
> > > MAINTAINERS | 5 +
> > > docs/system/device-emulation.rst| 1 +
> > > docs/system/devices/vdpa-net.rst| 121 +
> > > hw/net/virtio-net.c | 16 ++
> > > hw/virtio/virtio-pci.c | 189 +++-
I think those modifications should belong to a separate file as it
might conflict with virito features in the future.
> > > hw/virtio/virtio.c | 39
> > > include/hw/virtio/virtio-pci.h | 5 +
> > > include/hw/virtio/virtio.h | 19 ++
> > > include/standard-headers/linux/virtio_pci.h | 7 +
> > > 9 files changed, 399 insertions(+), 3 deletions(-)
> > > create mode 100644 docs/system/devices/vdpa-net.rst
> >
> > [..]
> >
> > > diff --git a/include/standard-headers/linux/virtio_pci.h
> > > b/include/standard-headers/linux/virtio_pci.h
> > > index b7fdfd0668..fb5391cef6 100644
> > > --- a/include/standard-headers/linux/virtio_pci.h
> > > +++ b/include/standard-headers/linux/virtio_pci.h
> > > @@ -216,6 +216,13 @@ struct virtio_pci_cfg_cap {
> > > #define VIRTIO_PCI_COMMON_Q_NDATA 56
> > > #define VIRTIO_PCI_COMMON_Q_RESET 58
> > >
> > > +#define LM_LOGGING_CTRL 0
> > > +#define LM_BASE_ADDR_LOW4
> > > +#define LM_BASE_ADDR_HIGH 8
> > > +#define LM_END_ADDR_LOW 12
> > > +#define LM_END_ADDR_HIGH16
> > > +#define LM_VRING_STATE_OFFSET 0x20
> >
> > These changes are not in upstream Linux and will be undone by
> > ./scripts/update-linux-headers.sh.
> >
> > Are they intentionally in this header?
>
>
> Good point. Pls move.
Right and this part, it's not a part of standard virtio.
Thanks
>
> > > +
> > > #endif /* VIRTIO_PCI_NO_MODERN */
> > >
> > > #endif
>
Re: [PATCH] vfio/iommufd: Fix memory leak
On 2024/3/14 05:06, Cédric Le Goater wrote:
Make sure variable contents is freed if scanf fails.
Cc: Eric Auger
Cc: Yi Liu
Cc: Zhenzhong Duan
Fixes: CID 1540007
Fixes: 5ee3dc7af785 ("vfio/iommufd: Implement the iommufd backend")
Signed-off-by: Cédric Le Goater
---
hw/vfio/iommufd.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index
a75a785e90c64cdcc4d10c88d217801b3f536cdb..cd549e0ee8573e75772c51cc96153762a6bc8550
100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -152,9 +152,8 @@ static int iommufd_cdev_getfd(const char *sysfs_path, Error
**errp)
if (sscanf(contents, "%d:%d", , ) != 2) {
error_setg(errp, "failed to get major:minor for \"%s\"",
vfio_dev_path);
-goto out_free_dev_path;
+goto out_free_contents;
}
-g_free(contents);
vfio_devt = makedev(major, minor);
vfio_path = g_strdup_printf("/dev/vfio/devices/%s", dent->d_name);
@@ -166,6 +165,8 @@ static int iommufd_cdev_getfd(const char *sysfs_path, Error
**errp)
trace_iommufd_cdev_getfd(vfio_path, ret);
g_free(vfio_path);
+out_free_contents:
+g_free(contents);
out_free_dev_path:
g_free(vfio_dev_path);
out_close_dir:
good catch.
Reviewed-by: Yi Liu
--
Regards,
Yi Liu
Re: [PATCH] vhost-vdpa: check vhost_vdpa_set_vring_ready() return value
On Wed, Feb 7, 2024 at 5:27 PM Stefano Garzarella wrote:
>
> vhost_vdpa_set_vring_ready() could already fail, but if Linux's
> patch [1] will be merged, it may fail with more chance if
> userspace does not activate virtqueues before DRIVER_OK when
> VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK is not negotiated.
I wonder what happens if we just leave it as is.
VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK: We do know enabling could be
done after driver_ok.
Without VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK: We don't know whether
enabling could be done after driver_ok or not.
Thanks
>
> So better check its return value anyway.
>
> [1]
> https://lore.kernel.org/virtualization/20240206145154.118044-1-sgarz...@redhat.com/T/#u
>
> Signed-off-by: Stefano Garzarella
> ---
> Note: This patch conflicts with [2], but the resolution is simple,
> so for now I sent a patch for the current master, but I'll rebase
> this patch if we merge the other one first.
>
> [2]
> https://lore.kernel.org/qemu-devel/20240202132521.32714-1-kw...@redhat.com/
> ---
> hw/virtio/vdpa-dev.c | 8 +++-
> net/vhost-vdpa.c | 15 ---
> 2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
> index eb9ecea83b..d57cd76c18 100644
> --- a/hw/virtio/vdpa-dev.c
> +++ b/hw/virtio/vdpa-dev.c
> @@ -259,7 +259,11 @@ static int vhost_vdpa_device_start(VirtIODevice *vdev,
> Error **errp)
> goto err_guest_notifiers;
> }
> for (i = 0; i < s->dev.nvqs; ++i) {
> -vhost_vdpa_set_vring_ready(>vdpa, i);
> +ret = vhost_vdpa_set_vring_ready(>vdpa, i);
> +if (ret < 0) {
> +error_setg_errno(errp, -ret, "Error starting vring %d", i);
> +goto err_dev_stop;
> +}
> }
> s->started = true;
>
> @@ -274,6 +278,8 @@ static int vhost_vdpa_device_start(VirtIODevice *vdev,
> Error **errp)
>
> return ret;
>
> +err_dev_stop:
> +vhost_dev_stop(>dev, vdev, false);
> err_guest_notifiers:
> k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false);
> err_host_notifiers:
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 3726ee5d67..e3d8036479 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -381,7 +381,10 @@ static int vhost_vdpa_net_data_load(NetClientState *nc)
> }
>
> for (int i = 0; i < v->dev->nvqs; ++i) {
> -vhost_vdpa_set_vring_ready(v, i + v->dev->vq_index);
> +int ret = vhost_vdpa_set_vring_ready(v, i + v->dev->vq_index);
> +if (ret < 0) {
> +return ret;
> +}
> }
> return 0;
> }
> @@ -1213,7 +1216,10 @@ static int vhost_vdpa_net_cvq_load(NetClientState *nc)
>
> assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
>
> -vhost_vdpa_set_vring_ready(v, v->dev->vq_index);
> +r = vhost_vdpa_set_vring_ready(v, v->dev->vq_index);
> +if (unlikely(r < 0)) {
> +return r;
> +}
>
> if (v->shadow_vqs_enabled) {
> n = VIRTIO_NET(v->dev->vdev);
> @@ -1252,7 +1258,10 @@ static int vhost_vdpa_net_cvq_load(NetClientState *nc)
> }
>
> for (int i = 0; i < v->dev->vq_index; ++i) {
> -vhost_vdpa_set_vring_ready(v, i);
> +r = vhost_vdpa_set_vring_ready(v, i);
> +if (unlikely(r < 0)) {
> +return r;
> +}
> }
>
> return 0;
> --
> 2.43.0
>
Re: [PATCH v2 1/6] virtio/virtio-pci: Handle extra notification data
On Wed, Mar 13, 2024 at 7:55 PM Jonah Palmer wrote:
>
> Add support to virtio-pci devices for handling the extra data sent
> from the driver to the device when the VIRTIO_F_NOTIFICATION_DATA
> transport feature has been negotiated.
>
> The extra data that's passed to the virtio-pci device when this
> feature is enabled varies depending on the device's virtqueue
> layout.
>
> In a split virtqueue layout, this data includes:
> - upper 16 bits: shadow_avail_idx
> - lower 16 bits: virtqueue index
>
> In a packed virtqueue layout, this data includes:
> - upper 16 bits: 1-bit wrap counter & 15-bit shadow_avail_idx
> - lower 16 bits: virtqueue index
>
> Tested-by: Lei Yang
> Reviewed-by: Eugenio Pérez
> Signed-off-by: Jonah Palmer
> ---
> hw/virtio/virtio-pci.c | 10 +++---
> hw/virtio/virtio.c | 18 ++
> include/hw/virtio/virtio.h | 1 +
> 3 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index cb6940fc0e..0f5c3c3b2f 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -384,7 +384,7 @@ static void virtio_ioport_write(void *opaque, uint32_t
> addr, uint32_t val)
> {
> VirtIOPCIProxy *proxy = opaque;
> VirtIODevice *vdev = virtio_bus_get_device(>bus);
> -uint16_t vector;
> +uint16_t vector, vq_idx;
> hwaddr pa;
>
> switch (addr) {
> @@ -408,8 +408,12 @@ static void virtio_ioport_write(void *opaque, uint32_t
> addr, uint32_t val)
> vdev->queue_sel = val;
> break;
> case VIRTIO_PCI_QUEUE_NOTIFY:
> -if (val < VIRTIO_QUEUE_MAX) {
> -virtio_queue_notify(vdev, val);
> +vq_idx = val;
> +if (vq_idx < VIRTIO_QUEUE_MAX) {
> +if (virtio_vdev_has_feature(vdev, VIRTIO_F_NOTIFICATION_DATA)) {
> +virtio_queue_set_shadow_avail_data(vdev, val);
> +}
> +virtio_queue_notify(vdev, vq_idx);
> }
> break;
> case VIRTIO_PCI_STATUS:
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index d229755eae..bcb9e09df0 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2255,6 +2255,24 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n,
> int align)
> }
> }
>
> +void virtio_queue_set_shadow_avail_data(VirtIODevice *vdev, uint32_t data)
> +{
> +/* Lower 16 bits is the virtqueue index */
> +uint16_t i = data;
> +VirtQueue *vq = >vq[i];
> +
> +if (!vq->vring.desc) {
> +return;
> +}
> +
> +if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
> +vq->shadow_avail_wrap_counter = (data >> 31) & 0x1;
> +vq->shadow_avail_idx = (data >> 16) & 0x7FFF;
> +} else {
> +vq->shadow_avail_idx = (data >> 16);
Do we need to do a sanity check for this value?
Thanks
> +}
> +}
> +
> static void virtio_queue_notify_vq(VirtQueue *vq)
> {
> if (vq->vring.desc && vq->handle_output) {
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index c8f72850bc..53915947a7 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -335,6 +335,7 @@ void virtio_queue_update_rings(VirtIODevice *vdev, int n);
> void virtio_init_region_cache(VirtIODevice *vdev, int n);
> void virtio_queue_set_align(VirtIODevice *vdev, int n, int align);
> void virtio_queue_notify(VirtIODevice *vdev, int n);
> +void virtio_queue_set_shadow_avail_data(VirtIODevice *vdev, uint32_t data);
> uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);
> void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t vector);
> int virtio_queue_set_host_notifier_mr(VirtIODevice *vdev, int n,
> --
> 2.39.3
>
Re: [PATCH v2 04/10] ppc/spapr: Remove copy-paste from pa-features
On Tue, Mar 12, 2024 at 11:14:13PM +1000, Nicholas Piggin wrote:
> TCG does not support copy/paste instructions. Remove it from
> ibm,pa-features. This has never been implemented under TCG or
> practically usable under KVM, so it won't be missed.
As with the previous patch, the specific circumstances here justify
breaking the general rule.
>
> Reviewed-by: Harsh Prateek Bora
> Signed-off-by: Nicholas Piggin
> ---
> hw/ppc/spapr.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3108d7c532..4192cd8d6c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -237,6 +237,10 @@ static void spapr_dt_pa_features(SpaprMachineState
> *spapr,
> * SSO (SAO) ordering is supported on KVM and thread=single hosts,
> * but not MTTCG, so disable it. To advertise it, a cap would have
> * to be added, or support implemented for MTTCG.
> + *
> + * Copy/paste is not supported by TCG, so it is not advertised. KVM
> + * can execute them but it has no accelerator drivers which are usable,
> + * so there isn't much need for it anyway.
> */
>
> uint8_t pa_features_206[] = { 6, 0,
> @@ -260,8 +264,8 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
> 0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
> /* 30: MMR, 32: LE atomic, 34: EBB + ext EBB */
> 0x80, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
> -/* 36: SPR SO, 38: Copy/Paste, 40: Radix MMU */
> -0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 36 - 41 */
> +/* 36: SPR SO, 40: Radix MMU */
> +0x80, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
> /* 42: PM, 44: PC RA, 46: SC vec'd */
> 0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
> /* 48: SIMD, 50: QP BFP, 52: String */
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH v2 03/10] ppc/spapr|pnv: Remove SAO from pa-features
On Tue, Mar 12, 2024 at 11:14:12PM +1000, Nicholas Piggin wrote:
> SAO is a page table attribute that strengthens the memory ordering of
> accesses. QEMU with MTTCG does not implement this, so clear it in
> ibm,pa-features. This is an obscure feature that has been removed from
> POWER10 ISA v3.1, there isn't much concern with removing it.
>
> Reviewed-by: Harsh Prateek Bora
> Signed-off-by: Nicholas Piggin
Usually altering a user visible feature like this without versioning
would be a no-no. However, I think it's probably ok here: AFAICT the
feature was basically never used, it didn't work in some cases anyway,
and it's now gone away.
> ---
> hw/ppc/pnv.c | 2 +-
> hw/ppc/spapr.c | 14 ++
> 2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 0b47b92baa..aa9786e970 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -150,7 +150,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void
> *fdt)
> uint32_t page_sizes_prop[64];
> size_t page_sizes_prop_size;
> const uint8_t pa_features[] = { 24, 0,
> -0xf6, 0x3f, 0xc7, 0xc0, 0x80, 0xf0,
> +0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0,
> 0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
> 0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 55263f0815..3108d7c532 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -233,17 +233,23 @@ static void spapr_dt_pa_features(SpaprMachineState
> *spapr,
> PowerPCCPU *cpu,
> void *fdt, int offset)
> {
> +/*
> + * SSO (SAO) ordering is supported on KVM and thread=single hosts,
> + * but not MTTCG, so disable it. To advertise it, a cap would have
> + * to be added, or support implemented for MTTCG.
> + */
> +
> uint8_t pa_features_206[] = { 6, 0,
> -0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 };
> +0xf6, 0x1f, 0xc7, 0x00, 0x00, 0xc0 };
> uint8_t pa_features_207[] = { 24, 0,
> -0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0,
> +0xf6, 0x1f, 0xc7, 0xc0, 0x00, 0xf0,
> 0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
> 0x80, 0x00, 0x80, 0x00, 0x00, 0x00 };
> uint8_t pa_features_300[] = { 66, 0,
> /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
> -/* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, SSO, 5: LE|CFAR|EB|LSQ
> */
> -0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0, /* 0 - 5 */
> +/* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
> +0xf6, 0x1f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
> /* 6: DS207 */
> 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
> /* 16: Vector */
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH v6 03/17] hw/loongarch: Add slave cpu boot_code
Song,
On Fri, Mar 8, 2024 at 12:51 AM Song Gao wrote:
>
> Signed-off-by: Song Gao
> Message-Id: <20240301093839.663947-4-gaos...@loongson.cn>
> ---
> hw/loongarch/boot.c | 70 -
> 1 file changed, 69 insertions(+), 1 deletion(-)
>
> diff --git a/hw/loongarch/boot.c b/hw/loongarch/boot.c
> index 149deb2e01..e560ac178a 100644
> --- a/hw/loongarch/boot.c
> +++ b/hw/loongarch/boot.c
> @@ -15,6 +15,54 @@
> #include "sysemu/reset.h"
> #include "sysemu/qtest.h"
>
> +static const unsigned int slave_boot_code[] = {
> + /* Configure reset ebase. */
> +0x0400302c, /* csrwr $r12,0xc*/
Use reg-names may be a little better than reg-nums.
Huacai
> +
> + /* Disable interrupt. */
> +0x0380100c, /* ori$r12,$r0,0x4*/
> +0x04000180, /* csrxchg$r0,$r12,0x0*/
> +
> + /* Clear mailbox. */
> +0x142d, /* lu12i.w$r13,1(0x1) */
> +0x038081ad, /* ori$r13,$r13,0x20 */
> +0x06481da0, /* iocsrwr.d $r0,$r13*/
> +
> + /* Enable IPI interrupt. */
> +0x142c, /* lu12i.w$r12,1(0x1) */
> +0x0400118c, /* csrxchg$r12,$r12,0x4 */
> +0x02fffc0c, /* addi.d $r12,$r0,-1(0xfff) */
> +0x142d, /* lu12i.w$r13,1(0x1) */
> +0x038011ad, /* ori$r13,$r13,0x4 */
> +0x064819ac, /* iocsrwr.w $r12,$r13 */
> +0x142d, /* lu12i.w$r13,1(0x1) */
> +0x038081ad, /* ori$r13,$r13,0x20 */
> +
> + /* Wait for wakeup <.L11>: */
> +0x06488000, /* idle 0x0 */
> +0x0340, /* andi $r0,$r0,0x0 */
> +0x064809ac, /* iocsrrd.w $r12,$r13 */
> +0x43fff59f, /* beqz $r12,-12(0x74) # 48 <.L11> */
> +
> + /* Read and clear IPI interrupt. */
> +0x142d, /* lu12i.w$r13,1(0x1) */
> +0x064809ac, /* iocsrrd.w $r12,$r13 */
> +0x142d, /* lu12i.w$r13,1(0x1) */
> +0x038031ad, /* ori$r13,$r13,0xc */
> +0x064819ac, /* iocsrwr.w $r12,$r13 */
> +
> + /* Disable IPI interrupt.*/
> +0x142c, /* lu12i.w$r12,1(0x1) */
> +0x04001180, /* csrxchg$r0,$r12,0x4*/
> +
> + /* Read mail buf and jump to specified entry */
> +0x142d, /* lu12i.w$r13,1(0x1) */
> +0x038081ad, /* ori$r13,$r13,0x20 */
> +0x06480dac, /* iocsrrd.d $r12,$r13 */
> +0x00150181, /* move $r1,$r12*/
> +0x4c20, /* jirl $r0,$r1,0 */
> +};
> +
> static uint64_t cpu_loongarch_virt_to_phys(void *opaque, uint64_t addr)
> {
> return addr & MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS);
> @@ -111,8 +159,15 @@ static void
> loongarch_firmware_boot(LoongArchMachineState *lams,
> fw_cfg_add_kernel_info(info, lams->fw_cfg);
> }
>
> +static void init_boot_rom(struct loongarch_boot_info *info, void *p)
> +{
> +memcpy(p, _boot_code, sizeof(slave_boot_code));
> +p += sizeof(slave_boot_code);
> +}
> +
> static void loongarch_direct_kernel_boot(struct loongarch_boot_info *info)
> {
> +void *p, *bp;
> int64_t kernel_addr = 0;
> LoongArchCPU *lacpu;
> CPUState *cs;
> @@ -126,11 +181,24 @@ static void loongarch_direct_kernel_boot(struct
> loongarch_boot_info *info)
> }
> }
>
> +/* Load 'boot_rom' at [0 - 1MiB] */
> +p = g_malloc0(1 * MiB);
> +bp = p;
> +init_boot_rom(info, p);
> +rom_add_blob_fixed("boot_rom", bp, 1 * MiB, 0);
> +
> CPU_FOREACH(cs) {
> lacpu = LOONGARCH_CPU(cs);
> lacpu->env.load_elf = true;
> -lacpu->env.elf_address = kernel_addr;
> +if (cs == first_cpu) {
> +lacpu->env.elf_address = kernel_addr;
> +} else {
> +lacpu->env.elf_address = 0;
> +}
> +lacpu->env.boot_info = info;
> }
> +
> +g_free(bp);
> }
>
> void loongarch_load_kernel(MachineState *ms, struct loongarch_boot_info
> *info)
> --
> 2.34.1
>
>
--
Huacai Chen
Re: [PATCH V4 1/1] target/loongarch: Fixed tlb huge page loading issue
Hi, Xianglai,
Generally, the subject should be "Fix tlb huge page loading issue"
rather than "Fixed tlb huge page loading issue".
On Thu, Mar 14, 2024 at 9:34 AM Xianglai Li wrote:
>
> When we use qemu tcg simulation, the page size of bios is 4KB.
> When using the level 2 super large page (page size is 1G) to create the page
> table,
> it is found that the content of the corresponding address space is abnormal,
> resulting in the bios can not start the operating system and graphical
> interface normally.
>
> The lddir and ldpte instruction emulation has
> a problem with the use of super large page processing above level 2.
> The page size is not correctly calculated,
> resulting in the wrong page size of the table entry found by tlb.
>
> Cc: maob...@loongson.cn
> Cc: Song Gao
> Cc: Xiaojuan Yang
> Cc: zhaotian...@loongson.cn
> Cc: yi...@loongson.cn
> Cc: wuruiy...@loongson.cn
>
> Signed-off-by: Xianglai Li
> ---
> target/loongarch/cpu-csr.h| 3 +
> target/loongarch/internals.h | 5 --
> target/loongarch/tcg/tlb_helper.c | 105 --
> 3 files changed, 74 insertions(+), 39 deletions(-)
>
> Changes log:
> V3->V4:
> Optimize the huge page calculation method,
> use the FIELD macro for bit calculation.
>
> V2->V3:
> Delete the intermediate variable LDDIR_PS, and implement lddir and ldpte
> huge pages by referring to the latest architecture reference manual.
>
> V1->V2:
> Modified the patch title format and Enrich the commit mesg description
>
> diff --git a/target/loongarch/cpu-csr.h b/target/loongarch/cpu-csr.h
> index c59d7a9fcb..b0775cf6bf 100644
> --- a/target/loongarch/cpu-csr.h
> +++ b/target/loongarch/cpu-csr.h
> @@ -67,6 +67,9 @@ FIELD(TLBENTRY, D, 1, 1)
> FIELD(TLBENTRY, PLV, 2, 2)
> FIELD(TLBENTRY, MAT, 4, 2)
> FIELD(TLBENTRY, G, 6, 1)
> +FIELD(TLBENTRY, HUGE, 6, 1)
> +FIELD(TLBENTRY, HG, 12, 1)
> +FIELD(TLBENTRY, LEVEL, 13, 2)
> FIELD(TLBENTRY_32, PPN, 8, 24)
> FIELD(TLBENTRY_64, PPN, 12, 36)
> FIELD(TLBENTRY_64, NR, 61, 1)
> diff --git a/target/loongarch/internals.h b/target/loongarch/internals.h
> index a2fc54c8a7..944153b180 100644
> --- a/target/loongarch/internals.h
> +++ b/target/loongarch/internals.h
> @@ -16,11 +16,6 @@
> #define TARGET_PHYS_MASK MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS)
> #define TARGET_VIRT_MASK MAKE_64BIT_MASK(0, TARGET_VIRT_ADDR_SPACE_BITS)
>
> -/* Global bit used for lddir/ldpte */
> -#define LOONGARCH_PAGE_HUGE_SHIFT 6
> -/* Global bit for huge page */
> -#define LOONGARCH_HGLOBAL_SHIFT 12
> -
> void loongarch_translate_init(void);
>
> void loongarch_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
> diff --git a/target/loongarch/tcg/tlb_helper.c
> b/target/loongarch/tcg/tlb_helper.c
> index 22be031ac7..b9a8633791 100644
> --- a/target/loongarch/tcg/tlb_helper.c
> +++ b/target/loongarch/tcg/tlb_helper.c
> @@ -17,6 +17,34 @@
> #include "exec/log.h"
> #include "cpu-csr.h"
>
> +static void get_dir_base_width(CPULoongArchState *env, uint64_t *dir_base,
> + uint64_t *dir_width, target_ulong level)
> +{
> +switch (level) {
> +case 1:
> +*dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_BASE);
> +*dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_WIDTH);
> +break;
> +case 2:
> +*dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_BASE);
> +*dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_WIDTH);
> +break;
> +case 3:
> +*dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_BASE);
> +*dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_WIDTH);
> +break;
> +case 4:
> +*dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_BASE);
> +*dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_WIDTH);
> +break;
> +default:
> +/* level may be zero for ldpte */
> +*dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTBASE);
> +*dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTWIDTH);
> +break;
> +}
> +}
> +
> static void raise_mmu_exception(CPULoongArchState *env, target_ulong address,
> MMUAccessType access_type, int tlb_error)
> {
> @@ -485,7 +513,23 @@ target_ulong helper_lddir(CPULoongArchState *env,
> target_ulong base,
> target_ulong badvaddr, index, phys, ret;
> int shift;
> uint64_t dir_base, dir_width;
> -bool huge = (base >> LOONGARCH_PAGE_HUGE_SHIFT) & 0x1;
> +
> +if (unlikely((level == 0) || (level > 4))) {
> +return base;
> +}
> +
> +if (FIELD_EX64(base, TLBENTRY, HUGE)) {
> +if (FIELD_EX64(base, TLBENTRY, LEVEL)) {
> +return base;
> +} else {
> +return FIELD_DP64(base, TLBENTRY, LEVEL, level);
> +}
> +
> +if (unlikely(level == 4)) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> + "Attempted use of level %lu huge page\n", level);
> +}
> +}
>
>
[PATCH V4] target/loongarch: Fixed tlb huge page loading issue
When we use qemu tcg simulation, the page size of bios is 4KB.
When using the level 2 super large page (page size is 1G) to create the page
table,
it is found that the content of the corresponding address space is abnormal,
resulting in the bios can not start the operating system and graphical
interface normally.
The lddir and ldpte instruction emulation has
a problem with the use of super large page processing above level 2.
The page size is not correctly calculated,
resulting in the wrong page size of the table entry found by tlb.
Signed-off-by: Xianglai Li
---
target/loongarch/cpu-csr.h| 3 +
target/loongarch/internals.h | 5 --
target/loongarch/tcg/tlb_helper.c | 105 --
3 files changed, 74 insertions(+), 39 deletions(-)
Changes log:
V3->V4:
Optimize the huge page calculation method,
use the FIELD macro for bit calculation.
V2->V3:
Delete the intermediate variable LDDIR_PS, and implement lddir and ldpte
huge pages by referring to the latest architecture reference manual.
V1->V2:
Modified the patch title format and Enrich the commit mesg description
diff --git a/target/loongarch/cpu-csr.h b/target/loongarch/cpu-csr.h
index c59d7a9fcb..b0775cf6bf 100644
--- a/target/loongarch/cpu-csr.h
+++ b/target/loongarch/cpu-csr.h
@@ -67,6 +67,9 @@ FIELD(TLBENTRY, D, 1, 1)
FIELD(TLBENTRY, PLV, 2, 2)
FIELD(TLBENTRY, MAT, 4, 2)
FIELD(TLBENTRY, G, 6, 1)
+FIELD(TLBENTRY, HUGE, 6, 1)
+FIELD(TLBENTRY, HG, 12, 1)
+FIELD(TLBENTRY, LEVEL, 13, 2)
FIELD(TLBENTRY_32, PPN, 8, 24)
FIELD(TLBENTRY_64, PPN, 12, 36)
FIELD(TLBENTRY_64, NR, 61, 1)
diff --git a/target/loongarch/internals.h b/target/loongarch/internals.h
index a2fc54c8a7..944153b180 100644
--- a/target/loongarch/internals.h
+++ b/target/loongarch/internals.h
@@ -16,11 +16,6 @@
#define TARGET_PHYS_MASK MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS)
#define TARGET_VIRT_MASK MAKE_64BIT_MASK(0, TARGET_VIRT_ADDR_SPACE_BITS)
-/* Global bit used for lddir/ldpte */
-#define LOONGARCH_PAGE_HUGE_SHIFT 6
-/* Global bit for huge page */
-#define LOONGARCH_HGLOBAL_SHIFT 12
-
void loongarch_translate_init(void);
void loongarch_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
diff --git a/target/loongarch/tcg/tlb_helper.c
b/target/loongarch/tcg/tlb_helper.c
index 22be031ac7..b9a8633791 100644
--- a/target/loongarch/tcg/tlb_helper.c
+++ b/target/loongarch/tcg/tlb_helper.c
@@ -17,6 +17,34 @@
#include "exec/log.h"
#include "cpu-csr.h"
+static void get_dir_base_width(CPULoongArchState *env, uint64_t *dir_base,
+ uint64_t *dir_width, target_ulong level)
+{
+switch (level) {
+case 1:
+*dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_BASE);
+*dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_WIDTH);
+break;
+case 2:
+*dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_BASE);
+*dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_WIDTH);
+break;
+case 3:
+*dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_BASE);
+*dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_WIDTH);
+break;
+case 4:
+*dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_BASE);
+*dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_WIDTH);
+break;
+default:
+/* level may be zero for ldpte */
+*dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTBASE);
+*dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTWIDTH);
+break;
+}
+}
+
static void raise_mmu_exception(CPULoongArchState *env, target_ulong address,
MMUAccessType access_type, int tlb_error)
{
@@ -485,7 +513,23 @@ target_ulong helper_lddir(CPULoongArchState *env,
target_ulong base,
target_ulong badvaddr, index, phys, ret;
int shift;
uint64_t dir_base, dir_width;
-bool huge = (base >> LOONGARCH_PAGE_HUGE_SHIFT) & 0x1;
+
+if (unlikely((level == 0) || (level > 4))) {
+return base;
+}
+
+if (FIELD_EX64(base, TLBENTRY, HUGE)) {
+if (FIELD_EX64(base, TLBENTRY, LEVEL)) {
+return base;
+} else {
+return FIELD_DP64(base, TLBENTRY, LEVEL, level);
+}
+
+if (unlikely(level == 4)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "Attempted use of level %lu huge page\n", level);
+}
+}
badvaddr = env->CSR_TLBRBADV;
base = base & TARGET_PHYS_MASK;
@@ -494,33 +538,12 @@ target_ulong helper_lddir(CPULoongArchState *env,
target_ulong base,
shift = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTEWIDTH);
shift = (shift + 1) * 3;
-if (huge) {
-return base;
-}
-switch (level) {
-case 1:
-dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_BASE);
-dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_WIDTH);
-break;
-case 2:
-dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_BASE);
RE: [PULL 2/3] xen: Drop out of coroutine context xen_invalidate_map_cache_entry
> Subject: Re: [PULL 2/3] xen: Drop out of coroutine context > xen_invalidate_map_cache_entry > > 13.03.2024 20:21, Michael Tokarev: > > 12.03.2024 17:27, Anthony PERARD wrote: > >> From: Peng Fan > >> > >> xen_invalidate_map_cache_entry is not expected to run in a coroutine. > >> Without this, there is crash: > > > > Hi! Is this a stable material? (It applies cleanly and builds on 8.2 > > and 7.2) > > Actually for 7.2 it needed a minor tweak: > > -void coroutine_mixed_fn xen_invalidate_map_cache_entry(uint8_t *buffer) > +void xen_invalidate_map_cache_entry(uint8_t *buffer) I only tested 8.2 with xen virtio enabled. Not sure whether 7.2 has the issue or not. Thanks, Peng. > > but the rest is okay. > > /mjt
[PATCH V4 1/1] target/loongarch: Fixed tlb huge page loading issue
When we use qemu tcg simulation, the page size of bios is 4KB.
When using the level 2 super large page (page size is 1G) to create the page
table,
it is found that the content of the corresponding address space is abnormal,
resulting in the bios can not start the operating system and graphical
interface normally.
The lddir and ldpte instruction emulation has
a problem with the use of super large page processing above level 2.
The page size is not correctly calculated,
resulting in the wrong page size of the table entry found by tlb.
Cc: maob...@loongson.cn
Cc: Song Gao
Cc: Xiaojuan Yang
Cc: zhaotian...@loongson.cn
Cc: yi...@loongson.cn
Cc: wuruiy...@loongson.cn
Signed-off-by: Xianglai Li
---
target/loongarch/cpu-csr.h| 3 +
target/loongarch/internals.h | 5 --
target/loongarch/tcg/tlb_helper.c | 105 --
3 files changed, 74 insertions(+), 39 deletions(-)
Changes log:
V3->V4:
Optimize the huge page calculation method,
use the FIELD macro for bit calculation.
V2->V3:
Delete the intermediate variable LDDIR_PS, and implement lddir and ldpte
huge pages by referring to the latest architecture reference manual.
V1->V2:
Modified the patch title format and Enrich the commit mesg description
diff --git a/target/loongarch/cpu-csr.h b/target/loongarch/cpu-csr.h
index c59d7a9fcb..b0775cf6bf 100644
--- a/target/loongarch/cpu-csr.h
+++ b/target/loongarch/cpu-csr.h
@@ -67,6 +67,9 @@ FIELD(TLBENTRY, D, 1, 1)
FIELD(TLBENTRY, PLV, 2, 2)
FIELD(TLBENTRY, MAT, 4, 2)
FIELD(TLBENTRY, G, 6, 1)
+FIELD(TLBENTRY, HUGE, 6, 1)
+FIELD(TLBENTRY, HG, 12, 1)
+FIELD(TLBENTRY, LEVEL, 13, 2)
FIELD(TLBENTRY_32, PPN, 8, 24)
FIELD(TLBENTRY_64, PPN, 12, 36)
FIELD(TLBENTRY_64, NR, 61, 1)
diff --git a/target/loongarch/internals.h b/target/loongarch/internals.h
index a2fc54c8a7..944153b180 100644
--- a/target/loongarch/internals.h
+++ b/target/loongarch/internals.h
@@ -16,11 +16,6 @@
#define TARGET_PHYS_MASK MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS)
#define TARGET_VIRT_MASK MAKE_64BIT_MASK(0, TARGET_VIRT_ADDR_SPACE_BITS)
-/* Global bit used for lddir/ldpte */
-#define LOONGARCH_PAGE_HUGE_SHIFT 6
-/* Global bit for huge page */
-#define LOONGARCH_HGLOBAL_SHIFT 12
-
void loongarch_translate_init(void);
void loongarch_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
diff --git a/target/loongarch/tcg/tlb_helper.c
b/target/loongarch/tcg/tlb_helper.c
index 22be031ac7..b9a8633791 100644
--- a/target/loongarch/tcg/tlb_helper.c
+++ b/target/loongarch/tcg/tlb_helper.c
@@ -17,6 +17,34 @@
#include "exec/log.h"
#include "cpu-csr.h"
+static void get_dir_base_width(CPULoongArchState *env, uint64_t *dir_base,
+ uint64_t *dir_width, target_ulong level)
+{
+switch (level) {
+case 1:
+*dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_BASE);
+*dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_WIDTH);
+break;
+case 2:
+*dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_BASE);
+*dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_WIDTH);
+break;
+case 3:
+*dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_BASE);
+*dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_WIDTH);
+break;
+case 4:
+*dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_BASE);
+*dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_WIDTH);
+break;
+default:
+/* level may be zero for ldpte */
+*dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTBASE);
+*dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTWIDTH);
+break;
+}
+}
+
static void raise_mmu_exception(CPULoongArchState *env, target_ulong address,
MMUAccessType access_type, int tlb_error)
{
@@ -485,7 +513,23 @@ target_ulong helper_lddir(CPULoongArchState *env,
target_ulong base,
target_ulong badvaddr, index, phys, ret;
int shift;
uint64_t dir_base, dir_width;
-bool huge = (base >> LOONGARCH_PAGE_HUGE_SHIFT) & 0x1;
+
+if (unlikely((level == 0) || (level > 4))) {
+return base;
+}
+
+if (FIELD_EX64(base, TLBENTRY, HUGE)) {
+if (FIELD_EX64(base, TLBENTRY, LEVEL)) {
+return base;
+} else {
+return FIELD_DP64(base, TLBENTRY, LEVEL, level);
+}
+
+if (unlikely(level == 4)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "Attempted use of level %lu huge page\n", level);
+}
+}
badvaddr = env->CSR_TLBRBADV;
base = base & TARGET_PHYS_MASK;
@@ -494,33 +538,12 @@ target_ulong helper_lddir(CPULoongArchState *env,
target_ulong base,
shift = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTEWIDTH);
shift = (shift + 1) * 3;
-if (huge) {
-return base;
-}
-switch (level) {
-case 1:
-dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_BASE);
-dir_width =
Re: [PATCH v6 03/17] hw/loongarch: Add slave cpu boot_code
On 2024/3/11 下午2:50, maobibo wrote:
On 2024/3/8 下午5:36, gaosong wrote:
在 2024/3/8 16:27, maobibo 写道:
On 2024/3/8 上午12:48, Song Gao wrote:
Signed-off-by: Song Gao
Message-Id: <20240301093839.663947-4-gaos...@loongson.cn>
---
hw/loongarch/boot.c | 70
-
1 file changed, 69 insertions(+), 1 deletion(-)
diff --git a/hw/loongarch/boot.c b/hw/loongarch/boot.c
index 149deb2e01..e560ac178a 100644
--- a/hw/loongarch/boot.c
+++ b/hw/loongarch/boot.c
@@ -15,6 +15,54 @@
#include "sysemu/reset.h"
#include "sysemu/qtest.h"
+static const unsigned int slave_boot_code[] = {
+ /* Configure reset ebase. */
+ 0x0400302c, /* csrwr $r12,0xc */
+
+ /* Disable interrupt. */
+ 0x0380100c, /* ori $r12,$r0,0x4 */
+ 0x04000180, /* csrxchg $r0,$r12,0x0 */
+
+ /* Clear mailbox. */
+ 0x142d, /* lu12i.w $r13,1(0x1) */
+ 0x038081ad, /* ori $r13,$r13,0x20 */
+ 0x06481da0, /* iocsrwr.d $r0,$r13 */
+
+ /* Enable IPI interrupt. */
+ 0x142c, /* lu12i.w $r12,1(0x1) */
+ 0x0400118c, /* csrxchg $r12,$r12,0x4 */
+ 0x02fffc0c, /* addi.d $r12,$r0,-1(0xfff) */
+ 0x142d, /* lu12i.w $r13,1(0x1) */
+ 0x038011ad, /* ori $r13,$r13,0x4 */
+ 0x064819ac, /* iocsrwr.w $r12,$r13 */
+ 0x142d, /* lu12i.w $r13,1(0x1) */
+ 0x038081ad, /* ori $r13,$r13,0x20 */
+
+ /* Wait for wakeup <.L11>: */
+ 0x06488000, /* idle 0x0 */
+ 0x0340, /* andi $r0,$r0,0x0 */
+ 0x064809ac, /* iocsrrd.w $r12,$r13 */
+ 0x43fff59f, /* beqz $r12,-12(0x74) # 48 <.L11> */
+
+ /* Read and clear IPI interrupt. */
+ 0x142d, /* lu12i.w $r13,1(0x1) */
+ 0x064809ac, /* iocsrrd.w $r12,$r13 */
+ 0x142d, /* lu12i.w $r13,1(0x1) */
+ 0x038031ad, /* ori $r13,$r13,0xc */
+ 0x064819ac, /* iocsrwr.w $r12,$r13 */
+
+ /* Disable IPI interrupt. */
+ 0x142c, /* lu12i.w $r12,1(0x1) */
+ 0x04001180, /* csrxchg $r0,$r12,0x4 */
+
+ /* Read mail buf and jump to specified entry */
+ 0x142d, /* lu12i.w $r13,1(0x1) */
+ 0x038081ad, /* ori $r13,$r13,0x20 */
+ 0x06480dac, /* iocsrrd.d $r12,$r13 */
+ 0x00150181, /* move $r1,$r12 */
+ 0x4c20, /* jirl $r0,$r1,0 */
+};
+
static uint64_t cpu_loongarch_virt_to_phys(void *opaque, uint64_t
addr)
{
return addr & MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS);
@@ -111,8 +159,15 @@ static void
loongarch_firmware_boot(LoongArchMachineState *lams,
fw_cfg_add_kernel_info(info, lams->fw_cfg);
}
+static void init_boot_rom(struct loongarch_boot_info *info, void *p)
+{
+ memcpy(p, _boot_code, sizeof(slave_boot_code));
+ p += sizeof(slave_boot_code);
+}
+
static void loongarch_direct_kernel_boot(struct
loongarch_boot_info *info)
{
+ void *p, *bp;
int64_t kernel_addr = 0;
LoongArchCPU *lacpu;
CPUState *cs;
@@ -126,11 +181,24 @@ static void
loongarch_direct_kernel_boot(struct loongarch_boot_info *info)
}
}
+ /* Load 'boot_rom' at [0 - 1MiB] */
+ p = g_malloc0(1 * MiB);
+ bp = p;
+ init_boot_rom(info, p);
+ rom_add_blob_fixed("boot_rom", bp, 1 * MiB, 0);
+
The secondary cpu waiting on the bootrom located memory address
0x0-0x10.
Is it possible that primary cpu clears the memory located at bootrom
and then wakeup the secondary cpu?
I think it impossible，0-1M is ROM。
I am not sure whether it is ok if area between 0-1M is ROM.
For the memory map table, low memory area (0 - 256M) is still ddr ram.
And it is passed to kernel with fdt system table, rather than
area(1-256M). Is that right?
There are some lines like this:
/* Node0 memory */
memmap_add_entry(VIRT_LOWMEM_BASE, VIRT_LOWMEM_SIZE, 1);
Song,
Can the base memory address of bootrom for secondary cpus be set as base
address of flash like bios, such as VIRT_FLASH0_BASE/VIRT_FLASH1_BASE?
And ddr memory map area is kept unchanged.
Regards
Bibo Mao
Regards
Bibo Mao
Thanks.
Song Gao
Regards
Bibo Mao
CPU_FOREACH(cs) {
lacpu = LOONGARCH_CPU(cs);
lacpu->env.load_elf = true;
- lacpu->env.elf_address = kernel_addr;
+ if (cs == first_cpu) {
+ lacpu->env.elf_address = kernel_addr;
+ } else {
+ lacpu->env.elf_address = 0;
+ }
+ lacpu->env.boot_info = info;
}
+
+ g_free(bp);
}
void loongarch_load_kernel(MachineState *ms, struct
Re: [PATCH for-9.0] mirror: Don't call job_pause_point() under graph lock
On Wed, Mar 13, 2024 at 04:30:00PM +0100, Kevin Wolf wrote: > Calling job_pause_point() while holding the graph reader lock > potentially results in a deadlock: bdrv_graph_wrlock() first drains > everything, including the mirror job, which pauses it. The job is only > unpaused at the end of the drain section, which is when the graph writer > lock has been successfully taken. However, if the job happens to be > paused at a pause point where it still holds the reader lock, the writer > lock can't be taken as long as the job is still paused. > > Mark job_pause_point() as GRAPH_UNLOCKED and fix mirror accordingly. > > Cc: qemu-sta...@nongnu.org > Buglink: https://issues.redhat.com/browse/RHEL-28125 > Fixes: 004915a96a7a40e942ac85e6d22518cbcd283506 > Signed-off-by: Kevin Wolf > --- > include/qemu/job.h | 2 +- > block/mirror.c | 10 ++ > 2 files changed, 7 insertions(+), 5 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
[PATCH for-9.0 v2 0/3] target/hppa: Fix some wide mode displacements
As reported by Sven Schnelle, fixed via decodetree functions. Changes for v2: - Fix extract_16 implementation (deller) - Adjust some local variables to match arch doc field names. r~ Richard Henderson (3): target/hppa: Fix assemble_16 insns for wide mode target/hppa: Fix assemble_11a insns for wide mode target/hppa: Fix assemble_12a insns for wide mode target/hppa/insns.decode | 49 ++- target/hppa/translate.c | 62 2 files changed, 85 insertions(+), 26 deletions(-) -- 2.34.1
[PATCH v2 3/3] target/hppa: Fix assemble_12a insns for wide mode
Tested-by: Helge Deller
Reported-by: Sven Schnelle
Signed-off-by: Richard Henderson
---
target/hppa/insns.decode | 27 ---
target/hppa/translate.c | 17 +
2 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index 9c6f92444c..5412ff9836 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -26,7 +26,7 @@
%assemble_11a 4:12 0:1 !function=expand_11a
%assemble_120:s1 2:1 3:10!function=expand_shl2
-%assemble_12a 0:s1 3:11!function=expand_shl2
+%assemble_12a 3:13 0:1 !function=expand_12a
%assemble_160:16 !function=expand_16
%assemble_170:s1 16:5 2:1 3:10 !function=expand_shl2
%assemble_220:s1 16:10 2:1 3:10 !function=expand_shl2
@@ -314,8 +314,9 @@ fstd001011 . . .. . 1 -- 100 0 . .
@fldstdi
@ldstim14m .. b:5 t:5 \
sp=%assemble_sp disp=%assemble_16 \
x=0 scale=0 m=%neg_to_m
-@ldstim12m .. b:5 t:5 sp:2 .. \
- disp=%assemble_12a x=0 scale=0 m=%pos_to_m
+@ldstim12m .. b:5 t:5 \
+ sp=%assemble_sp disp=%assemble_12a \
+x=0 scale=0 m=%pos_to_m
# LDB, LDH, LDW, LDWM
ld 01 . . .. ..@ldstim14 size=0
@@ -331,15 +332,19 @@ st 011010 . . .. ..
@ldstim14 size=2
st 011011 . . .. ..@ldstim14m size=2
st 01 . . .. ...10.@ldstim12m size=2
-fldw010110 b:5 . sp:2 ..\
- disp=%assemble_12a t=%rm64 m=%a_to_m x=0 scale=0 size=2
-fldw010111 b:5 . sp:2 ...0..\
- disp=%assemble_12a t=%rm64 m=0 x=0 scale=0 size=2
+fldw010110 b:5 . \
+ disp=%assemble_12a sp=%assemble_sp \
+t=%rm64 m=%a_to_m x=0 scale=0 size=2
+fldw010111 b:5 . .0..\
+ disp=%assemble_12a sp=%assemble_sp \
+t=%rm64 m=0 x=0 scale=0 size=2
-fstw00 b:5 . sp:2 ..\
- disp=%assemble_12a t=%rm64 m=%a_to_m x=0 scale=0 size=2
-fstw01 b:5 . sp:2 ...0..\
- disp=%assemble_12a t=%rm64 m=0 x=0 scale=0 size=2
+fstw00 b:5 . \
+ disp=%assemble_12a sp=%assemble_sp \
+t=%rm64 m=%a_to_m x=0 scale=0 size=2
+fstw01 b:5 . .0..\
+ disp=%assemble_12a sp=%assemble_sp \
+t=%rm64 m=0 x=0 scale=0 size=2
ld 010100 . . .. 0.@ldstim11
fldd010100 . . .. 1.@ldstim11
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 40b9ff6d59..be0b0494d0 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -155,6 +155,23 @@ static int expand_11a(DisasContext *ctx, int val)
return i;
}
+/* Expander for assemble_16a(s,im11a,i). */
+static int expand_12a(DisasContext *ctx, int val)
+{
+/*
+ * @val is bit 0 and bits [3:15].
+ * Swizzle thing around depending on PSW.W.
+ */
+int im11a = extract32(val, 1, 11);
+int s = extract32(val, 12, 2);
+int i = (-(val & 1) << 13) | (im11a << 2);
+
+if (ctx->tb_flags & PSW_W) {
+i ^= s << 13;
+}
+return i;
+}
+
/* Expander for assemble_16(s,im14). */
static int expand_16(DisasContext *ctx, int val)
{
--
2.34.1
Re: [PATCH v3 00/20] Workaround Windows failing to find 64bit SMBIOS entry point with SeaBIOS
On Wed, Mar 13, 2024 at 09:49:39AM +0100, Igor Mammedov wrote: > On Tue, 12 Mar 2024 13:31:39 -0400 > "Michael S. Tsirkin" wrote: > > > On Tue, Mar 12, 2024 at 05:10:30PM +0100, Igor Mammedov wrote: > > > Changelog: > > > v3: > > >* whitespace missed by checkpatch > > >* fix idndent in QAPI > > >* reorder 17/20 before 1st 'auto' can be used > > >* pick up acks > > > v2: > > >* QAPI style fixes (Markus Armbruster ) > > >* squash 11/19 into 10/19 (Ani Sinha ) > > >* split '[PATCH 09/19] smbios: build legacy mode code only for 'pc' > > > machine' > > > in 3 smaller patches, to make it more readable > > >smbios: add smbios_add_usr_blob_size() helper > > > > > >smbios: rename/expose structures/bitmaps used by both legacy and > > > modern code > > > > > >smbios: build legacy mode code only for 'pc' machine > > >* pick up acks > > > > thanks! > > of course this conflicts with > > SMBIOS type 9 > > and I am trying to figure out how to resolve this again. > > I'll rebase once your pull req is merged. Note it's merged already. > > Do you ack SMBIOS type 9 btw? > nope, and it seems it's too late do so now. > > > > > > Windows (10) bootloader when running on top of SeaBIOS, fails to find > > > > > > SMBIOSv3 entry point. Tracing it shows that it looks for v2 anchor > > > markers > > > only and not v3. Tricking it into believing that entry point is found > > > > > > lets Windows successfully locate and parse SMBIOSv3 tables. Whether it > > > > > > will be fixed on Windows side is not clear so here goes a workaround. > > > > > > > > > > > > Idea is to try build v2 tables if QEMU configuration permits, > > > > > > and fallback to v3 tables otherwise. That will mask Windows issue > > > > > > form majority of users. > > > > > > However if VM configuration can't be described (typically large VMs) > > > > > > by v2 tables, QEMU will use SMBIOSv3 and Windows will hit the issue > > > > > > again. In this case complain to Microsoft and/or use UEFI instead of > > > > > > SeaBIOS (requires reinstall). > > > > > > > > > > > > Default compat setting of smbios-entry-point-type after series > > > > > > for pc/q35 machines: > > > > > > * 9.0-newer: 'auto' > > > > > > * 8.1-8.2: '64' > > > > > > * 8.0-older: '32' > > > > > > > > > > > > Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2008 > > > > > > CC: imamm...@redhat.com > > > > > > CC: m...@redhat.com > > > > > > Igor Mammedov (20): > > > tests: smbios: make it possible to write SMBIOS only test > > > tests: smbios: add test for -smbios type=11 option > > > tests: smbios: add test for legacy mode CLI options > > > smbios: cleanup smbios_get_tables() from legacy handling > > > smbios: get rid of smbios_smp_sockets global > > > smbios: get rid of smbios_legacy global > > > smbios: avoid mangling user provided tables > > > smbios: don't check type4 structures in legacy mode > > > smbios: add smbios_add_usr_blob_size() helper > > > smbios: rename/expose structures/bitmaps used by both legacy and > > > modern code > > > smbios: build legacy mode code only for 'pc' machine > > > smbios: handle errors consistently > > > smbios: get rid of global smbios_ep_type > > > smbios: clear smbios_type4_count before building tables > > > smbios: extend smbios-entry-point-type with 'auto' value > > > smbios: in case of entry point is 'auto' try to build v2 tables 1st > > > smbios: error out when building type 4 table is not possible > > > tests: acpi/smbios: whitelist expected blobs > > > pc/q35: set SMBIOS entry point type to 'auto' by default > > > tests: acpi: update expected SSDT.dimmpxm blob > > > > > > hw/i386/fw_cfg.h | 3 +- > > > include/hw/firmware/smbios.h | 28 +- > > > hw/arm/virt.c| 6 +- > > > hw/i386/Kconfig | 1 + > > > hw/i386/fw_cfg.c | 14 +- > > > hw/i386/pc.c | 4 +- > > >
[PATCH v2 2/3] target/hppa: Fix assemble_11a insns for wide mode
Tested-by: Helge Deller
Reviewed-by: Helge Deller
Reported-by: Sven Schnelle
Signed-off-by: Richard Henderson
---
target/hppa/insns.decode | 7 ---
target/hppa/translate.c | 23 +--
2 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index 0d9f8159ec..9c6f92444c 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -24,7 +24,7 @@
%assemble_sr3 13:1 14:2
%assemble_sr3x 13:1 14:2 !function=expand_sr3x
-%assemble_11a 0:s1 4:10!function=expand_shl3
+%assemble_11a 4:12 0:1 !function=expand_11a
%assemble_120:s1 2:1 3:10!function=expand_shl2
%assemble_12a 0:s1 3:11!function=expand_shl2
%assemble_160:16 !function=expand_16
@@ -305,8 +305,9 @@ fstd001011 . . .. . 1 -- 100 0 . .
@fldstdi
# Offset Mem
-@ldstim11 .. b:5 t:5 sp:2 .. \
- disp=%assemble_11a m=%ma2_to_m x=0 scale=0 size=3
+@ldstim11 .. b:5 t:5 \
+ sp=%assemble_sp disp=%assemble_11a \
+m=%ma2_to_m x=0 scale=0 size=3
@ldstim14 .. b:5 t:5 \
sp=%assemble_sp disp=%assemble_16 \
x=0 scale=0 m=0
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index cbe44ef75a..40b9ff6d59 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -121,12 +121,6 @@ static int expand_shl2(DisasContext *ctx, int val)
return val << 2;
}
-/* Used for fp memory ops. */
-static int expand_shl3(DisasContext *ctx, int val)
-{
-return val << 3;
-}
-
/* Used for assemble_21. */
static int expand_shl11(DisasContext *ctx, int val)
{
@@ -144,6 +138,23 @@ static int assemble_6(DisasContext *ctx, int val)
return (val ^ 31) + 1;
}
+/* Expander for assemble_16a(s,cat(im10a,0),i). */
+static int expand_11a(DisasContext *ctx, int val)
+{
+/*
+ * @val is bit 0 and bits [4:15].
+ * Swizzle thing around depending on PSW.W.
+ */
+int im10a = extract32(val, 1, 10);
+int s = extract32(val, 11, 2);
+int i = (-(val & 1) << 13) | (im10a << 3);
+
+if (ctx->tb_flags & PSW_W) {
+i ^= s << 13;
+}
+return i;
+}
+
/* Expander for assemble_16(s,im14). */
static int expand_16(DisasContext *ctx, int val)
{
--
2.34.1
[PATCH v2 1/3] target/hppa: Fix assemble_16 insns for wide mode
Reported-by: Sven Schnelle
Signed-off-by: Richard Henderson
---
target/hppa/insns.decode | 15 +--
target/hppa/translate.c | 22 ++
2 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index f5a3f02fd1..0d9f8159ec 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -27,13 +27,14 @@
%assemble_11a 0:s1 4:10!function=expand_shl3
%assemble_120:s1 2:1 3:10!function=expand_shl2
%assemble_12a 0:s1 3:11!function=expand_shl2
+%assemble_160:16 !function=expand_16
%assemble_170:s1 16:5 2:1 3:10 !function=expand_shl2
%assemble_220:s1 16:10 2:1 3:10 !function=expand_shl2
+%assemble_sp14:2 !function=sp0_if_wide
%assemble_210:s1 1:11 14:2 16:5 12:2 !function=expand_shl11
%lowsign_11 0:s1 1:10
-%lowsign_14 0:s1 1:13
%sm_imm 16:10 !function=expand_sm_imm
@@ -221,7 +222,7 @@ sub_b_tsv 10 . . 110100 . .
@rrr_cf_d
ldil001000 t:5 .i=%assemble_21
addil 001010 r:5 .i=%assemble_21
-ldo 001101 b:5 t:5 -- ..i=%lowsign_14
+ldo 001101 b:5 t:5 i=%assemble_16
addi101101 . . 0 ... @rri_cf
addi_tsv101101 . . 1 ... @rri_cf
@@ -306,10 +307,12 @@ fstd001011 . . .. . 1 -- 100 0 .
. @fldstdi
@ldstim11 .. b:5 t:5 sp:2 .. \
disp=%assemble_11a m=%ma2_to_m x=0 scale=0 size=3
-@ldstim14 .. b:5 t:5 sp:2 .. \
- disp=%lowsign_14 x=0 scale=0 m=0
-@ldstim14m .. b:5 t:5 sp:2 .. \
- disp=%lowsign_14 x=0 scale=0 m=%neg_to_m
+@ldstim14 .. b:5 t:5 \
+ sp=%assemble_sp disp=%assemble_16 \
+x=0 scale=0 m=0
+@ldstim14m .. b:5 t:5 \
+ sp=%assemble_sp disp=%assemble_16 \
+x=0 scale=0 m=%neg_to_m
@ldstim12m .. b:5 t:5 sp:2 .. \
disp=%assemble_12a x=0 scale=0 m=%pos_to_m
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index eb2046c5ad..cbe44ef75a 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -144,6 +144,28 @@ static int assemble_6(DisasContext *ctx, int val)
return (val ^ 31) + 1;
}
+/* Expander for assemble_16(s,im14). */
+static int expand_16(DisasContext *ctx, int val)
+{
+/*
+ * @val is bits [0:15], containing both im14 and s.
+ * Swizzle thing around depending on PSW.W.
+ */
+int s = extract32(val, 14, 2);
+int i = (-(val & 1) << 13) | extract32(val, 1, 13);
+
+if (ctx->tb_flags & PSW_W) {
+i ^= s << 13;
+}
+return i;
+}
+
+/* The sp field is only present with !PSW_W. */
+static int sp0_if_wide(DisasContext *ctx, int sp)
+{
+return ctx->tb_flags & PSW_W ? 0 : sp;
+}
+
/* Translate CMPI doubleword conditions to standard. */
static int cmpbid_c(DisasContext *ctx, int val)
{
--
2.34.1
Re: [PATCH for-9.0 v14 3/8] target/riscv: always clear vstart in whole vec move insns
On 3/13/24 12:01, Daniel Henrique Barboza wrote:
These insns have 2 paths: we'll either have vstart already cleared if
vstart_eq_zero or we'll do a brcond to check if vstart >= maxsz to call
the 'vmvr_v' helper. The helper will clear vstart if it executes until
the end, or if vstart >= vl.
However, if vstart >= maxsz, the helper will be skipped, and vstart
won't be cleared since the helper is being responsible from doing it.
We want to make the helpers responsible to manage vstart, including
these corner cases, precisely to avoid these situations. Move the vstart
= maxsz cond to the helper, and be sure to clear vstart if that
happens. This way we're now sure that vstart is being cleared in the end
of the execution, regardless of the path taken.
Fixes: f714361ed7 ("target/riscv: rvv-1.0: implement vstart CSR")
Signed-off-by: Daniel Henrique Barboza
---
target/riscv/insn_trans/trans_rvv.c.inc | 3 ---
target/riscv/vector_helper.c| 5 +
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index 8c16a9f5b3..52c26a7834 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -3664,12 +3664,9 @@ static bool trans_##NAME(DisasContext *s, arg_##NAME *
a) \
vreg_ofs(s, a->rs2), maxsz, maxsz);\
mark_vs_dirty(s); \
} else {\
-TCGLabel *over = gen_new_label(); \
-tcg_gen_brcondi_tl(TCG_COND_GEU, cpu_vstart, maxsz, over); \
tcg_gen_gvec_2_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2), \
tcg_env, maxsz, maxsz, 0, gen_helper_vmvr_v); \
mark_vs_dirty(s); \
-gen_set_label(over);\
} \
return true;\
} \
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index ca79571ae2..cd8235ea98 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -5075,6 +5075,11 @@ void HELPER(vmvr_v)(void *vd, void *vs2, CPURISCVState
*env, uint32_t desc)
uint32_t startb = env->vstart * sewb;
uint32_t i = startb;
+if (env->vstart >= maxsz) {
+env->vstart = 0;
+return;
+}
I think you were right to be concerned about vlmax -- you need to use startb not vstart in
this comparison, otherwise you've got mixed units on the comparison.
memcpy((uint8_t *)vd + H1(i),
(uint8_t *)vs2 + H1(i),
maxsz - startb);
Unrelated to this patch series, this has a big-endian host error.
With big-endian, the bytes to be copied may not be sequential.
if (HOST_BIG_ENDIAN && i % 8 != 0) {
uint32_t j = ROUND_UP(i, 8);
memcpy(vd + H(j - 1), vs2 + H1(j - 1), j - i);
i = j;
}
memcpy(vd + i, vs2 + i, maxsz - i);
r~
[PATCH for-9.0 v14 4/8] target/riscv/vector_helpers: do early exit when vstart >= vl
We're going to make changes that will required each helper to be
responsible for the 'vstart' management, i.e. we will relieve the
'vstart < vl' assumption that helpers have today.
Helpers are usually able to deal with vstart >= vl, i.e. doing nothing
aside from setting vstart = 0 at the end, but the tail update functions
will update the tail regardless of vstart being valid or not.
Unifying the tail update process in a single function that would handle
the vstart >= vl case isn't trivial. We have 2 functions that are used
to update tail: vext_set_tail_elems_1s() and vext_set_elems_1s(). The
latter is a more generic function that is also used to mask elements.
There's no easy way of making all callers using vext_set_tail_elems_1s()
because we're not encoding NF properly in all cases [1].
This patch takes a blunt approach: do an early exit in every single
vector helper if vstart >= vl. We can worry about unifying the tail
update process later.
[1]
https://lore.kernel.org/qemu-riscv/1590234b-0291-432a-a0fa-c5a687609...@linux.alibaba.com/
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Richard Henderson
---
target/riscv/vcrypto_helper.c | 32
target/riscv/vector_helper.c| 88 +
target/riscv/vector_internals.c | 4 ++
target/riscv/vector_internals.h | 9
4 files changed, 133 insertions(+)
diff --git a/target/riscv/vcrypto_helper.c b/target/riscv/vcrypto_helper.c
index e2d719b13b..f7423df226 100644
--- a/target/riscv/vcrypto_helper.c
+++ b/target/riscv/vcrypto_helper.c
@@ -222,6 +222,8 @@ static inline void xor_round_key(AESState *round_state,
AESState *round_key)
uint32_t total_elems = vext_get_total_elems(env, desc, 4);\
uint32_t vta = vext_vta(desc);\
\
+VSTART_CHECK_EARLY_EXIT(env); \
+ \
for (uint32_t i = env->vstart / 4; i < env->vl / 4; i++) {\
AESState round_key; \
round_key.d[0] = *((uint64_t *)vs2 + H8(i * 2 + 0)); \
@@ -246,6 +248,8 @@ static inline void xor_round_key(AESState *round_state,
AESState *round_key)
uint32_t total_elems = vext_get_total_elems(env, desc, 4);\
uint32_t vta = vext_vta(desc);\
\
+VSTART_CHECK_EARLY_EXIT(env); \
+ \
for (uint32_t i = env->vstart / 4; i < env->vl / 4; i++) {\
AESState round_key; \
round_key.d[0] = *((uint64_t *)vs2 + H8(0)); \
@@ -305,6 +309,8 @@ void HELPER(vaeskf1_vi)(void *vd_vptr, void *vs2_vptr,
uint32_t uimm,
uint32_t total_elems = vext_get_total_elems(env, desc, 4);
uint32_t vta = vext_vta(desc);
+VSTART_CHECK_EARLY_EXIT(env);
+
uimm &= 0b;
if (uimm > 10 || uimm == 0) {
uimm ^= 0b1000;
@@ -351,6 +357,8 @@ void HELPER(vaeskf2_vi)(void *vd_vptr, void *vs2_vptr,
uint32_t uimm,
uint32_t total_elems = vext_get_total_elems(env, desc, 4);
uint32_t vta = vext_vta(desc);
+VSTART_CHECK_EARLY_EXIT(env);
+
uimm &= 0b;
if (uimm > 14 || uimm < 2) {
uimm ^= 0b1000;
@@ -457,6 +465,8 @@ void HELPER(vsha2ms_vv)(void *vd, void *vs1, void *vs2,
CPURISCVState *env,
uint32_t total_elems;
uint32_t vta = vext_vta(desc);
+VSTART_CHECK_EARLY_EXIT(env);
+
for (uint32_t i = env->vstart / 4; i < env->vl / 4; i++) {
if (sew == MO_32) {
vsha2ms_e32(((uint32_t *)vd) + i * 4, ((uint32_t *)vs1) + i * 4,
@@ -572,6 +582,8 @@ void HELPER(vsha2ch32_vv)(void *vd, void *vs1, void *vs2,
CPURISCVState *env,
uint32_t total_elems;
uint32_t vta = vext_vta(desc);
+VSTART_CHECK_EARLY_EXIT(env);
+
for (uint32_t i = env->vstart / 4; i < env->vl / 4; i++) {
vsha2c_32(((uint32_t *)vs2) + 4 * i, ((uint32_t *)vd) + 4 * i,
((uint32_t *)vs1) + 4 * i + 2);
@@ -590,6 +602,8 @@ void HELPER(vsha2ch64_vv)(void *vd, void *vs1, void *vs2,
CPURISCVState *env,
uint32_t total_elems;
uint32_t vta = vext_vta(desc);
+VSTART_CHECK_EARLY_EXIT(env);
+
for (uint32_t i = env->vstart / 4; i < env->vl / 4; i++) {
vsha2c_64(((uint64_t *)vs2) + 4 * i, ((uint64_t *)vd) + 4 * i,
((uint64_t *)vs1) + 4 * i + 2);
@@ -608,6 +622,8 @@ void HELPER(vsha2cl32_vv)(void *vd, void *vs1, void *vs2,
CPURISCVState *env,
uint32_t total_elems;
uint32_t vta = vext_vta(desc);
+VSTART_CHECK_EARLY_EXIT(env);
+
for (uint32_t i = env->vstart / 4; i
[PATCH for-9.0 v14 5/8] target/riscv: remove 'over' brconds from vector trans
The previous patch added an early vstart >= vl exit in all vector
helpers, most of them using the VSTART_CHECK_EARLY_EXIT() macro,
and now we're left with a lot of 'brcond' that has not use. The
pattern goes like this:
VSTART_CHECK_EARLY_EXIT(env);
(...)
tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
(...)
gen_set_label(over);
return true;
The early exit makes the 'brcond' unneeded since it's already granted that
vstart < vl. Remove all 'over' conditionals from the vector helpers.
Note that not all insns uses helpers, and for those cases the 'brcond'
jump is the only way to filter vstart >= vl. This is the case of
trans_vmv_s_x() and trans_vfmv_s_f(). We won't remove the 'brcond'
conditionals from them.
While we're at it, remove the (vl == 0) brconds from trans_rvbf16.c.inc
too since they're unneeded.
Suggested-by: Richard Henderson
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Richard Henderson
Reviewed-by: Alistair Francis
---
target/riscv/insn_trans/trans_rvbf16.c.inc | 12 ---
target/riscv/insn_trans/trans_rvv.c.inc| 105 -
target/riscv/insn_trans/trans_rvvk.c.inc | 18
3 files changed, 135 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvbf16.c.inc
b/target/riscv/insn_trans/trans_rvbf16.c.inc
index 8ee99df3f3..a842e76a6b 100644
--- a/target/riscv/insn_trans/trans_rvbf16.c.inc
+++ b/target/riscv/insn_trans/trans_rvbf16.c.inc
@@ -71,11 +71,8 @@ static bool trans_vfncvtbf16_f_f_w(DisasContext *ctx,
arg_vfncvtbf16_f_f_w *a)
if (opfv_narrow_check(ctx, a) && (ctx->sew == MO_16)) {
uint32_t data = 0;
-TCGLabel *over = gen_new_label();
gen_set_rm_chkfrm(ctx, RISCV_FRM_DYN);
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, ctx->lmul);
@@ -87,7 +84,6 @@ static bool trans_vfncvtbf16_f_f_w(DisasContext *ctx,
arg_vfncvtbf16_f_f_w *a)
ctx->cfg_ptr->vlenb, data,
gen_helper_vfncvtbf16_f_f_w);
mark_vs_dirty(ctx);
-gen_set_label(over);
return true;
}
return false;
@@ -100,11 +96,8 @@ static bool trans_vfwcvtbf16_f_f_v(DisasContext *ctx,
arg_vfwcvtbf16_f_f_v *a)
if (opfv_widen_check(ctx, a) && (ctx->sew == MO_16)) {
uint32_t data = 0;
-TCGLabel *over = gen_new_label();
gen_set_rm_chkfrm(ctx, RISCV_FRM_DYN);
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, ctx->lmul);
@@ -116,7 +109,6 @@ static bool trans_vfwcvtbf16_f_f_v(DisasContext *ctx,
arg_vfwcvtbf16_f_f_v *a)
ctx->cfg_ptr->vlenb, data,
gen_helper_vfwcvtbf16_f_f_v);
mark_vs_dirty(ctx);
-gen_set_label(over);
return true;
}
return false;
@@ -130,11 +122,8 @@ static bool trans_vfwmaccbf16_vv(DisasContext *ctx,
arg_vfwmaccbf16_vv *a)
if (require_rvv(ctx) && vext_check_isa_ill(ctx) && (ctx->sew == MO_16) &&
vext_check_dss(ctx, a->rd, a->rs1, a->rs2, a->vm)) {
uint32_t data = 0;
-TCGLabel *over = gen_new_label();
gen_set_rm_chkfrm(ctx, RISCV_FRM_DYN);
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, ctx->lmul);
@@ -147,7 +136,6 @@ static bool trans_vfwmaccbf16_vv(DisasContext *ctx,
arg_vfwmaccbf16_vv *a)
ctx->cfg_ptr->vlenb, data,
gen_helper_vfwmaccbf16_vv);
mark_vs_dirty(ctx);
-gen_set_label(over);
return true;
}
return false;
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index 52c26a7834..4c1a064cf6 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -616,9 +616,6 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1,
uint32_t data,
TCGv base;
TCGv_i32 desc;
-TCGLabel *over = gen_new_label();
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
-
dest = tcg_temp_new_ptr();
mask = tcg_temp_new_ptr();
base = get_gpr(s, rs1, EXT_NONE);
@@ -660,7 +657,6 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1,
uint32_t data,
tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ);
}
-gen_set_label(over);
return true;
}
@@ -802,9 +798,6 @@ static bool ldst_stride_trans(uint32_t vd, uint32_t rs1,
uint32_t rs2,
TCGv base, stride;
TCGv_i32 desc;
-TCGLabel *over = gen_new_label();
-
[PATCH for-9.0 v14 8/8] target/riscv/vector_helper.c: optimize loops in ldst helpers
Change the for loops in ldst helpers to do a single increment in the
counter, and assign it env->vstart, to avoid re-reading from vstart
every time.
Suggested-by: Richard Henderson
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
Reviewed-by: Richard Henderson
---
target/riscv/vector_helper.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 48d041dd4e..6ee5380d0e 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -209,7 +209,7 @@ vext_ldst_stride(void *vd, void *v0, target_ulong base,
VSTART_CHECK_EARLY_EXIT(env);
-for (i = env->vstart; i < env->vl; i++, env->vstart++) {
+for (i = env->vstart; i < env->vl; env->vstart = ++i) {
k = 0;
while (k < nf) {
if (!vm && !vext_elem_mask(v0, i)) {
@@ -277,7 +277,7 @@ vext_ldst_us(void *vd, target_ulong base, CPURISCVState
*env, uint32_t desc,
VSTART_CHECK_EARLY_EXIT(env);
/* load bytes from guest memory */
-for (i = env->vstart; i < evl; i++, env->vstart++) {
+for (i = env->vstart; i < evl; env->vstart = ++i) {
k = 0;
while (k < nf) {
target_ulong addr = base + ((i * nf + k) << log2_esz);
@@ -393,7 +393,7 @@ vext_ldst_index(void *vd, void *v0, target_ulong base,
VSTART_CHECK_EARLY_EXIT(env);
/* load bytes from guest memory */
-for (i = env->vstart; i < env->vl; i++, env->vstart++) {
+for (i = env->vstart; i < env->vl; env->vstart = ++i) {
k = 0;
while (k < nf) {
if (!vm && !vext_elem_mask(v0, i)) {
--
2.43.2
[PATCH for-9.0 v14 2/8] trans_rvv.c.inc: set vstart = 0 in int scalar move insns
trans_vmv_x_s, trans_vmv_s_x, trans_vfmv_f_s and trans_vfmv_s_f aren't
setting vstart = 0 after execution. This is usually done by a helper in
vector_helper.c but these functions don't use helpers.
We'll set vstart after any potential 'over' brconds, and that will also
mandate a mark_vs_dirty() too.
Fixes: dedc53cbc9 ("target/riscv: rvv-1.0: integer scalar move instructions")
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Richard Henderson
---
target/riscv/insn_trans/trans_rvv.c.inc | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index e42728990e..8c16a9f5b3 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -3373,6 +3373,8 @@ static bool trans_vmv_x_s(DisasContext *s, arg_vmv_x_s *a)
vec_element_loadi(s, t1, a->rs2, 0, true);
tcg_gen_trunc_i64_tl(dest, t1);
gen_set_gpr(s, a->rd, dest);
+tcg_gen_movi_tl(cpu_vstart, 0);
+mark_vs_dirty(s);
return true;
}
return false;
@@ -3399,8 +3401,9 @@ static bool trans_vmv_s_x(DisasContext *s, arg_vmv_s_x *a)
s1 = get_gpr(s, a->rs1, EXT_NONE);
tcg_gen_ext_tl_i64(t1, s1);
vec_element_storei(s, a->rd, 0, t1);
-mark_vs_dirty(s);
gen_set_label(over);
+tcg_gen_movi_tl(cpu_vstart, 0);
+mark_vs_dirty(s);
return true;
}
return false;
@@ -3427,6 +3430,8 @@ static bool trans_vfmv_f_s(DisasContext *s, arg_vfmv_f_s
*a)
}
mark_fs_dirty(s);
+tcg_gen_movi_tl(cpu_vstart, 0);
+mark_vs_dirty(s);
return true;
}
return false;
@@ -3452,8 +3457,9 @@ static bool trans_vfmv_s_f(DisasContext *s, arg_vfmv_s_f
*a)
do_nanbox(s, t1, cpu_fpr[a->rs1]);
vec_element_storei(s, a->rd, 0, t1);
-mark_vs_dirty(s);
gen_set_label(over);
+tcg_gen_movi_tl(cpu_vstart, 0);
+mark_vs_dirty(s);
return true;
}
return false;
--
2.43.2
[PATCH for-9.0 v14 0/8] riscv: set vstart_eq_zero on vector insns
Hi, In this version we're fixing a redundant check in the vmvr_v helper that was pointed out by in v13. To make this change easier patches 3 and 4 switched places. A trivial change was made in patch 4 that don't warrant another review. We're missing acks in patch 3 only. Series based on master. Changes from v13: - patches 3 and 4: switched places - patch 3: - fixed commit msg: from "(...) now sure that vstart is being clearer" to "(...) now sure that vstart is being cleared" - patch 4: - do not check for vstart >= vl (i.e. add VSTART_CHECK_EARLY_EXIT()) in the 'vmvr_v' helper - v13 link: https://lore.kernel.org/qemu-riscv/20240313193059.405329-1-dbarb...@ventanamicro.com/ Daniel Henrique Barboza (7): target/riscv/vector_helper.c: set vstart = 0 in GEN_VEXT_VSLIDEUP_VX() trans_rvv.c.inc: set vstart = 0 in int scalar move insns target/riscv: always clear vstart in whole vec move insns target/riscv/vector_helpers: do early exit when vstart >= vl target/riscv: remove 'over' brconds from vector trans trans_rvv.c.inc: remove redundant mark_vs_dirty() calls target/riscv/vector_helper.c: optimize loops in ldst helpers Ivan Klokov (1): target/riscv: enable 'vstart_eq_zero' in the end of insns target/riscv/insn_trans/trans_rvbf16.c.inc | 18 +- target/riscv/insn_trans/trans_rvv.c.inc| 198 + target/riscv/insn_trans/trans_rvvk.c.inc | 30 +--- target/riscv/translate.c | 6 + target/riscv/vcrypto_helper.c | 32 target/riscv/vector_helper.c | 100 ++- target/riscv/vector_internals.c| 4 + target/riscv/vector_internals.h| 9 + 8 files changed, 205 insertions(+), 192 deletions(-) -- 2.43.2
[PATCH for-9.0 v14 7/8] target/riscv: enable 'vstart_eq_zero' in the end of insns
From: Ivan Klokov The vstart_eq_zero flag is updated at the beginning of the translation phase from the env->vstart variable. During the execution phase all functions will set env->vstart = 0 after a successful execution, but the vstart_eq_zero flag remains the same as at the start of the block. This will wrongly cause SIGILLs in translations that requires env->vstart = 0 and might be reading vstart_eq_zero = false. This patch adds a new finalize_rvv_inst() helper that is called at the end of each vector instruction that will both update vstart_eq_zero and do a mark_vs_dirty(). Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1976 Signed-off-by: Ivan Klokov Signed-off-by: Daniel Henrique Barboza Reviewed-by: Richard Henderson Reviewed-by: Alistair Francis --- target/riscv/insn_trans/trans_rvbf16.c.inc | 6 +- target/riscv/insn_trans/trans_rvv.c.inc| 83 -- target/riscv/insn_trans/trans_rvvk.c.inc | 12 ++-- target/riscv/translate.c | 6 ++ 4 files changed, 59 insertions(+), 48 deletions(-) diff --git a/target/riscv/insn_trans/trans_rvbf16.c.inc b/target/riscv/insn_trans/trans_rvbf16.c.inc index a842e76a6b..0a9cd1ec31 100644 --- a/target/riscv/insn_trans/trans_rvbf16.c.inc +++ b/target/riscv/insn_trans/trans_rvbf16.c.inc @@ -83,7 +83,7 @@ static bool trans_vfncvtbf16_f_f_w(DisasContext *ctx, arg_vfncvtbf16_f_f_w *a) ctx->cfg_ptr->vlenb, ctx->cfg_ptr->vlenb, data, gen_helper_vfncvtbf16_f_f_w); -mark_vs_dirty(ctx); +finalize_rvv_inst(ctx); return true; } return false; @@ -108,7 +108,7 @@ static bool trans_vfwcvtbf16_f_f_v(DisasContext *ctx, arg_vfwcvtbf16_f_f_v *a) ctx->cfg_ptr->vlenb, ctx->cfg_ptr->vlenb, data, gen_helper_vfwcvtbf16_f_f_v); -mark_vs_dirty(ctx); +finalize_rvv_inst(ctx); return true; } return false; @@ -135,7 +135,7 @@ static bool trans_vfwmaccbf16_vv(DisasContext *ctx, arg_vfwmaccbf16_vv *a) ctx->cfg_ptr->vlenb, ctx->cfg_ptr->vlenb, data, gen_helper_vfwmaccbf16_vv); -mark_vs_dirty(ctx); +finalize_rvv_inst(ctx); return true; } return false; diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc index b0f19dcd85..b3d467a874 100644 --- a/target/riscv/insn_trans/trans_rvv.c.inc +++ b/target/riscv/insn_trans/trans_rvv.c.inc @@ -167,7 +167,7 @@ static bool do_vsetvl(DisasContext *s, int rd, int rs1, TCGv s2) gen_helper_vsetvl(dst, tcg_env, s1, s2); gen_set_gpr(s, rd, dst); -mark_vs_dirty(s); +finalize_rvv_inst(s); gen_update_pc(s, s->cur_insn_len); lookup_and_goto_ptr(s); @@ -187,7 +187,7 @@ static bool do_vsetivli(DisasContext *s, int rd, TCGv s1, TCGv s2) gen_helper_vsetvl(dst, tcg_env, s1, s2); gen_set_gpr(s, rd, dst); -mark_vs_dirty(s); +finalize_rvv_inst(s); gen_update_pc(s, s->cur_insn_len); lookup_and_goto_ptr(s); s->base.is_jmp = DISAS_NORETURN; @@ -657,6 +657,7 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1, uint32_t data, tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ); } +finalize_rvv_inst(s); return true; } @@ -812,6 +813,7 @@ static bool ldst_stride_trans(uint32_t vd, uint32_t rs1, uint32_t rs2, fn(dest, mask, base, stride, tcg_env, desc); +finalize_rvv_inst(s); return true; } @@ -913,6 +915,7 @@ static bool ldst_index_trans(uint32_t vd, uint32_t rs1, uint32_t vs2, fn(dest, mask, base, index, tcg_env, desc); +finalize_rvv_inst(s); return true; } @@ -1043,7 +1046,7 @@ static bool ldff_trans(uint32_t vd, uint32_t rs1, uint32_t data, fn(dest, mask, base, tcg_env, desc); -mark_vs_dirty(s); +finalize_rvv_inst(s); return true; } @@ -1100,6 +1103,7 @@ static bool ldst_whole_trans(uint32_t vd, uint32_t rs1, uint32_t nf, fn(dest, base, tcg_env, desc); +finalize_rvv_inst(s); return true; } @@ -1189,7 +1193,7 @@ do_opivv_gvec(DisasContext *s, arg_rmrr *a, GVecGen3Fn *gvec_fn, tcg_env, s->cfg_ptr->vlenb, s->cfg_ptr->vlenb, data, fn); } -mark_vs_dirty(s); +finalize_rvv_inst(s); return true; } @@ -1240,7 +1244,7 @@ static bool opivx_trans(uint32_t vd, uint32_t rs1, uint32_t vs2, uint32_t vm, fn(dest, mask, src1, src2, tcg_env, desc); -mark_vs_dirty(s); +finalize_rvv_inst(s); return true; } @@ -1265,7 +1269,7 @@ do_opivx_gvec(DisasContext *s, arg_rmrr *a, GVecGen2sFn *gvec_fn, gvec_fn(s->sew, vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2), src1, MAXSZ(s), MAXSZ(s)); -mark_vs_dirty(s); +finalize_rvv_inst(s);
[PATCH for-9.0 v14 6/8] trans_rvv.c.inc: remove redundant mark_vs_dirty() calls
trans_vmv_v_i , trans_vfmv_v_f and the trans_##NAME macro from
GEN_VMV_WHOLE_TRANS() are calling mark_vs_dirty() in both branches of
their 'ifs'. conditionals.
Call it just once in the end like other functions are doing.
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Richard Henderson
Reviewed-by: Alistair Francis
Reviewed-by: Philippe Mathieu-Daudé
---
target/riscv/insn_trans/trans_rvv.c.inc | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index 4c1a064cf6..b0f19dcd85 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -2065,7 +2065,6 @@ static bool trans_vmv_v_i(DisasContext *s, arg_vmv_v_i *a)
if (s->vl_eq_vlmax && !(s->vta && s->lmul < 0)) {
tcg_gen_gvec_dup_imm(s->sew, vreg_ofs(s, a->rd),
MAXSZ(s), MAXSZ(s), simm);
-mark_vs_dirty(s);
} else {
TCGv_i32 desc;
TCGv_i64 s1;
@@ -2083,9 +2082,8 @@ static bool trans_vmv_v_i(DisasContext *s, arg_vmv_v_i *a)
s->cfg_ptr->vlenb, data));
tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, a->rd));
fns[s->sew](dest, s1, tcg_env, desc);
-
-mark_vs_dirty(s);
}
+mark_vs_dirty(s);
return true;
}
return false;
@@ -2612,7 +2610,6 @@ static bool trans_vfmv_v_f(DisasContext *s, arg_vfmv_v_f
*a)
tcg_gen_gvec_dup_i64(s->sew, vreg_ofs(s, a->rd),
MAXSZ(s), MAXSZ(s), t1);
-mark_vs_dirty(s);
} else {
TCGv_ptr dest;
TCGv_i32 desc;
@@ -2635,9 +2632,8 @@ static bool trans_vfmv_v_f(DisasContext *s, arg_vfmv_v_f
*a)
tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, a->rd));
fns[s->sew - 1](dest, t1, tcg_env, desc);
-
-mark_vs_dirty(s);
}
+mark_vs_dirty(s);
return true;
}
return false;
@@ -3560,12 +3556,11 @@ static bool trans_##NAME(DisasContext *s, arg_##NAME *
a) \
if (s->vstart_eq_zero) {\
tcg_gen_gvec_mov(s->sew, vreg_ofs(s, a->rd),\
vreg_ofs(s, a->rs2), maxsz, maxsz);\
-mark_vs_dirty(s); \
} else {\
tcg_gen_gvec_2_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2), \
tcg_env, maxsz, maxsz, 0, gen_helper_vmvr_v); \
-mark_vs_dirty(s); \
} \
+mark_vs_dirty(s); \
return true;\
} \
return false; \
--
2.43.2
[PATCH for-9.0 v14 3/8] target/riscv: always clear vstart in whole vec move insns
These insns have 2 paths: we'll either have vstart already cleared if
vstart_eq_zero or we'll do a brcond to check if vstart >= maxsz to call
the 'vmvr_v' helper. The helper will clear vstart if it executes until
the end, or if vstart >= vl.
However, if vstart >= maxsz, the helper will be skipped, and vstart
won't be cleared since the helper is being responsible from doing it.
We want to make the helpers responsible to manage vstart, including
these corner cases, precisely to avoid these situations. Move the vstart
>= maxsz cond to the helper, and be sure to clear vstart if that
happens. This way we're now sure that vstart is being cleared in the end
of the execution, regardless of the path taken.
Fixes: f714361ed7 ("target/riscv: rvv-1.0: implement vstart CSR")
Signed-off-by: Daniel Henrique Barboza
---
target/riscv/insn_trans/trans_rvv.c.inc | 3 ---
target/riscv/vector_helper.c| 5 +
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index 8c16a9f5b3..52c26a7834 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -3664,12 +3664,9 @@ static bool trans_##NAME(DisasContext *s, arg_##NAME *
a) \
vreg_ofs(s, a->rs2), maxsz, maxsz);\
mark_vs_dirty(s); \
} else {\
-TCGLabel *over = gen_new_label(); \
-tcg_gen_brcondi_tl(TCG_COND_GEU, cpu_vstart, maxsz, over); \
tcg_gen_gvec_2_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2), \
tcg_env, maxsz, maxsz, 0, gen_helper_vmvr_v); \
mark_vs_dirty(s); \
-gen_set_label(over);\
} \
return true;\
} \
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index ca79571ae2..cd8235ea98 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -5075,6 +5075,11 @@ void HELPER(vmvr_v)(void *vd, void *vs2, CPURISCVState
*env, uint32_t desc)
uint32_t startb = env->vstart * sewb;
uint32_t i = startb;
+if (env->vstart >= maxsz) {
+env->vstart = 0;
+return;
+}
+
memcpy((uint8_t *)vd + H1(i),
(uint8_t *)vs2 + H1(i),
maxsz - startb);
--
2.43.2
[PATCH for-9.0 v14 1/8] target/riscv/vector_helper.c: set vstart = 0 in GEN_VEXT_VSLIDEUP_VX()
The helper isn't setting env->vstart = 0 after its execution, as it is expected from every vector instruction that completes successfully. Signed-off-by: Daniel Henrique Barboza Reviewed-by: Richard Henderson Reviewed-by: Alistair Francis --- target/riscv/vector_helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c index fe56c007d5..ca79571ae2 100644 --- a/target/riscv/vector_helper.c +++ b/target/riscv/vector_helper.c @@ -4781,6 +4781,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \ } \ *((ETYPE *)vd + H(i)) = *((ETYPE *)vs2 + H(i - offset)); \ } \ +env->vstart = 0; \ /* set tail elements to 1s */ \ vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz); \ } -- 2.43.2
Re: [PATCH v2 02/16] esp.c: replace cmdfifo use of esp_fifo_pop_buf() in do_message_phase()
On 13/3/24 12:03, Philippe Mathieu-Daudé wrote:
On 13/3/24 09:57, Mark Cave-Ayland wrote:
The aim is to restrict the esp_fifo_*() functions so that they only
operate on
the hardware FIFO. When reading from cmdfifo in do_message_phase() use
the
underlying Fifo8 functions directly.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f8230c74b3..100560244b 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -309,6 +309,8 @@ static void do_command_phase(ESPState *s)
static void do_message_phase(ESPState *s)
{
+ uint32_t n;
+
if (s->cmdfifo_cdb_offset) {
uint8_t message = esp_fifo_pop(>cmdfifo);
@@ -320,7 +322,10 @@ static void do_message_phase(ESPState *s)
/* Ignore extended messages for now */
if (s->cmdfifo_cdb_offset) {
int len = MIN(s->cmdfifo_cdb_offset,
fifo8_num_used(>cmdfifo));
- esp_fifo_pop_buf(>cmdfifo, NULL, len);
+
+ if (len) {
+ fifo8_pop_buf(>cmdfifo, len, );
'n' is unused, use NULL?
Using NULL:
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 01/16] esp.c: replace cmdfifo use of esp_fifo_pop_buf() in do_command_phase()
On 13/3/24 22:08, Mark Cave-Ayland wrote:
On 13/03/2024 11:03, Philippe Mathieu-Daudé wrote:
On 13/3/24 09:57, Mark Cave-Ayland wrote:
The aim is to restrict the esp_fifo_*() functions so that they only
operate on
the hardware FIFO. When reading from cmdfifo in do_command_phase()
use the
underlying Fifo8 functions directly.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 590ff99744..f8230c74b3 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -265,7 +265,7 @@ static void esp_do_nodma(ESPState *s);
static void do_command_phase(ESPState *s)
{
- uint32_t cmdlen;
+ uint32_t cmdlen, n;
int32_t datalen;
SCSIDevice *current_lun;
uint8_t buf[ESP_CMDFIFO_SZ];
@@ -275,7 +275,7 @@ static void do_command_phase(ESPState *s)
if (!cmdlen || !s->current_dev) {
return;
}
- esp_fifo_pop_buf(>cmdfifo, buf, cmdlen);
+ memcpy(buf, fifo8_pop_buf(>cmdfifo, cmdlen, ), cmdlen);
'n' is unused, use NULL?
I was sure I had tried that before and it had failed, but I see that you
made it work with NULL in commit cd04033dbe ("util/fifo8: Allow
fifo8_pop_buf() to not populate popped length") - thanks!
Ah!
So using NULL in patches 1 and 2:
Reviewed-by: Philippe Mathieu-Daudé
I'll make the change for v3, but I'll wait a couple of days first to see
if there are any further comments, in particular R-B tags for patches 10
and 11.
I still have them in my TOREVIEW queue and need to digest them.
current_lun = scsi_device_find(>bus, 0, s->current_dev->id,
s->lun);
if (!current_lun) {
ATB,
Mark.
Re: [PATCH for-9.0 v13 4/8] target/riscv: always clear vstart in whole vec move insns
On 3/13/24 18:16, Richard Henderson wrote:
On 3/13/24 09:30, Daniel Henrique Barboza wrote:
These insns have 2 paths: we'll either have vstart already cleared if
vstart_eq_zero or we'll do a brcond to check if vstart >= maxsz to call
the 'vmvr_v' helper. The helper will clear vstart if it executes until
the end, or if vstart >= vl.
However, if vstart >= maxsz, the helper will be skipped, and vstart
won't be cleared since the helper is being responsible from doing it.
We want to make the helpers responsible to manage vstart, including
these corner cases, precisely to avoid these situations. Move the vstart
= maxsz cond to the helper, and be sure to clear vstart if that
happens. This way we're now 100% sure that vstart is being clearer in
the end of the execution, regardless of the path taken.
Fixes: f714361ed7 ("target/riscv: rvv-1.0: implement vstart CSR")
Signed-off-by: Daniel Henrique Barboza
---
target/riscv/insn_trans/trans_rvv.c.inc | 3 ---
target/riscv/vector_helper.c | 5 +
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index 8c16a9f5b3..52c26a7834 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -3664,12 +3664,9 @@ static bool trans_##NAME(DisasContext *s, arg_##NAME *
a) \
vreg_ofs(s, a->rs2), maxsz, maxsz); \
mark_vs_dirty(s); \
} else { \
- TCGLabel *over = gen_new_label(); \
- tcg_gen_brcondi_tl(TCG_COND_GEU, cpu_vstart, maxsz, over); \
tcg_gen_gvec_2_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2), \
tcg_env, maxsz, maxsz, 0, gen_helper_vmvr_v); \
mark_vs_dirty(s); \
- gen_set_label(over); \
} \
return true; \
} \
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index b4360dbd52..7260a5972b 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -5163,6 +5163,11 @@ void HELPER(vmvr_v)(void *vd, void *vs2, CPURISCVState
*env, uint32_t desc)
VSTART_CHECK_EARLY_EXIT(env);
+ if (env->vstart >= maxsz) {
+ env->vstart = 0;
+ return;
+ }
Did you need the VSTART_CHECK_EARLY_EXIT here then?
It certainly seems like the vstart >= vl check is redundant...
Hmm right. I thought about maxsz being calculated via vlmax, and vlmax not
necessarily
being == vl ... but vlmax will be always >= vl. So the vstart >= vl check s
supersed
by vstart >= maxsz.
I'll re-send. Thanks,
Daniel
r~
[PATCH-for-9.1 06/12] tcg/sparc64: Check for USER_ONLY definition instead of SOFTMMU one
Since we *might* have user emulation with softmmu,
replace the system emulation check by !user emulation one.
Signed-off-by: Philippe Mathieu-Daudé
---
tcg/sparc64/tcg-target.c.inc | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tcg/sparc64/tcg-target.c.inc b/tcg/sparc64/tcg-target.c.inc
index 176c98740b..56915a913b 100644
--- a/tcg/sparc64/tcg-target.c.inc
+++ b/tcg/sparc64/tcg-target.c.inc
@@ -78,7 +78,7 @@ static const char * const
tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
#define TCG_REG_T2 TCG_REG_G2
#define TCG_REG_T3 TCG_REG_O7
-#ifndef CONFIG_SOFTMMU
+#ifdef CONFIG_USER_ONLY
# define TCG_GUEST_BASE_REG TCG_REG_I5
#endif
@@ -961,7 +961,7 @@ static void tcg_target_qemu_prologue(TCGContext *s)
tcg_out32(s, SAVE | INSN_RD(TCG_REG_O6) | INSN_RS1(TCG_REG_O6) |
INSN_IMM13(-frame_size));
-#ifndef CONFIG_SOFTMMU
+#ifdef CONFIG_USER_ONLY
if (guest_base != 0) {
tcg_out_movi_int(s, TCG_TYPE_PTR, TCG_GUEST_BASE_REG,
guest_base, true, TCG_REG_T1);
@@ -1075,7 +1075,7 @@ static TCGLabelQemuLdst *prepare_host_addr(TCGContext *s,
HostAddress *h,
h->aa.align = MAX(h->aa.align, s_bits);
a_mask = (1u << h->aa.align) - 1;
-#ifdef CONFIG_SOFTMMU
+#ifndef CONFIG_USER_ONLY
int mem_index = get_mmuidx(oi);
int fast_off = tlb_mask_table_ofs(s, mem_index);
int mask_off = fast_off + offsetof(CPUTLBDescFast, mask);
@@ -1147,7 +1147,7 @@ static TCGLabelQemuLdst *prepare_host_addr(TCGContext *s,
HostAddress *h,
tcg_out_bpcc0(s, COND_NE, BPCC_PN | BPCC_ICC, 0);
}
h->base = guest_base ? TCG_GUEST_BASE_REG : TCG_REG_G0;
-#endif
+#endif /* CONFIG_USER_ONLY */
/* If the guest address must be zero-extended, do in the delay slot. */
if (addr_type == TCG_TYPE_I32) {
--
2.41.0
[PATCH-for-9.1 10/12] exec/cpu-defs: Restrict SOFTMMU specific definitions to accel/tcg/
CPU_TLB_foo definitions are specific to SoftMMU and only used in accel/tcg/. Signed-off-by: Philippe Mathieu-Daudé --- accel/tcg/internal-target.h | 26 ++ include/exec/cpu-defs.h | 26 -- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/accel/tcg/internal-target.h b/accel/tcg/internal-target.h index b22b29c461..9b5cc9168b 100644 --- a/accel/tcg/internal-target.h +++ b/accel/tcg/internal-target.h @@ -12,6 +12,32 @@ #include "exec/exec-all.h" #include "exec/translate-all.h" +#if defined(CONFIG_SOFTMMU) && defined(CONFIG_TCG) +#define CPU_TLB_DYN_MIN_BITS 6 +#define CPU_TLB_DYN_DEFAULT_BITS 8 + +# if HOST_LONG_BITS == 32 +/* Make sure we do not require a double-word shift for the TLB load */ +# define CPU_TLB_DYN_MAX_BITS (32 - TARGET_PAGE_BITS) +# else /* HOST_LONG_BITS == 64 */ +/* + * Assuming TARGET_PAGE_BITS==12, with 2**22 entries we can cover 2**(22+12) == + * 2**34 == 16G of address space. This is roughly what one would expect a + * TLB to cover in a modern (as of 2018) x86_64 CPU. For instance, Intel + * Skylake's Level-2 STLB has 16 1G entries. + * Also, make sure we do not size the TLB past the guest's address space. + */ +# ifdef TARGET_PAGE_BITS_VARY +# define CPU_TLB_DYN_MAX_BITS \ +MIN(22, TARGET_VIRT_ADDR_SPACE_BITS - TARGET_PAGE_BITS) +# else +# define CPU_TLB_DYN_MAX_BITS \ +MIN_CONST(22, TARGET_VIRT_ADDR_SPACE_BITS - TARGET_PAGE_BITS) +# endif +# endif + +#endif /* CONFIG_SOFTMMU && CONFIG_TCG */ + /* * Access to the various translations structures need to be serialised * via locks for consistency. In user-mode emulation access to the diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h index 3915438b83..955cbefe81 100644 --- a/include/exec/cpu-defs.h +++ b/include/exec/cpu-defs.h @@ -54,30 +54,4 @@ #include "exec/target_long.h" -#if defined(CONFIG_SOFTMMU) && defined(CONFIG_TCG) -#define CPU_TLB_DYN_MIN_BITS 6 -#define CPU_TLB_DYN_DEFAULT_BITS 8 - -# if HOST_LONG_BITS == 32 -/* Make sure we do not require a double-word shift for the TLB load */ -# define CPU_TLB_DYN_MAX_BITS (32 - TARGET_PAGE_BITS) -# else /* HOST_LONG_BITS == 64 */ -/* - * Assuming TARGET_PAGE_BITS==12, with 2**22 entries we can cover 2**(22+12) == - * 2**34 == 16G of address space. This is roughly what one would expect a - * TLB to cover in a modern (as of 2018) x86_64 CPU. For instance, Intel - * Skylake's Level-2 STLB has 16 1G entries. - * Also, make sure we do not size the TLB past the guest's address space. - */ -# ifdef TARGET_PAGE_BITS_VARY -# define CPU_TLB_DYN_MAX_BITS \ -MIN(22, TARGET_VIRT_ADDR_SPACE_BITS - TARGET_PAGE_BITS) -# else -# define CPU_TLB_DYN_MAX_BITS \ -MIN_CONST(22, TARGET_VIRT_ADDR_SPACE_BITS - TARGET_PAGE_BITS) -# endif -# endif - -#endif /* CONFIG_SOFTMMU && CONFIG_TCG */ - #endif -- 2.41.0
[PATCH-for-9.1 07/12] plugins/api: Check for USER_ONLY definition instead of SOFTMMU one
Since we *might* have user emulation with softmmu,
replace the system emulation check by !user emulation one.
Signed-off-by: Philippe Mathieu-Daudé
---
plugins/api.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/plugins/api.c b/plugins/api.c
index 8fa5a600ac..06d3e95da2 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -294,14 +294,14 @@ bool qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info)
* Virtual Memory queries
*/
-#ifdef CONFIG_SOFTMMU
+#ifndef CONFIG_USER_ONLY
static __thread struct qemu_plugin_hwaddr hwaddr_info;
#endif
struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
uint64_t vaddr)
{
-#ifdef CONFIG_SOFTMMU
+#ifndef CONFIG_USER_ONLY
CPUState *cpu = current_cpu;
unsigned int mmu_idx = get_mmuidx(info);
enum qemu_plugin_mem_rw rw = get_plugin_meminfo_rw(info);
@@ -323,7 +323,7 @@ struct qemu_plugin_hwaddr
*qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr)
{
-#ifdef CONFIG_SOFTMMU
+#ifndef CONFIG_USER_ONLY
return haddr->is_io;
#else
return false;
@@ -332,7 +332,7 @@ bool qemu_plugin_hwaddr_is_io(const struct
qemu_plugin_hwaddr *haddr)
uint64_t qemu_plugin_hwaddr_phys_addr(const struct qemu_plugin_hwaddr *haddr)
{
-#ifdef CONFIG_SOFTMMU
+#ifndef CONFIG_USER_ONLY
if (haddr) {
return haddr->phys_addr;
}
@@ -342,7 +342,7 @@ uint64_t qemu_plugin_hwaddr_phys_addr(const struct
qemu_plugin_hwaddr *haddr)
const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h)
{
-#ifdef CONFIG_SOFTMMU
+#ifndef CONFIG_USER_ONLY
if (h && h->is_io) {
MemoryRegion *mr = h->mr;
if (!mr->name) {
--
2.41.0
[PATCH-for-9.1 09/12] accel/tcg/internal: Check for USER_ONLY definition instead of SOFTMMU
Since we *might* have user emulation with softmmu,
replace the system emulation check by !user emulation one.
Signed-off-by: Philippe Mathieu-Daudé
---
accel/tcg/internal-target.h | 6 +++---
accel/tcg/tb-hash.h | 4 ++--
accel/tcg/tcg-all.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/accel/tcg/internal-target.h b/accel/tcg/internal-target.h
index 4e36cf858e..b22b29c461 100644
--- a/accel/tcg/internal-target.h
+++ b/accel/tcg/internal-target.h
@@ -24,7 +24,7 @@
#define assert_memory_lock()
#endif
-#if defined(CONFIG_SOFTMMU) && defined(CONFIG_DEBUG_TCG)
+#if !defined(CONFIG_USER_ONLY) && defined(CONFIG_DEBUG_TCG)
void assert_no_pages_locked(void);
#else
static inline void assert_no_pages_locked(void) { }
@@ -62,12 +62,12 @@ void tb_unlock_page1(tb_page_addr_t, tb_page_addr_t);
void tb_unlock_pages(TranslationBlock *);
#endif
-#ifdef CONFIG_SOFTMMU
+#ifndef CONFIG_USER_ONLY
void tb_invalidate_phys_range_fast(ram_addr_t ram_addr,
unsigned size,
uintptr_t retaddr);
G_NORETURN void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
-#endif /* CONFIG_SOFTMMU */
+#endif /* !CONFIG_USER_ONLY */
TranslationBlock *tb_gen_code(CPUState *cpu, vaddr pc,
uint64_t cs_base, uint32_t flags,
diff --git a/accel/tcg/tb-hash.h b/accel/tcg/tb-hash.h
index a0c61f25cd..45a484ce82 100644
--- a/accel/tcg/tb-hash.h
+++ b/accel/tcg/tb-hash.h
@@ -25,7 +25,7 @@
#include "qemu/xxhash.h"
#include "tb-jmp-cache.h"
-#ifdef CONFIG_SOFTMMU
+#ifndef CONFIG_USER_ONLY
/* Only the bottom TB_JMP_PAGE_BITS of the jump cache hash bits vary for
addresses on the same page. The top bits are the same. This allows
@@ -58,7 +58,7 @@ static inline unsigned int tb_jmp_cache_hash_func(vaddr pc)
return (pc ^ (pc >> TB_JMP_CACHE_BITS)) & (TB_JMP_CACHE_SIZE - 1);
}
-#endif /* CONFIG_SOFTMMU */
+#endif /* CONFIG_USER_ONLY */
static inline
uint32_t tb_hash_func(tb_page_addr_t phys_pc, vaddr pc,
diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index c6619f5b98..929af1f64c 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -116,7 +116,7 @@ static int tcg_init_machine(MachineState *ms)
tb_htable_init();
tcg_init(s->tb_size * MiB, s->splitwx_enabled, max_cpus);
-#if defined(CONFIG_SOFTMMU)
+#if !defined(CONFIG_USER_ONLY)
/*
* There's no guest base to take into account, so go ahead and
* initialize the prologue now.
--
2.41.0
[PATCH-for-9.0? 08/12] accel/tcg/tb-maint: Add comments around system emulation
Signed-off-by: Philippe Mathieu-Daudé --- accel/tcg/tb-maint.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c index da39a43bd8..2fef7db9e1 100644 --- a/accel/tcg/tb-maint.c +++ b/accel/tcg/tb-maint.c @@ -147,7 +147,8 @@ static PageForEachNext foreach_tb_next(PageForEachNext tb, return NULL; } -#else +#else /* !CONFIG_USER_ONLY */ + /* * In system mode we want L1_MAP to be based on ram offsets. */ @@ -1088,7 +1089,7 @@ bool tb_invalidate_phys_page_unwind(tb_page_addr_t addr, uintptr_t pc) } return false; } -#else +#else /* !CONFIG_USER_ONLY */ /* * @p must be non-NULL. * Call with all @pages locked. @@ -1226,4 +1227,4 @@ void tb_invalidate_phys_range_fast(ram_addr_t ram_addr, page_collection_unlock(pages); } -#endif /* CONFIG_USER_ONLY */ +#endif /* !CONFIG_USER_ONLY */ -- 2.41.0
[PATCH-for-9.1 12/12] exec/poison: Poison CONFIG_SOFTMMU again
Now that the confusion around SOFTMMU vs SYSTEM emulation
was clarified, we can restore the CONFIG_SOFTMMU poison
pragma.
This reverts commit d31b84041d4353ef310ffde23c87b78c2aa32ead
("exec/poison: Do not poison CONFIG_SOFTMMU").
Signed-off-by: Philippe Mathieu-Daudé
---
include/exec/poison.h | 1 +
scripts/make-config-poison.sh | 1 -
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/exec/poison.h b/include/exec/poison.h
index 1ea5633eb3..fbec710f6c 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -84,6 +84,7 @@
#pragma GCC poison CONFIG_HVF
#pragma GCC poison CONFIG_LINUX_USER
#pragma GCC poison CONFIG_KVM
+#pragma GCC poison CONFIG_SOFTMMU
#pragma GCC poison CONFIG_WHPX
#pragma GCC poison CONFIG_XEN
diff --git a/scripts/make-config-poison.sh b/scripts/make-config-poison.sh
index 2b36907e23..6ef5580f84 100755
--- a/scripts/make-config-poison.sh
+++ b/scripts/make-config-poison.sh
@@ -9,7 +9,6 @@ fi
exec sed -n \
-e' /CONFIG_TCG/d' \
-e '/CONFIG_USER_ONLY/d' \
- -e '/CONFIG_SOFTMMU/d' \
-e '/^#define / {' \
-e's///' \
-e's/ .*//' \
--
2.41.0
Re: [PATCH-for-9.0 3/4] qtest/libqos: Un-inline size_to_prdtl()
On 13/3/24 20:39, Peter Maydell wrote:
On Wed, 13 Mar 2024 at 18:50, Philippe Mathieu-Daudé wrote:
See previous commit and commit 9de9fa5cf2 ("Avoid using inlined
functions with external linkage") for rationale.
Signed-off-by: Philippe Mathieu-Daudé
---
tests/qtest/libqos/ahci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qtest/libqos/ahci.c b/tests/qtest/libqos/ahci.c
index a2c94c6e06..135b23ffd9 100644
--- a/tests/qtest/libqos/ahci.c
+++ b/tests/qtest/libqos/ahci.c
@@ -662,7 +662,7 @@ unsigned ahci_pick_cmd(AHCIQState *ahci, uint8_t port)
g_assert_not_reached();
}
-inline unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd)
+unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd)
{
/* Each PRD can describe up to 4MiB */
g_assert_cmphex(bytes_per_prd, <=, 4096 * 1024);
It looks like this function is only used in this file, so
we could make it static ?
Eh I did check for that, but sure how I missed it...
[PATCH-for-9.0? 04/12] gdbstub/system: Rename 'user_ctx' argument as 'ctx'
Signed-off-by: Philippe Mathieu-Daudé
---
gdbstub/internals.h | 8
gdbstub/system.c| 8
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index 7055138dee..e39c4b113c 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -187,7 +187,7 @@ typedef union GdbCmdVariant {
#define get_param(p, i)(_array_index(p, GdbCmdVariant, i))
-void gdb_handle_query_rcmd(GArray *params, void *user_ctx); /* system */
+void gdb_handle_query_rcmd(GArray *params, void *ctx); /* system */
void gdb_handle_query_offsets(GArray *params, void *user_ctx); /* user */
void gdb_handle_query_xfer_auxv(GArray *params, void *user_ctx); /*user */
void gdb_handle_v_file_open(GArray *params, void *user_ctx); /* user */
@@ -200,11 +200,11 @@ void gdb_handle_query_supported_user(const char
*gdb_supported); /* user */
bool gdb_handle_set_thread_user(uint32_t pid, uint32_t tid); /* user */
bool gdb_handle_detach_user(uint32_t pid); /* user */
-void gdb_handle_query_attached(GArray *params, void *user_ctx); /* both */
+void gdb_handle_query_attached(GArray *params, void *ctx); /* both */
/* system only */
-void gdb_handle_query_qemu_phy_mem_mode(GArray *params, void *user_ctx);
-void gdb_handle_set_qemu_phy_mem_mode(GArray *params, void *user_ctx);
+void gdb_handle_query_qemu_phy_mem_mode(GArray *params, void *ctx);
+void gdb_handle_set_qemu_phy_mem_mode(GArray *params, void *ctx);
/* sycall handling */
void gdb_handle_file_io(GArray *params, void *user_ctx);
diff --git a/gdbstub/system.c b/gdbstub/system.c
index a3ce384cd1..d235403855 100644
--- a/gdbstub/system.c
+++ b/gdbstub/system.c
@@ -488,13 +488,13 @@ bool gdb_can_reverse(void)
*/
void gdb_handle_query_qemu_phy_mem_mode(GArray *params,
-void *user_ctx)
+void *ctx)
{
g_string_printf(gdbserver_state.str_buf, "%d", phy_memory_mode);
gdb_put_strbuf();
}
-void gdb_handle_set_qemu_phy_mem_mode(GArray *params, void *user_ctx)
+void gdb_handle_set_qemu_phy_mem_mode(GArray *params, void *ctx)
{
if (!params->len) {
gdb_put_packet("E22");
@@ -509,7 +509,7 @@ void gdb_handle_set_qemu_phy_mem_mode(GArray *params, void
*user_ctx)
gdb_put_packet("OK");
}
-void gdb_handle_query_rcmd(GArray *params, void *user_ctx)
+void gdb_handle_query_rcmd(GArray *params, void *ctx)
{
const guint8 zero = 0;
int len;
@@ -539,7 +539,7 @@ void gdb_handle_query_rcmd(GArray *params, void *user_ctx)
* Execution state helpers
*/
-void gdb_handle_query_attached(GArray *params, void *user_ctx)
+void gdb_handle_query_attached(GArray *params, void *ctx)
{
gdb_put_packet("1");
}
--
2.41.0
[PATCH-for-9.0? 03/12] gdbstub: Correct invalid mentions of 'softmmu' by 'system'
Signed-off-by: Philippe Mathieu-Daudé
---
gdbstub/internals.h | 20 ++--
gdbstub/system.c| 2 +-
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index b472459838..7055138dee 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -115,7 +115,7 @@ void gdb_read_byte(uint8_t ch);
/*
* Packet acknowledgement - we handle this slightly differently
- * between user and softmmu mode, mainly to deal with the differences
+ * between user and system mode, mainly to deal with the differences
* between the flexible chardev and the direct fd approaches.
*
* We currently don't support a negotiated QStartNoAckMode
@@ -125,7 +125,7 @@ void gdb_read_byte(uint8_t ch);
* gdb_got_immediate_ack() - check ok to continue
*
* Returns true to continue, false to re-transmit for user only, the
- * softmmu stub always returns true.
+ * system stub always returns true.
*/
bool gdb_got_immediate_ack(void);
/* utility helpers */
@@ -135,12 +135,12 @@ CPUState *gdb_first_attached_cpu(void);
void gdb_append_thread_id(CPUState *cpu, GString *buf);
int gdb_get_cpu_index(CPUState *cpu);
unsigned int gdb_get_max_cpus(void); /* both */
-bool gdb_can_reverse(void); /* softmmu, stub for user */
+bool gdb_can_reverse(void); /* system emulation, stub for user */
int gdb_target_sigtrap(void); /* user */
void gdb_create_default_process(GDBState *s);
-/* signal mapping, common for softmmu, specialised for user-mode */
+/* signal mapping, common for system, specialised for user-mode */
int gdb_signal_to_target(int sig);
int gdb_target_signal_to_gdb(int sig);
@@ -157,12 +157,12 @@ void gdb_continue(void);
int gdb_continue_partial(char *newstates);
/*
- * Helpers with separate softmmu and user implementations
+ * Helpers with separate system and user implementations
*/
void gdb_put_buffer(const uint8_t *buf, int len);
/*
- * Command handlers - either specialised or softmmu or user only
+ * Command handlers - either specialised or system or user only
*/
void gdb_init_gdbserver_state(void);
@@ -187,7 +187,7 @@ typedef union GdbCmdVariant {
#define get_param(p, i)(_array_index(p, GdbCmdVariant, i))
-void gdb_handle_query_rcmd(GArray *params, void *user_ctx); /* softmmu */
+void gdb_handle_query_rcmd(GArray *params, void *user_ctx); /* system */
void gdb_handle_query_offsets(GArray *params, void *user_ctx); /* user */
void gdb_handle_query_xfer_auxv(GArray *params, void *user_ctx); /*user */
void gdb_handle_v_file_open(GArray *params, void *user_ctx); /* user */
@@ -202,7 +202,7 @@ bool gdb_handle_detach_user(uint32_t pid); /* user */
void gdb_handle_query_attached(GArray *params, void *user_ctx); /* both */
-/* softmmu only */
+/* system only */
void gdb_handle_query_qemu_phy_mem_mode(GArray *params, void *user_ctx);
void gdb_handle_set_qemu_phy_mem_mode(GArray *params, void *user_ctx);
@@ -212,11 +212,11 @@ bool gdb_handled_syscall(void);
void gdb_disable_syscalls(void);
void gdb_syscall_reset(void);
-/* user/softmmu specific syscall handling */
+/* user/system specific syscall handling */
void gdb_syscall_handling(const char *syscall_packet);
/*
- * Break/Watch point support - there is an implementation for softmmu
+ * Break/Watch point support - there is an implementation for system
* and user mode.
*/
bool gdb_supports_guest_debug(void);
diff --git a/gdbstub/system.c b/gdbstub/system.c
index 83fd452800..a3ce384cd1 100644
--- a/gdbstub/system.c
+++ b/gdbstub/system.c
@@ -1,5 +1,5 @@
/*
- * gdb server stub - softmmu specific bits
+ * gdb server stub - system specific bits
*
* Debug integration depends on support from the individual
* accelerators so most of this involves calling the ops helpers.
--
2.41.0
[PATCH-for-9.1 11/12] tcg: Remove unused CONFIG_SOFTMMU definition from libtcg_system.fa
Signed-off-by: Philippe Mathieu-Daudé
---
tcg/meson.build | 1 -
1 file changed, 1 deletion(-)
diff --git a/tcg/meson.build b/tcg/meson.build
index 8251589fd4..b5246676c6 100644
--- a/tcg/meson.build
+++ b/tcg/meson.build
@@ -42,7 +42,6 @@ user_ss.add(tcg_user)
libtcg_system = static_library('tcg_system',
tcg_ss.sources() + genh,
name_suffix: 'fa',
-c_args: '-DCONFIG_SOFTMMU',
build_by_default: false)
tcg_system = declare_dependency(link_with: libtcg_system,
--
2.41.0
[PATCH-for-9.0? 05/12] target/ppc: Rename init_excp_4xx_softmmu() -> init_excp_4xx()
Unify with other init_excp_FOO() in the same file.
Signed-off-by: Philippe Mathieu-Daudé
---
target/ppc/cpu_init.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 7e65f08147..b208bd91a0 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -1642,7 +1642,7 @@ static void register_8xx_sprs(CPUPPCState *env)
/*/
/* Exception vectors models */
-static void init_excp_4xx_softmmu(CPUPPCState *env)
+static void init_excp_4xx(CPUPPCState *env)
{
#if !defined(CONFIG_USER_ONLY)
env->excp_vectors[POWERPC_EXCP_CRITICAL] = 0x0100;
@@ -2120,7 +2120,7 @@ static void init_proc_405(CPUPPCState *env)
env->id_tlbs = 0;
env->tlb_type = TLB_EMB;
#endif
-init_excp_4xx_softmmu(env);
+init_excp_4xx(env);
env->dcache_line_size = 32;
env->icache_line_size = 32;
/* Allocate hardware IRQ controller */
--
2.41.0
[PATCH-for-9.0? 02/12] travis-ci: Rename SOFTMMU -> SYSTEM
Since we *might* have user emulation with softmmu,
rename MAIN_SOFTMMU_TARGETS as MAIN_SYSTEM_TARGETS
to express 'system emulation targets'.
Signed-off-by: Philippe Mathieu-Daudé
---
.travis.yml | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index 76859d48da..597d151b80 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -35,7 +35,7 @@ env:
- TEST_BUILD_CMD=""
- TEST_CMD="make check V=1"
# This is broadly a list of "mainline" system targets which have support
across the major distros
--
MAIN_SOFTMMU_TARGETS="aarch64-softmmu,mips64-softmmu,ppc64-softmmu,riscv64-softmmu,s390x-softmmu,x86_64-softmmu"
+-
MAIN_SYSTEM_TARGETS="aarch64-softmmu,mips64-softmmu,ppc64-softmmu,riscv64-softmmu,s390x-softmmu,x86_64-softmmu"
- CCACHE_SLOPPINESS="include_file_ctime,include_file_mtime"
- CCACHE_MAXSIZE=1G
- G_MESSAGES_DEBUG=error
@@ -114,7 +114,7 @@ jobs:
env:
- TEST_CMD="make check check-tcg V=1"
- CONFIG="--disable-containers --enable-fdt=system
- --target-list=${MAIN_SOFTMMU_TARGETS} --cxx=/bin/false"
+ --target-list=${MAIN_SYSTEM_TARGETS} --cxx=/bin/false"
- UNRELIABLE=true
- name: "[ppc64] GCC check-tcg"
@@ -185,7 +185,7 @@ jobs:
env:
- TEST_CMD="make check check-tcg V=1"
- CONFIG="--disable-containers --enable-fdt=system
- --target-list=${MAIN_SOFTMMU_TARGETS},s390x-linux-user"
+ --target-list=${MAIN_SYSTEM_TARGETS},s390x-linux-user"
- UNRELIABLE=true
script:
- BUILD_RC=0 && make -j${JOBS} || BUILD_RC=$?
@@ -226,7 +226,7 @@ jobs:
- genisoimage
env:
- CONFIG="--disable-containers --enable-fdt=system --audio-drv-list=sdl
- --disable-user --target-list-exclude=${MAIN_SOFTMMU_TARGETS}"
+ --disable-user --target-list-exclude=${MAIN_SYSTEM_TARGETS}"
- name: "[s390x] GCC (user)"
arch: s390x
--
2.41.0
[PATCH-for-9.0? 01/12] accel/tcg/plugin: Remove CONFIG_SOFTMMU_GATE definition
The CONFIG_SOFTMMU_GATE definition was never used, remove it. Signed-off-by: Philippe Mathieu-Daudé --- accel/tcg/plugin-gen.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c index 8028786c7b..cd78ef94a1 100644 --- a/accel/tcg/plugin-gen.c +++ b/accel/tcg/plugin-gen.c @@ -57,12 +57,6 @@ #include "exec/helper-info.c.inc" #undef HELPER_H -#ifdef CONFIG_SOFTMMU -# define CONFIG_SOFTMMU_GATE 1 -#else -# define CONFIG_SOFTMMU_GATE 0 -#endif - /* * plugin_cb_start TCG op args[]: * 0: enum plugin_gen_from -- 2.41.0
[PATCH-for-9.1 00/12] accel/tcg: Finish replacing SOFTMMU -> SYSTEM
Finish the softmmu/system clarification. Poison CONFIG_SOFTMMU at the end, we can still check for system mode with !CONFIG_USER_ONLY. Philippe Mathieu-Daudé (12): accel/tcg/plugin: Remove CONFIG_SOFTMMU_GATE definition travis-ci: Rename SOFTMMU -> SYSTEM gdbstub: Correct invalid mentions of 'softmmu' by 'system' gdbstub/system: Rename 'user_ctx' argument as 'ctx' target/ppc: Rename init_excp_4xx_softmmu() -> init_excp_4xx() tcg/sparc64: Check for USER_ONLY definition instead of SOFTMMU one plugins/api: Check for USER_ONLY definition instead of SOFTMMU one accel/tcg/tb-maint: Add comments around system emulation accel/tcg/internal: Check for USER_ONLY definition instead of SOFTMMU exec/cpu-defs: Restrict SOFTMMU specific definitions to accel/tcg/ tcg: Remove unused CONFIG_SOFTMMU definition from libtcg_system.fa exec/poison: Poison CONFIG_SOFTMMU again accel/tcg/internal-target.h | 32 +--- accel/tcg/tb-hash.h | 4 ++-- gdbstub/internals.h | 26 +- include/exec/cpu-defs.h | 26 -- include/exec/poison.h | 1 + accel/tcg/plugin-gen.c| 6 -- accel/tcg/tb-maint.c | 7 --- accel/tcg/tcg-all.c | 2 +- gdbstub/system.c | 10 +- plugins/api.c | 10 +- target/ppc/cpu_init.c | 4 ++-- tcg/sparc64/tcg-target.c.inc | 8 .travis.yml | 8 scripts/make-config-poison.sh | 1 - tcg/meson.build | 1 - 15 files changed, 70 insertions(+), 76 deletions(-) -- 2.41.0
[PATCH v2 0/2] migration mapped-ram fixes
Hi, In this v2: patch 1 - The fix for the ioc leaks, now including the main channel patch 2 - A fix for an fd: migration case I thought I had written code for, but obviously didn't. Thank you for your patience. based-on: https://gitlab.com/peterx/qemu/-/commits/migration-stable CI run: https://gitlab.com/farosas/qemu/-/pipelines/1212483701 Fabiano Rosas (2): migration: Fix iocs leaks during file and fd migration migration/multifd: Ensure we're not given a socket for file migration migration/fd.c | 35 +++--- migration/file.c | 65 migration/file.h | 1 + 3 files changed, 60 insertions(+), 41 deletions(-) -- 2.35.3
[PATCH v2 2/2] migration/multifd: Ensure we're not given a socket for file migration
When doing migration using the fd: URI, the incoming migration starts
before the user has passed the file descriptor to QEMU. This means
that the checks at migration_channels_and_transport_compatible()
happen too soon and we need to allow a migration channel of type
SOCKET_ADDRESS_TYPE_FD even though socket migration is not supported
with multifd.
The commit decdc76772 ("migration/multifd: Add mapped-ram support to
fd: URI") was supposed to add a second check prior to starting
migration to make sure a socket fd is not passed instead of a file fd,
but failed to do so.
Add the missing verification.
Fixes: decdc76772 ("migration/multifd: Add mapped-ram support to fd: URI")
Signed-off-by: Fabiano Rosas
---
migration/fd.c | 8
migration/file.c | 7 +++
2 files changed, 15 insertions(+)
diff --git a/migration/fd.c b/migration/fd.c
index 39a52e5c90..c07030f715 100644
--- a/migration/fd.c
+++ b/migration/fd.c
@@ -22,6 +22,7 @@
#include "migration.h"
#include "monitor/monitor.h"
#include "io/channel-file.h"
+#include "io/channel-socket.h"
#include "io/channel-util.h"
#include "options.h"
#include "trace.h"
@@ -95,6 +96,13 @@ void fd_start_incoming_migration(const char *fdname, Error
**errp)
}
if (migrate_multifd()) {
+if (fd_is_socket(fd)) {
+error_setg(errp,
+ "Multifd migration to a socket FD is not supported");
+object_unref(ioc);
+return;
+}
+
file_create_incoming_channels(ioc, errp);
} else {
qio_channel_set_name(ioc, "migration-fd-incoming");
diff --git a/migration/file.c b/migration/file.c
index ddde0ca818..b6e8ba13f2 100644
--- a/migration/file.c
+++ b/migration/file.c
@@ -15,6 +15,7 @@
#include "file.h"
#include "migration.h"
#include "io/channel-file.h"
+#include "io/channel-socket.h"
#include "io/channel-util.h"
#include "options.h"
#include "trace.h"
@@ -58,6 +59,12 @@ bool file_send_channel_create(gpointer opaque, Error **errp)
int fd = fd_args_get_fd();
if (fd && fd != -1) {
+if (fd_is_socket(fd)) {
+error_setg(errp,
+ "Multifd migration to a socket FD is not supported");
+goto out;
+}
+
ioc = qio_channel_file_new_dupfd(fd, errp);
} else {
ioc = qio_channel_file_new_path(outgoing_args.fname, flags, 0, errp);
--
2.35.3
[PATCH v2 1/2] migration: Fix iocs leaks during file and fd migration
The memory for the io channels is being leaked in three different ways
during file migration:
1) if the offset check fails we never drop the ioc reference;
2) we allocate an extra channel for no reason;
3) if multifd is enabled but channel creation fails when calling
dup(), we leave the previous channels around along with the glib
polling;
Fix all issues by restructuring the code to first allocate the
channels and only register the watches when all channels have been
created.
For multifd, the file and fd migrations can share code because both
are backed by a QIOChannelFile. For the non-multifd case, the fd needs
to be separate because it is backed by a QIOChannelSocket.
Fixes: 2dd7ee7a51 ("migration/multifd: Add incoming QIOChannelFile support")
Fixes: decdc76772 ("migration/multifd: Add mapped-ram support to fd: URI")
Reported-by: Peter Xu
Signed-off-by: Fabiano Rosas
---
migration/fd.c | 29 +++-
migration/file.c | 58 ++--
migration/file.h | 1 +
3 files changed, 46 insertions(+), 42 deletions(-)
diff --git a/migration/fd.c b/migration/fd.c
index 4e2a63a73d..39a52e5c90 100644
--- a/migration/fd.c
+++ b/migration/fd.c
@@ -18,6 +18,7 @@
#include "qapi/error.h"
#include "channel.h"
#include "fd.h"
+#include "file.h"
#include "migration.h"
#include "monitor/monitor.h"
#include "io/channel-file.h"
@@ -80,7 +81,6 @@ static gboolean fd_accept_incoming_migration(QIOChannel *ioc,
void fd_start_incoming_migration(const char *fdname, Error **errp)
{
QIOChannel *ioc;
-QIOChannelFile *fioc;
int fd = monitor_fd_param(monitor_cur(), fdname, errp);
if (fd == -1) {
return;
@@ -94,26 +94,13 @@ void fd_start_incoming_migration(const char *fdname, Error
**errp)
return;
}
-qio_channel_set_name(ioc, "migration-fd-incoming");
-qio_channel_add_watch_full(ioc, G_IO_IN,
- fd_accept_incoming_migration,
- NULL, NULL,
- g_main_context_get_thread_default());
-
if (migrate_multifd()) {
-int channels = migrate_multifd_channels();
-
-while (channels--) {
-fioc = qio_channel_file_new_dupfd(fd, errp);
-if (!fioc) {
-return;
-}
-
-qio_channel_set_name(ioc, "migration-fd-incoming");
-qio_channel_add_watch_full(QIO_CHANNEL(fioc), G_IO_IN,
- fd_accept_incoming_migration,
- NULL, NULL,
- g_main_context_get_thread_default());
-}
+file_create_incoming_channels(ioc, errp);
+} else {
+qio_channel_set_name(ioc, "migration-fd-incoming");
+qio_channel_add_watch_full(ioc, G_IO_IN,
+ fd_accept_incoming_migration,
+ NULL, NULL,
+ g_main_context_get_thread_default());
}
}
diff --git a/migration/file.c b/migration/file.c
index e56c5eb0a5..ddde0ca818 100644
--- a/migration/file.c
+++ b/migration/file.c
@@ -115,13 +115,46 @@ static gboolean file_accept_incoming_migration(QIOChannel
*ioc,
return G_SOURCE_REMOVE;
}
+void file_create_incoming_channels(QIOChannel *ioc, Error **errp)
+{
+int i, fd, channels = 1;
+g_autofree QIOChannel **iocs = NULL;
+
+if (migrate_multifd()) {
+channels += migrate_multifd_channels();
+}
+
+iocs = g_new0(QIOChannel *, channels);
+fd = QIO_CHANNEL_FILE(ioc)->fd;
+iocs[0] = ioc;
+
+for (i = 1; i < channels; i++) {
+QIOChannelFile *fioc = qio_channel_file_new_dupfd(fd, errp);
+
+if (!fioc) {
+while (i) {
+object_unref(iocs[--i]);
+}
+return;
+}
+
+iocs[i] = QIO_CHANNEL(fioc);
+}
+
+for (i = 0; i < channels; i++) {
+qio_channel_set_name(iocs[i], "migration-file-incoming");
+qio_channel_add_watch_full(iocs[i], G_IO_IN,
+ file_accept_incoming_migration,
+ NULL, NULL,
+ g_main_context_get_thread_default());
+}
+}
+
void file_start_incoming_migration(FileMigrationArgs *file_args, Error **errp)
{
g_autofree char *filename = g_strdup(file_args->filename);
QIOChannelFile *fioc = NULL;
uint64_t offset = file_args->offset;
-int channels = 1;
-int i = 0;
trace_migration_file_incoming(filename);
@@ -132,28 +165,11 @@ void file_start_incoming_migration(FileMigrationArgs
*file_args, Error **errp)
if (offset &&
qio_channel_io_seek(QIO_CHANNEL(fioc), offset, SEEK_SET, errp) < 0) {
+object_unref(OBJECT(fioc));
return;
}
-if (migrate_multifd()) {
-channels += migrate_multifd_channels();
-}
-
-do {
-QIOChannel
Re: [PATCH V4 10/14] migration: stop vm for cpr
On 3/13/24 15:18, Steven Sistare wrote: On 2/29/2024 8:28 PM, Peter Xu wrote: On Thu, Feb 29, 2024 at 10:21:14AM -0500, Steven Sistare wrote: On 2/25/2024 9:08 PM, Peter Xu wrote: On Thu, Feb 22, 2024 at 09:28:36AM -0800, Steve Sistare wrote: When migration for cpr is initiated, stop the vm and set state RUN_STATE_FINISH_MIGRATE before ram is saved. This eliminates the possibility of ram and device state being out of sync, and guarantees that a guest in the suspended state remains suspended, because qmp_cont rejects a cont command in the RUN_STATE_FINISH_MIGRATE state. Signed-off-by: Steve Sistare Reviewed-by: Peter Xu cpr-reboot mode keeps changing behavior. Could we declare it "experimental" until it's solid? Maybe a patch to document this? Normally IMHO we shouldn't merge a feature if it's not complete, however cpr-reboot is so special that the mode itself is already merged in 8.2 before I started to merge patches, and it keeps changing things. I don't know what else we can do here besides declaring it experimental and not declare it a stable feature. Hi Peter, the planned/committed functionality for cpr-reboot changed only once, in: migration: stop vm for cpr Suspension to support vfio is an enhancement which adds to the basic functionality, it does not change it. This was planned all along, but submitted as a separate If VFIO used to migrate without suspension and now it won't, it's a behavior change? VFIO could not cpr-reboot migrate without suspension. The existing vfio migration blockers applied to all modes: Error: :3a:10.0: VFIO migration is not supported in kernel Now, with suspension, it will. An addition, not a change. Still, I wonder if we should not have a per-device toggle to block migration for CPR_REBOOT mode. This to maintain the pre-9.0 behavior and to manage possible incompatibilities we haven't thought of yet. A config option to deactivate CPR_REBOOT mode in downstream could be useful too. Thanks, C. series to manage complexity, as I outlined in my qemu community presentation, which I emailed you at the time. Other "changes" that arose during review were just clarifications and explanations. So, I don't think cpr-reboot deserves to be condemned to experimental limbo. IMHO it's not about a feature being condemned, it's about a kindful heads-up to the users that one needs to take risk on using it until it becomes stable, it also makes developers easier because of no limitation on behavior change. If all the changes are landing, then there's no need for such a patch. If so, please propose the planned complete document patch. I had a feeling that cpr is still not fully understood by even many developers on the list. It'll be great that such document will contain all the details one needs to know on the feature, etc. meaning of the name cpr-reboot (what is "cpr"), when to use it, how to use it, how it differs from "normal" mode (etc. lifted limitations on some devices that used to block migration?), what is enforced (vm stop, suspension, etc.) and what is optionally offered (VFIO, shared-mem, etc.), the expected behaviors, etc. When send it, please copy relevant people (Alex & Cedric for VFIO, while Markus could also be a good candidate considering the QMP involvement). Submitted. - Steve
Re: [PATCH for-9.0 v13 4/8] target/riscv: always clear vstart in whole vec move insns
On 3/13/24 09:30, Daniel Henrique Barboza wrote:
These insns have 2 paths: we'll either have vstart already cleared if
vstart_eq_zero or we'll do a brcond to check if vstart >= maxsz to call
the 'vmvr_v' helper. The helper will clear vstart if it executes until
the end, or if vstart >= vl.
However, if vstart >= maxsz, the helper will be skipped, and vstart
won't be cleared since the helper is being responsible from doing it.
We want to make the helpers responsible to manage vstart, including
these corner cases, precisely to avoid these situations. Move the vstart
= maxsz cond to the helper, and be sure to clear vstart if that
happens. This way we're now 100% sure that vstart is being clearer in
the end of the execution, regardless of the path taken.
Fixes: f714361ed7 ("target/riscv: rvv-1.0: implement vstart CSR")
Signed-off-by: Daniel Henrique Barboza
---
target/riscv/insn_trans/trans_rvv.c.inc | 3 ---
target/riscv/vector_helper.c| 5 +
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index 8c16a9f5b3..52c26a7834 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -3664,12 +3664,9 @@ static bool trans_##NAME(DisasContext *s, arg_##NAME *
a) \
vreg_ofs(s, a->rs2), maxsz, maxsz);\
mark_vs_dirty(s); \
} else {\
-TCGLabel *over = gen_new_label(); \
-tcg_gen_brcondi_tl(TCG_COND_GEU, cpu_vstart, maxsz, over); \
tcg_gen_gvec_2_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2), \
tcg_env, maxsz, maxsz, 0, gen_helper_vmvr_v); \
mark_vs_dirty(s); \
-gen_set_label(over);\
} \
return true;\
} \
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index b4360dbd52..7260a5972b 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -5163,6 +5163,11 @@ void HELPER(vmvr_v)(void *vd, void *vs2, CPURISCVState
*env, uint32_t desc)
VSTART_CHECK_EARLY_EXIT(env);
+if (env->vstart >= maxsz) {
+env->vstart = 0;
+return;
+}
Did you need the VSTART_CHECK_EARLY_EXIT here then?
It certainly seems like the vstart >= vl check is redundant...
r~
Re: [PATCH] vfio/iommufd: Fix memory leak
On 3/13/24 22:06, Cédric Le Goater wrote:
> Make sure variable contents is freed if scanf fails.
>
> Cc: Eric Auger
> Cc: Yi Liu
> Cc: Zhenzhong Duan
> Fixes: CID 1540007
> Fixes: 5ee3dc7af785 ("vfio/iommufd: Implement the iommufd backend")
> Signed-off-by: Cédric Le Goater
Reviewed-by: Eric Auger
Thanks!
Eric
> ---
> hw/vfio/iommufd.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index
> a75a785e90c64cdcc4d10c88d217801b3f536cdb..cd549e0ee8573e75772c51cc96153762a6bc8550
> 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -152,9 +152,8 @@ static int iommufd_cdev_getfd(const char *sysfs_path,
> Error **errp)
>
> if (sscanf(contents, "%d:%d", , ) != 2) {
> error_setg(errp, "failed to get major:minor for \"%s\"",
> vfio_dev_path);
> -goto out_free_dev_path;
> +goto out_free_contents;
> }
> -g_free(contents);
> vfio_devt = makedev(major, minor);
>
> vfio_path = g_strdup_printf("/dev/vfio/devices/%s", dent->d_name);
> @@ -166,6 +165,8 @@ static int iommufd_cdev_getfd(const char *sysfs_path,
> Error **errp)
> trace_iommufd_cdev_getfd(vfio_path, ret);
> g_free(vfio_path);
>
> +out_free_contents:
> +g_free(contents);
> out_free_dev_path:
> g_free(vfio_dev_path);
> out_close_dir:
Re: [PATCH-for-9.0 2/4] accel/hvf: Un-inline hvf_arch_supports_guest_debug()
On 3/13/24 08:49, Philippe Mathieu-Daudé wrote:
See previous commit and commit 9de9fa5cf2 ("Avoid using inlined
functions with external linkage") for rationale.
Signed-off-by: Philippe Mathieu-Daudé
---
target/arm/hvf/hvf.c | 2 +-
target/i386/hvf/hvf.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Richard Henderson
r~
Re: [PATCH-for-9.0 4/4] meson: Enable -Wstatic-in-inline
On 3/13/24 08:49, Philippe Mathieu-Daudé wrote: Compilers are clever enough to inline code when necessary. The only case we accept an inline function is static in header (we use C, not C++). Add the -Wstatic-in-inline CPPFLAG to prevent public and inline function to be added in the code base. Signed-off-by: Philippe Mathieu-Daudé --- meson.build | 1 + 1 file changed, 1 insertion(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 01/16] esp.c: replace cmdfifo use of esp_fifo_pop_buf() in do_command_phase()
On 13/03/2024 11:03, Philippe Mathieu-Daudé wrote:
On 13/3/24 09:57, Mark Cave-Ayland wrote:
The aim is to restrict the esp_fifo_*() functions so that they only operate on
the hardware FIFO. When reading from cmdfifo in do_command_phase() use the
underlying Fifo8 functions directly.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 590ff99744..f8230c74b3 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -265,7 +265,7 @@ static void esp_do_nodma(ESPState *s);
static void do_command_phase(ESPState *s)
{
- uint32_t cmdlen;
+ uint32_t cmdlen, n;
int32_t datalen;
SCSIDevice *current_lun;
uint8_t buf[ESP_CMDFIFO_SZ];
@@ -275,7 +275,7 @@ static void do_command_phase(ESPState *s)
if (!cmdlen || !s->current_dev) {
return;
}
- esp_fifo_pop_buf(>cmdfifo, buf, cmdlen);
+ memcpy(buf, fifo8_pop_buf(>cmdfifo, cmdlen, ), cmdlen);
'n' is unused, use NULL?
I was sure I had tried that before and it had failed, but I see that you made it work
with NULL in commit cd04033dbe ("util/fifo8: Allow fifo8_pop_buf() to not populate
popped length") - thanks!
I'll make the change for v3, but I'll wait a couple of days first to see if there are
any further comments, in particular R-B tags for patches 10 and 11.
current_lun = scsi_device_find(>bus, 0, s->current_dev->id, s->lun);
if (!current_lun) {
ATB,
Mark.
Re: [PATCH-for-9.0 1/4] hw/arm/smmu: Avoid using inlined functions with external linkage again
On 3/13/24 08:49, Philippe Mathieu-Daudé wrote:
Similarly to commit 9de9fa5cf2 ("hw/arm/smmu-common: Avoid using
inlined functions with external linkage"):
None of our code base require / use inlined functions with external
linkage. Some places use internal inlining in the hot path. These
two functions are certainly not in any hot path and don't justify
any inlining, so these are likely oversights rather than intentional.
Fix:
C compiler for the host machine: clang (clang 15.0.0 "Apple clang version 15.0.0
(clang-1500.3.9.4)")
...
hw/arm/smmu-common.c:203:43: error: static function
'smmu_hash_remove_by_vmid' is
used in an inline function with external linkage [-Werror,-Wstatic-in-inline]
g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_vmid, );
^
include/hw/arm/smmu-common.h:197:1: note: use 'static' to give inline
function 'smmu_iotlb_inv_vmid' internal linkage
void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid);
^
static
hw/arm/smmu-common.c:139:17: note: 'smmu_hash_remove_by_vmid' declared here
static gboolean smmu_hash_remove_by_vmid(gpointer key, gpointer value,
^
Fixes: ccc3ee3871 ("hw/arm/smmuv3: Add CMDs related to stage-2")
Signed-off-by: Philippe Mathieu-Daudé
---
hw/arm/smmu-common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Richard Henderson
r~
[PATCH] vfio/iommufd: Fix memory leak
Make sure variable contents is freed if scanf fails.
Cc: Eric Auger
Cc: Yi Liu
Cc: Zhenzhong Duan
Fixes: CID 1540007
Fixes: 5ee3dc7af785 ("vfio/iommufd: Implement the iommufd backend")
Signed-off-by: Cédric Le Goater
---
hw/vfio/iommufd.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index
a75a785e90c64cdcc4d10c88d217801b3f536cdb..cd549e0ee8573e75772c51cc96153762a6bc8550
100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -152,9 +152,8 @@ static int iommufd_cdev_getfd(const char *sysfs_path, Error
**errp)
if (sscanf(contents, "%d:%d", , ) != 2) {
error_setg(errp, "failed to get major:minor for \"%s\"",
vfio_dev_path);
-goto out_free_dev_path;
+goto out_free_contents;
}
-g_free(contents);
vfio_devt = makedev(major, minor);
vfio_path = g_strdup_printf("/dev/vfio/devices/%s", dent->d_name);
@@ -166,6 +165,8 @@ static int iommufd_cdev_getfd(const char *sysfs_path, Error
**errp)
trace_iommufd_cdev_getfd(vfio_path, ret);
g_free(vfio_path);
+out_free_contents:
+g_free(contents);
out_free_dev_path:
g_free(vfio_dev_path);
out_close_dir:
--
2.44.0
Re: [PATCH 12/12] vdpa: fix network breakage after cancelling migration
13.03.2024 22:10, Si-Wei Liu wrote: On 3/13/2024 11:12 AM, Michael Tokarev wrote: .. Is this a -stable material? Probably yes, the pre-requisites of this patch are PATCH #10 and #11 from this series (where SVQ_TSTATE_DISABLING gets defined and set). If yes, is it also applicable for stable-7.2 (mentioned commit is in 7.2.0), which lacks v7.2.0-2327-gb276524386 "vdpa: Remember last call fd set", or should this one also be picked up? Eugenio can judge, but seems to me the relevant code path cannot be effectively called as the dynamic SVQ feature (switching over to SVQ dynamically when migration is started) is not supported from 7.2. Maybe not worth it to cherry-pick this one to 7.2. Cherry-pick to stable-8.0 and above should be applicable though (it needs some tweaks on patch #10 to move svq_switching from @struct VhostVDPAShared to @struct vhost_vdpa). Uhh. That's a bit larger than I thought. With this amount of prepreqs and manual tweaking, and with the fact this whole thing introduces Yet Another State value, - let's omit this one. Or at the very least I'd love to have actual real-life testcase and a clean backport to 8.2 of all the required changes. Thanks, /mjt
Re: [PATCH v3] target/riscv: Implement dynamic establishment of custom decoder
On 3/12/24 23:57, Huang Tao wrote: In this patch, we modify the decoder to be a freely composable data structure instead of a hardcoded one. It can be dynamically builded up according to the extensions. This approach has several benefits: 1. Provides support for heterogeneous cpu architectures. As we add decoder in RISCVCPU, each cpu can have their own decoder, and the decoders can be different due to cpu's features. 2. Improve the decoding efficiency. We run the guard_func to see if the decoder can be added to the dynamic_decoder when building up the decoder. Therefore, there is no need to run the guard_func when decoding each instruction. It can improve the decoding efficiency 3. For vendor or dynamic cpus, it allows them to customize their own decoder functions to improve decoding efficiency, especially when vendor-defined instruction sets increase. Because of dynamic building up, it can skip the other decoder guard functions when decoding. 4. Pre patch for allowing adding a vendor decoder before decode_insn32() with minimal overhead for users that don't need this particular vendor deocder. Signed-off-by: Huang Tao Suggested-by: Christoph Muellner Co-authored-by: LIU Zhiwei --- Changes in v3: - use GPtrArray to save decode function poionter list. --- target/riscv/cpu.c | 18 ++ target/riscv/cpu.h | 2 ++ target/riscv/cpu_decoder.h | 34 ++ target/riscv/translate.c | 29 + 4 files changed, 67 insertions(+), 16 deletions(-) create mode 100644 target/riscv/cpu_decoder.h Reviewed-by: Richard Henderson r~
Re: udp guestfwd
Hi Louai, Are you using IPv6 or IPv4? The IPv4 is actually broken (if you want to send multiple requests to slirp and get them forwarded). You can check the latest comments in following tickets: https://gitlab.freedesktop.org/slirp/libslirp/-/issues/67 https://gitlab.com/qemu-project/qemu/-/issues/1835 If you want to use IPv6, let me know and I can create pull requests in libslirp so you can try it. Thanks, Felix On Fri, Dec 8, 2023 at 9:33 AM Patrick Venture wrote: > > On Fri, Oct 27, 2023 at 11:44 PM Louai Al-Khanji > wrote: > >> Hi, >> >> I'm interested in having the guestfwd option work for udp. My >> understanding is that currently it's restricted to only tcp. >> >> I'm not familiar with libslirp internals. What would need to be changed >> to implement this? I'm potentially interested in doing the work. >> >> I did a tiny amount of digging around libslirp and saw this comment in >> `udp.c': >> >> /* >> * X Here, check if it's in udpexec_list, >> * and if it is, do the fork_exec() etc. >> */ >> >> I wonder whether that is related. In any case any help is much >> appreciated. >> > > Felix has been working in this space and it may take time to get the CLs > landed in libslirp and qemu. > > Patrick > >> >> Thanks, >> Louai Al-Khanji >> >
Re: [PATCH for-9.0] target/riscv: do not enable all named features by default
On Tue, Mar 12, 2024 at 05:32:14PM -0300, Daniel Henrique Barboza wrote:
> Commit 3b8022269c added the capability of named features/profile
> extensions to be added in riscv,isa. To do that we had to assign priv
> versions for each one of them in isa_edata_arr[]. But this resulted in a
> side-effect: vendor CPUs that aren't running priv_version_latest started
> to experience warnings for these profile extensions [1]:
>
> | $ qemu-system-riscv32 -M sifive_e
> | qemu-system-riscv32: warning: disabling zic64b extension for hart
> 0x because privilege spec version does not match
> | qemu-system-riscv32: warning: disabling ziccamoa extension for
> hart 0x because privilege spec version does not match
>
> This is benign as far as the CPU behavior is concerned since disabling
> both extensions is a no-op (aside from riscv,isa). But the warnings are
> unpleasant to deal with, especially because we're sending user warnings
> for extensions that users can't enable/disable.
>
> Instead of enabling all named features all the time, separate them by
> priv version. During finalize() time, after we decided which
> priv_version the CPU is running, enable/disable all the named extensions
> based on the priv spec chosen. This will be enough for a bug fix, but as
> a future work we should look into how we can name these extensions in a
> way that we don't need an explicit ext_name => priv_ver as we're doing
> here.
>
> The named extensions being added in isa_edata_arr[] that will be
> enabled/disabled based solely on priv version can be removed from
> riscv_cpu_named_features[]. 'zic64b' is an extension that can be
> disabled based on block sizes so it'll retain its own flag and entry.
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2024-03/msg02592.html
>
> Reported-by: Clément Chigot
> Fixes: 3b8022269c ("target/riscv: add riscv,isa to named features")
> Suggested-by: Andrew Jones
> Signed-off-by: Daniel Henrique Barboza
> ---
> target/riscv/cpu.c | 40 +-
> target/riscv/cpu_cfg.h | 8 +---
> target/riscv/tcg/tcg-cpu.c | 14 ++---
> 3 files changed, 25 insertions(+), 37 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 5a48d30828..1da5417764 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -102,10 +102,10 @@ const RISCVIsaExtData isa_edata_arr[] = {
> ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_zicbom),
> ISA_EXT_DATA_ENTRY(zicbop, PRIV_VERSION_1_12_0, ext_zicbop),
> ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_zicboz),
> -ISA_EXT_DATA_ENTRY(ziccamoa, PRIV_VERSION_1_11_0, ext_always_enabled),
> -ISA_EXT_DATA_ENTRY(ziccif, PRIV_VERSION_1_11_0, ext_always_enabled),
> -ISA_EXT_DATA_ENTRY(zicclsm, PRIV_VERSION_1_11_0, ext_always_enabled),
> -ISA_EXT_DATA_ENTRY(ziccrse, PRIV_VERSION_1_11_0, ext_always_enabled),
> +ISA_EXT_DATA_ENTRY(ziccamoa, PRIV_VERSION_1_11_0, has_priv_1_11),
> +ISA_EXT_DATA_ENTRY(ziccif, PRIV_VERSION_1_11_0, has_priv_1_11),
> +ISA_EXT_DATA_ENTRY(zicclsm, PRIV_VERSION_1_11_0, has_priv_1_11),
> +ISA_EXT_DATA_ENTRY(ziccrse, PRIV_VERSION_1_11_0, has_priv_1_11),
> ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond),
> ISA_EXT_DATA_ENTRY(zicntr, PRIV_VERSION_1_12_0, ext_zicntr),
> ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_zicsr),
> @@ -114,7 +114,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
> ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause),
> ISA_EXT_DATA_ENTRY(zihpm, PRIV_VERSION_1_12_0, ext_zihpm),
> ISA_EXT_DATA_ENTRY(zmmul, PRIV_VERSION_1_12_0, ext_zmmul),
> -ISA_EXT_DATA_ENTRY(za64rs, PRIV_VERSION_1_12_0, ext_always_enabled),
> +ISA_EXT_DATA_ENTRY(za64rs, PRIV_VERSION_1_12_0, has_priv_1_11),
> ISA_EXT_DATA_ENTRY(zaamo, PRIV_VERSION_1_12_0, ext_zaamo),
> ISA_EXT_DATA_ENTRY(zacas, PRIV_VERSION_1_12_0, ext_zacas),
> ISA_EXT_DATA_ENTRY(zalrsc, PRIV_VERSION_1_12_0, ext_zalrsc),
> @@ -179,12 +179,12 @@ const RISCVIsaExtData isa_edata_arr[] = {
> ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
> ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
> ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia),
> -ISA_EXT_DATA_ENTRY(ssccptr, PRIV_VERSION_1_11_0, ext_always_enabled),
> +ISA_EXT_DATA_ENTRY(ssccptr, PRIV_VERSION_1_11_0, has_priv_1_11),
> ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf),
> -ISA_EXT_DATA_ENTRY(sscounterenw, PRIV_VERSION_1_12_0,
> ext_always_enabled),
> +ISA_EXT_DATA_ENTRY(sscounterenw, PRIV_VERSION_1_12_0, has_priv_1_12),
> ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
> -ISA_EXT_DATA_ENTRY(sstvala, PRIV_VERSION_1_12_0, ext_always_enabled),
> -ISA_EXT_DATA_ENTRY(sstvecd, PRIV_VERSION_1_12_0, ext_always_enabled),
> +ISA_EXT_DATA_ENTRY(sstvala, PRIV_VERSION_1_12_0, has_priv_1_12),
> +
Re: [PATCH] hw/virtio: Add support for VDPA network simulation devices
On Wed, Mar 13, 2024 at 07:51:08PM +0100, Thomas Weißschuh wrote:
> On 2024-02-21 15:38:02+0800, Hao Chen wrote:
> > This patch adds support for VDPA network simulation devices.
> > The device is developed based on virtio-net and tap backend,
> > and supports hardware live migration function.
> >
> > For more details, please refer to "docs/system/devices/vdpa-net.rst"
> >
> > Signed-off-by: Hao Chen
> > ---
> > MAINTAINERS | 5 +
> > docs/system/device-emulation.rst| 1 +
> > docs/system/devices/vdpa-net.rst| 121 +
> > hw/net/virtio-net.c | 16 ++
> > hw/virtio/virtio-pci.c | 189 +++-
> > hw/virtio/virtio.c | 39
> > include/hw/virtio/virtio-pci.h | 5 +
> > include/hw/virtio/virtio.h | 19 ++
> > include/standard-headers/linux/virtio_pci.h | 7 +
> > 9 files changed, 399 insertions(+), 3 deletions(-)
> > create mode 100644 docs/system/devices/vdpa-net.rst
>
> [..]
>
> > diff --git a/include/standard-headers/linux/virtio_pci.h
> > b/include/standard-headers/linux/virtio_pci.h
> > index b7fdfd0668..fb5391cef6 100644
> > --- a/include/standard-headers/linux/virtio_pci.h
> > +++ b/include/standard-headers/linux/virtio_pci.h
> > @@ -216,6 +216,13 @@ struct virtio_pci_cfg_cap {
> > #define VIRTIO_PCI_COMMON_Q_NDATA 56
> > #define VIRTIO_PCI_COMMON_Q_RESET 58
> >
> > +#define LM_LOGGING_CTRL 0
> > +#define LM_BASE_ADDR_LOW4
> > +#define LM_BASE_ADDR_HIGH 8
> > +#define LM_END_ADDR_LOW 12
> > +#define LM_END_ADDR_HIGH16
> > +#define LM_VRING_STATE_OFFSET 0x20
>
> These changes are not in upstream Linux and will be undone by
> ./scripts/update-linux-headers.sh.
>
> Are they intentionally in this header?
Good point. Pls move.
> > +
> > #endif /* VIRTIO_PCI_NO_MODERN */
> >
> > #endif
Re: [PATCH-for-9.0 2/4] accel/hvf: Un-inline hvf_arch_supports_guest_debug()
On Wed, 13 Mar 2024 at 18:50, Philippe Mathieu-Daudé wrote:
>
> See previous commit and commit 9de9fa5cf2 ("Avoid using inlined
> functions with external linkage") for rationale.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Peter Maydell
thanks
-- PMM
Re: [PATCH-for-9.0 3/4] qtest/libqos: Un-inline size_to_prdtl()
On Wed, 13 Mar 2024 at 18:50, Philippe Mathieu-Daudé wrote:
>
> See previous commit and commit 9de9fa5cf2 ("Avoid using inlined
> functions with external linkage") for rationale.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> tests/qtest/libqos/ahci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/qtest/libqos/ahci.c b/tests/qtest/libqos/ahci.c
> index a2c94c6e06..135b23ffd9 100644
> --- a/tests/qtest/libqos/ahci.c
> +++ b/tests/qtest/libqos/ahci.c
> @@ -662,7 +662,7 @@ unsigned ahci_pick_cmd(AHCIQState *ahci, uint8_t port)
> g_assert_not_reached();
> }
>
> -inline unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd)
> +unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd)
> {
> /* Each PRD can describe up to 4MiB */
> g_assert_cmphex(bytes_per_prd, <=, 4096 * 1024);
It looks like this function is only used in this file, so
we could make it static ?
-- PMM
[PATCH for-9.0 v13 3/8] target/riscv/vector_helpers: do early exit when vstart >= vl
We're going to make changes that will required each helper to be
responsible for the 'vstart' management, i.e. we will relieve the
'vstart < vl' assumption that helpers have today.
Helpers are usually able to deal with vstart >= vl, i.e. doing nothing
aside from setting vstart = 0 at the end, but the tail update functions
will update the tail regardless of vstart being valid or not.
Unifying the tail update process in a single function that would handle
the vstart >= vl case isn't trivial. We have 2 functions that are used
to update tail: vext_set_tail_elems_1s() and vext_set_elems_1s(). The
latter is a more generic function that is also used to mask elements.
There's no easy way of making all callers using vext_set_tail_elems_1s()
because we're not encoding NF properly in all cases [1].
This patch takes a blunt approach: do an early exit in every single
vector helper if vstart >= vl. We can worry about unifying the tail
update process later.
[1]
https://lore.kernel.org/qemu-riscv/1590234b-0291-432a-a0fa-c5a687609...@linux.alibaba.com/
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Richard Henderson
---
target/riscv/vcrypto_helper.c | 32
target/riscv/vector_helper.c| 90 +
target/riscv/vector_internals.c | 4 ++
target/riscv/vector_internals.h | 9
4 files changed, 135 insertions(+)
diff --git a/target/riscv/vcrypto_helper.c b/target/riscv/vcrypto_helper.c
index e2d719b13b..f7423df226 100644
--- a/target/riscv/vcrypto_helper.c
+++ b/target/riscv/vcrypto_helper.c
@@ -222,6 +222,8 @@ static inline void xor_round_key(AESState *round_state,
AESState *round_key)
uint32_t total_elems = vext_get_total_elems(env, desc, 4);\
uint32_t vta = vext_vta(desc);\
\
+VSTART_CHECK_EARLY_EXIT(env); \
+ \
for (uint32_t i = env->vstart / 4; i < env->vl / 4; i++) {\
AESState round_key; \
round_key.d[0] = *((uint64_t *)vs2 + H8(i * 2 + 0)); \
@@ -246,6 +248,8 @@ static inline void xor_round_key(AESState *round_state,
AESState *round_key)
uint32_t total_elems = vext_get_total_elems(env, desc, 4);\
uint32_t vta = vext_vta(desc);\
\
+VSTART_CHECK_EARLY_EXIT(env); \
+ \
for (uint32_t i = env->vstart / 4; i < env->vl / 4; i++) {\
AESState round_key; \
round_key.d[0] = *((uint64_t *)vs2 + H8(0)); \
@@ -305,6 +309,8 @@ void HELPER(vaeskf1_vi)(void *vd_vptr, void *vs2_vptr,
uint32_t uimm,
uint32_t total_elems = vext_get_total_elems(env, desc, 4);
uint32_t vta = vext_vta(desc);
+VSTART_CHECK_EARLY_EXIT(env);
+
uimm &= 0b;
if (uimm > 10 || uimm == 0) {
uimm ^= 0b1000;
@@ -351,6 +357,8 @@ void HELPER(vaeskf2_vi)(void *vd_vptr, void *vs2_vptr,
uint32_t uimm,
uint32_t total_elems = vext_get_total_elems(env, desc, 4);
uint32_t vta = vext_vta(desc);
+VSTART_CHECK_EARLY_EXIT(env);
+
uimm &= 0b;
if (uimm > 14 || uimm < 2) {
uimm ^= 0b1000;
@@ -457,6 +465,8 @@ void HELPER(vsha2ms_vv)(void *vd, void *vs1, void *vs2,
CPURISCVState *env,
uint32_t total_elems;
uint32_t vta = vext_vta(desc);
+VSTART_CHECK_EARLY_EXIT(env);
+
for (uint32_t i = env->vstart / 4; i < env->vl / 4; i++) {
if (sew == MO_32) {
vsha2ms_e32(((uint32_t *)vd) + i * 4, ((uint32_t *)vs1) + i * 4,
@@ -572,6 +582,8 @@ void HELPER(vsha2ch32_vv)(void *vd, void *vs1, void *vs2,
CPURISCVState *env,
uint32_t total_elems;
uint32_t vta = vext_vta(desc);
+VSTART_CHECK_EARLY_EXIT(env);
+
for (uint32_t i = env->vstart / 4; i < env->vl / 4; i++) {
vsha2c_32(((uint32_t *)vs2) + 4 * i, ((uint32_t *)vd) + 4 * i,
((uint32_t *)vs1) + 4 * i + 2);
@@ -590,6 +602,8 @@ void HELPER(vsha2ch64_vv)(void *vd, void *vs1, void *vs2,
CPURISCVState *env,
uint32_t total_elems;
uint32_t vta = vext_vta(desc);
+VSTART_CHECK_EARLY_EXIT(env);
+
for (uint32_t i = env->vstart / 4; i < env->vl / 4; i++) {
vsha2c_64(((uint64_t *)vs2) + 4 * i, ((uint64_t *)vd) + 4 * i,
((uint64_t *)vs1) + 4 * i + 2);
@@ -608,6 +622,8 @@ void HELPER(vsha2cl32_vv)(void *vd, void *vs1, void *vs2,
CPURISCVState *env,
uint32_t total_elems;
uint32_t vta = vext_vta(desc);
+VSTART_CHECK_EARLY_EXIT(env);
+
for (uint32_t i = env->vstart / 4; i
[PATCH for-9.0 v13 7/8] target/riscv: enable 'vstart_eq_zero' in the end of insns
From: Ivan Klokov The vstart_eq_zero flag is updated at the beginning of the translation phase from the env->vstart variable. During the execution phase all functions will set env->vstart = 0 after a successful execution, but the vstart_eq_zero flag remains the same as at the start of the block. This will wrongly cause SIGILLs in translations that requires env->vstart = 0 and might be reading vstart_eq_zero = false. This patch adds a new finalize_rvv_inst() helper that is called at the end of each vector instruction that will both update vstart_eq_zero and do a mark_vs_dirty(). Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1976 Signed-off-by: Ivan Klokov Signed-off-by: Daniel Henrique Barboza Reviewed-by: Richard Henderson Reviewed-by: Alistair Francis --- target/riscv/insn_trans/trans_rvbf16.c.inc | 6 +- target/riscv/insn_trans/trans_rvv.c.inc| 83 -- target/riscv/insn_trans/trans_rvvk.c.inc | 12 ++-- target/riscv/translate.c | 6 ++ 4 files changed, 59 insertions(+), 48 deletions(-) diff --git a/target/riscv/insn_trans/trans_rvbf16.c.inc b/target/riscv/insn_trans/trans_rvbf16.c.inc index a842e76a6b..0a9cd1ec31 100644 --- a/target/riscv/insn_trans/trans_rvbf16.c.inc +++ b/target/riscv/insn_trans/trans_rvbf16.c.inc @@ -83,7 +83,7 @@ static bool trans_vfncvtbf16_f_f_w(DisasContext *ctx, arg_vfncvtbf16_f_f_w *a) ctx->cfg_ptr->vlenb, ctx->cfg_ptr->vlenb, data, gen_helper_vfncvtbf16_f_f_w); -mark_vs_dirty(ctx); +finalize_rvv_inst(ctx); return true; } return false; @@ -108,7 +108,7 @@ static bool trans_vfwcvtbf16_f_f_v(DisasContext *ctx, arg_vfwcvtbf16_f_f_v *a) ctx->cfg_ptr->vlenb, ctx->cfg_ptr->vlenb, data, gen_helper_vfwcvtbf16_f_f_v); -mark_vs_dirty(ctx); +finalize_rvv_inst(ctx); return true; } return false; @@ -135,7 +135,7 @@ static bool trans_vfwmaccbf16_vv(DisasContext *ctx, arg_vfwmaccbf16_vv *a) ctx->cfg_ptr->vlenb, ctx->cfg_ptr->vlenb, data, gen_helper_vfwmaccbf16_vv); -mark_vs_dirty(ctx); +finalize_rvv_inst(ctx); return true; } return false; diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc index b0f19dcd85..b3d467a874 100644 --- a/target/riscv/insn_trans/trans_rvv.c.inc +++ b/target/riscv/insn_trans/trans_rvv.c.inc @@ -167,7 +167,7 @@ static bool do_vsetvl(DisasContext *s, int rd, int rs1, TCGv s2) gen_helper_vsetvl(dst, tcg_env, s1, s2); gen_set_gpr(s, rd, dst); -mark_vs_dirty(s); +finalize_rvv_inst(s); gen_update_pc(s, s->cur_insn_len); lookup_and_goto_ptr(s); @@ -187,7 +187,7 @@ static bool do_vsetivli(DisasContext *s, int rd, TCGv s1, TCGv s2) gen_helper_vsetvl(dst, tcg_env, s1, s2); gen_set_gpr(s, rd, dst); -mark_vs_dirty(s); +finalize_rvv_inst(s); gen_update_pc(s, s->cur_insn_len); lookup_and_goto_ptr(s); s->base.is_jmp = DISAS_NORETURN; @@ -657,6 +657,7 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1, uint32_t data, tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ); } +finalize_rvv_inst(s); return true; } @@ -812,6 +813,7 @@ static bool ldst_stride_trans(uint32_t vd, uint32_t rs1, uint32_t rs2, fn(dest, mask, base, stride, tcg_env, desc); +finalize_rvv_inst(s); return true; } @@ -913,6 +915,7 @@ static bool ldst_index_trans(uint32_t vd, uint32_t rs1, uint32_t vs2, fn(dest, mask, base, index, tcg_env, desc); +finalize_rvv_inst(s); return true; } @@ -1043,7 +1046,7 @@ static bool ldff_trans(uint32_t vd, uint32_t rs1, uint32_t data, fn(dest, mask, base, tcg_env, desc); -mark_vs_dirty(s); +finalize_rvv_inst(s); return true; } @@ -1100,6 +1103,7 @@ static bool ldst_whole_trans(uint32_t vd, uint32_t rs1, uint32_t nf, fn(dest, base, tcg_env, desc); +finalize_rvv_inst(s); return true; } @@ -1189,7 +1193,7 @@ do_opivv_gvec(DisasContext *s, arg_rmrr *a, GVecGen3Fn *gvec_fn, tcg_env, s->cfg_ptr->vlenb, s->cfg_ptr->vlenb, data, fn); } -mark_vs_dirty(s); +finalize_rvv_inst(s); return true; } @@ -1240,7 +1244,7 @@ static bool opivx_trans(uint32_t vd, uint32_t rs1, uint32_t vs2, uint32_t vm, fn(dest, mask, src1, src2, tcg_env, desc); -mark_vs_dirty(s); +finalize_rvv_inst(s); return true; } @@ -1265,7 +1269,7 @@ do_opivx_gvec(DisasContext *s, arg_rmrr *a, GVecGen2sFn *gvec_fn, gvec_fn(s->sew, vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2), src1, MAXSZ(s), MAXSZ(s)); -mark_vs_dirty(s); +finalize_rvv_inst(s);
[PATCH for-9.0 v13 6/8] trans_rvv.c.inc: remove redundant mark_vs_dirty() calls
trans_vmv_v_i , trans_vfmv_v_f and the trans_##NAME macro from
GEN_VMV_WHOLE_TRANS() are calling mark_vs_dirty() in both branches of
their 'ifs'. conditionals.
Call it just once in the end like other functions are doing.
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Richard Henderson
Reviewed-by: Alistair Francis
Reviewed-by: Philippe Mathieu-Daudé
---
target/riscv/insn_trans/trans_rvv.c.inc | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index 4c1a064cf6..b0f19dcd85 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -2065,7 +2065,6 @@ static bool trans_vmv_v_i(DisasContext *s, arg_vmv_v_i *a)
if (s->vl_eq_vlmax && !(s->vta && s->lmul < 0)) {
tcg_gen_gvec_dup_imm(s->sew, vreg_ofs(s, a->rd),
MAXSZ(s), MAXSZ(s), simm);
-mark_vs_dirty(s);
} else {
TCGv_i32 desc;
TCGv_i64 s1;
@@ -2083,9 +2082,8 @@ static bool trans_vmv_v_i(DisasContext *s, arg_vmv_v_i *a)
s->cfg_ptr->vlenb, data));
tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, a->rd));
fns[s->sew](dest, s1, tcg_env, desc);
-
-mark_vs_dirty(s);
}
+mark_vs_dirty(s);
return true;
}
return false;
@@ -2612,7 +2610,6 @@ static bool trans_vfmv_v_f(DisasContext *s, arg_vfmv_v_f
*a)
tcg_gen_gvec_dup_i64(s->sew, vreg_ofs(s, a->rd),
MAXSZ(s), MAXSZ(s), t1);
-mark_vs_dirty(s);
} else {
TCGv_ptr dest;
TCGv_i32 desc;
@@ -2635,9 +2632,8 @@ static bool trans_vfmv_v_f(DisasContext *s, arg_vfmv_v_f
*a)
tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, a->rd));
fns[s->sew - 1](dest, t1, tcg_env, desc);
-
-mark_vs_dirty(s);
}
+mark_vs_dirty(s);
return true;
}
return false;
@@ -3560,12 +3556,11 @@ static bool trans_##NAME(DisasContext *s, arg_##NAME *
a) \
if (s->vstart_eq_zero) {\
tcg_gen_gvec_mov(s->sew, vreg_ofs(s, a->rd),\
vreg_ofs(s, a->rs2), maxsz, maxsz);\
-mark_vs_dirty(s); \
} else {\
tcg_gen_gvec_2_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2), \
tcg_env, maxsz, maxsz, 0, gen_helper_vmvr_v); \
-mark_vs_dirty(s); \
} \
+mark_vs_dirty(s); \
return true;\
} \
return false; \
--
2.43.2
[PATCH for-9.0 v13 8/8] target/riscv/vector_helper.c: optimize loops in ldst helpers
Change the for loops in ldst helpers to do a single increment in the
counter, and assign it env->vstart, to avoid re-reading from vstart
every time.
Suggested-by: Richard Henderson
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
Reviewed-by: Richard Henderson
---
target/riscv/vector_helper.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 7260a5972b..b29b8f9116 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -209,7 +209,7 @@ vext_ldst_stride(void *vd, void *v0, target_ulong base,
VSTART_CHECK_EARLY_EXIT(env);
-for (i = env->vstart; i < env->vl; i++, env->vstart++) {
+for (i = env->vstart; i < env->vl; env->vstart = ++i) {
k = 0;
while (k < nf) {
if (!vm && !vext_elem_mask(v0, i)) {
@@ -277,7 +277,7 @@ vext_ldst_us(void *vd, target_ulong base, CPURISCVState
*env, uint32_t desc,
VSTART_CHECK_EARLY_EXIT(env);
/* load bytes from guest memory */
-for (i = env->vstart; i < evl; i++, env->vstart++) {
+for (i = env->vstart; i < evl; env->vstart = ++i) {
k = 0;
while (k < nf) {
target_ulong addr = base + ((i * nf + k) << log2_esz);
@@ -393,7 +393,7 @@ vext_ldst_index(void *vd, void *v0, target_ulong base,
VSTART_CHECK_EARLY_EXIT(env);
/* load bytes from guest memory */
-for (i = env->vstart; i < env->vl; i++, env->vstart++) {
+for (i = env->vstart; i < env->vl; env->vstart = ++i) {
k = 0;
while (k < nf) {
if (!vm && !vext_elem_mask(v0, i)) {
--
2.43.2
[PATCH for-9.0 v13 4/8] target/riscv: always clear vstart in whole vec move insns
These insns have 2 paths: we'll either have vstart already cleared if
vstart_eq_zero or we'll do a brcond to check if vstart >= maxsz to call
the 'vmvr_v' helper. The helper will clear vstart if it executes until
the end, or if vstart >= vl.
However, if vstart >= maxsz, the helper will be skipped, and vstart
won't be cleared since the helper is being responsible from doing it.
We want to make the helpers responsible to manage vstart, including
these corner cases, precisely to avoid these situations. Move the vstart
>= maxsz cond to the helper, and be sure to clear vstart if that
happens. This way we're now 100% sure that vstart is being clearer in
the end of the execution, regardless of the path taken.
Fixes: f714361ed7 ("target/riscv: rvv-1.0: implement vstart CSR")
Signed-off-by: Daniel Henrique Barboza
---
target/riscv/insn_trans/trans_rvv.c.inc | 3 ---
target/riscv/vector_helper.c| 5 +
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index 8c16a9f5b3..52c26a7834 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -3664,12 +3664,9 @@ static bool trans_##NAME(DisasContext *s, arg_##NAME *
a) \
vreg_ofs(s, a->rs2), maxsz, maxsz);\
mark_vs_dirty(s); \
} else {\
-TCGLabel *over = gen_new_label(); \
-tcg_gen_brcondi_tl(TCG_COND_GEU, cpu_vstart, maxsz, over); \
tcg_gen_gvec_2_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2), \
tcg_env, maxsz, maxsz, 0, gen_helper_vmvr_v); \
mark_vs_dirty(s); \
-gen_set_label(over);\
} \
return true;\
} \
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index b4360dbd52..7260a5972b 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -5163,6 +5163,11 @@ void HELPER(vmvr_v)(void *vd, void *vs2, CPURISCVState
*env, uint32_t desc)
VSTART_CHECK_EARLY_EXIT(env);
+if (env->vstart >= maxsz) {
+env->vstart = 0;
+return;
+}
+
memcpy((uint8_t *)vd + H1(i),
(uint8_t *)vs2 + H1(i),
maxsz - startb);
--
2.43.2
[PATCH for-9.0 v13 5/8] target/riscv: remove 'over' brconds from vector trans
The previous patch added an early vstart >= vl exit in all vector
helpers, most of them using the VSTART_CHECK_EARLY_EXIT() macro,
and now we're left with a lot of 'brcond' that has not use. The
pattern goes like this:
VSTART_CHECK_EARLY_EXIT(env);
(...)
tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
(...)
gen_set_label(over);
return true;
The early exit makes the 'brcond' unneeded since it's already granted that
vstart < vl. Remove all 'over' conditionals from the vector helpers.
Note that not all insns uses helpers, and for those cases the 'brcond'
jump is the only way to filter vstart >= vl. This is the case of
trans_vmv_s_x() and trans_vfmv_s_f(). We won't remove the 'brcond'
conditionals from them.
While we're at it, remove the (vl == 0) brconds from trans_rvbf16.c.inc
too since they're unneeded.
Suggested-by: Richard Henderson
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Richard Henderson
Reviewed-by: Alistair Francis
---
target/riscv/insn_trans/trans_rvbf16.c.inc | 12 ---
target/riscv/insn_trans/trans_rvv.c.inc| 105 -
target/riscv/insn_trans/trans_rvvk.c.inc | 18
3 files changed, 135 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvbf16.c.inc
b/target/riscv/insn_trans/trans_rvbf16.c.inc
index 8ee99df3f3..a842e76a6b 100644
--- a/target/riscv/insn_trans/trans_rvbf16.c.inc
+++ b/target/riscv/insn_trans/trans_rvbf16.c.inc
@@ -71,11 +71,8 @@ static bool trans_vfncvtbf16_f_f_w(DisasContext *ctx,
arg_vfncvtbf16_f_f_w *a)
if (opfv_narrow_check(ctx, a) && (ctx->sew == MO_16)) {
uint32_t data = 0;
-TCGLabel *over = gen_new_label();
gen_set_rm_chkfrm(ctx, RISCV_FRM_DYN);
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, ctx->lmul);
@@ -87,7 +84,6 @@ static bool trans_vfncvtbf16_f_f_w(DisasContext *ctx,
arg_vfncvtbf16_f_f_w *a)
ctx->cfg_ptr->vlenb, data,
gen_helper_vfncvtbf16_f_f_w);
mark_vs_dirty(ctx);
-gen_set_label(over);
return true;
}
return false;
@@ -100,11 +96,8 @@ static bool trans_vfwcvtbf16_f_f_v(DisasContext *ctx,
arg_vfwcvtbf16_f_f_v *a)
if (opfv_widen_check(ctx, a) && (ctx->sew == MO_16)) {
uint32_t data = 0;
-TCGLabel *over = gen_new_label();
gen_set_rm_chkfrm(ctx, RISCV_FRM_DYN);
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, ctx->lmul);
@@ -116,7 +109,6 @@ static bool trans_vfwcvtbf16_f_f_v(DisasContext *ctx,
arg_vfwcvtbf16_f_f_v *a)
ctx->cfg_ptr->vlenb, data,
gen_helper_vfwcvtbf16_f_f_v);
mark_vs_dirty(ctx);
-gen_set_label(over);
return true;
}
return false;
@@ -130,11 +122,8 @@ static bool trans_vfwmaccbf16_vv(DisasContext *ctx,
arg_vfwmaccbf16_vv *a)
if (require_rvv(ctx) && vext_check_isa_ill(ctx) && (ctx->sew == MO_16) &&
vext_check_dss(ctx, a->rd, a->rs1, a->rs2, a->vm)) {
uint32_t data = 0;
-TCGLabel *over = gen_new_label();
gen_set_rm_chkfrm(ctx, RISCV_FRM_DYN);
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, ctx->lmul);
@@ -147,7 +136,6 @@ static bool trans_vfwmaccbf16_vv(DisasContext *ctx,
arg_vfwmaccbf16_vv *a)
ctx->cfg_ptr->vlenb, data,
gen_helper_vfwmaccbf16_vv);
mark_vs_dirty(ctx);
-gen_set_label(over);
return true;
}
return false;
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index 52c26a7834..4c1a064cf6 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -616,9 +616,6 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1,
uint32_t data,
TCGv base;
TCGv_i32 desc;
-TCGLabel *over = gen_new_label();
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
-
dest = tcg_temp_new_ptr();
mask = tcg_temp_new_ptr();
base = get_gpr(s, rs1, EXT_NONE);
@@ -660,7 +657,6 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1,
uint32_t data,
tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ);
}
-gen_set_label(over);
return true;
}
@@ -802,9 +798,6 @@ static bool ldst_stride_trans(uint32_t vd, uint32_t rs1,
uint32_t rs2,
TCGv base, stride;
TCGv_i32 desc;
-TCGLabel *over = gen_new_label();
-
[PATCH for-9.0 v13 2/8] trans_rvv.c.inc: set vstart = 0 in int scalar move insns
trans_vmv_x_s, trans_vmv_s_x, trans_vfmv_f_s and trans_vfmv_s_f aren't
setting vstart = 0 after execution. This is usually done by a helper in
vector_helper.c but these functions don't use helpers.
We'll set vstart after any potential 'over' brconds, and that will also
mandate a mark_vs_dirty() too.
Fixes: dedc53cbc9 ("target/riscv: rvv-1.0: integer scalar move instructions")
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Richard Henderson
---
target/riscv/insn_trans/trans_rvv.c.inc | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index e42728990e..8c16a9f5b3 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -3373,6 +3373,8 @@ static bool trans_vmv_x_s(DisasContext *s, arg_vmv_x_s *a)
vec_element_loadi(s, t1, a->rs2, 0, true);
tcg_gen_trunc_i64_tl(dest, t1);
gen_set_gpr(s, a->rd, dest);
+tcg_gen_movi_tl(cpu_vstart, 0);
+mark_vs_dirty(s);
return true;
}
return false;
@@ -3399,8 +3401,9 @@ static bool trans_vmv_s_x(DisasContext *s, arg_vmv_s_x *a)
s1 = get_gpr(s, a->rs1, EXT_NONE);
tcg_gen_ext_tl_i64(t1, s1);
vec_element_storei(s, a->rd, 0, t1);
-mark_vs_dirty(s);
gen_set_label(over);
+tcg_gen_movi_tl(cpu_vstart, 0);
+mark_vs_dirty(s);
return true;
}
return false;
@@ -3427,6 +3430,8 @@ static bool trans_vfmv_f_s(DisasContext *s, arg_vfmv_f_s
*a)
}
mark_fs_dirty(s);
+tcg_gen_movi_tl(cpu_vstart, 0);
+mark_vs_dirty(s);
return true;
}
return false;
@@ -3452,8 +3457,9 @@ static bool trans_vfmv_s_f(DisasContext *s, arg_vfmv_s_f
*a)
do_nanbox(s, t1, cpu_fpr[a->rs1]);
vec_element_storei(s, a->rd, 0, t1);
-mark_vs_dirty(s);
gen_set_label(over);
+tcg_gen_movi_tl(cpu_vstart, 0);
+mark_vs_dirty(s);
return true;
}
return false;
--
2.43.2
[PATCH for-9.0 v13 1/8] target/riscv/vector_helper.c: set vstart = 0 in GEN_VEXT_VSLIDEUP_VX()
The helper isn't setting env->vstart = 0 after its execution, as it is expected from every vector instruction that completes successfully. Signed-off-by: Daniel Henrique Barboza Reviewed-by: Richard Henderson Reviewed-by: Alistair Francis --- target/riscv/vector_helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c index fe56c007d5..ca79571ae2 100644 --- a/target/riscv/vector_helper.c +++ b/target/riscv/vector_helper.c @@ -4781,6 +4781,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \ } \ *((ETYPE *)vd + H(i)) = *((ETYPE *)vs2 + H(i - offset)); \ } \ +env->vstart = 0; \ /* set tail elements to 1s */ \ vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz); \ } -- 2.43.2
[PATCH for-9.0 v13 0/8] riscv: set vstart_eq_zero on vector insns
Hi, In this new version I added a new patch (patch 4) to handle the case pointed out by LIU Zhiwei in v12. I decided to do it in separate since it's a distinct case from what we're dealing with in patch 5. No other changes made. Series based on master. Patches missing acks: patch 4. Changes from v12: - patch 4 (new): - move vstart >= maxsz cond to the vmvr_v helper - clear vstart when vstart >= maxsz - v12 link: https://lore.kernel.org/qemu-riscv/20240311180821.250469-1-dbarb...@ventanamicro.com/ Daniel Henrique Barboza (7): target/riscv/vector_helper.c: set vstart = 0 in GEN_VEXT_VSLIDEUP_VX() trans_rvv.c.inc: set vstart = 0 in int scalar move insns target/riscv/vector_helpers: do early exit when vstart >= vl target/riscv: always clear vstart in whole vec move insns target/riscv: remove 'over' brconds from vector trans trans_rvv.c.inc: remove redundant mark_vs_dirty() calls target/riscv/vector_helper.c: optimize loops in ldst helpers Ivan Klokov (1): target/riscv: enable 'vstart_eq_zero' in the end of insns target/riscv/insn_trans/trans_rvbf16.c.inc | 18 +- target/riscv/insn_trans/trans_rvv.c.inc| 198 + target/riscv/insn_trans/trans_rvvk.c.inc | 30 +--- target/riscv/translate.c | 6 + target/riscv/vcrypto_helper.c | 32 target/riscv/vector_helper.c | 102 ++- target/riscv/vector_internals.c| 4 + target/riscv/vector_internals.h| 9 + 8 files changed, 207 insertions(+), 192 deletions(-) -- 2.43.2
Re: [PATCH v3 1/1] target/i386: Enable page walking from MMIO memory
On 3/7/24 05:53, Jonathan Cameron wrote: From: Gregory Price CXL emulation of interleave requires read and write hooks due to requirement for subpage granularity. The Linux kernel stack now enables using this memory as conventional memory in a separate NUMA node. If a process is deliberately forced to run from that node $ numactl --membind=1 ls the page table walk on i386 fails. Useful part of backtrace: (cpu=cpu@entry=0x56fd9000, fmt=fmt@entry=0x55fe3378 "cpu_io_recompile: could not find TB for pc=%p") at ../../cpu-target.c:359 (retaddr=0, addr=19595792376, attrs=..., xlat=, cpu=0x56fd9000, out_offset=) at ../../accel/tcg/cputlb.c:1339 (cpu=0x56fd9000, full=0x7fffee0d96e0, ret_be=ret_be@entry=0, addr=19595792376, size=size@entry=8, mmu_idx=4, type=MMU_DATA_LOAD, ra=0) at ../../accel/tcg/cputlb.c:2030 (cpu=cpu@entry=0x56fd9000, p=p@entry=0x756fddc0, mmu_idx=, type=type@entry=MMU_DATA_LOAD, memop=, ra=ra@entry=0) at ../../accel/tcg/cputlb.c:2356 (cpu=cpu@entry=0x56fd9000, addr=addr@entry=19595792376, oi=oi@entry=52, ra=ra@entry=0, access_type=access_type@entry=MMU_DATA_LOAD) at ../../accel/tcg/cputlb.c:2439 at ../../accel/tcg/ldst_common.c.inc:301 at ../../target/i386/tcg/sysemu/excp_helper.c:173 (err=0x756fdf80, out=0x756fdf70, mmu_idx=0, access_type=MMU_INST_FETCH, addr=18446744072116178925, env=0x56fdb7c0) at ../../target/i386/tcg/sysemu/excp_helper.c:578 (cs=0x56fd9000, addr=18446744072116178925, size=, access_type=MMU_INST_FETCH, mmu_idx=0, probe=, retaddr=0) at ../../target/i386/tcg/sysemu/excp_helper.c:604 Avoid this by plumbing the address all the way down from x86_cpu_tlb_fill() where is available as retaddr to the actual accessors which provide it to probe_access_full() which already handles MMIO accesses. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Suggested-by: Peter Maydell Signed-off-by: Gregory Price Signed-off-by: Jonathan Cameron --- v3: No change. Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2180 Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2220 r~
Re: [PATCH v2] target/arm: Fix 32-bit SMOPA
On 3/13/24 09:12, Michael Tokarev wrote: warning: TCG temporary leaks before 00400730 qemu-aarch64: ../tcg/tcg.c:1052: tcg_temp_free_internal: Assertion `ts->temp_allocated != 0' failed. timeout: the monitored command dumped core Trace/breakpoint trap Does it make sense to pick this for 7.2 with the above failures? No, it doesn't. I guess it was in the 8.x series that we eliminated the need for freeing tcg temporaries. The patch set would need adjustment for that, and I don't think it's worth the effort. r~
Re: [PATCH v5 3/3] target/riscv: Enable sdtrig for Ventana's Veyron CPUs
On Wed, Mar 13, 2024 at 11:50:09PM +0530, Himanshu Chauhan wrote: > Ventana's Veyron CPUs support sdtrig ISA extension. By default, enable > the sdtrig extension and disable the debug property for these CPUs. > > Signed-off-by: Himanshu Chauhan > --- > target/riscv/cpu.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index e0710010f5..a7ea66c7fa 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -568,7 +568,9 @@ static void rv64_veyron_v1_cpu_init(Object *obj) > cpu->cfg.ext_zicbom = true; > cpu->cfg.cbom_blocksize = 64; > cpu->cfg.cboz_blocksize = 64; > +cpu->cfg.debug = false; We don't want/need the above line. Veyron does support 'debug' since it supports 'sdtrig'. And removing the line above allows all the '|| cfg->ext_sdtrig' to also be removed. Thanks, drew > cpu->cfg.ext_zicboz = true; > +cpu->cfg.ext_sdtrig = true; > cpu->cfg.ext_smaia = true; > cpu->cfg.ext_ssaia = true; > cpu->cfg.ext_sscofpmf = true; > -- > 2.34.1 > >
Re: [PATCH v5 2/3] target/riscv: Expose sdtrig ISA extension
On Wed, Mar 13, 2024 at 11:50:08PM +0530, Himanshu Chauhan wrote:
> This patch adds "sdtrig" in the ISA string when sdtrig extension is enabled.
> The sdtrig extension may or may not be implemented in a system. Therefore, the
>-cpu rv64,sdtrig=
> option can be used to dynamically turn sdtrig extension on or off.
>
> Since, the sdtrig ISA extension is a superset of debug specification, disable
> the debug property when sdtrig is enabled. A warning is printed when this is
> done.
>
> By default, the sdtrig extension is disabled and debug property enabled as
> usual.
>
> Signed-off-by: Himanshu Chauhan
> ---
> target/riscv/cpu.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 2602aae9f5..e0710010f5 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -175,6 +175,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
> ISA_EXT_DATA_ENTRY(zvkt, PRIV_VERSION_1_12_0, ext_zvkt),
> ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx),
> ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
> +ISA_EXT_DATA_ENTRY(sdtrig, PRIV_VERSION_1_12_0, ext_sdtrig),
> ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
> ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
> ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
> @@ -1008,6 +1009,10 @@ static void riscv_cpu_reset_hold(Object *obj)
> set_default_nan_mode(1, >fp_status);
>
> #ifndef CONFIG_USER_ONLY
> +if (!cpu->cfg.debug && cpu->cfg.ext_sdtrig) {
> + cpu->cfg.debug = 1;
nit: '= true'
I also wonder if we need a warning here that says something like
"reenabling 'debug' since 'sdtrig' is enabled...", since the only way we'd
get here is if the user did 'debug=off,sdtrig=on'. But, I think I might be
OK with just silently ignoring that 'debug=off' too.
Thanks,
drew
> +}
> +
> if (cpu->cfg.debug || cpu->cfg.ext_sdtrig) {
> riscv_trigger_reset_hold(env);
> }
> @@ -1480,6 +1485,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
> MULTI_EXT_CFG_BOOL("zvfhmin", ext_zvfhmin, false),
> MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
>
> +MULTI_EXT_CFG_BOOL("sdtrig", ext_sdtrig, false),
> MULTI_EXT_CFG_BOOL("smaia", ext_smaia, false),
> MULTI_EXT_CFG_BOOL("smepmp", ext_smepmp, false),
> MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
> --
> 2.34.1
>
>
Re: [PATCH v2 1/2] vhost: dirty log should be per backend type
On 3/12/2024 8:07 AM, Michael S. Tsirkin wrote: On Wed, Feb 14, 2024 at 10:42:29AM -0800, Si-Wei Liu wrote: Hi Michael, I'm taking off for 2+ weeks, but please feel free to provide comment and feedback while I'm off. I'll be checking emails still, and am about to address any opens as soon as I am back. Thanks, -Siwei Eugenio sent some comments. I don't have more, just address these please. Thanks! Thanks Michael, good to know you don't have more other than the one from Eugenio. I will post a v3 shortly to address his comments. -Siwei
Re: [PATCH v2] target/arm: Fix 32-bit SMOPA
10.03.2024 21:13, Richard Henderson wrote: On 3/9/24 08:40, Michael Tokarev wrote: ... I tried to pick this one up for stable-7.2 (since the fix is for older commit), and faced a fun issue in this change to tests/tcg/aarch64/Makefile.target, since 7.2. doesn't have CROSS_AS_HAS_ARMV9_SME yet. I went on and found out that the following commits: v7.2.0-374-gbc6bd20ee3 target/arm: align exposed ID registers with Linux v8.0.0-2358-g3dc2afeab2 tests/tcg/aarch64/sysregs.c: Use S syntax for id_aa64zfr0_el1 and id_aa64smfr0_el1 v8.0.0-2361-g1f51573f79 target/arm: Fix SME full tile indexing applies to 7.2, and lets this "Fix 32-bit SMOPA" change to apply cleanly, including this very change in Makefile.target. Now, 1f51573f79 "Fix SME full tile indexing" is Cc'd qemu-stable already, but it is not in 7.2.x for some reason which I don't remember anymore, so it is a good one to pick up already. 3dc2afeab is tests-only. Oh wow, I didn't expect the fix to get propagated back that far. I was expecting only back into the 8.x series... And bc6bd20ee3 (from Dec-2022) seems like a good candidate too, is it not? Sure, couldn't hurt. If it all applies without drama, all is well. While it goes fine, it doesn't quite work actually: https://gitlab.com/qemu-project/qemu/-/jobs/6386966720#L2482 timeout --foreground 90 /home/gitlab-runner/builds/E8PpwMky/0/qemu-project/qemu/build/qemu-aarch64 sme-smopa-1 > sme-smopa-1.out warning: TCG temporary leaks before 00400714 qemu-aarch64: ../tcg/tcg.c:1052: tcg_temp_free_internal: Assertion `ts->temp_allocated != 0' failed. timeout: the monitored command dumped core Trace/breakpoint trap make[1]: *** [Makefile:170: run-sme-smopa-1] Error 133 make[1]: Leaving directory '/home/gitlab-runner/builds/E8PpwMky/0/qemu-project/qemu/build/tests/tcg/aarch64-linux-user' make[1]: *** Waiting for unfinished jobs make[1]: Entering directory '/home/gitlab-runner/builds/E8PpwMky/0/qemu-project/qemu/build/tests/tcg/aarch64-linux-user' timeout --foreground 90 /home/gitlab-runner/builds/E8PpwMky/0/qemu-project/qemu/build/qemu-aarch64 sme-smopa-2 > sme-smopa-2.out warning: TCG temporary leaks before 00400730 qemu-aarch64: ../tcg/tcg.c:1052: tcg_temp_free_internal: Assertion `ts->temp_allocated != 0' failed. timeout: the monitored command dumped core Trace/breakpoint trap Does it make sense to pick this for 7.2 with the above failures? Unfortunately this test does not run on a private repository clone on gitlab, that's why I haven't noticed this immediately. Thanks, /mjt
Re: [PATCH 12/12] vdpa: fix network breakage after cancelling migration
On 3/13/2024 11:12 AM, Michael Tokarev wrote:
14.02.2024 14:28, Si-Wei Liu wrote:
Fix an issue where cancellation of ongoing migration ends up
with no network connectivity.
When canceling migration, SVQ will be switched back to the
passthrough mode, but the right call fd is not programed to
the device and the svq's own call fd is still used. At the
point of this transitioning period, the shadow_vqs_enabled
hadn't been set back to false yet, causing the installation
of call fd inadvertently bypassed.
Fixes: a8ac88585da1 ("vhost: Add Shadow VirtQueue call forwarding
capabilities")
Cc: Eugenio Pérez
Acked-by: Jason Wang
Signed-off-by: Si-Wei Liu
---
hw/virtio/vhost-vdpa.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
Is this a -stable material?
Probably yes, the pre-requisites of this patch are PATCH #10 and #11
from this series (where SVQ_TSTATE_DISABLING gets defined and set).
If yes, is it also applicable for stable-7.2 (mentioned commit is in
7.2.0),
which lacks v7.2.0-2327-gb276524386 "vdpa: Remember last call fd set",
or should this one also be picked up?
Eugenio can judge, but seems to me the relevant code path cannot be
effectively called as the dynamic SVQ feature (switching over to SVQ
dynamically when migration is started) is not supported from 7.2. Maybe
not worth it to cherry-pick this one to 7.2. Cherry-pick to stable-8.0
and above should be applicable though (it needs some tweaks on patch #10
to move svq_switching from @struct VhostVDPAShared to @struct vhost_vdpa).
Regards,
-Siwei
Thanks,
/mjt
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 004110f..dfeca8b 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1468,7 +1468,15 @@ static int vhost_vdpa_set_vring_call(struct
vhost_dev *dev,
/* Remember last call fd because we can switch to SVQ
anytime. */
vhost_svq_set_svq_call_fd(svq, file->fd);
- if (v->shadow_vqs_enabled) {
+ /*
+ * When SVQ is transitioning to off, shadow_vqs_enabled has
+ * not been set back to false yet, but the underlying call fd
+ * will have to switch back to the guest notifier to signal the
+ * passthrough virtqueues. In other situations, SVQ's own call
+ * fd shall be used to signal the device model.
+ */
+ if (v->shadow_vqs_enabled &&
+ v->shared->svq_switching != SVQ_TSTATE_DISABLING) {
return 0;
}
Re: [PATCH v5 1/3] target/riscv: Enable mcontrol6 triggers only when sdtrig is selected
On Wed, Mar 13, 2024 at 11:50:07PM +0530, Himanshu Chauhan wrote:
> The mcontrol6 triggers are not defined in debug specification v0.13
> These triggers are defined in sdtrig ISA extension.
>
> This patch:
>* Adds ext_sdtrig capability which is used to select mcontrol6 triggers
>* Keeps the debug property. All triggers that are defined in v0.13 are
> exposed.
>
> Signed-off-by: Himanshu Chauhan
> ---
> target/riscv/cpu.c | 4 +-
> target/riscv/cpu_cfg.h | 1 +
> target/riscv/csr.c | 2 +-
> target/riscv/debug.c | 90 +-
> target/riscv/machine.c | 2 +-
> 5 files changed, 58 insertions(+), 41 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index c160b9216b..2602aae9f5 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1008,7 +1008,7 @@ static void riscv_cpu_reset_hold(Object *obj)
> set_default_nan_mode(1, >fp_status);
>
> #ifndef CONFIG_USER_ONLY
> -if (cpu->cfg.debug) {
> +if (cpu->cfg.debug || cpu->cfg.ext_sdtrig) {
> riscv_trigger_reset_hold(env);
> }
>
> @@ -1168,7 +1168,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> **errp)
> riscv_cpu_register_gdb_regs_for_features(cs);
>
> #ifndef CONFIG_USER_ONLY
> -if (cpu->cfg.debug) {
> +if (cpu->cfg.debug || cpu->cfg.ext_sdtrig) {
> riscv_trigger_realize(>env);
> }
> #endif
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index 2040b90da0..0c57e1acd4 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -114,6 +114,7 @@ struct RISCVCPUConfig {
> bool ext_zvfbfwma;
> bool ext_zvfh;
> bool ext_zvfhmin;
> +bool ext_sdtrig;
> bool ext_smaia;
> bool ext_ssaia;
> bool ext_sscofpmf;
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 726096444f..26623d3640 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -546,7 +546,7 @@ static RISCVException have_mseccfg(CPURISCVState *env,
> int csrno)
>
> static RISCVException debug(CPURISCVState *env, int csrno)
> {
> -if (riscv_cpu_cfg(env)->debug) {
> +if (riscv_cpu_cfg(env)->debug || riscv_cpu_cfg(env)->ext_sdtrig) {
> return RISCV_EXCP_NONE;
> }
>
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index e30d99cc2f..674223e966 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -100,13 +100,15 @@ static trigger_action_t
> get_trigger_action(CPURISCVState *env,
> target_ulong tdata1 = env->tdata1[trigger_index];
> int trigger_type = get_trigger_type(env, trigger_index);
> trigger_action_t action = DBG_ACTION_NONE;
> +const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
>
> switch (trigger_type) {
> case TRIGGER_TYPE_AD_MATCH:
> action = (tdata1 & TYPE2_ACTION) >> 12;
> break;
> case TRIGGER_TYPE_AD_MATCH6:
> -action = (tdata1 & TYPE6_ACTION) >> 12;
> +if (cfg->ext_sdtrig)
> +action = (tdata1 & TYPE6_ACTION) >> 12;
> break;
> case TRIGGER_TYPE_INST_CNT:
> case TRIGGER_TYPE_INT:
> @@ -727,7 +729,12 @@ void tdata_csr_write(CPURISCVState *env, int
> tdata_index, target_ulong val)
> type2_reg_write(env, env->trigger_cur, tdata_index, val);
> break;
> case TRIGGER_TYPE_AD_MATCH6:
> -type6_reg_write(env, env->trigger_cur, tdata_index, val);
> +if (riscv_cpu_cfg(env)->ext_sdtrig) {
> +type6_reg_write(env, env->trigger_cur, tdata_index, val);
> +} else {
> +qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
> + trigger_type);
> +}
> break;
> case TRIGGER_TYPE_INST_CNT:
> itrigger_reg_write(env, env->trigger_cur, tdata_index, val);
> @@ -750,9 +757,14 @@ void tdata_csr_write(CPURISCVState *env, int
> tdata_index, target_ulong val)
>
> target_ulong tinfo_csr_read(CPURISCVState *env)
> {
> -/* assume all triggers support the same types of triggers */
> -return BIT(TRIGGER_TYPE_AD_MATCH) |
> - BIT(TRIGGER_TYPE_AD_MATCH6);
> +target_ulong ts = 0;
> +
> +ts = BIT(TRIGGER_TYPE_AD_MATCH);
> +
> +if (riscv_cpu_cfg(env)->ext_sdtrig)
> +ts |= BIT(TRIGGER_TYPE_AD_MATCH6);
> +
> +return ts;
> }
>
> void riscv_cpu_debug_excp_handler(CPUState *cs)
> @@ -803,19 +815,21 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
> }
> break;
> case TRIGGER_TYPE_AD_MATCH6:
> -ctrl = env->tdata1[i];
> -pc = env->tdata2[i];
> -
> -if ((ctrl & TYPE6_EXEC) && (bp->pc == pc)) {
> -if (env->virt_enabled) {
> -/* check VU/VS bit against current privilege level */
> -if ((ctrl >> 23) & BIT(env->priv)) {
> -return true;
> -}
> -
Re: [PATCH 12/12] vdpa: fix network breakage after cancelling migration
13.03.2024 21:12, Michael Tokarev пишет:
14.02.2024 14:28, Si-Wei Liu wrote:
Fix an issue where cancellation of ongoing migration ends up
with no network connectivity.
When canceling migration, SVQ will be switched back to the
passthrough mode, but the right call fd is not programed to
the device and the svq's own call fd is still used. At the
point of this transitioning period, the shadow_vqs_enabled
hadn't been set back to false yet, causing the installation
of call fd inadvertently bypassed.
Fixes: a8ac88585da1 ("vhost: Add Shadow VirtQueue call forwarding capabilities")
Cc: Eugenio Pérez
Acked-by: Jason Wang
Signed-off-by: Si-Wei Liu
---
hw/virtio/vhost-vdpa.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
Is this a -stable material?
If yes, is it also applicable for stable-7.2 (mentioned commit is in 7.2.0),
which lacks v7.2.0-2327-gb276524386 "vdpa: Remember last call fd set",
or should this one also be picked up?
Aha, this does not actually work without v8.2.0-171-gf6fe3e333f
"vdpa: move memory listener to vhost_vdpa_shared".
/mjt
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 004110f..dfeca8b 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1468,7 +1468,15 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev
*dev,
/* Remember last call fd because we can switch to SVQ anytime. */
vhost_svq_set_svq_call_fd(svq, file->fd);
- if (v->shadow_vqs_enabled) {
+ /*
+ * When SVQ is transitioning to off, shadow_vqs_enabled has
+ * not been set back to false yet, but the underlying call fd
+ * will have to switch back to the guest notifier to signal the
+ * passthrough virtqueues. In other situations, SVQ's own call
+ * fd shall be used to signal the device model.
+ */
+ if (v->shadow_vqs_enabled &&
+ v->shared->svq_switching != SVQ_TSTATE_DISABLING) {
return 0;
}
Re: [PATCH] hw/virtio: Add support for VDPA network simulation devices
On 2024-02-21 15:38:02+0800, Hao Chen wrote:
> This patch adds support for VDPA network simulation devices.
> The device is developed based on virtio-net and tap backend,
> and supports hardware live migration function.
>
> For more details, please refer to "docs/system/devices/vdpa-net.rst"
>
> Signed-off-by: Hao Chen
> ---
> MAINTAINERS | 5 +
> docs/system/device-emulation.rst| 1 +
> docs/system/devices/vdpa-net.rst| 121 +
> hw/net/virtio-net.c | 16 ++
> hw/virtio/virtio-pci.c | 189 +++-
> hw/virtio/virtio.c | 39
> include/hw/virtio/virtio-pci.h | 5 +
> include/hw/virtio/virtio.h | 19 ++
> include/standard-headers/linux/virtio_pci.h | 7 +
> 9 files changed, 399 insertions(+), 3 deletions(-)
> create mode 100644 docs/system/devices/vdpa-net.rst
[..]
> diff --git a/include/standard-headers/linux/virtio_pci.h
> b/include/standard-headers/linux/virtio_pci.h
> index b7fdfd0668..fb5391cef6 100644
> --- a/include/standard-headers/linux/virtio_pci.h
> +++ b/include/standard-headers/linux/virtio_pci.h
> @@ -216,6 +216,13 @@ struct virtio_pci_cfg_cap {
> #define VIRTIO_PCI_COMMON_Q_NDATA56
> #define VIRTIO_PCI_COMMON_Q_RESET58
>
> +#define LM_LOGGING_CTRL 0
> +#define LM_BASE_ADDR_LOW4
> +#define LM_BASE_ADDR_HIGH 8
> +#define LM_END_ADDR_LOW 12
> +#define LM_END_ADDR_HIGH16
> +#define LM_VRING_STATE_OFFSET 0x20
These changes are not in upstream Linux and will be undone by
./scripts/update-linux-headers.sh.
Are they intentionally in this header?
> +
> #endif /* VIRTIO_PCI_NO_MODERN */
>
> #endif
[PATCH-for-9.0 4/4] meson: Enable -Wstatic-in-inline
Compilers are clever enough to inline code when necessary. The only case we accept an inline function is static in header (we use C, not C++). Add the -Wstatic-in-inline CPPFLAG to prevent public and inline function to be added in the code base. Signed-off-by: Philippe Mathieu-Daudé --- meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/meson.build b/meson.build index b375248a76..f57397aa53 100644 --- a/meson.build +++ b/meson.build @@ -591,6 +591,7 @@ warn_flags = [ '-Wold-style-definition', '-Wredundant-decls', '-Wshadow=local', + '-Wstatic-in-inline', '-Wstrict-prototypes', '-Wtype-limits', '-Wundef', -- 2.41.0
[PATCH-for-9.0 1/4] hw/arm/smmu: Avoid using inlined functions with external linkage again
Similarly to commit 9de9fa5cf2 ("hw/arm/smmu-common: Avoid using
inlined functions with external linkage"):
None of our code base require / use inlined functions with external
linkage. Some places use internal inlining in the hot path. These
two functions are certainly not in any hot path and don't justify
any inlining, so these are likely oversights rather than intentional.
Fix:
C compiler for the host machine: clang (clang 15.0.0 "Apple clang version
15.0.0 (clang-1500.3.9.4)")
...
hw/arm/smmu-common.c:203:43: error: static function
'smmu_hash_remove_by_vmid' is
used in an inline function with external linkage [-Werror,-Wstatic-in-inline]
g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_vmid, );
^
include/hw/arm/smmu-common.h:197:1: note: use 'static' to give inline
function 'smmu_iotlb_inv_vmid' internal linkage
void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid);
^
static
hw/arm/smmu-common.c:139:17: note: 'smmu_hash_remove_by_vmid' declared here
static gboolean smmu_hash_remove_by_vmid(gpointer key, gpointer value,
^
Fixes: ccc3ee3871 ("hw/arm/smmuv3: Add CMDs related to stage-2")
Signed-off-by: Philippe Mathieu-Daudé
---
hw/arm/smmu-common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 4caedb4998..c4b540656c 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -197,7 +197,7 @@ void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid)
g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_asid, );
}
-inline void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid)
+void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid)
{
trace_smmu_iotlb_inv_vmid(vmid);
g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_vmid, );
--
2.41.0
[PATCH-for-9.0 2/4] accel/hvf: Un-inline hvf_arch_supports_guest_debug()
See previous commit and commit 9de9fa5cf2 ("Avoid using inlined
functions with external linkage") for rationale.
Signed-off-by: Philippe Mathieu-Daudé
---
target/arm/hvf/hvf.c | 2 +-
target/i386/hvf/hvf.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index e5f0f60093..65a5601804 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -2246,7 +2246,7 @@ void hvf_arch_update_guest_debug(CPUState *cpu)
hvf_arch_set_traps();
}
-inline bool hvf_arch_supports_guest_debug(void)
+bool hvf_arch_supports_guest_debug(void)
{
return true;
}
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 11ffdd4c69..1ed8ed5154 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -708,7 +708,7 @@ void hvf_arch_update_guest_debug(CPUState *cpu)
{
}
-inline bool hvf_arch_supports_guest_debug(void)
+bool hvf_arch_supports_guest_debug(void)
{
return false;
}
--
2.41.0
[PATCH-for-9.0 3/4] qtest/libqos: Un-inline size_to_prdtl()
See previous commit and commit 9de9fa5cf2 ("Avoid using inlined
functions with external linkage") for rationale.
Signed-off-by: Philippe Mathieu-Daudé
---
tests/qtest/libqos/ahci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qtest/libqos/ahci.c b/tests/qtest/libqos/ahci.c
index a2c94c6e06..135b23ffd9 100644
--- a/tests/qtest/libqos/ahci.c
+++ b/tests/qtest/libqos/ahci.c
@@ -662,7 +662,7 @@ unsigned ahci_pick_cmd(AHCIQState *ahci, uint8_t port)
g_assert_not_reached();
}
-inline unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd)
+unsigned size_to_prdtl(unsigned bytes, unsigned bytes_per_prd)
{
/* Each PRD can describe up to 4MiB */
g_assert_cmphex(bytes_per_prd, <=, 4096 * 1024);
--
2.41.0
[PATCH-for-9.0 0/4] overall: Avoid using inlined functions with external linkage again
Mostly as a C style cleanup, use -Wstatic-in-inline to avoid using inlined function with external linkage. Philippe Mathieu-Daudé (4): hw/arm/smmu: Avoid using inlined functions with external linkage again accel/hvf: Un-inline hvf_arch_supports_guest_debug() qtest/libqos: Un-inline size_to_prdtl() meson: Enable -Wstatic-in-inline meson.build | 1 + hw/arm/smmu-common.c | 2 +- target/arm/hvf/hvf.c | 2 +- target/i386/hvf/hvf.c | 2 +- tests/qtest/libqos/ahci.c | 2 +- 5 files changed, 5 insertions(+), 4 deletions(-) -- 2.41.0
[PATCH] docs/specs/pvpanic: mark shutdown event as not implemented
Mention the fact that this event is not yet implemented to avoid confusion. As requested by Michael. Signed-off-by: Thomas Weißschuh --- docs/specs/pvpanic.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/specs/pvpanic.rst b/docs/specs/pvpanic.rst index 61a80480edb8..b0f27860ec3b 100644 --- a/docs/specs/pvpanic.rst +++ b/docs/specs/pvpanic.rst @@ -29,7 +29,7 @@ bit 1 a guest panic has happened and will be handled by the guest; the host should record it or report it, but should not affect the execution of the guest. -bit 2 +bit 2 (to be implemented) a regular guest shutdown has happened and should be processed by the host PCI Interface --- base-commit: ba49d760eb04630e7b15f423ebecf6c871b8f77b change-id: 20240313-pvpanic-note-fa3ce8d2165a Best regards, -- Thomas Weißschuh
Re: [PATCH v2 2/2] hmat acpi: Fix out of bounds access due to missing use of indirection
07.03.2024 19:03, Jonathan Cameron via wrote:
With a numa set up such as
-numa nodeid=0,cpus=0 \
-numa nodeid=1,memdev=mem \
-numa nodeid=2,cpus=1
and appropriate hmat_lb entries the initiator list is correctly
computed and writen to HMAT as 0,2 but then the LB data is accessed
using the node id (here 2), landing outside the entry_list array.
Stash the reverse lookup when writing the initiator list and use
it to get the correct array index index.
Fixes: 4586a2cb83 ("hmat acpi: Build System Locality Latency and Bandwidth
Information Structure(s)")
Signed-off-by: Jonathan Cameron
This seems like a -stable material, is it not?
Thanks,
/mjt
---
hw/acpi/hmat.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
index 723ae28d32..b933ae3c06 100644
--- a/hw/acpi/hmat.c
+++ b/hw/acpi/hmat.c
@@ -78,6 +78,7 @@ static void build_hmat_lb(GArray *table_data, HMAT_LB_Info
*hmat_lb,
uint32_t *initiator_list)
{
int i, index;
+uint32_t initiator_to_index[MAX_NODES] = {};
HMAT_LB_Data *lb_data;
uint16_t *entry_list;
uint32_t base;
@@ -121,6 +122,8 @@ static void build_hmat_lb(GArray *table_data, HMAT_LB_Info
*hmat_lb,
/* Initiator Proximity Domain List */
for (i = 0; i < num_initiator; i++) {
build_append_int_noprefix(table_data, initiator_list[i], 4);
+/* Reverse mapping for array possitions */
+initiator_to_index[initiator_list[i]] = i;
}
/* Target Proximity Domain List */
@@ -132,7 +135,8 @@ static void build_hmat_lb(GArray *table_data, HMAT_LB_Info
*hmat_lb,
entry_list = g_new0(uint16_t, num_initiator * num_target);
for (i = 0; i < hmat_lb->list->len; i++) {
lb_data = _array_index(hmat_lb->list, HMAT_LB_Data, i);
-index = lb_data->initiator * num_target + lb_data->target;
+index = initiator_to_index[lb_data->initiator] * num_target +
+lb_data->target;
entry_list[index] = (uint16_t)(lb_data->data / hmat_lb->base);
}
[PATCH v5 1/3] target/riscv: Enable mcontrol6 triggers only when sdtrig is selected
The mcontrol6 triggers are not defined in debug specification v0.13
These triggers are defined in sdtrig ISA extension.
This patch:
* Adds ext_sdtrig capability which is used to select mcontrol6 triggers
* Keeps the debug property. All triggers that are defined in v0.13 are
exposed.
Signed-off-by: Himanshu Chauhan
---
target/riscv/cpu.c | 4 +-
target/riscv/cpu_cfg.h | 1 +
target/riscv/csr.c | 2 +-
target/riscv/debug.c | 90 +-
target/riscv/machine.c | 2 +-
5 files changed, 58 insertions(+), 41 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index c160b9216b..2602aae9f5 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1008,7 +1008,7 @@ static void riscv_cpu_reset_hold(Object *obj)
set_default_nan_mode(1, >fp_status);
#ifndef CONFIG_USER_ONLY
-if (cpu->cfg.debug) {
+if (cpu->cfg.debug || cpu->cfg.ext_sdtrig) {
riscv_trigger_reset_hold(env);
}
@@ -1168,7 +1168,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error
**errp)
riscv_cpu_register_gdb_regs_for_features(cs);
#ifndef CONFIG_USER_ONLY
-if (cpu->cfg.debug) {
+if (cpu->cfg.debug || cpu->cfg.ext_sdtrig) {
riscv_trigger_realize(>env);
}
#endif
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 2040b90da0..0c57e1acd4 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -114,6 +114,7 @@ struct RISCVCPUConfig {
bool ext_zvfbfwma;
bool ext_zvfh;
bool ext_zvfhmin;
+bool ext_sdtrig;
bool ext_smaia;
bool ext_ssaia;
bool ext_sscofpmf;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 726096444f..26623d3640 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -546,7 +546,7 @@ static RISCVException have_mseccfg(CPURISCVState *env, int
csrno)
static RISCVException debug(CPURISCVState *env, int csrno)
{
-if (riscv_cpu_cfg(env)->debug) {
+if (riscv_cpu_cfg(env)->debug || riscv_cpu_cfg(env)->ext_sdtrig) {
return RISCV_EXCP_NONE;
}
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index e30d99cc2f..674223e966 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -100,13 +100,15 @@ static trigger_action_t get_trigger_action(CPURISCVState
*env,
target_ulong tdata1 = env->tdata1[trigger_index];
int trigger_type = get_trigger_type(env, trigger_index);
trigger_action_t action = DBG_ACTION_NONE;
+const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
switch (trigger_type) {
case TRIGGER_TYPE_AD_MATCH:
action = (tdata1 & TYPE2_ACTION) >> 12;
break;
case TRIGGER_TYPE_AD_MATCH6:
-action = (tdata1 & TYPE6_ACTION) >> 12;
+if (cfg->ext_sdtrig)
+action = (tdata1 & TYPE6_ACTION) >> 12;
break;
case TRIGGER_TYPE_INST_CNT:
case TRIGGER_TYPE_INT:
@@ -727,7 +729,12 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index,
target_ulong val)
type2_reg_write(env, env->trigger_cur, tdata_index, val);
break;
case TRIGGER_TYPE_AD_MATCH6:
-type6_reg_write(env, env->trigger_cur, tdata_index, val);
+if (riscv_cpu_cfg(env)->ext_sdtrig) {
+type6_reg_write(env, env->trigger_cur, tdata_index, val);
+} else {
+qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
+ trigger_type);
+}
break;
case TRIGGER_TYPE_INST_CNT:
itrigger_reg_write(env, env->trigger_cur, tdata_index, val);
@@ -750,9 +757,14 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index,
target_ulong val)
target_ulong tinfo_csr_read(CPURISCVState *env)
{
-/* assume all triggers support the same types of triggers */
-return BIT(TRIGGER_TYPE_AD_MATCH) |
- BIT(TRIGGER_TYPE_AD_MATCH6);
+target_ulong ts = 0;
+
+ts = BIT(TRIGGER_TYPE_AD_MATCH);
+
+if (riscv_cpu_cfg(env)->ext_sdtrig)
+ts |= BIT(TRIGGER_TYPE_AD_MATCH6);
+
+return ts;
}
void riscv_cpu_debug_excp_handler(CPUState *cs)
@@ -803,19 +815,21 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
}
break;
case TRIGGER_TYPE_AD_MATCH6:
-ctrl = env->tdata1[i];
-pc = env->tdata2[i];
-
-if ((ctrl & TYPE6_EXEC) && (bp->pc == pc)) {
-if (env->virt_enabled) {
-/* check VU/VS bit against current privilege level */
-if ((ctrl >> 23) & BIT(env->priv)) {
-return true;
-}
-} else {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
+if (cpu->cfg.ext_sdtrig) {
+ctrl = env->tdata1[i];
+pc = env->tdata2[i];
+
+
[PATCH v5 3/3] target/riscv: Enable sdtrig for Ventana's Veyron CPUs
Ventana's Veyron CPUs support sdtrig ISA extension. By default, enable the sdtrig extension and disable the debug property for these CPUs. Signed-off-by: Himanshu Chauhan --- target/riscv/cpu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index e0710010f5..a7ea66c7fa 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -568,7 +568,9 @@ static void rv64_veyron_v1_cpu_init(Object *obj) cpu->cfg.ext_zicbom = true; cpu->cfg.cbom_blocksize = 64; cpu->cfg.cboz_blocksize = 64; +cpu->cfg.debug = false; cpu->cfg.ext_zicboz = true; +cpu->cfg.ext_sdtrig = true; cpu->cfg.ext_smaia = true; cpu->cfg.ext_ssaia = true; cpu->cfg.ext_sscofpmf = true; -- 2.34.1
[PATCH v5 2/3] target/riscv: Expose sdtrig ISA extension
This patch adds "sdtrig" in the ISA string when sdtrig extension is enabled.
The sdtrig extension may or may not be implemented in a system. Therefore, the
-cpu rv64,sdtrig=
option can be used to dynamically turn sdtrig extension on or off.
Since, the sdtrig ISA extension is a superset of debug specification, disable
the debug property when sdtrig is enabled. A warning is printed when this is
done.
By default, the sdtrig extension is disabled and debug property enabled as
usual.
Signed-off-by: Himanshu Chauhan
---
target/riscv/cpu.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 2602aae9f5..e0710010f5 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -175,6 +175,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zvkt, PRIV_VERSION_1_12_0, ext_zvkt),
ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx),
ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
+ISA_EXT_DATA_ENTRY(sdtrig, PRIV_VERSION_1_12_0, ext_sdtrig),
ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
@@ -1008,6 +1009,10 @@ static void riscv_cpu_reset_hold(Object *obj)
set_default_nan_mode(1, >fp_status);
#ifndef CONFIG_USER_ONLY
+if (!cpu->cfg.debug && cpu->cfg.ext_sdtrig) {
+ cpu->cfg.debug = 1;
+}
+
if (cpu->cfg.debug || cpu->cfg.ext_sdtrig) {
riscv_trigger_reset_hold(env);
}
@@ -1480,6 +1485,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
MULTI_EXT_CFG_BOOL("zvfhmin", ext_zvfhmin, false),
MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
+MULTI_EXT_CFG_BOOL("sdtrig", ext_sdtrig, false),
MULTI_EXT_CFG_BOOL("smaia", ext_smaia, false),
MULTI_EXT_CFG_BOOL("smepmp", ext_smepmp, false),
MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
--
2.34.1
[PATCH v5 0/3] Introduce sdtrig ISA extension
All the CPUs may or may not implement the debug triggers. Some CPUs may implement only debug specification v0.13 and not sdtrig ISA extension. This patchset, adds sdtrig ISA as an extension which can be turned on or off by sdtrig= option. It is turned off by default. When debug is true and sdtrig is false, the behaviour is as defined in debug specification v0.13. If sdtrig is turned on, the behaviour is as defined in the sdtrig ISA extension. The "sdtrig" string is concatenated to ISA string when debug or sdtrig is enabled. Changes from v1: - Replaced the debug property with ext_sdtrig - Marked it experimenatal by naming it x-sdtrig - x-sdtrig is added to ISA string - Reversed the patch order Changes from v2: - Mark debug property as deprecated and replace internally with sdtrig extension - setting/unsetting debug property shows warning and sets/unsets ext_sdtrig - sdtrig is added to ISA string as RISC-V debug specification is frozen Changes from v3: - debug propery is not deprecated but it is superceded by sdtrig extension - Mcontrol6 support is not published when only debug property is turned on as debug spec v0.13 doesn't define mcontrol6 match triggers. - Enabling sdtrig extension turns of debug property and a warning is printed. This doesn't break debug specification implemenation since sdtrig is backward compatible with debug specification. - Disable debug property and enable sdtrig by default for Ventana's Veyron CPUs. Changes from v4: - Enable debug flag if sdtrig was enabled but debug was disabled. - Other cosmetic changes. Himanshu Chauhan (3): target/riscv: Enable mcontrol6 triggers only when sdtrig is selected target/riscv: Expose sdtrig ISA extension target/riscv: Enable sdtrig for Ventana's Veyron CPUs target/riscv/cpu.c | 12 +- target/riscv/cpu_cfg.h | 1 + target/riscv/csr.c | 2 +- target/riscv/debug.c | 90 +- target/riscv/machine.c | 2 +- 5 files changed, 66 insertions(+), 41 deletions(-) -- 2.34.1
Re: [PATCH 12/12] vdpa: fix network breakage after cancelling migration
14.02.2024 14:28, Si-Wei Liu wrote:
Fix an issue where cancellation of ongoing migration ends up
with no network connectivity.
When canceling migration, SVQ will be switched back to the
passthrough mode, but the right call fd is not programed to
the device and the svq's own call fd is still used. At the
point of this transitioning period, the shadow_vqs_enabled
hadn't been set back to false yet, causing the installation
of call fd inadvertently bypassed.
Fixes: a8ac88585da1 ("vhost: Add Shadow VirtQueue call forwarding capabilities")
Cc: Eugenio Pérez
Acked-by: Jason Wang
Signed-off-by: Si-Wei Liu
---
hw/virtio/vhost-vdpa.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
Is this a -stable material?
If yes, is it also applicable for stable-7.2 (mentioned commit is in 7.2.0),
which lacks v7.2.0-2327-gb276524386 "vdpa: Remember last call fd set",
or should this one also be picked up?
Thanks,
/mjt
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 004110f..dfeca8b 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1468,7 +1468,15 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev
*dev,
/* Remember last call fd because we can switch to SVQ anytime. */
vhost_svq_set_svq_call_fd(svq, file->fd);
-if (v->shadow_vqs_enabled) {
+/*
+ * When SVQ is transitioning to off, shadow_vqs_enabled has
+ * not been set back to false yet, but the underlying call fd
+ * will have to switch back to the guest notifier to signal the
+ * passthrough virtqueues. In other situations, SVQ's own call
+ * fd shall be used to signal the device model.
+ */
+if (v->shadow_vqs_enabled &&
+v->shared->svq_switching != SVQ_TSTATE_DISABLING) {
return 0;
}
