Re: [External] Re: [PATCH 3/5] migration: Introduce unimplemented 'qatzip' compression method
On Fri, Jan 5, 2024 at 3:52 PM Hao Xiang wrote:
>
> On Fri, Jan 5, 2024 at 12:07 PM Fabiano Rosas wrote:
> >
> > Bryan Zhang writes:
> >
> > +cc Yuan Liu, Daniel Berrangé
> >
> > > Adds support for 'qatzip' as an option for the multifd compression
> > > method parameter, but copy-pastes the no-op logic to leave the actual
> > > methods effectively unimplemented. This is in preparation of a
> > > subsequent commit that will implement actually using QAT for compression
> > > and decompression.
> > >
> > > Signed-off-by: Bryan Zhang
> > > Signed-off-by: Hao Xiang
> > > ---
> > > hw/core/qdev-properties-system.c | 6 ++-
> > > migration/meson.build| 1 +
> > > migration/multifd-qatzip.c | 81
> > > migration/multifd.h | 1 +
> > > qapi/migration.json | 5 +-
> > > 5 files changed, 92 insertions(+), 2 deletions(-)
> > > create mode 100644 migration/multifd-qatzip.c
> > >
> > > diff --git a/hw/core/qdev-properties-system.c
> > > b/hw/core/qdev-properties-system.c
> > > index 1a396521d5..d8e48dcb0e 100644
> > > --- a/hw/core/qdev-properties-system.c
> > > +++ b/hw/core/qdev-properties-system.c
> > > @@ -658,7 +658,11 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
> > > const PropertyInfo qdev_prop_multifd_compression = {
> > > .name = "MultiFDCompression",
> > > .description = "multifd_compression values, "
> > > - "none/zlib/zstd",
> > > + "none/zlib/zstd"
> > > +#ifdef CONFIG_QATZIP
> > > + "/qatzip"
> > > +#endif
> > > + ,
> > > .enum_table = _lookup,
> > > .get = qdev_propinfo_get_enum,
> > > .set = qdev_propinfo_set_enum,
> > > diff --git a/migration/meson.build b/migration/meson.build
> > > index 92b1cc4297..e20f318379 100644
> > > --- a/migration/meson.build
> > > +++ b/migration/meson.build
> > > @@ -40,6 +40,7 @@ if get_option('live_block_migration').allowed()
> > >system_ss.add(files('block.c'))
> > > endif
> > > system_ss.add(when: zstd, if_true: files('multifd-zstd.c'))
> > > +system_ss.add(when: qatzip, if_true: files('multifd-qatzip.c'))
> > >
> > > specific_ss.add(when: 'CONFIG_SYSTEM_ONLY',
> > > if_true: files('ram.c',
> > > diff --git a/migration/multifd-qatzip.c b/migration/multifd-qatzip.c
> > > new file mode 100644
> > > index 00..1733bbddb7
> > > --- /dev/null
> > > +++ b/migration/multifd-qatzip.c
> > > @@ -0,0 +1,81 @@
> > > +/*
> > > + * Multifd QATzip compression implementation
> > > + *
> > > + * Copyright (c) Bytedance
> > > + *
> > > + * Authors:
> > > + * Bryan Zhang
> > > + * Hao Xiang
> > > + *
> > > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > > later.
> > > + * See the COPYING file in the top-level directory.
> > > + */
> > > +
> > > +#include "qemu/osdep.h"
> > > +#include "exec/ramblock.h"
> > > +#include "exec/target_page.h"
> > > +#include "qapi/error.h"
> > > +#include "migration.h"
> > > +#include "options.h"
> > > +#include "multifd.h"
> > > +
> > > +static int qatzip_send_setup(MultiFDSendParams *p, Error **errp)
> > > +{
> > > +return 0;
> > > +}
> > > +
> > > +static void qatzip_send_cleanup(MultiFDSendParams *p, Error **errp) {};
> > > +
> > > +static int qatzip_send_prepare(MultiFDSendParams *p, Error **errp)
> > > +{
> > > +MultiFDPages_t *pages = p->pages;
> > > +
> > > +for (int i = 0; i < p->normal_num; i++) {
> > > +p->iov[p->iovs_num].iov_base = pages->block->host + p->normal[i];
> > > +p->iov[p->iovs_num].iov_len = p->page_size;
> > > +p->iovs_num++;
> > > +}
> > > +
> > > +p->next_packet_size = p->normal_num * p->page_size;
> > > +p->flags |= MULTIFD_FLAG_NOCOMP;
> > > +return 0;
> > > +}
> > > +
> > > +static int qatzip_recv_setup(MultiFDRecvParams *p, Error **errp)
> > > +{
> > > +return 0;
> > > +}
> > > +
> > > +static void qatzip_recv_cleanup(MultiFDRecvParams *p) {};
> > > +
> > > +static int qatzip_recv_pages(MultiFDRecvParams *p, Error **errp)
> > > +{
> > > +uint32_t flags = p->flags & MULTIFD_FLAG_COMPRESSION_MASK;
> > > +
> > > +if (flags != MULTIFD_FLAG_NOCOMP) {
> > > +error_setg(errp, "multifd %u: flags received %x flags expected
> > > %x",
> > > + p->id, flags, MULTIFD_FLAG_NOCOMP);
> > > +return -1;
> > > +}
> > > +for (int i = 0; i < p->normal_num; i++) {
> > > +p->iov[i].iov_base = p->host + p->normal[i];
> > > +p->iov[i].iov_len = p->page_size;
> > > +}
> > > +return qio_channel_readv_all(p->c, p->iov, p->normal_num, errp);
> > > +}
> > > +
> > > +static MultiFDMethods multifd_qatzip_ops = {
> > > +.send_setup = qatzip_send_setup,
> > > +.send_cleanup = qatzip_send_cleanup,
> > > +.send_prepare = qatzip_send_prepare,
> > > +.recv_setup = qatzip_recv_setup,
> > > +.recv_cleanup = qatzip_recv_cleanup,
> > > +.recv_pages =
Re: [External] Re: [QEMU-devel][RFC PATCH 1/1] backends/hostmem: qapi/qom: Add an ObjectOption for memory-backend-* called HostMemType and its arg 'cxlram'
On Wed, Jan 3, 2024 at 1:56 PM Gregory Price wrote: > > On Sun, Dec 31, 2023 at 11:53:15PM -0800, Ho-Ren (Jack) Chuang wrote: > > Introduce a new configuration option 'host-mem-type=' in the > > '-object memory-backend-ram', allowing users to specify > > from which type of memory to allocate. > > > > Users can specify 'cxlram' as an argument, and QEMU will then > > automatically locate CXL RAM NUMA nodes and use them as the backend memory. > > For example: > > -object memory-backend-ram,id=vmem0,size=19G,host-mem-type=cxlram \ > > Stupid questions: > > Why not just use `host-nodes` and pass in the numa node you want to > allocate from? Why should QEMU be made "CXL-aware" in the sense that > QEMU is responsible for figuring out what host node has CXL memory? > > This feels like an "upper level software" operation (orchestration), rather > than something qemu should internally understand. I don't have a "big picture" and I am learning. Maybe we proposed something not useful :-) I replied to the same question on a fork of this thread. > > > -device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1 \ > > -device cxl-rp,port=0,bus=cxl.1,id=root_port13,chassis=0,slot=2 \ > > -device cxl-type3,bus=root_port13,volatile-memdev=vmem0,id=cxl-vmem0 \ > > For a variety of performance reasons, this will not work the way you > want it to. You are essentially telling QEMU to map the vmem0 into a > virtual cxl device, and now any memory accesses to that memory region > will end up going through the cxl-type3 device logic - which is an IO > path from the perspective of QEMU. I didn't understand exactly how the virtual cxl-type3 device works. I thought it would go with the same "guest virtual address -> guest physical address -> host physical address" translation totally done by CPU. But if it is going through an emulation path handled by virtual cxl-type3, I agree the performance would be bad. Do you know why accessing memory on a virtual cxl-type3 device can't go with the nested page table translation? > > You may want to double check that your tests aren't using swap space in > the guest, because I found in my testing that linux would prefer to use > swap rather than attempt to use virtual cxl-type3 device memory because > of how god-awful slow it is (even if it is backed by DRAM). We didn't enable swap in our current test setup. I think there was a kernel change making the mm page reclamation path to use cxl memory instead of swap if you enable memory tiering. Did you try that? Swap is persistent storage. I would be very surprised if virtual cxl is actually slower than swap. > > > Additionally, this configuration will not (should not) presently work > with VMX enabled. Unless I missed some other update, QEMU w/ CXL memory > presently crashes when VMX is enabled for not-yet-debugged reasons. When we had a discussion with Intel, they told us to not use the KVM option in QEMU while using virtual cxl type3 device. That's probably related to the issue you described here? We enabled KVM though but haven't seen the crash yet. > > Another possiblity: You mapped this memory-backend into another numa > node explicitly and never onlined the memory via cxlcli. I've done > this, and it works, but it's a "hidden feature" that probably should > not exist / be supported. I thought general purpose memory nodes are onlined by default? > > > > If I understand the goal here, it's to pass CXL-hosted DRAM through to > the guest in a way that the system can manage it according to its > performance attributes. Yes. > > You're better off just using the `host-nodes` field of host-memory > and passing bandwidth/latency attributes though via `-numa hmat-lb` We tried this but it doesn't work from end to end right now. I described the issue in another fork of this thread. > > In that scenario, the guest software doesn't even need to know CXL > exists at all, it can just read the attributes of the numa node > that QEMU created for it. We thought about this before. But the current kernel implementation requires a devdax device to be probed and recognized as a slow tier (by reading the memory attributes). I don't think this can be done via the path you described. Have you tried this before? > > In the future to deal with proper dynamic capacity, we may need to > consider a different backend object altogether that allows sparse > allocations, and a virtual cxl device which pre-allocates the CFMW > can at least be registered to manage it. I'm not quite sure how that > looks just yet. Are we talking about CXL memory pooling? > > For example: 1-socket, 4 CPU QEMU instance w/ 4GB on a cpu-node and 4GB > on a cpuless node. > > qemu-system-x86_64 \ > -nographic \ > -accel kvm \ > -machine type=q35,hmat=on \ > -drive file=./myvm.qcow2,format=qcow2,index=0,media=disk,id=hd \ > -m 8G,slots=4,maxmem=16G \ > -smp cpus=4 \ > -object memory-backend-ram,size=4G,id=ram-node0,numa=X \ <-- extend here > -object
Re: acpiBitsTest.test_acpi_smbios_bits test intermittently times out
On Sat, Jan 6, 2024 at 10:05 AM Ani Sinha wrote: > > On Sat, Jan 6, 2024 at 12:11 AM Peter Maydell > wrote: > > > > The avocado test acpiBitsTest.test_acpi_smbios_bits seems to be > > flaky in CI -- sometimes it appears to time out. > > > > https://gitlab.com/qemu-project/qemu/-/issues/2077 > > has the details (including links to jobs etc). > > Do you have more data points in terms of the jobs that failed? I just noticed that you attached three examples of failed tests. In all of them the test seems to be stuck at the ami latency test. The one > you point to seems to be stuck in SMI latency tests: > > SMI latency test > 16:45:49 DEBUG| > Warning: touching the keyboard can affect the results of this test. > 16:45:50 DEBUG| > Starting test. Wait here, I will be back in 15 seconds. > > If this is consistently timing out here, we can disable the smi > latency test. This test was a little problematic from the get go. > > Meanwhile I have assigned > https://gitlab.com/qemu-project/qemu/-/issues/2077 to myself and will > take a look. > > > As far as I can > > see, the test is still running when after about a minute it > > gets timed out. (Though the python tracebacks in the logs are > > not easy for me to interpret, so I might be wrong). This I find > > a bit confusing, because tests/avocado/acpi-bits.py sets > > "timeout = 200". So maybe that isn't taking effect properly? > > > > Does anybody have time to investigate this? If not, we can disable > > the test as flaky until somebody does have the time. > > > > thanks > > -- PMM > >
Re: acpiBitsTest.test_acpi_smbios_bits test intermittently times out
On Sat, Jan 6, 2024 at 12:11 AM Peter Maydell wrote: > > The avocado test acpiBitsTest.test_acpi_smbios_bits seems to be > flaky in CI -- sometimes it appears to time out. > > https://gitlab.com/qemu-project/qemu/-/issues/2077 > has the details (including links to jobs etc). Do you have more data points in terms of the jobs that failed? The one you point to seems to be stuck in SMI latency tests: SMI latency test 16:45:49 DEBUG| Warning: touching the keyboard can affect the results of this test. 16:45:50 DEBUG| Starting test. Wait here, I will be back in 15 seconds. If this is consistently timing out here, we can disable the smi latency test. This test was a little problematic from the get go. Meanwhile I have assigned https://gitlab.com/qemu-project/qemu/-/issues/2077 to myself and will take a look. > As far as I can > see, the test is still running when after about a minute it > gets timed out. (Though the python tracebacks in the logs are > not easy for me to interpret, so I might be wrong). This I find > a bit confusing, because tests/avocado/acpi-bits.py sets > "timeout = 200". So maybe that isn't taking effect properly? > > Does anybody have time to investigate this? If not, we can disable > the test as flaky until somebody does have the time. > > thanks > -- PMM >
[PULL v2 0/2] loongarch-to-apply queue
The following changes since commit 0c1eccd368af8805ec0fb11e6cf25d0684d37328:
Merge tag 'hw-cpus-20240105' of https://github.com/philmd/qemu into staging
(2024-01-05 16:08:58 +)
are available in the Git repository at:
https://gitlab.com/gaosong/qemu.git tags/pull-loongarch-20240106
for you to fetch changes up to 5c23704e4725f935b3171787f00e9922a7fc58cb:
target/loongarch: move translate modules to tcg/ (2024-01-06 10:18:52 +0800)
pull-loongarch-20240106
Fixes patch conflict
Song Gao (2):
target/loongarch/meson: move gdbstub.c to loongarch.ss
target/loongarch: move translate modules to tcg/
target/loongarch/meson.build | 15 +--
target/loongarch/{ => tcg}/constant_timer.c | 0
target/loongarch/{ => tcg}/csr_helper.c | 0
target/loongarch/{ => tcg}/fpu_helper.c | 0
.../loongarch/{ => tcg}/insn_trans/trans_arith.c.inc | 0
.../loongarch/{ => tcg}/insn_trans/trans_atomic.c.inc | 0
target/loongarch/{ => tcg}/insn_trans/trans_bit.c.inc | 0
.../loongarch/{ => tcg}/insn_trans/trans_branch.c.inc | 0
.../loongarch/{ => tcg}/insn_trans/trans_extra.c.inc | 0
.../loongarch/{ => tcg}/insn_trans/trans_farith.c.inc | 0
.../loongarch/{ => tcg}/insn_trans/trans_fcmp.c.inc | 0
.../loongarch/{ => tcg}/insn_trans/trans_fcnv.c.inc | 0
.../{ => tcg}/insn_trans/trans_fmemory.c.inc | 0
.../loongarch/{ => tcg}/insn_trans/trans_fmov.c.inc | 0
.../loongarch/{ => tcg}/insn_trans/trans_memory.c.inc | 0
.../{ => tcg}/insn_trans/trans_privileged.c.inc | 0
.../loongarch/{ => tcg}/insn_trans/trans_shift.c.inc | 0
target/loongarch/{ => tcg}/insn_trans/trans_vec.c.inc | 0
target/loongarch/{ => tcg}/iocsr_helper.c | 0
target/loongarch/tcg/meson.build | 19 +++
target/loongarch/{ => tcg}/op_helper.c| 0
target/loongarch/{ => tcg}/tlb_helper.c | 0
target/loongarch/{ => tcg}/translate.c| 0
target/loongarch/{ => tcg}/vec_helper.c | 0
24 files changed, 20 insertions(+), 14 deletions(-)
rename target/loongarch/{ => tcg}/constant_timer.c (100%)
rename target/loongarch/{ => tcg}/csr_helper.c (100%)
rename target/loongarch/{ => tcg}/fpu_helper.c (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_arith.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_atomic.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_bit.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_branch.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_extra.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_farith.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_fcmp.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_fcnv.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_fmemory.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_fmov.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_memory.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_privileged.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_shift.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_vec.c.inc (100%)
rename target/loongarch/{ => tcg}/iocsr_helper.c (100%)
create mode 100644 target/loongarch/tcg/meson.build
rename target/loongarch/{ => tcg}/op_helper.c (100%)
rename target/loongarch/{ => tcg}/tlb_helper.c (100%)
rename target/loongarch/{ => tcg}/translate.c (100%)
rename target/loongarch/{ => tcg}/vec_helper.c (100%)
[PULL v2 1/2] target/loongarch/meson: move gdbstub.c to loongarch.ss
gdbstub.c is not specific to TCG and can be used by
other accelerators, such as KVM accelerator
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Song Gao
Message-Id: <20240102020200.3462097-1-gaos...@loongson.cn>
---
target/loongarch/meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/loongarch/meson.build b/target/loongarch/meson.build
index 18e8191e2b..b3a0fb12fb 100644
--- a/target/loongarch/meson.build
+++ b/target/loongarch/meson.build
@@ -3,6 +3,7 @@ gen = decodetree.process('insns.decode')
loongarch_ss = ss.source_set()
loongarch_ss.add(files(
'cpu.c',
+ 'gdbstub.c',
))
loongarch_tcg_ss = ss.source_set()
loongarch_tcg_ss.add(gen)
@@ -10,7 +11,6 @@ loongarch_tcg_ss.add(files(
'fpu_helper.c',
'op_helper.c',
'translate.c',
- 'gdbstub.c',
'vec_helper.c',
))
loongarch_tcg_ss.add(zlib)
--
2.25.1
[PULL v2 2/2] target/loongarch: move translate modules to tcg/
Introduce the target/loongarch/tcg directory. Its purpose is to hold the TCG
code that is selected by CONFIG_TCG
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Song Gao
Message-Id: <20240102020200.3462097-2-gaos...@loongson.cn>
---
target/loongarch/meson.build | 15 +--
target/loongarch/{ => tcg}/constant_timer.c | 0
target/loongarch/{ => tcg}/csr_helper.c | 0
target/loongarch/{ => tcg}/fpu_helper.c | 0
.../{ => tcg}/insn_trans/trans_arith.c.inc| 0
.../{ => tcg}/insn_trans/trans_atomic.c.inc | 0
.../{ => tcg}/insn_trans/trans_bit.c.inc | 0
.../{ => tcg}/insn_trans/trans_branch.c.inc | 0
.../{ => tcg}/insn_trans/trans_extra.c.inc| 0
.../{ => tcg}/insn_trans/trans_farith.c.inc | 0
.../{ => tcg}/insn_trans/trans_fcmp.c.inc | 0
.../{ => tcg}/insn_trans/trans_fcnv.c.inc | 0
.../{ => tcg}/insn_trans/trans_fmemory.c.inc | 0
.../{ => tcg}/insn_trans/trans_fmov.c.inc | 0
.../{ => tcg}/insn_trans/trans_memory.c.inc | 0
.../insn_trans/trans_privileged.c.inc | 0
.../{ => tcg}/insn_trans/trans_shift.c.inc| 0
.../{ => tcg}/insn_trans/trans_vec.c.inc | 0
target/loongarch/{ => tcg}/iocsr_helper.c | 0
target/loongarch/tcg/meson.build | 19 +++
target/loongarch/{ => tcg}/op_helper.c| 0
target/loongarch/{ => tcg}/tlb_helper.c | 0
target/loongarch/{ => tcg}/translate.c| 0
target/loongarch/{ => tcg}/vec_helper.c | 0
24 files changed, 20 insertions(+), 14 deletions(-)
rename target/loongarch/{ => tcg}/constant_timer.c (100%)
rename target/loongarch/{ => tcg}/csr_helper.c (100%)
rename target/loongarch/{ => tcg}/fpu_helper.c (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_arith.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_atomic.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_bit.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_branch.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_extra.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_farith.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_fcmp.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_fcnv.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_fmemory.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_fmov.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_memory.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_privileged.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_shift.c.inc (100%)
rename target/loongarch/{ => tcg}/insn_trans/trans_vec.c.inc (100%)
rename target/loongarch/{ => tcg}/iocsr_helper.c (100%)
create mode 100644 target/loongarch/tcg/meson.build
rename target/loongarch/{ => tcg}/op_helper.c (100%)
rename target/loongarch/{ => tcg}/tlb_helper.c (100%)
rename target/loongarch/{ => tcg}/translate.c (100%)
rename target/loongarch/{ => tcg}/vec_helper.c (100%)
diff --git a/target/loongarch/meson.build b/target/loongarch/meson.build
index b3a0fb12fb..e84e4c51f4 100644
--- a/target/loongarch/meson.build
+++ b/target/loongarch/meson.build
@@ -5,29 +5,16 @@ loongarch_ss.add(files(
'cpu.c',
'gdbstub.c',
))
-loongarch_tcg_ss = ss.source_set()
-loongarch_tcg_ss.add(gen)
-loongarch_tcg_ss.add(files(
- 'fpu_helper.c',
- 'op_helper.c',
- 'translate.c',
- 'vec_helper.c',
-))
-loongarch_tcg_ss.add(zlib)
loongarch_system_ss = ss.source_set()
loongarch_system_ss.add(files(
'loongarch-qmp-cmds.c',
'machine.c',
- 'tlb_helper.c',
- 'constant_timer.c',
- 'csr_helper.c',
- 'iocsr_helper.c',
))
common_ss.add(when: 'CONFIG_LOONGARCH_DIS', if_true: [files('disas.c'), gen])
-loongarch_ss.add_all(when: 'CONFIG_TCG', if_true: [loongarch_tcg_ss])
+subdir('tcg')
target_arch += {'loongarch': loongarch_ss}
target_system_arch += {'loongarch': loongarch_system_ss}
diff --git a/target/loongarch/constant_timer.c
b/target/loongarch/tcg/constant_timer.c
similarity index 100%
rename from target/loongarch/constant_timer.c
rename to target/loongarch/tcg/constant_timer.c
diff --git a/target/loongarch/csr_helper.c b/target/loongarch/tcg/csr_helper.c
similarity index 100%
rename from target/loongarch/csr_helper.c
rename to target/loongarch/tcg/csr_helper.c
diff --git a/target/loongarch/fpu_helper.c b/target/loongarch/tcg/fpu_helper.c
similarity index 100%
rename from target/loongarch/fpu_helper.c
rename to target/loongarch/tcg/fpu_helper.c
diff --git a/target/loongarch/insn_trans/trans_arith.c.inc
b/target/loongarch/tcg/insn_trans/trans_arith.c.inc
similarity index 100%
rename from target/loongarch/insn_trans/trans_arith.c.inc
rename to target/loongarch/tcg/insn_trans/trans_arith.c.inc
diff --git a/target/loongarch/insn_trans/trans_atomic.c.inc
b/target/loongarch/tcg/insn_trans/trans_atomic.c.inc
similarity index 100%
rename from
Re: [PULL 0/2] loongarch-to-apply queue
在 2024/1/5 下午9:34, Peter Maydell 写道: On Fri, 5 Jan 2024 at 01:30, Song Gao wrote: The following changes since commit d328fef93ae757a0dd65ed786a4086e27952eef3: Merge tag 'pull-20231230' of https://gitlab.com/rth7680/qemu into staging (2024-01-04 10:23:34 +) are available in the Git repository at: https://gitlab.com/gaosong/qemu.git tags/pull-loongarch-20240105 for you to fetch changes up to 0cd8b379081fa71c23836052feb65da4685f8ec7: target/loongarch: move translate modules to tcg/ (2024-01-05 09:31:05 +0800) pull-loongarch-20240105 Song Gao (2): target/loongarch/meson: move gdbstub.c to loongarch.ss target/loongarch: move translate modules to tcg/ Hi; this fails to build, with ../target/loongarch/tcg/meson.build:1:3: ERROR: Unknown variable "config_all". (eg https://gitlab.com/qemu-project/qemu/-/jobs/5868662017) I think your pullreq has unfortunately got a conflict with the meson cleanup patches that I just applied from Paolo. Could you have a look at this and respin the pullreq, please? Sure, I will. Thanks. Song Gao.
Re: [External] Re: [QEMU-devel][RFC PATCH 1/1] backends/hostmem: qapi/qom: Add an ObjectOption for memory-backend-* called HostMemType and its arg 'cxlram'
On Tue, Jan 2, 2024 at 5:04 AM David Hildenbrand wrote: > > On 01.01.24 08:53, Ho-Ren (Jack) Chuang wrote: > > Introduce a new configuration option 'host-mem-type=' in the > > '-object memory-backend-ram', allowing users to specify > > from which type of memory to allocate. > > > > Users can specify 'cxlram' as an argument, and QEMU will then > > automatically locate CXL RAM NUMA nodes and use them as the backend memory. > > For example: > > -object memory-backend-ram,id=vmem0,size=19G,host-mem-type=cxlram \ > > -device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1 \ > > -device cxl-rp,port=0,bus=cxl.1,id=root_port13,chassis=0,slot=2 \ > > -device cxl-type3,bus=root_port13,volatile-memdev=vmem0,id=cxl-vmem0 \ > > -M > > cxl-fmw.0.targets.0=cxl.1,cxl-fmw.0.size=19G,cxl-fmw.0.interleave-granularity=8k > > \ > > > > You can achieve the exact same thing already simply by using memory > policies and detecting the node(s) before calling QEMU, no? Yes, I agree this can be done with memory policy and bind to the CXL memory numa nodes on host. > > There has to be a good reason to add such a shortcut into QEMU, and it > should be spelled out here. So our end goal here is to enable CXL memory in the guest VM and have the guest kernel to recognize the CXL memory to the correct memory tier (slow tier) in the Linux kernel tiered memory system. Here is what we observed: * The kernel tiered memory system relies on calculating the memory attributes (read/write latency, bandwidth from ACPI) for fast vs slow tier. * The kernel tiered memory system has two path to recognize a memory tier 1) in mm subsystem init, memory_tier_init 2) in kmem driver device probe dev_dax_kmem_probe. Since the ACPI subsystem is initialized after mm, reading the memory attributes from ACPI can only be done in 2). CXL memory has to be presented as a devdax device, which can then be probed by the kmem driver in the guest and recognized as the slow tier. We do see that QEMU has this option "-numa hmat-lb" to set the memory attributes per VM's numa node. The problem is that setting the memory attributes per numa node means that the numa node is created during guest kernel initialization. A CXL devdax device can only be created post kernel initialization and new numa nodes are created for the CXL devdax devices. The guest kernel is not reading the memory attributes from "-numa hmat-lb" for the CXL devdax devices. So we thought if we create an explicit CXL memory backend, and associate that with the virtual CXL type-3 frontend, we can pass the CXL memory attributes from the host into the guest VM and avoid using memory policy and "-numa hmat-lb", thus simplifying the configuration. We are still figuring out exactly how to pass the memory attributes from the CXL backend into the VM. There is probably a solution to "-numa hmat-lb" for the CXL devdax devices as well and we are also looking into it. > > -- > Cheers, > > David / dhildenb >
Re: [External] Re: [PATCH 3/5] migration: Introduce unimplemented 'qatzip' compression method
On Fri, Jan 5, 2024 at 12:07 PM Fabiano Rosas wrote:
>
> Bryan Zhang writes:
>
> +cc Yuan Liu, Daniel Berrangé
>
> > Adds support for 'qatzip' as an option for the multifd compression
> > method parameter, but copy-pastes the no-op logic to leave the actual
> > methods effectively unimplemented. This is in preparation of a
> > subsequent commit that will implement actually using QAT for compression
> > and decompression.
> >
> > Signed-off-by: Bryan Zhang
> > Signed-off-by: Hao Xiang
> > ---
> > hw/core/qdev-properties-system.c | 6 ++-
> > migration/meson.build| 1 +
> > migration/multifd-qatzip.c | 81
> > migration/multifd.h | 1 +
> > qapi/migration.json | 5 +-
> > 5 files changed, 92 insertions(+), 2 deletions(-)
> > create mode 100644 migration/multifd-qatzip.c
> >
> > diff --git a/hw/core/qdev-properties-system.c
> > b/hw/core/qdev-properties-system.c
> > index 1a396521d5..d8e48dcb0e 100644
> > --- a/hw/core/qdev-properties-system.c
> > +++ b/hw/core/qdev-properties-system.c
> > @@ -658,7 +658,11 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
> > const PropertyInfo qdev_prop_multifd_compression = {
> > .name = "MultiFDCompression",
> > .description = "multifd_compression values, "
> > - "none/zlib/zstd",
> > + "none/zlib/zstd"
> > +#ifdef CONFIG_QATZIP
> > + "/qatzip"
> > +#endif
> > + ,
> > .enum_table = _lookup,
> > .get = qdev_propinfo_get_enum,
> > .set = qdev_propinfo_set_enum,
> > diff --git a/migration/meson.build b/migration/meson.build
> > index 92b1cc4297..e20f318379 100644
> > --- a/migration/meson.build
> > +++ b/migration/meson.build
> > @@ -40,6 +40,7 @@ if get_option('live_block_migration').allowed()
> >system_ss.add(files('block.c'))
> > endif
> > system_ss.add(when: zstd, if_true: files('multifd-zstd.c'))
> > +system_ss.add(when: qatzip, if_true: files('multifd-qatzip.c'))
> >
> > specific_ss.add(when: 'CONFIG_SYSTEM_ONLY',
> > if_true: files('ram.c',
> > diff --git a/migration/multifd-qatzip.c b/migration/multifd-qatzip.c
> > new file mode 100644
> > index 00..1733bbddb7
> > --- /dev/null
> > +++ b/migration/multifd-qatzip.c
> > @@ -0,0 +1,81 @@
> > +/*
> > + * Multifd QATzip compression implementation
> > + *
> > + * Copyright (c) Bytedance
> > + *
> > + * Authors:
> > + * Bryan Zhang
> > + * Hao Xiang
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "exec/ramblock.h"
> > +#include "exec/target_page.h"
> > +#include "qapi/error.h"
> > +#include "migration.h"
> > +#include "options.h"
> > +#include "multifd.h"
> > +
> > +static int qatzip_send_setup(MultiFDSendParams *p, Error **errp)
> > +{
> > +return 0;
> > +}
> > +
> > +static void qatzip_send_cleanup(MultiFDSendParams *p, Error **errp) {};
> > +
> > +static int qatzip_send_prepare(MultiFDSendParams *p, Error **errp)
> > +{
> > +MultiFDPages_t *pages = p->pages;
> > +
> > +for (int i = 0; i < p->normal_num; i++) {
> > +p->iov[p->iovs_num].iov_base = pages->block->host + p->normal[i];
> > +p->iov[p->iovs_num].iov_len = p->page_size;
> > +p->iovs_num++;
> > +}
> > +
> > +p->next_packet_size = p->normal_num * p->page_size;
> > +p->flags |= MULTIFD_FLAG_NOCOMP;
> > +return 0;
> > +}
> > +
> > +static int qatzip_recv_setup(MultiFDRecvParams *p, Error **errp)
> > +{
> > +return 0;
> > +}
> > +
> > +static void qatzip_recv_cleanup(MultiFDRecvParams *p) {};
> > +
> > +static int qatzip_recv_pages(MultiFDRecvParams *p, Error **errp)
> > +{
> > +uint32_t flags = p->flags & MULTIFD_FLAG_COMPRESSION_MASK;
> > +
> > +if (flags != MULTIFD_FLAG_NOCOMP) {
> > +error_setg(errp, "multifd %u: flags received %x flags expected %x",
> > + p->id, flags, MULTIFD_FLAG_NOCOMP);
> > +return -1;
> > +}
> > +for (int i = 0; i < p->normal_num; i++) {
> > +p->iov[i].iov_base = p->host + p->normal[i];
> > +p->iov[i].iov_len = p->page_size;
> > +}
> > +return qio_channel_readv_all(p->c, p->iov, p->normal_num, errp);
> > +}
> > +
> > +static MultiFDMethods multifd_qatzip_ops = {
> > +.send_setup = qatzip_send_setup,
> > +.send_cleanup = qatzip_send_cleanup,
> > +.send_prepare = qatzip_send_prepare,
> > +.recv_setup = qatzip_recv_setup,
> > +.recv_cleanup = qatzip_recv_cleanup,
> > +.recv_pages = qatzip_recv_pages
> > +};
> > +
> > +static void multifd_qatzip_register(void)
> > +{
> > +multifd_register_ops(MULTIFD_COMPRESSION_QATZIP, _qatzip_ops);
> > +}
> > +
> > +migration_init(multifd_qatzip_register);
> > diff --git a/migration/multifd.h b/migration/multifd.h
> > index a835643b48..5600f7fc82 100644
> > ---
[PATCH v4 02/17] target/riscv: make riscv_cpu_is_vendor() public
We'll use this function in target/riscv/cpu.c to implement setters that
won't allow vendor CPU options to be changed.
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
---
target/riscv/cpu.c | 5 +
target/riscv/cpu.h | 1 +
target/riscv/tcg/tcg-cpu.c | 5 -
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cce8165d69..025c6cb23c 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -190,6 +190,11 @@ void isa_ext_update_enabled(RISCVCPU *cpu, uint32_t
ext_offset, bool en)
*ext_enabled = en;
}
+bool riscv_cpu_is_vendor(Object *cpu_obj)
+{
+return object_dynamic_cast(cpu_obj, TYPE_RISCV_VENDOR_CPU) != NULL;
+}
+
const char * const riscv_int_regnames[] = {
"x0/zero", "x1/ra", "x2/sp", "x3/gp", "x4/tp", "x5/t0", "x6/t1",
"x7/t2", "x8/s0", "x9/s1", "x10/a0", "x11/a1", "x12/a2", "x13/a3",
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 1c5ab8bd0e..1ece80a604 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -773,6 +773,7 @@ enum riscv_pmu_event_idx {
void isa_ext_update_enabled(RISCVCPU *cpu, uint32_t ext_offset, bool en);
bool isa_ext_is_enabled(RISCVCPU *cpu, uint32_t ext_offset);
void riscv_cpu_set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext);
+bool riscv_cpu_is_vendor(Object *cpu_obj);
typedef struct RISCVCPUMultiExtConfig {
const char *name;
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 14133ff665..2c2ce51b19 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -917,11 +917,6 @@ static bool riscv_cpu_is_generic(Object *cpu_obj)
return object_dynamic_cast(cpu_obj, TYPE_RISCV_DYNAMIC_CPU) != NULL;
}
-static bool riscv_cpu_is_vendor(Object *cpu_obj)
-{
-return object_dynamic_cast(cpu_obj, TYPE_RISCV_VENDOR_CPU) != NULL;
-}
-
/*
* We'll get here via the following path:
*
--
2.43.0
[PATCH v4 15/17] target/riscv/cpu.c: move 'mvendorid' to riscv_cpu_properties[]
Keep all class properties in riscv_cpu_properties[].
Signed-off-by: Daniel Henrique Barboza
---
target/riscv/cpu.c | 69 +-
1 file changed, 37 insertions(+), 32 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 9d4243891c..c725a4839d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1974,6 +1974,41 @@ static const PropertyInfo prop_cboz_blksize = {
.set = prop_cboz_blksize_set,
};
+static void prop_mvendorid_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+bool dynamic_cpu = riscv_cpu_is_dynamic(obj);
+RISCVCPU *cpu = RISCV_CPU(obj);
+uint32_t prev_val = cpu->cfg.mvendorid;
+uint32_t value;
+
+if (!visit_type_uint32(v, name, , errp)) {
+return;
+}
+
+if (!dynamic_cpu && prev_val != value) {
+error_setg(errp, "Unable to change %s mvendorid (0x%x)",
+ object_get_typename(obj), prev_val);
+return;
+}
+
+cpu->cfg.mvendorid = value;
+}
+
+static void prop_mvendorid_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+uint32_t value = RISCV_CPU(obj)->cfg.mvendorid;
+
+visit_type_uint32(v, name, , errp);
+}
+
+static const PropertyInfo prop_mvendorid = {
+.name = "mvendorid",
+.get = prop_mvendorid_get,
+.set = prop_mvendorid_set,
+};
+
/*
* RVA22U64 defines some 'named features' or 'synthetic extensions'
* that are cache related: Za64rs, Zic64b, Ziccif, Ziccrse, Ziccamoa
@@ -2060,6 +2095,8 @@ static Property riscv_cpu_properties[] = {
{.name = "cbop_blocksize", .info = _cbop_blksize},
{.name = "cboz_blocksize", .info = _cboz_blksize},
+ {.name = "mvendorid", .info = _mvendorid},
+
#ifndef CONFIG_USER_ONLY
DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
#endif
@@ -2140,35 +2177,6 @@ static const struct SysemuCPUOps riscv_sysemu_ops = {
};
#endif
-static void cpu_set_mvendorid(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp)
-{
-bool dynamic_cpu = riscv_cpu_is_dynamic(obj);
-RISCVCPU *cpu = RISCV_CPU(obj);
-uint32_t prev_val = cpu->cfg.mvendorid;
-uint32_t value;
-
-if (!visit_type_uint32(v, name, , errp)) {
-return;
-}
-
-if (!dynamic_cpu && prev_val != value) {
-error_setg(errp, "Unable to change %s mvendorid (0x%x)",
- object_get_typename(obj), prev_val);
-return;
-}
-
-cpu->cfg.mvendorid = value;
-}
-
-static void cpu_get_mvendorid(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp)
-{
-uint32_t value = RISCV_CPU(obj)->cfg.mvendorid;
-
-visit_type_uint32(v, name, , errp);
-}
-
static void cpu_set_mimpid(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
@@ -2278,9 +2286,6 @@ static void riscv_cpu_class_init(ObjectClass *c, void
*data)
cc->gdb_arch_name = riscv_gdb_arch_name;
cc->gdb_get_dynamic_xml = riscv_gdb_get_dynamic_xml;
-object_class_property_add(c, "mvendorid", "uint32", cpu_get_mvendorid,
- cpu_set_mvendorid, NULL, NULL);
-
object_class_property_add(c, "mimpid", "uint64", cpu_get_mimpid,
cpu_set_mimpid, NULL, NULL);
--
2.43.0
[PATCH v4 01/17] target/riscv/cpu_cfg.h: remove unused fields
user_spec, bext_spec and bext_ver aren't being used.
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
---
target/riscv/cpu.h | 1 -
target/riscv/cpu_cfg.h | 2 --
2 files changed, 3 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 44fb0a9ca8..1c5ab8bd0e 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -180,7 +180,6 @@ struct CPUArchState {
target_ulong guest_phys_fault_addr;
target_ulong priv_ver;
-target_ulong bext_ver;
target_ulong vext_ver;
/* RISCVMXL, but uint32_t for vmstate migration */
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 780ae6ef17..0612668144 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -140,8 +140,6 @@ struct RISCVCPUConfig {
uint32_t pmu_mask;
char *priv_spec;
-char *user_spec;
-char *bext_spec;
char *vext_spec;
uint16_t vlen;
uint16_t elen;
--
2.43.0
[PATCH v4 13/17] target/riscv: move 'cboz_blocksize' to riscv_cpu_properties[]
And remove the now unused kvm_cpu_set_cbomz_blksize() setter.
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
---
target/riscv/cpu.c | 38 +-
target/riscv/kvm/kvm-cpu.c | 28
2 files changed, 37 insertions(+), 29 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index e3cbe9b1b6..f84c3fc4a2 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1318,6 +1318,7 @@ static void riscv_cpu_init(Object *obj)
cpu->cfg.elen = 64;
cpu->cfg.cbom_blocksize = 64;
cpu->cfg.cbop_blocksize = 64;
+cpu->cfg.cboz_blocksize = 64;
cpu->env.vext_ver = VEXT_VERSION_1_00_0;
}
@@ -1938,8 +1939,42 @@ static const PropertyInfo prop_cbop_blksize = {
.set = prop_cbop_blksize_set,
};
+static void prop_cboz_blksize_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+RISCVCPU *cpu = RISCV_CPU(obj);
+uint16_t value;
+
+if (!visit_type_uint16(v, name, , errp)) {
+return;
+}
+
+if (value != cpu->cfg.cboz_blocksize && riscv_cpu_is_vendor(obj)) {
+cpu_set_prop_err(cpu, name, errp);
+error_append_hint(errp, "Current '%s' val: %u\n",
+ name, cpu->cfg.cboz_blocksize);
+return;
+}
+
+cpu_option_add_user_setting(name, value);
+cpu->cfg.cboz_blocksize = value;
+}
+
+static void prop_cboz_blksize_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+uint16_t value = RISCV_CPU(obj)->cfg.cboz_blocksize;
+
+visit_type_uint16(v, name, , errp);
+}
+
+static const PropertyInfo prop_cboz_blksize = {
+.name = "cboz_blocksize",
+.get = prop_cboz_blksize_get,
+.set = prop_cboz_blksize_set,
+};
+
Property riscv_cpu_options[] = {
-DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
DEFINE_PROP_END_OF_LIST(),
};
@@ -2028,6 +2063,7 @@ static Property riscv_cpu_properties[] = {
{.name = "cbom_blocksize", .info = _cbom_blksize},
{.name = "cbop_blocksize", .info = _cbop_blksize},
+{.name = "cboz_blocksize", .info = _cboz_blksize},
#ifndef CONFIG_USER_ONLY
DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 9a6a007931..c9b4a6a7e8 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -343,30 +343,6 @@ static KVMCPUConfig kvm_cboz_blocksize = {
.kvm_reg_id = KVM_REG_RISCV_CONFIG_REG(zicboz_block_size)
};
-static void kvm_cpu_set_cbomz_blksize(Object *obj, Visitor *v,
- const char *name,
- void *opaque, Error **errp)
-{
-KVMCPUConfig *cbomz_cfg = opaque;
-RISCVCPU *cpu = RISCV_CPU(obj);
-uint16_t value, *host_val;
-
-if (!visit_type_uint16(v, name, , errp)) {
-return;
-}
-
-host_val = kvmconfig_get_cfg_addr(cpu, cbomz_cfg);
-
-if (value != *host_val) {
-error_report("Unable to set %s to a different value than "
- "the host (%u)",
- cbomz_cfg->name, *host_val);
-exit(EXIT_FAILURE);
-}
-
-cbomz_cfg->user_set = true;
-}
-
static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
{
CPURISCVState *env = >env;
@@ -484,10 +460,6 @@ static void kvm_riscv_add_cpu_user_properties(Object
*cpu_obj)
NULL, multi_cfg);
}
-object_property_add(cpu_obj, "cboz_blocksize", "uint16",
-NULL, kvm_cpu_set_cbomz_blksize,
-NULL, _cboz_blocksize);
-
riscv_cpu_add_kvm_unavail_prop_array(cpu_obj, riscv_cpu_extensions);
riscv_cpu_add_kvm_unavail_prop_array(cpu_obj, riscv_cpu_vendor_exts);
riscv_cpu_add_kvm_unavail_prop_array(cpu_obj, riscv_cpu_experimental_exts);
--
2.43.0
[PATCH v4 11/17] target/riscv: move 'cbom_blocksize' to riscv_cpu_properties[]
After adding a KVM finalize() implementation, turn cbom_blocksize into a
class property. Follow the same design we used with 'vlen' and 'elen'.
The duplicated 'cbom_blocksize' KVM property can be removed from
kvm_riscv_add_cpu_user_properties().
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
---
target/riscv/cpu.c | 39 +-
target/riscv/kvm/kvm-cpu.c | 4
2 files changed, 38 insertions(+), 5 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cd91966ea7..b77d26231c 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1316,6 +1316,7 @@ static void riscv_cpu_init(Object *obj)
cpu->cfg.pmu_mask = MAKE_64BIT_MASK(3, 16);
cpu->cfg.vlen = 128;
cpu->cfg.elen = 64;
+cpu->cfg.cbom_blocksize = 64;
cpu->env.vext_ver = VEXT_VERSION_1_00_0;
}
@@ -1866,8 +1867,42 @@ static const PropertyInfo prop_elen = {
.set = prop_elen_set,
};
+static void prop_cbom_blksize_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+RISCVCPU *cpu = RISCV_CPU(obj);
+uint16_t value;
+
+if (!visit_type_uint16(v, name, , errp)) {
+return;
+}
+
+if (value != cpu->cfg.cbom_blocksize && riscv_cpu_is_vendor(obj)) {
+cpu_set_prop_err(cpu, name, errp);
+error_append_hint(errp, "Current '%s' val: %u\n",
+ name, cpu->cfg.cbom_blocksize);
+return;
+}
+
+cpu_option_add_user_setting(name, value);
+cpu->cfg.cbom_blocksize = value;
+}
+
+static void prop_cbom_blksize_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+uint16_t value = RISCV_CPU(obj)->cfg.cbom_blocksize;
+
+visit_type_uint16(v, name, , errp);
+}
+
+static const PropertyInfo prop_cbom_blksize = {
+.name = "cbom_blocksize",
+.get = prop_cbom_blksize_get,
+.set = prop_cbom_blksize_set,
+};
+
Property riscv_cpu_options[] = {
-DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64),
DEFINE_PROP_UINT16("cbop_blocksize", RISCVCPU, cfg.cbop_blocksize, 64),
DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
@@ -1956,6 +1991,8 @@ static Property riscv_cpu_properties[] = {
{.name = "vlen", .info = _vlen},
{.name = "elen", .info = _elen},
+{.name = "cbom_blocksize", .info = _cbom_blksize},
+
#ifndef CONFIG_USER_ONLY
DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
#endif
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 2713f4b2ba..9a6a007931 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -484,10 +484,6 @@ static void kvm_riscv_add_cpu_user_properties(Object
*cpu_obj)
NULL, multi_cfg);
}
-object_property_add(cpu_obj, "cbom_blocksize", "uint16",
-NULL, kvm_cpu_set_cbomz_blksize,
-NULL, _cbom_blocksize);
-
object_property_add(cpu_obj, "cboz_blocksize", "uint16",
NULL, kvm_cpu_set_cbomz_blksize,
NULL, _cboz_blocksize);
--
2.43.0
[PATCH v4 10/17] target/riscv: create finalize_features() for KVM
To turn cbom_blocksize and cboz_blocksize into class properties we need
KVM specific changes.
KVM is creating its own version of these options with a customized
setter() that prevents users from picking an invalid value during init()
time. This comes at the cost of duplicating each option that KVM
supports. This will keep happening for each new shared option KVM
implements in the future.
We can avoid that by using the same property TCG uses and adding
specific KVM handling during finalize() time, like TCG already does with
riscv_tcg_cpu_finalize_features(). To do that, the common CPU property
offers a way of knowing if an option was user set or not, sparing us
from doing unneeded syscalls.
riscv_kvm_cpu_finalize_features() is then created using the same
KVMScratch CPU we already use during init() time, since finalize() time
is still too early to use the official KVM CPU for it. cbom_blocksize
and cboz_blocksize are then handled during finalize() in the same way
they're handled by their KVM specific setter.
With this change we can proceed with the blocksize changes in the common
code without breaking the KVM driver.
Signed-off-by: Daniel Henrique Barboza
---
target/riscv/cpu.c | 16 +++---
target/riscv/cpu.h | 1 +
target/riscv/kvm/kvm-cpu.c | 59
target/riscv/kvm/kvm_riscv.h | 1 +
4 files changed, 72 insertions(+), 5 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 2bb4828324..cd91966ea7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -68,6 +68,11 @@ static void cpu_option_add_user_setting(const char *optname,
uint32_t value)
GUINT_TO_POINTER(value));
}
+bool riscv_cpu_option_set(const char *optname)
+{
+return g_hash_table_contains(general_user_opts, optname);
+}
+
#define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \
{#_name, _min_ver, CPU_CFG_OFFSET(_prop)}
@@ -1104,17 +1109,18 @@ void riscv_cpu_finalize_features(RISCVCPU *cpu, Error
**errp)
}
#endif
-/*
- * KVM accel does not have a specialized finalize()
- * callback because its extensions are validated
- * in the get()/set() callbacks of each property.
- */
if (tcg_enabled()) {
riscv_tcg_cpu_finalize_features(cpu, _err);
if (local_err != NULL) {
error_propagate(errp, local_err);
return;
}
+} else if (kvm_enabled()) {
+riscv_kvm_cpu_finalize_features(cpu, _err);
+if (local_err != NULL) {
+error_propagate(errp, local_err);
+return;
+}
}
}
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 3cec85069f..1c19fa84bb 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -510,6 +510,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
bool probe, uintptr_t retaddr);
char *riscv_isa_string(RISCVCPU *cpu);
void riscv_cpu_list(void);
+bool riscv_cpu_option_set(const char *optname);
#define cpu_list riscv_cpu_list
#define cpu_mmu_index riscv_cpu_mmu_index
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 2c5217102c..2713f4b2ba 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1493,6 +1493,65 @@ static void kvm_cpu_instance_init(CPUState *cs)
}
}
+void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
+{
+CPURISCVState *env = >env;
+KVMScratchCPU kvmcpu;
+struct kvm_one_reg reg;
+uint64_t val;
+int ret;
+
+/* short-circuit without spinning the scratch CPU */
+if (!cpu->cfg.ext_zicbom && !cpu->cfg.ext_zicboz) {
+return;
+}
+
+if (!kvm_riscv_create_scratch_vcpu()) {
+error_setg(errp, "Unable to create scratch KVM cpu");
+return;
+}
+
+if (cpu->cfg.ext_zicbom &&
+riscv_cpu_option_set(kvm_cbom_blocksize.name)) {
+
+reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG,
+kvm_cbom_blocksize.kvm_reg_id);
+reg.addr = (uint64_t)
+ret = ioctl(kvmcpu.cpufd, KVM_GET_ONE_REG, );
+if (ret != 0) {
+error_setg(errp, "Unable to read cbom_blocksize, error %d", errno);
+return;
+}
+
+if (cpu->cfg.cbom_blocksize != val) {
+error_setg(errp, "Unable to set cbom_blocksize to a different "
+ "value than the host (%lu)", val);
+return;
+}
+}
+
+if (cpu->cfg.ext_zicboz &&
+riscv_cpu_option_set(kvm_cboz_blocksize.name)) {
+
+reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG,
+kvm_cboz_blocksize.kvm_reg_id);
+reg.addr = (uint64_t)
+ret = ioctl(kvmcpu.cpufd, KVM_GET_ONE_REG, );
+if (ret != 0) {
+error_setg(errp, "Unable to read cboz_blocksize, error %d", errno);
+return;
+}
+
+if
[PATCH v4 07/17] target/riscv: rework 'vext_spec'
The same rework did in 'priv_spec' is done for 'vext_spec'. This time is
simpler, since we only accept one value ("v1.0") and we'll always have
env->vext_ver set to VEXT_VERSION_1_00_0, thus we don't need helpers to
convert string to 'vext_ver' back and forth like we needed for
'priv_spec'.
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
---
target/riscv/cpu.c | 35 +--
target/riscv/cpu.h | 1 +
target/riscv/cpu_cfg.h | 1 -
target/riscv/tcg/tcg-cpu.c | 15 ---
4 files changed, 34 insertions(+), 18 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f3316c5082..a4e5863de7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1307,6 +1307,7 @@ static void riscv_cpu_init(Object *obj)
/* Default values for non-bool cpu properties */
cpu->cfg.pmu_mask = MAKE_64BIT_MASK(3, 16);
+cpu->env.vext_ver = VEXT_VERSION_1_00_0;
}
typedef struct misa_ext_info {
@@ -1745,9 +1746,38 @@ static const PropertyInfo prop_priv_spec = {
.set = prop_priv_spec_set,
};
-Property riscv_cpu_options[] = {
-DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
+static void prop_vext_spec_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+RISCVCPU *cpu = RISCV_CPU(obj);
+g_autofree char *value = NULL;
+visit_type_str(v, name, , errp);
+
+if (g_strcmp0(value, VEXT_VER_1_00_0_STR) != 0) {
+error_setg(errp, "Unsupported vector spec version '%s'", value);
+return;
+}
+
+cpu_option_add_user_setting(name, VEXT_VERSION_1_00_0);
+cpu->env.vext_ver = VEXT_VERSION_1_00_0;
+}
+
+static void prop_vext_spec_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+const char *value = VEXT_VER_1_00_0_STR;
+
+visit_type_str(v, name, (char **), errp);
+}
+
+static const PropertyInfo prop_vext_spec = {
+.name = "vext_spec",
+.get = prop_vext_spec_get,
+.set = prop_vext_spec_set,
+};
+
+Property riscv_cpu_options[] = {
DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
@@ -1835,6 +1865,7 @@ static Property riscv_cpu_properties[] = {
{.name = "pmp", .info = _pmp},
{.name = "priv_spec", .info = _priv_spec},
+{.name = "vext_spec", .info = _vext_spec},
#ifndef CONFIG_USER_ONLY
DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index ee65ed555a..3cec85069f 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -105,6 +105,7 @@ enum {
};
#define VEXT_VERSION_1_00_0 0x0001
+#define VEXT_VER_1_00_0_STR "v1.0"
enum {
TRANSLATE_SUCCESS,
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 68965743b6..fea14c275f 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -139,7 +139,6 @@ struct RISCVCPUConfig {
bool ext_XVentanaCondOps;
uint32_t pmu_mask;
-char *vext_spec;
uint16_t vlen;
uint16_t elen;
uint16_t cbom_blocksize;
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index a24c5e7e58..0fb054b20e 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -321,21 +321,6 @@ static void riscv_cpu_validate_v(CPURISCVState *env,
RISCVCPUConfig *cfg,
"in the range [8, 64]");
return;
}
-
-if (cfg->vext_spec) {
-if (!g_strcmp0(cfg->vext_spec, "v1.0")) {
-env->vext_ver = VEXT_VERSION_1_00_0;
-} else {
-error_setg(errp, "Unsupported vector spec version '%s'",
- cfg->vext_spec);
-return;
-}
-} else if (env->vext_ver == 0) {
-qemu_log("vector version is not specified, "
- "use the default value v1.0\n");
-
-env->vext_ver = VEXT_VERSION_1_00_0;
-}
}
static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
--
2.43.0
[PATCH v4 12/17] target/riscv: move 'cbop_blocksize' to riscv_cpu_properties[]
Do the same we did with 'cbom_blocksize' in the previous patch.
Signed-off-by: Daniel Henrique Barboza
---
target/riscv/cpu.c | 38 +-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index b77d26231c..e3cbe9b1b6 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1317,6 +1317,7 @@ static void riscv_cpu_init(Object *obj)
cpu->cfg.vlen = 128;
cpu->cfg.elen = 64;
cpu->cfg.cbom_blocksize = 64;
+cpu->cfg.cbop_blocksize = 64;
cpu->env.vext_ver = VEXT_VERSION_1_00_0;
}
@@ -1902,8 +1903,42 @@ static const PropertyInfo prop_cbom_blksize = {
.set = prop_cbom_blksize_set,
};
+static void prop_cbop_blksize_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+RISCVCPU *cpu = RISCV_CPU(obj);
+uint16_t value;
+
+if (!visit_type_uint16(v, name, , errp)) {
+return;
+}
+
+if (value != cpu->cfg.cbop_blocksize && riscv_cpu_is_vendor(obj)) {
+cpu_set_prop_err(cpu, name, errp);
+error_append_hint(errp, "Current '%s' val: %u\n",
+ name, cpu->cfg.cbop_blocksize);
+return;
+}
+
+cpu_option_add_user_setting(name, value);
+cpu->cfg.cbop_blocksize = value;
+}
+
+static void prop_cbop_blksize_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+uint16_t value = RISCV_CPU(obj)->cfg.cbop_blocksize;
+
+visit_type_uint16(v, name, , errp);
+}
+
+static const PropertyInfo prop_cbop_blksize = {
+.name = "cbop_blocksize",
+.get = prop_cbop_blksize_get,
+.set = prop_cbop_blksize_set,
+};
+
Property riscv_cpu_options[] = {
-DEFINE_PROP_UINT16("cbop_blocksize", RISCVCPU, cfg.cbop_blocksize, 64),
DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
DEFINE_PROP_END_OF_LIST(),
@@ -1992,6 +2027,7 @@ static Property riscv_cpu_properties[] = {
{.name = "elen", .info = _elen},
{.name = "cbom_blocksize", .info = _cbom_blksize},
+{.name = "cbop_blocksize", .info = _cbop_blksize},
#ifndef CONFIG_USER_ONLY
DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
--
2.43.0
[PATCH v4 08/17] target/riscv: move 'vlen' to riscv_cpu_properties[]
Turning 'vlen' into a class property will allow its default value to be
overwritten by cpu_init() later on, solving the issue we have now where
CPU specific settings are getting overwritten by the default.
Common validation bits are moved from riscv_cpu_validate_v() to
prop_vlen_set() to be shared with KVM.
And, as done with every option we migrated to riscv_cpu_properties[],
vendor CPUs can't have their 'vlen' value changed.
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
---
target/riscv/cpu.c | 45 +-
target/riscv/tcg/tcg-cpu.c | 5 -
2 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index a4e5863de7..fd55064c3b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -29,6 +29,7 @@
#include "qapi/visitor.h"
#include "qemu/error-report.h"
#include "hw/qdev-properties.h"
+#include "hw/core/qdev-prop-internal.h"
#include "migration/vmstate.h"
#include "fpu/softfloat-helpers.h"
#include "sysemu/kvm.h"
@@ -1307,6 +1308,7 @@ static void riscv_cpu_init(Object *obj)
/* Default values for non-bool cpu properties */
cpu->cfg.pmu_mask = MAKE_64BIT_MASK(3, 16);
+cpu->cfg.vlen = 128;
cpu->env.vext_ver = VEXT_VERSION_1_00_0;
}
@@ -1777,8 +1779,47 @@ static const PropertyInfo prop_vext_spec = {
.set = prop_vext_spec_set,
};
+static void prop_vlen_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+RISCVCPU *cpu = RISCV_CPU(obj);
+uint16_t value;
+
+if (!visit_type_uint16(v, name, , errp)) {
+return;
+}
+
+if (!is_power_of_2(value)) {
+error_setg(errp, "Vector extension VLEN must be power of 2");
+return;
+}
+
+if (value != cpu->cfg.vlen && riscv_cpu_is_vendor(obj)) {
+cpu_set_prop_err(cpu, name, errp);
+error_append_hint(errp, "Current '%s' val: %u\n",
+ name, cpu->cfg.vlen);
+return;
+}
+
+cpu_option_add_user_setting(name, value);
+cpu->cfg.vlen = value;
+}
+
+static void prop_vlen_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+uint16_t value = RISCV_CPU(obj)->cfg.vlen;
+
+visit_type_uint16(v, name, , errp);
+}
+
+static const PropertyInfo prop_vlen = {
+.name = "vlen",
+.get = prop_vlen_get,
+.set = prop_vlen_set,
+};
+
Property riscv_cpu_options[] = {
-DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64),
@@ -1867,6 +1908,8 @@ static Property riscv_cpu_properties[] = {
{.name = "priv_spec", .info = _priv_spec},
{.name = "vext_spec", .info = _vext_spec},
+{.name = "vlen", .info = _vlen},
+
#ifndef CONFIG_USER_ONLY
DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
#endif
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 0fb054b20e..71a364453e 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -298,11 +298,6 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu,
Error **errp)
static void riscv_cpu_validate_v(CPURISCVState *env, RISCVCPUConfig *cfg,
Error **errp)
{
-if (!is_power_of_2(cfg->vlen)) {
-error_setg(errp, "Vector extension VLEN must be power of 2");
-return;
-}
-
if (cfg->vlen > RV_VLEN_MAX || cfg->vlen < 128) {
error_setg(errp,
"Vector extension implementation only supports VLEN "
--
2.43.0
[PATCH v4 03/17] target/riscv: move 'pmu-mask' and 'pmu-num' to riscv_cpu_properties[]
Every property in riscv_cpu_options[] will be migrated to
riscv_cpu_properties[]. This will make their default values init
earlier, allowing cpu_init() functions to overwrite them. We'll also
implement common getters and setters that both accelerators will use,
allowing them to share validations that TCG is doing.
At the same time, some options (namely 'vlen', 'elen' and the cache
blocksizes) need a way of tracking if the user set a value for them.
This is benign for TCG since the cost of always validating these values
are small, but for KVM we need syscalls to read the host values to make
the validations, thus knowing whether the user didn't touch the values
makes a difference.
We'll track user setting for these properties using a hash, like we do
in the TCG driver. All riscv cpu options will update this hash in case
the user sets it. The KVM driver will use this hash to minimize the
amount of syscalls done.
For now, both 'pmu-mask' and 'pmu-num' shouldn't be changed for vendor
CPUs. The existing setter for 'pmu-num' is changed to add this
restriction. New getters and setters are required for 'pmu-mask'
While we're at it, add a 'static' modifier to 'prop_pmu_num' since we're
not exporting it.
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
---
target/riscv/cpu.c | 91 ++
1 file changed, 84 insertions(+), 7 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 025c6cb23c..e5885e0dfb 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -58,6 +58,15 @@ bool riscv_cpu_is_32bit(RISCVCPU *cpu)
return riscv_cpu_mxl(>env) == MXL_RV32;
}
+/* Hash that stores general user set numeric options */
+static GHashTable *general_user_opts;
+
+static void cpu_option_add_user_setting(const char *optname, uint32_t value)
+{
+g_hash_table_insert(general_user_opts, (gpointer)optname,
+GUINT_TO_POINTER(value));
+}
+
#define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \
{#_name, _min_ver, CPU_CFG_OFFSET(_prop)}
@@ -1258,11 +1267,15 @@ static void riscv_cpu_post_init(Object *obj)
static void riscv_cpu_init(Object *obj)
{
+RISCVCPU *cpu = RISCV_CPU(obj);
+
#ifndef CONFIG_USER_ONLY
qdev_init_gpio_in(DEVICE(obj), riscv_cpu_set_irq,
IRQ_LOCAL_MAX + IRQ_LOCAL_GUEST_MAX);
#endif /* CONFIG_USER_ONLY */
+general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
+
/*
* The timer and performance counters extensions were supported
* in QEMU before they were added as discrete extensions in the
@@ -1272,6 +1285,9 @@ static void riscv_cpu_init(Object *obj)
*/
RISCV_CPU(obj)->cfg.ext_zicntr = true;
RISCV_CPU(obj)->cfg.ext_zihpm = true;
+
+/* Default values for non-bool cpu properties */
+cpu->cfg.pmu_mask = MAKE_64BIT_MASK(3, 16);
}
typedef struct misa_ext_info {
@@ -1480,26 +1496,46 @@ const RISCVCPUMultiExtConfig
riscv_cpu_deprecated_exts[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static void cpu_set_prop_err(RISCVCPU *cpu, const char *propname,
+ Error **errp)
+{
+g_autofree char *cpuname = riscv_cpu_get_name(cpu);
+error_setg(errp, "CPU '%s' does not allow changing the value of '%s'",
+ cpuname, propname);
+}
+
static void prop_pmu_num_set(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
RISCVCPU *cpu = RISCV_CPU(obj);
-uint8_t pmu_num;
+uint8_t pmu_num, curr_pmu_num;
+uint32_t pmu_mask;
visit_type_uint8(v, name, _num, errp);
+curr_pmu_num = ctpop32(cpu->cfg.pmu_mask);
+
+if (pmu_num != curr_pmu_num && riscv_cpu_is_vendor(obj)) {
+cpu_set_prop_err(cpu, name, errp);
+error_append_hint(errp, "Current '%s' val: %u\n",
+ name, curr_pmu_num);
+return;
+}
+
if (pmu_num > (RV_MAX_MHPMCOUNTERS - 3)) {
error_setg(errp, "Number of counters exceeds maximum available");
return;
}
if (pmu_num == 0) {
-cpu->cfg.pmu_mask = 0;
+pmu_mask = 0;
} else {
-cpu->cfg.pmu_mask = MAKE_64BIT_MASK(3, pmu_num);
+pmu_mask = MAKE_64BIT_MASK(3, pmu_num);
}
warn_report("\"pmu-num\" property is deprecated; use \"pmu-mask\"");
+cpu->cfg.pmu_mask = pmu_mask;
+cpu_option_add_user_setting("pmu-mask", pmu_mask);
}
static void prop_pmu_num_get(Object *obj, Visitor *v, const char *name,
@@ -1511,16 +1547,54 @@ static void prop_pmu_num_get(Object *obj, Visitor *v,
const char *name,
visit_type_uint8(v, name, _num, errp);
}
-const PropertyInfo prop_pmu_num = {
+static const PropertyInfo prop_pmu_num = {
.name = "pmu-num",
.get = prop_pmu_num_get,
.set = prop_pmu_num_set,
};
-Property riscv_cpu_options[] = {
-DEFINE_PROP_UINT32("pmu-mask", RISCVCPU, cfg.pmu_mask, MAKE_64BIT_MASK(3,
16)),
-{.name = "pmu-num", .info = _pmu_num}, /*
[PATCH v4 05/17] target/riscv: move 'pmp' to riscv_cpu_properties[]
Move 'pmp' to riscv_cpu_properties[], creating a new setter() for it
that forbids 'pmp' to be changed in vendor CPUs, like we did with the
'mmu' option.
We'll also have to manually set 'pmp = true' to generic CPUs that were
still relying on the previous default to set it.
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
---
target/riscv/cpu.c | 38 --
1 file changed, 36 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 891a3b630b..df8e0b43f7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -438,6 +438,7 @@ static void riscv_max_cpu_init(Object *obj)
RISCVMXL mlx = MXL_RV64;
cpu->cfg.mmu = true;
+cpu->cfg.pmp = true;
#ifdef TARGET_RISCV32
mlx = MXL_RV32;
@@ -457,6 +458,7 @@ static void rv64_base_cpu_init(Object *obj)
CPURISCVState *env = >env;
cpu->cfg.mmu = true;
+cpu->cfg.pmp = true;
/* We set this in the realise function */
riscv_cpu_set_misa(env, MXL_RV64, 0);
@@ -586,6 +588,7 @@ static void rv128_base_cpu_init(Object *obj)
}
cpu->cfg.mmu = true;
+cpu->cfg.pmp = true;
/* We set this in the realise function */
riscv_cpu_set_misa(env, MXL_RV128, 0);
@@ -624,6 +627,7 @@ static void rv32_base_cpu_init(Object *obj)
CPURISCVState *env = >env;
cpu->cfg.mmu = true;
+cpu->cfg.pmp = true;
/* We set this in the realise function */
riscv_cpu_set_misa(env, MXL_RV32, 0);
@@ -1640,9 +1644,38 @@ static const PropertyInfo prop_mmu = {
.set = prop_mmu_set,
};
-Property riscv_cpu_options[] = {
-DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
+static void prop_pmp_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+RISCVCPU *cpu = RISCV_CPU(obj);
+bool value;
+
+visit_type_bool(v, name, , errp);
+if (cpu->cfg.pmp != value && riscv_cpu_is_vendor(obj)) {
+cpu_set_prop_err(cpu, name, errp);
+return;
+}
+
+cpu_option_add_user_setting(name, value);
+cpu->cfg.pmp = value;
+}
+
+static void prop_pmp_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+bool value = RISCV_CPU(obj)->cfg.pmp;
+
+visit_type_bool(v, name, , errp);
+}
+
+static const PropertyInfo prop_pmp = {
+.name = "pmp",
+.get = prop_pmp_get,
+.set = prop_pmp_set,
+};
+
+Property riscv_cpu_options[] = {
DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
@@ -1730,6 +1763,7 @@ static Property riscv_cpu_properties[] = {
{.name = "pmu-num", .info = _pmu_num}, /* Deprecated */
{.name = "mmu", .info = _mmu},
+{.name = "pmp", .info = _pmp},
#ifndef CONFIG_USER_ONLY
DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
--
2.43.0
[PATCH v4 17/17] target/riscv/cpu.c: move 'marchid' to riscv_cpu_properties[]
Keep all class properties in riscv_cpu_properties[].
Signed-off-by: Daniel Henrique Barboza
---
target/riscv/cpu.c | 110 +++--
1 file changed, 57 insertions(+), 53 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 6149f5960e..3870c3a433 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2044,6 +2044,62 @@ static const PropertyInfo prop_mimpid = {
.set = prop_mimpid_set,
};
+static void prop_marchid_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+bool dynamic_cpu = riscv_cpu_is_dynamic(obj);
+RISCVCPU *cpu = RISCV_CPU(obj);
+uint64_t prev_val = cpu->cfg.marchid;
+uint64_t value, invalid_val;
+uint32_t mxlen = 0;
+
+if (!visit_type_uint64(v, name, , errp)) {
+return;
+}
+
+if (!dynamic_cpu && prev_val != value) {
+error_setg(errp, "Unable to change %s marchid (0x%" PRIu64 ")",
+ object_get_typename(obj), prev_val);
+return;
+}
+
+switch (riscv_cpu_mxl(>env)) {
+case MXL_RV32:
+mxlen = 32;
+break;
+case MXL_RV64:
+case MXL_RV128:
+mxlen = 64;
+break;
+default:
+g_assert_not_reached();
+}
+
+invalid_val = 1LL << (mxlen - 1);
+
+if (value == invalid_val) {
+error_setg(errp, "Unable to set marchid with MSB (%u) bit set "
+ "and the remaining bits zero", mxlen);
+return;
+}
+
+cpu->cfg.marchid = value;
+}
+
+static void prop_marchid_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+uint64_t value = RISCV_CPU(obj)->cfg.marchid;
+
+visit_type_uint64(v, name, , errp);
+}
+
+static const PropertyInfo prop_marchid = {
+.name = "marchid",
+.get = prop_marchid_get,
+.set = prop_marchid_set,
+};
+
/*
* RVA22U64 defines some 'named features' or 'synthetic extensions'
* that are cache related: Za64rs, Zic64b, Ziccif, Ziccrse, Ziccamoa
@@ -2132,6 +2188,7 @@ static Property riscv_cpu_properties[] = {
{.name = "mvendorid", .info = _mvendorid},
{.name = "mimpid", .info = _mimpid},
+ {.name = "marchid", .info = _marchid},
#ifndef CONFIG_USER_ONLY
DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
@@ -2213,56 +2270,6 @@ static const struct SysemuCPUOps riscv_sysemu_ops = {
};
#endif
-static void cpu_set_marchid(Object *obj, Visitor *v, const char *name,
-void *opaque, Error **errp)
-{
-bool dynamic_cpu = riscv_cpu_is_dynamic(obj);
-RISCVCPU *cpu = RISCV_CPU(obj);
-uint64_t prev_val = cpu->cfg.marchid;
-uint64_t value, invalid_val;
-uint32_t mxlen = 0;
-
-if (!visit_type_uint64(v, name, , errp)) {
-return;
-}
-
-if (!dynamic_cpu && prev_val != value) {
-error_setg(errp, "Unable to change %s marchid (0x%" PRIu64 ")",
- object_get_typename(obj), prev_val);
-return;
-}
-
-switch (riscv_cpu_mxl(>env)) {
-case MXL_RV32:
-mxlen = 32;
-break;
-case MXL_RV64:
-case MXL_RV128:
-mxlen = 64;
-break;
-default:
-g_assert_not_reached();
-}
-
-invalid_val = 1LL << (mxlen - 1);
-
-if (value == invalid_val) {
-error_setg(errp, "Unable to set marchid with MSB (%u) bit set "
- "and the remaining bits zero", mxlen);
-return;
-}
-
-cpu->cfg.marchid = value;
-}
-
-static void cpu_get_marchid(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp)
-{
-uint64_t value = RISCV_CPU(obj)->cfg.marchid;
-
-visit_type_uint64(v, name, , errp);
-}
-
static void riscv_cpu_class_init(ObjectClass *c, void *data)
{
RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
@@ -2293,9 +2300,6 @@ static void riscv_cpu_class_init(ObjectClass *c, void
*data)
cc->gdb_arch_name = riscv_gdb_arch_name;
cc->gdb_get_dynamic_xml = riscv_gdb_get_dynamic_xml;
-object_class_property_add(c, "marchid", "uint64", cpu_get_marchid,
- cpu_set_marchid, NULL, NULL);
-
device_class_set_props(dc, riscv_cpu_properties);
}
--
2.43.0
[PATCH v4 14/17] target/riscv: remove riscv_cpu_options[]
The array is empty and can be removed.
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
---
target/riscv/cpu.c | 5 -
target/riscv/cpu.h | 1 -
target/riscv/kvm/kvm-cpu.c | 9 -
target/riscv/tcg/tcg-cpu.c | 4
4 files changed, 19 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f84c3fc4a2..9d4243891c 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1974,11 +1974,6 @@ static const PropertyInfo prop_cboz_blksize = {
.set = prop_cboz_blksize_set,
};
-Property riscv_cpu_options[] = {
-
-DEFINE_PROP_END_OF_LIST(),
-};
-
/*
* RVA22U64 defines some 'named features' or 'synthetic extensions'
* that are cache related: Za64rs, Zic64b, Ziccif, Ziccrse, Ziccamoa
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 1c19fa84bb..c0336b8c0d 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -791,7 +791,6 @@ extern const RISCVCPUMultiExtConfig riscv_cpu_vendor_exts[];
extern const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[];
extern const RISCVCPUMultiExtConfig riscv_cpu_named_features[];
extern const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[];
-extern Property riscv_cpu_options[];
typedef struct isa_ext_data {
const char *name;
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index c9b4a6a7e8..8c81049169 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1446,19 +1446,10 @@ void kvm_riscv_aia_create(MachineState *machine,
uint64_t group_shift,
static void kvm_cpu_instance_init(CPUState *cs)
{
Object *obj = OBJECT(RISCV_CPU(cs));
-DeviceState *dev = DEVICE(obj);
riscv_init_kvm_registers(obj);
kvm_riscv_add_cpu_user_properties(obj);
-
-for (Property *prop = riscv_cpu_options; prop && prop->name; prop++) {
-/* Check if we have a specific KVM handler for the option */
-if (object_property_find(obj, prop->name)) {
-continue;
-}
-qdev_property_add_static(dev, prop);
-}
}
void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index c7c2a28f10..27591ad2f2 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -1267,10 +1267,6 @@ static void riscv_cpu_add_user_properties(Object *obj)
riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_deprecated_exts);
riscv_cpu_add_profiles(obj);
-
-for (Property *prop = riscv_cpu_options; prop && prop->name; prop++) {
-qdev_property_add_static(DEVICE(obj), prop);
-}
}
/*
--
2.43.0
[PATCH v4 00/17] target/riscv: deprecate riscv_cpu_options[]
Hi, This new version contains changes due to a rebase with current riscv-to-apply.next, after "[PATCH v13 00/26] riscv: RVA22 profiles support" was queued. Most notable change is a new patch (12) that was added to handle 'cbop_blocksize' - zicbop was added by the profile work that just got queued and was missing from v3. A wrong 'cbom_blocksize' reference in patch 10 was also fixed. Patches based on Alistair's riscv-to-apply.next. Patches missing acks: 10, 12, 15, 16, 17 Changes from v3: - patch 10: - changed wrong cbom_blocksize ref to cboz_blocksize - patch 12 (new): - move cbop_blocksize to riscv_cpu_properties[] - v3 link: https://lore.kernel.org/qemu-riscv/20240103174013.147279-1-dbarb...@ventanamicro.com/ Daniel Henrique Barboza (17): target/riscv/cpu_cfg.h: remove unused fields target/riscv: make riscv_cpu_is_vendor() public target/riscv: move 'pmu-mask' and 'pmu-num' to riscv_cpu_properties[] target/riscv: move 'mmu' to riscv_cpu_properties[] target/riscv: move 'pmp' to riscv_cpu_properties[] target/riscv: rework 'priv_spec' target/riscv: rework 'vext_spec' target/riscv: move 'vlen' to riscv_cpu_properties[] target/riscv: move 'elen' to riscv_cpu_properties[] target/riscv: create finalize_features() for KVM target/riscv: move 'cbom_blocksize' to riscv_cpu_properties[] target/riscv: move 'cbop_blocksize' to riscv_cpu_properties[] target/riscv: move 'cboz_blocksize' to riscv_cpu_properties[] target/riscv: remove riscv_cpu_options[] target/riscv/cpu.c: move 'mvendorid' to riscv_cpu_properties[] target/riscv/cpu.c: move 'mimpid' to riscv_cpu_properties[] target/riscv/cpu.c: move 'marchid' to riscv_cpu_properties[] target/riscv/cpu.c | 755 --- target/riscv/cpu.h | 8 +- target/riscv/cpu_cfg.h | 4 - target/riscv/kvm/kvm-cpu.c | 94 +++-- target/riscv/kvm/kvm_riscv.h | 1 + target/riscv/tcg/tcg-cpu.c | 63 --- 6 files changed, 676 insertions(+), 249 deletions(-) -- 2.43.0
[PATCH v4 16/17] target/riscv/cpu.c: move 'mimpid' to riscv_cpu_properties[]
Keep all class properties in riscv_cpu_properties[].
Signed-off-by: Daniel Henrique Barboza
---
target/riscv/cpu.c | 68 --
1 file changed, 36 insertions(+), 32 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index c725a4839d..6149f5960e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2009,6 +2009,41 @@ static const PropertyInfo prop_mvendorid = {
.set = prop_mvendorid_set,
};
+static void prop_mimpid_set(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+bool dynamic_cpu = riscv_cpu_is_dynamic(obj);
+RISCVCPU *cpu = RISCV_CPU(obj);
+uint64_t prev_val = cpu->cfg.mimpid;
+uint64_t value;
+
+if (!visit_type_uint64(v, name, , errp)) {
+return;
+}
+
+if (!dynamic_cpu && prev_val != value) {
+error_setg(errp, "Unable to change %s mimpid (0x%" PRIu64 ")",
+ object_get_typename(obj), prev_val);
+return;
+}
+
+cpu->cfg.mimpid = value;
+}
+
+static void prop_mimpid_get(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+uint64_t value = RISCV_CPU(obj)->cfg.mimpid;
+
+visit_type_uint64(v, name, , errp);
+}
+
+static const PropertyInfo prop_mimpid = {
+.name = "mimpid",
+.get = prop_mimpid_get,
+.set = prop_mimpid_set,
+};
+
/*
* RVA22U64 defines some 'named features' or 'synthetic extensions'
* that are cache related: Za64rs, Zic64b, Ziccif, Ziccrse, Ziccamoa
@@ -2096,6 +2131,7 @@ static Property riscv_cpu_properties[] = {
{.name = "cboz_blocksize", .info = _cboz_blksize},
{.name = "mvendorid", .info = _mvendorid},
+ {.name = "mimpid", .info = _mimpid},
#ifndef CONFIG_USER_ONLY
DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
@@ -2177,35 +2213,6 @@ static const struct SysemuCPUOps riscv_sysemu_ops = {
};
#endif
-static void cpu_set_mimpid(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp)
-{
-bool dynamic_cpu = riscv_cpu_is_dynamic(obj);
-RISCVCPU *cpu = RISCV_CPU(obj);
-uint64_t prev_val = cpu->cfg.mimpid;
-uint64_t value;
-
-if (!visit_type_uint64(v, name, , errp)) {
-return;
-}
-
-if (!dynamic_cpu && prev_val != value) {
-error_setg(errp, "Unable to change %s mimpid (0x%" PRIu64 ")",
- object_get_typename(obj), prev_val);
-return;
-}
-
-cpu->cfg.mimpid = value;
-}
-
-static void cpu_get_mimpid(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp)
-{
-uint64_t value = RISCV_CPU(obj)->cfg.mimpid;
-
-visit_type_uint64(v, name, , errp);
-}
-
static void cpu_set_marchid(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
@@ -2286,9 +2293,6 @@ static void riscv_cpu_class_init(ObjectClass *c, void
*data)
cc->gdb_arch_name = riscv_gdb_arch_name;
cc->gdb_get_dynamic_xml = riscv_gdb_get_dynamic_xml;
-object_class_property_add(c, "mimpid", "uint64", cpu_get_mimpid,
- cpu_set_mimpid, NULL, NULL);
-
object_class_property_add(c, "marchid", "uint64", cpu_get_marchid,
cpu_set_marchid, NULL, NULL);
--
2.43.0
[PATCH v4 06/17] target/riscv: rework 'priv_spec'
'priv_spec' and 'vext_spec' are two string options used as a fancy way
of setting integers in the CPU state (cpu->env.priv_ver and
cpu->env.vext_ver). It requires us to deal with string parsing and to
store them in cpu_cfg.
We must support these string options, but we don't need to store them.
We have a precedence for this kind of arrangement in target/ppc/compat.c,
ppc_compat_prop_get|set, getters and setters used for the
'max-cpu-compat' class property of the pseries ppc64 machine. We'll do
the same with both 'priv_spec' and 'vext_spec'.
For 'priv_spec', the validation from riscv_cpu_validate_priv_spec() will
be done by the prop_priv_spec_set() setter, while also preventing it to
be changed for vendor CPUs. Add two helpers that converts env->priv_ver
back and forth to its string representation. These helpers allow us to
get a string and set 'env->priv_ver' and return a string giving the
current env->priv_ver value. In other words, make the cpu->cfg.priv_spec
string obsolete.
Last but not the least, move the reworked 'priv_spec' option to
riscv_cpu_properties[].
After all said and done, we don't need to store the 'priv_spec' string in
the CPU state, and we're now protecting vendor CPUs from priv_ver
changes:
$ ./build/qemu-system-riscv64 -M virt -cpu sifive-e51,priv_spec="v1.12.0"
qemu-system-riscv64: can't apply global sifive-e51-riscv-cpu.priv_spec=v1.12.0:
CPU 'sifive-e51' does not allow changing the value of 'priv_spec'
Current 'priv_spec' val: v1.10.0
$
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
---
target/riscv/cpu.c | 73 +-
target/riscv/cpu.h | 3 ++
target/riscv/cpu_cfg.h | 1 -
target/riscv/tcg/tcg-cpu.c | 29 ---
4 files changed, 75 insertions(+), 31 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index df8e0b43f7..f3316c5082 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1675,8 +1675,77 @@ static const PropertyInfo prop_pmp = {
.set = prop_pmp_set,
};
+static int priv_spec_from_str(const char *priv_spec_str)
+{
+int priv_version = -1;
+
+if (!g_strcmp0(priv_spec_str, PRIV_VER_1_12_0_STR)) {
+priv_version = PRIV_VERSION_1_12_0;
+} else if (!g_strcmp0(priv_spec_str, PRIV_VER_1_11_0_STR)) {
+priv_version = PRIV_VERSION_1_11_0;
+} else if (!g_strcmp0(priv_spec_str, PRIV_VER_1_10_0_STR)) {
+priv_version = PRIV_VERSION_1_10_0;
+}
+
+return priv_version;
+}
+
+static const char *priv_spec_to_str(int priv_version)
+{
+switch (priv_version) {
+case PRIV_VERSION_1_10_0:
+return PRIV_VER_1_10_0_STR;
+case PRIV_VERSION_1_11_0:
+return PRIV_VER_1_11_0_STR;
+case PRIV_VERSION_1_12_0:
+return PRIV_VER_1_12_0_STR;
+default:
+return NULL;
+}
+}
+
+static void prop_priv_spec_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+RISCVCPU *cpu = RISCV_CPU(obj);
+g_autofree char *value = NULL;
+int priv_version = -1;
+
+visit_type_str(v, name, , errp);
+
+priv_version = priv_spec_from_str(value);
+if (priv_version < 0) {
+error_setg(errp, "Unsupported privilege spec version '%s'", value);
+return;
+}
+
+if (priv_version != cpu->env.priv_ver && riscv_cpu_is_vendor(obj)) {
+cpu_set_prop_err(cpu, name, errp);
+error_append_hint(errp, "Current '%s' val: %s\n", name,
+ object_property_get_str(obj, name, NULL));
+return;
+}
+
+cpu_option_add_user_setting(name, priv_version);
+cpu->env.priv_ver = priv_version;
+}
+
+static void prop_priv_spec_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+RISCVCPU *cpu = RISCV_CPU(obj);
+const char *value = priv_spec_to_str(cpu->env.priv_ver);
+
+visit_type_str(v, name, (char **), errp);
+}
+
+static const PropertyInfo prop_priv_spec = {
+.name = "priv_spec",
+.get = prop_priv_spec_get,
+.set = prop_priv_spec_set,
+};
+
Property riscv_cpu_options[] = {
-DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
@@ -1765,6 +1834,8 @@ static Property riscv_cpu_properties[] = {
{.name = "mmu", .info = _mmu},
{.name = "pmp", .info = _pmp},
+{.name = "priv_spec", .info = _priv_spec},
+
#ifndef CONFIG_USER_ONLY
DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
#endif
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 1ece80a604..ee65ed555a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -93,6 +93,9 @@ typedef struct riscv_cpu_profile {
extern RISCVCPUProfile *riscv_profiles[];
/* Privileged specification version */
+#define PRIV_VER_1_10_0_STR "v1.10.0"
+#define PRIV_VER_1_11_0_STR "v1.11.0"
+#define
[PATCH v4 04/17] target/riscv: move 'mmu' to riscv_cpu_properties[]
Commit 7f0bdfb5bfc ("target/riscv/cpu.c: remove cfg setup from
riscv_cpu_init()") already did some of the work by making some
cpu_init() functions to explictly enable their own 'mmu' default.
The generic CPUs didn't get update by that commit, so they are still
relying on the defaults set by the 'mmu' option. But having 'mmu' and
'pmp' being default=true will force CPUs that doesn't implement these
options to set them to 'false' in their cpu_init(), which isn't ideal.
We'll move 'mmu' to riscv_cpu_properties[] without any defaults, i.e.
the default will be 'false'. Compensate it by manually setting 'mmu =
true' to the generic CPUs that requires it.
Implement a setter for it to forbid the 'mmu' setting to be changed for
vendor CPUs. This will allow the option to exist for all CPUs and, at
the same time, protect vendor CPUs from undesired changes:
$ ./build/qemu-system-riscv64 -M virt -cpu sifive-e51,mmu=true
qemu-system-riscv64: can't apply global sifive-e51-riscv-cpu.mmu=true:
CPU 'sifive-e51' does not allow changing the value of 'mmu'
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
---
target/riscv/cpu.c | 55 ++
1 file changed, 51 insertions(+), 4 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index e5885e0dfb..891a3b630b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -437,6 +437,8 @@ static void riscv_max_cpu_init(Object *obj)
CPURISCVState *env = >env;
RISCVMXL mlx = MXL_RV64;
+cpu->cfg.mmu = true;
+
#ifdef TARGET_RISCV32
mlx = MXL_RV32;
#endif
@@ -451,7 +453,11 @@ static void riscv_max_cpu_init(Object *obj)
#if defined(TARGET_RISCV64)
static void rv64_base_cpu_init(Object *obj)
{
-CPURISCVState *env = _CPU(obj)->env;
+RISCVCPU *cpu = RISCV_CPU(obj);
+CPURISCVState *env = >env;
+
+cpu->cfg.mmu = true;
+
/* We set this in the realise function */
riscv_cpu_set_misa(env, MXL_RV64, 0);
/* Set latest version of privileged specification */
@@ -569,13 +575,18 @@ static void rv64_veyron_v1_cpu_init(Object *obj)
static void rv128_base_cpu_init(Object *obj)
{
+RISCVCPU *cpu = RISCV_CPU(obj);
+CPURISCVState *env = >env;
+
if (qemu_tcg_mttcg_enabled()) {
/* Missing 128-bit aligned atomics */
error_report("128-bit RISC-V currently does not work with Multi "
"Threaded TCG. Please use: -accel tcg,thread=single");
exit(EXIT_FAILURE);
}
-CPURISCVState *env = _CPU(obj)->env;
+
+cpu->cfg.mmu = true;
+
/* We set this in the realise function */
riscv_cpu_set_misa(env, MXL_RV128, 0);
/* Set latest version of privileged specification */
@@ -609,7 +620,11 @@ static void rv64i_bare_cpu_init(Object *obj)
#else
static void rv32_base_cpu_init(Object *obj)
{
-CPURISCVState *env = _CPU(obj)->env;
+RISCVCPU *cpu = RISCV_CPU(obj);
+CPURISCVState *env = >env;
+
+cpu->cfg.mmu = true;
+
/* We set this in the realise function */
riscv_cpu_set_misa(env, MXL_RV32, 0);
/* Set latest version of privileged specification */
@@ -1594,8 +1609,38 @@ static const PropertyInfo prop_pmu_mask = {
.set = prop_pmu_mask_set,
};
+static void prop_mmu_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+RISCVCPU *cpu = RISCV_CPU(obj);
+bool value;
+
+visit_type_bool(v, name, , errp);
+
+if (cpu->cfg.mmu != value && riscv_cpu_is_vendor(obj)) {
+cpu_set_prop_err(cpu, "mmu", errp);
+return;
+}
+
+cpu_option_add_user_setting(name, value);
+cpu->cfg.mmu = value;
+}
+
+static void prop_mmu_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+bool value = RISCV_CPU(obj)->cfg.mmu;
+
+visit_type_bool(v, name, , errp);
+}
+
+static const PropertyInfo prop_mmu = {
+.name = "mmu",
+.get = prop_mmu_get,
+.set = prop_mmu_set,
+};
+
Property riscv_cpu_options[] = {
-DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
@@ -1684,6 +1729,8 @@ static Property riscv_cpu_properties[] = {
{.name = "pmu-mask", .info = _pmu_mask},
{.name = "pmu-num", .info = _pmu_num}, /* Deprecated */
+{.name = "mmu", .info = _mmu},
+
#ifndef CONFIG_USER_ONLY
DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
#endif
--
2.43.0
[PATCH v4 09/17] target/riscv: move 'elen' to riscv_cpu_properties[]
Do the same thing we did with 'vlen' in the previous patch with 'elen'.
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
---
target/riscv/cpu.c | 44 --
target/riscv/tcg/tcg-cpu.c | 5 -
2 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index fd55064c3b..2bb4828324 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1309,6 +1309,7 @@ static void riscv_cpu_init(Object *obj)
/* Default values for non-bool cpu properties */
cpu->cfg.pmu_mask = MAKE_64BIT_MASK(3, 16);
cpu->cfg.vlen = 128;
+cpu->cfg.elen = 64;
cpu->env.vext_ver = VEXT_VERSION_1_00_0;
}
@@ -1819,9 +1820,47 @@ static const PropertyInfo prop_vlen = {
.set = prop_vlen_set,
};
-Property riscv_cpu_options[] = {
-DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
+static void prop_elen_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+RISCVCPU *cpu = RISCV_CPU(obj);
+uint16_t value;
+
+if (!visit_type_uint16(v, name, , errp)) {
+return;
+}
+
+if (!is_power_of_2(value)) {
+error_setg(errp, "Vector extension ELEN must be power of 2");
+return;
+}
+
+if (value != cpu->cfg.elen && riscv_cpu_is_vendor(obj)) {
+cpu_set_prop_err(cpu, name, errp);
+error_append_hint(errp, "Current '%s' val: %u\n",
+ name, cpu->cfg.elen);
+return;
+}
+
+cpu_option_add_user_setting(name, value);
+cpu->cfg.elen = value;
+}
+static void prop_elen_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+uint16_t value = RISCV_CPU(obj)->cfg.elen;
+
+visit_type_uint16(v, name, , errp);
+}
+
+static const PropertyInfo prop_elen = {
+.name = "elen",
+.get = prop_elen_get,
+.set = prop_elen_set,
+};
+
+Property riscv_cpu_options[] = {
DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64),
DEFINE_PROP_UINT16("cbop_blocksize", RISCVCPU, cfg.cbop_blocksize, 64),
DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
@@ -1909,6 +1948,7 @@ static Property riscv_cpu_properties[] = {
{.name = "vext_spec", .info = _vext_spec},
{.name = "vlen", .info = _vlen},
+{.name = "elen", .info = _elen},
#ifndef CONFIG_USER_ONLY
DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 71a364453e..c7c2a28f10 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -305,11 +305,6 @@ static void riscv_cpu_validate_v(CPURISCVState *env,
RISCVCPUConfig *cfg,
return;
}
-if (!is_power_of_2(cfg->elen)) {
-error_setg(errp, "Vector extension ELEN must be power of 2");
-return;
-}
-
if (cfg->elen > 64 || cfg->elen < 8) {
error_setg(errp,
"Vector extension implementation only supports ELEN "
--
2.43.0
Re: [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase
On 5/1/24 11:24, Daniel P. Berrangé wrote:
On Thu, Jan 04, 2024 at 01:37:22PM +, inesvarhol wrote:
Le jeudi 4 janvier 2024 à 14:05, Philippe Mathieu-Daudé a
écrit :
Hello,
+static void test_edge_selector(void)
+{
+ enable_nvic_irq(EXTI0_IRQ);
+
+ / Configure EXTI line 0 irq on rising edge */
+ qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",
Markus, this qtest use seems to expect some stability in QOM path...
Inès, Arnaud, having the SoC unattached is dubious, it belongs to
the machine.
Noted, we will fix that.
Should we be concerned about the "stability in QOM path" ?
QTest is a functional test harness that intentionally has knowledge
about QEMU internals.
IOW, usage of particular QOM path in qtest does *not* imply that
QOM path needs to be stable. If QEMU internals change for whatever
reason, it is expected that QTests may need some updates to match.
Good point.
QOM path stability only matters if there's a mgmt app facing use
case, which requires the app to have hardcoded knowledge of the
path.
Even a mgmt app can use unstable QOM paths, provided it has a way
to dynamically detect the path to be used, instead of hardcoding
it.
I can understand this use to lookup "on which CDROM tray is
inserted the blkdrv named FOO", but to find a component on a
well specified system on chip, this is overkill.
None the less, you may still choose to move it out of /unattached
at your discretion.
Yeah we should clean those...
Thanks for clarifying,
Phil.
[PATCH v3 3/5] target/riscv: Add cycle & instret privilege mode filtering definitions
From: Kaiwen Xue
This adds the definitions for ISA extension smcntrpmf.
Signed-off-by: Kaiwen Xue
Signed-off-by: Atish Patra
---
target/riscv/cpu.h | 6 ++
target/riscv/cpu_bits.h | 29 +
2 files changed, 35 insertions(+)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index d74b361be641..34617c4c4bab 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -319,6 +319,12 @@ struct CPUArchState {
target_ulong mcountinhibit;
+/* PMU cycle & instret privilege mode filtering */
+target_ulong mcyclecfg;
+target_ulong mcyclecfgh;
+target_ulong minstretcfg;
+target_ulong minstretcfgh;
+
/* PMU counter state */
PMUCTRState pmu_ctrs[RV_MAX_MHPMCOUNTERS];
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index ebd7917d490a..0ee91e502e8f 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -401,6 +401,10 @@
/* Machine counter-inhibit register */
#define CSR_MCOUNTINHIBIT 0x320
+/* Machine counter configuration registers */
+#define CSR_MCYCLECFG 0x321
+#define CSR_MINSTRETCFG 0x322
+
#define CSR_MHPMEVENT3 0x323
#define CSR_MHPMEVENT4 0x324
#define CSR_MHPMEVENT5 0x325
@@ -431,6 +435,9 @@
#define CSR_MHPMEVENT30 0x33e
#define CSR_MHPMEVENT31 0x33f
+#define CSR_MCYCLECFGH 0x721
+#define CSR_MINSTRETCFGH0x722
+
#define CSR_MHPMEVENT3H 0x723
#define CSR_MHPMEVENT4H 0x724
#define CSR_MHPMEVENT5H 0x725
@@ -885,6 +892,28 @@ typedef enum RISCVException {
/* PMU related bits */
#define MIE_LCOFIE (1 << IRQ_PMU_OVF)
+#define MCYCLECFG_BIT_MINH BIT_ULL(62)
+#define MCYCLECFGH_BIT_MINHBIT(30)
+#define MCYCLECFG_BIT_SINH BIT_ULL(61)
+#define MCYCLECFGH_BIT_SINHBIT(29)
+#define MCYCLECFG_BIT_UINH BIT_ULL(60)
+#define MCYCLECFGH_BIT_UINHBIT(28)
+#define MCYCLECFG_BIT_VSINHBIT_ULL(59)
+#define MCYCLECFGH_BIT_VSINH BIT(27)
+#define MCYCLECFG_BIT_VUINHBIT_ULL(58)
+#define MCYCLECFGH_BIT_VUINH BIT(26)
+
+#define MINSTRETCFG_BIT_MINH BIT_ULL(62)
+#define MINSTRETCFGH_BIT_MINH BIT(30)
+#define MINSTRETCFG_BIT_SINH BIT_ULL(61)
+#define MINSTRETCFGH_BIT_SINH BIT(29)
+#define MINSTRETCFG_BIT_UINH BIT_ULL(60)
+#define MINSTRETCFGH_BIT_UINH BIT(28)
+#define MINSTRETCFG_BIT_VSINH BIT_ULL(59)
+#define MINSTRETCFGH_BIT_VSINH BIT(27)
+#define MINSTRETCFG_BIT_VUINH BIT_ULL(58)
+#define MINSTRETCFGH_BIT_VUINH BIT(26)
+
#define MHPMEVENT_BIT_OF BIT_ULL(63)
#define MHPMEVENTH_BIT_OF BIT(31)
#define MHPMEVENT_BIT_MINH BIT_ULL(62)
--
2.34.1
[PATCH v3 4/5] target/riscv: Add cycle & instret privilege mode filtering support
From: Kaiwen Xue
QEMU only calculates dummy cycles and instructions, so there is no
actual means to stop the icount in QEMU. Hence this patch merely adds
the functionality of accessing the cfg registers, and cause no actual
effects on the counting of cycle and instret counters.
Signed-off-by: Atish Patra
Signed-off-by: Kaiwen Xue
---
target/riscv/csr.c | 80 ++
1 file changed, 80 insertions(+)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 283468bbc652..3bd4aa22374f 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -233,6 +233,24 @@ static RISCVException sscofpmf_32(CPURISCVState *env, int
csrno)
return sscofpmf(env, csrno);
}
+static RISCVException smcntrpmf(CPURISCVState *env, int csrno)
+{
+if (!riscv_cpu_cfg(env)->ext_smcntrpmf) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException smcntrpmf_32(CPURISCVState *env, int csrno)
+{
+if (riscv_cpu_mxl(env) != MXL_RV32) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+return smcntrpmf(env, csrno);
+}
+
static RISCVException any(CPURISCVState *env, int csrno)
{
return RISCV_EXCP_NONE;
@@ -818,6 +836,54 @@ static int read_hpmcounterh(CPURISCVState *env, int csrno,
target_ulong *val)
#else /* CONFIG_USER_ONLY */
+static int read_mcyclecfg(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->mcyclecfg;
+return RISCV_EXCP_NONE;
+}
+
+static int write_mcyclecfg(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->mcyclecfg = val;
+return RISCV_EXCP_NONE;
+}
+
+static int read_mcyclecfgh(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->mcyclecfgh;
+return RISCV_EXCP_NONE;
+}
+
+static int write_mcyclecfgh(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->mcyclecfgh = val;
+return RISCV_EXCP_NONE;
+}
+
+static int read_minstretcfg(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->minstretcfg;
+return RISCV_EXCP_NONE;
+}
+
+static int write_minstretcfg(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->minstretcfg = val;
+return RISCV_EXCP_NONE;
+}
+
+static int read_minstretcfgh(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->minstretcfgh;
+return RISCV_EXCP_NONE;
+}
+
+static int write_minstretcfgh(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->minstretcfgh = val;
+return RISCV_EXCP_NONE;
+}
+
static int read_mhpmevent(CPURISCVState *env, int csrno, target_ulong *val)
{
int evt_index = csrno - CSR_MCOUNTINHIBIT;
@@ -4922,6 +4988,13 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
write_mcountinhibit,
.min_priv_ver = PRIV_VERSION_1_11_0 },
+[CSR_MCYCLECFG] = { "mcyclecfg", smcntrpmf, read_mcyclecfg,
+ write_mcyclecfg,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+[CSR_MINSTRETCFG]= { "minstretcfg", smcntrpmf, read_minstretcfg,
+ write_minstretcfg,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
+
[CSR_MHPMEVENT3] = { "mhpmevent3", any,read_mhpmevent,
write_mhpmevent },
[CSR_MHPMEVENT4] = { "mhpmevent4", any,read_mhpmevent,
@@ -4981,6 +5054,13 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_MHPMEVENT31]= { "mhpmevent31",any,read_mhpmevent,
write_mhpmevent },
+[CSR_MCYCLECFGH] = { "mcyclecfgh", smcntrpmf_32, read_mcyclecfgh,
+ write_mcyclecfgh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
+[CSR_MINSTRETCFGH] = { "minstretcfgh", smcntrpmf_32, read_minstretcfgh,
+ write_minstretcfgh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
+
[CSR_MHPMEVENT3H]= { "mhpmevent3h",sscofpmf_32, read_mhpmeventh,
write_mhpmeventh,
.min_priv_ver = PRIV_VERSION_1_12_0},
--
2.34.1
Re: [v2 3/5] target/riscv: Add cycle & instret privilege mode filtering definitions
On Thu, Jan 4, 2024 at 6:46 PM Alistair Francis wrote:
>
> On Fri, Dec 29, 2023 at 12:08 PM Atish Patra wrote:
> >
> > From: Kaiwen Xue
> >
> > This adds the definitions for ISA extension smcntrpmf.
> >
> > Signed-off-by: Kaiwen Xue
> > Signed-off-by: Atish Patra
> > ---
> > target/riscv/cpu.c | 1 -
> > target/riscv/cpu.h | 6 ++
> > target/riscv/cpu_bits.h | 29 +
> > 3 files changed, 35 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index da3f05cd5373..54395f95b299 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -1297,7 +1297,6 @@ const char *riscv_get_misa_ext_description(uint32_t
> > bit)
> > const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
> > /* Defaults for standard extensions */
> > MULTI_EXT_CFG_BOOL("sscofpmf", ext_sscofpmf, false),
> > -DEFINE_PROP_BOOL("smcntrpmf", RISCVCPU, cfg.ext_smcntrpmf, false),
>
> Can you explain why you are removing this in the commit message?
>
That is just a rebasing error I overlooked at first attempt. I fixed
it and sent a v3. Sorry for the confusion.
> Alistair
>
> > MULTI_EXT_CFG_BOOL("zifencei", ext_zifencei, true),
> > MULTI_EXT_CFG_BOOL("zicsr", ext_zicsr, true),
> > MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index d74b361be641..34617c4c4bab 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -319,6 +319,12 @@ struct CPUArchState {
> >
> > target_ulong mcountinhibit;
> >
> > +/* PMU cycle & instret privilege mode filtering */
> > +target_ulong mcyclecfg;
> > +target_ulong mcyclecfgh;
> > +target_ulong minstretcfg;
> > +target_ulong minstretcfgh;
> > +
> > /* PMU counter state */
> > PMUCTRState pmu_ctrs[RV_MAX_MHPMCOUNTERS];
> >
> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > index ebd7917d490a..0ee91e502e8f 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -401,6 +401,10 @@
> > /* Machine counter-inhibit register */
> > #define CSR_MCOUNTINHIBIT 0x320
> >
> > +/* Machine counter configuration registers */
> > +#define CSR_MCYCLECFG 0x321
> > +#define CSR_MINSTRETCFG 0x322
> > +
> > #define CSR_MHPMEVENT3 0x323
> > #define CSR_MHPMEVENT4 0x324
> > #define CSR_MHPMEVENT5 0x325
> > @@ -431,6 +435,9 @@
> > #define CSR_MHPMEVENT30 0x33e
> > #define CSR_MHPMEVENT31 0x33f
> >
> > +#define CSR_MCYCLECFGH 0x721
> > +#define CSR_MINSTRETCFGH0x722
> > +
> > #define CSR_MHPMEVENT3H 0x723
> > #define CSR_MHPMEVENT4H 0x724
> > #define CSR_MHPMEVENT5H 0x725
> > @@ -885,6 +892,28 @@ typedef enum RISCVException {
> > /* PMU related bits */
> > #define MIE_LCOFIE (1 << IRQ_PMU_OVF)
> >
> > +#define MCYCLECFG_BIT_MINH BIT_ULL(62)
> > +#define MCYCLECFGH_BIT_MINHBIT(30)
> > +#define MCYCLECFG_BIT_SINH BIT_ULL(61)
> > +#define MCYCLECFGH_BIT_SINHBIT(29)
> > +#define MCYCLECFG_BIT_UINH BIT_ULL(60)
> > +#define MCYCLECFGH_BIT_UINHBIT(28)
> > +#define MCYCLECFG_BIT_VSINHBIT_ULL(59)
> > +#define MCYCLECFGH_BIT_VSINH BIT(27)
> > +#define MCYCLECFG_BIT_VUINHBIT_ULL(58)
> > +#define MCYCLECFGH_BIT_VUINH BIT(26)
> > +
> > +#define MINSTRETCFG_BIT_MINH BIT_ULL(62)
> > +#define MINSTRETCFGH_BIT_MINH BIT(30)
> > +#define MINSTRETCFG_BIT_SINH BIT_ULL(61)
> > +#define MINSTRETCFGH_BIT_SINH BIT(29)
> > +#define MINSTRETCFG_BIT_UINH BIT_ULL(60)
> > +#define MINSTRETCFGH_BIT_UINH BIT(28)
> > +#define MINSTRETCFG_BIT_VSINH BIT_ULL(59)
> > +#define MINSTRETCFGH_BIT_VSINH BIT(27)
> > +#define MINSTRETCFG_BIT_VUINH BIT_ULL(58)
> > +#define MINSTRETCFGH_BIT_VUINH BIT(26)
> > +
> > #define MHPMEVENT_BIT_OF BIT_ULL(63)
> > #define MHPMEVENTH_BIT_OF BIT(31)
> > #define MHPMEVENT_BIT_MINH BIT_ULL(62)
> > --
> > 2.34.1
> >
> >
[PATCH v3 2/5] target/riscv: Add cycle & instret privilege mode filtering properties
From: Kaiwen Xue
This adds the properties for ISA extension smcntrpmf. Patches
implementing it will follow.
Signed-off-by: Atish Patra
Signed-off-by: Kaiwen Xue
---
target/riscv/cpu.c | 2 ++
target/riscv/cpu_cfg.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 83c7c0cf07be..ea34ff2ae983 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -148,6 +148,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
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(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf),
+ISA_EXT_DATA_ENTRY(smcntrpmf, PRIV_VERSION_1_12_0, ext_smcntrpmf),
ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
ISA_EXT_DATA_ENTRY(svadu, PRIV_VERSION_1_12_0, ext_svadu),
ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval),
@@ -1296,6 +1297,7 @@ const char *riscv_get_misa_ext_description(uint32_t bit)
const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
/* Defaults for standard extensions */
MULTI_EXT_CFG_BOOL("sscofpmf", ext_sscofpmf, false),
+MULTI_EXT_CFG_BOOL("smcntrpmf", ext_smcntrpmf, false),
MULTI_EXT_CFG_BOOL("zifencei", ext_zifencei, true),
MULTI_EXT_CFG_BOOL("zicsr", ext_zicsr, true),
MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index f4605fb190b9..00c34fdd3209 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -72,6 +72,7 @@ struct RISCVCPUConfig {
bool ext_zihpm;
bool ext_smstateen;
bool ext_sstc;
+bool ext_smcntrpmf;
bool ext_svadu;
bool ext_svinval;
bool ext_svnapot;
--
2.34.1
[PATCH v3 0/5] Add ISA extension smcntrpmf support
This patch series adds the support for RISC-V ISA extension smcntrpmf (cycle and privilege mode filtering) [1]. It is based on Kevin's earlier work but improves it by actually implement privilege mode filtering by tracking the privilege mode switches. This enables the privilege mode filtering for mhpmcounters as well. However, Smcntrpmf/Sscofpmf must be enabled to leverage this. This series also modified to report the raw instruction count instead of virtual cpu time based on the instruction count when icount is enabled. The former seems to be the preferred approach for instruction count for other architectures as well. Please let me know if anybody thinks that's incorrect. The series is also available at Changes from v2->v3: 1. Fixed the rebasing error in PATCH2. 2. Added RB tags. 3. Addressed other review comments. Changes from v1->v2: 1. Implemented actual mode filtering for both icount and host ticks mode. 1. Addressed comments in v1. 2. Added Kevin's personal email address. [1] https://github.com/riscv/riscv-smcntrpmf [2] https://github.com/atishp04/qemu/tree/smcntrpmf_v3 Atish Patra (2): target/riscv: Fix the predicate functions for mhpmeventhX CSRs target/riscv: Implement privilege mode filtering for cycle/instret Kaiwen Xue (3): target/riscv: Add cycle & instret privilege mode filtering properties target/riscv: Add cycle & instret privilege mode filtering definitions target/riscv: Add cycle & instret privilege mode filtering support target/riscv/cpu.c| 2 + target/riscv/cpu.h| 17 +++ target/riscv/cpu_bits.h | 29 + target/riscv/cpu_cfg.h| 1 + target/riscv/cpu_helper.c | 9 +- target/riscv/csr.c| 242 ++ target/riscv/pmu.c| 43 +++ target/riscv/pmu.h| 2 + 8 files changed, 292 insertions(+), 53 deletions(-) -- 2.34.1
[PATCH v3 5/5] target/riscv: Implement privilege mode filtering for cycle/instret
Privilege mode filtering can also be emulated for cycle/instret by
tracking host_ticks/icount during each privilege mode switch. This
patch implements that for both cycle/instret and mhpmcounters. The
first one requires Smcntrpmf while the other one requires Sscofpmf
to be enabled.
The cycle/instret are still computed using host ticks when icount
is not enabled. Otherwise, they are computed using raw icount which
is more accurate in icount mode.
Reviewed-by: Daniel Henrique Barboza
Signed-off-by: Atish Patra
---
target/riscv/cpu.h| 11 +
target/riscv/cpu_helper.c | 9 +++-
target/riscv/csr.c| 95 ++-
target/riscv/pmu.c| 43 ++
target/riscv/pmu.h| 2 +
5 files changed, 136 insertions(+), 24 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 34617c4c4bab..40d10726155b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -136,6 +136,15 @@ typedef struct PMUCTRState {
target_ulong irq_overflow_left;
} PMUCTRState;
+typedef struct PMUFixedCtrState {
+/* Track cycle and icount for each privilege mode */
+uint64_t counter[4];
+uint64_t counter_prev[4];
+/* Track cycle and icount for each privilege mode when V = 1*/
+uint64_t counter_virt[2];
+uint64_t counter_virt_prev[2];
+} PMUFixedCtrState;
+
struct CPUArchState {
target_ulong gpr[32];
target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */
@@ -334,6 +343,8 @@ struct CPUArchState {
/* PMU event selector configured values for RV32 */
target_ulong mhpmeventh_val[RV_MAX_MHPMEVENTS];
+PMUFixedCtrState pmu_fixed_ctrs[2];
+
target_ulong sscratch;
target_ulong mscratch;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index e7e23b34f455..3dddb1b433e8 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -715,8 +715,13 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong
newpriv)
{
g_assert(newpriv <= PRV_M && newpriv != PRV_RESERVED);
-if (icount_enabled() && newpriv != env->priv) {
-riscv_itrigger_update_priv(env);
+if (newpriv != env->priv) {
+if (icount_enabled()) {
+riscv_itrigger_update_priv(env);
+riscv_pmu_icount_update_priv(env, newpriv);
+} else {
+riscv_pmu_cycle_update_priv(env, newpriv);
+}
}
/* tlb_flush is unnecessary as mode is contained in mmu_idx */
env->priv = newpriv;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 3bd4aa22374f..307d052021c5 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -782,32 +782,16 @@ static int write_vcsr(CPURISCVState *env, int csrno,
target_ulong val)
return RISCV_EXCP_NONE;
}
+#if defined(CONFIG_USER_ONLY)
/* User Timers and Counters */
static target_ulong get_ticks(bool shift)
{
-int64_t val;
-target_ulong result;
-
-#if !defined(CONFIG_USER_ONLY)
-if (icount_enabled()) {
-val = icount_get();
-} else {
-val = cpu_get_host_ticks();
-}
-#else
-val = cpu_get_host_ticks();
-#endif
-
-if (shift) {
-result = val >> 32;
-} else {
-result = val;
-}
+int64_t val = cpu_get_host_ticks();
+target_ulong result = shift ? val >> 32 : val;
return result;
}
-#if defined(CONFIG_USER_ONLY)
static RISCVException read_time(CPURISCVState *env, int csrno,
target_ulong *val)
{
@@ -932,6 +916,70 @@ static int write_mhpmeventh(CPURISCVState *env, int csrno,
target_ulong val)
return RISCV_EXCP_NONE;
}
+static target_ulong riscv_pmu_ctr_get_fixed_counters_val(CPURISCVState *env,
+ int counter_idx,
+ bool upper_half)
+{
+uint64_t curr_val = 0;
+target_ulong result = 0;
+uint64_t *counter_arr = icount_enabled() ? env->pmu_fixed_ctrs[1].counter :
+env->pmu_fixed_ctrs[0].counter;
+uint64_t *counter_arr_virt = icount_enabled() ?
+ env->pmu_fixed_ctrs[1].counter_virt :
+ env->pmu_fixed_ctrs[0].counter_virt;
+uint64_t cfg_val = 0;
+
+if (counter_idx == 0) {
+cfg_val = upper_half ? ((uint64_t)env->mcyclecfgh << 32) :
+ env->mcyclecfg;
+} else if (counter_idx == 2) {
+cfg_val = upper_half ? ((uint64_t)env->minstretcfgh << 32) :
+ env->minstretcfg;
+} else {
+cfg_val = upper_half ?
+ ((uint64_t)env->mhpmeventh_val[counter_idx] << 32) :
+ env->mhpmevent_val[counter_idx];
+}
+
+if (!cfg_val) {
+if (icount_enabled()) {
+curr_val = icount_get_raw();
+} else {
+curr_val = cpu_get_host_ticks();
+}
+goto done;
+}
+
+if (!(cfg_val &
[PATCH v3 1/5] target/riscv: Fix the predicate functions for mhpmeventhX CSRs
mhpmeventhX CSRs are available for RV32. The predicate function
should check that first before checking sscofpmf extension.
Fixes: 14664483457b ("target/riscv: Add sscofpmf extension support")
Reviewed-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
Signed-off-by: Atish Patra
---
target/riscv/csr.c | 67 ++
1 file changed, 38 insertions(+), 29 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index fde7ce1a5336..283468bbc652 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -224,6 +224,15 @@ static RISCVException sscofpmf(CPURISCVState *env, int
csrno)
return RISCV_EXCP_NONE;
}
+static RISCVException sscofpmf_32(CPURISCVState *env, int csrno)
+{
+if (riscv_cpu_mxl(env) != MXL_RV32) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+return sscofpmf(env, csrno);
+}
+
static RISCVException any(CPURISCVState *env, int csrno)
{
return RISCV_EXCP_NONE;
@@ -4972,91 +4981,91 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_MHPMEVENT31]= { "mhpmevent31",any,read_mhpmevent,
write_mhpmevent },
-[CSR_MHPMEVENT3H]= { "mhpmevent3h",sscofpmf, read_mhpmeventh,
+[CSR_MHPMEVENT3H]= { "mhpmevent3h",sscofpmf_32, read_mhpmeventh,
write_mhpmeventh,
.min_priv_ver = PRIV_VERSION_1_12_0},
-[CSR_MHPMEVENT4H]= { "mhpmevent4h",sscofpmf, read_mhpmeventh,
+[CSR_MHPMEVENT4H]= { "mhpmevent4h",sscofpmf_32, read_mhpmeventh,
write_mhpmeventh,
.min_priv_ver = PRIV_VERSION_1_12_0},
-[CSR_MHPMEVENT5H]= { "mhpmevent5h",sscofpmf, read_mhpmeventh,
+[CSR_MHPMEVENT5H]= { "mhpmevent5h",sscofpmf_32, read_mhpmeventh,
write_mhpmeventh,
.min_priv_ver = PRIV_VERSION_1_12_0},
-[CSR_MHPMEVENT6H]= { "mhpmevent6h",sscofpmf, read_mhpmeventh,
+[CSR_MHPMEVENT6H]= { "mhpmevent6h",sscofpmf_32, read_mhpmeventh,
write_mhpmeventh,
.min_priv_ver = PRIV_VERSION_1_12_0},
-[CSR_MHPMEVENT7H]= { "mhpmevent7h",sscofpmf, read_mhpmeventh,
+[CSR_MHPMEVENT7H]= { "mhpmevent7h",sscofpmf_32, read_mhpmeventh,
write_mhpmeventh,
.min_priv_ver = PRIV_VERSION_1_12_0},
-[CSR_MHPMEVENT8H]= { "mhpmevent8h",sscofpmf, read_mhpmeventh,
+[CSR_MHPMEVENT8H]= { "mhpmevent8h",sscofpmf_32, read_mhpmeventh,
write_mhpmeventh,
.min_priv_ver = PRIV_VERSION_1_12_0},
-[CSR_MHPMEVENT9H]= { "mhpmevent9h",sscofpmf, read_mhpmeventh,
+[CSR_MHPMEVENT9H]= { "mhpmevent9h",sscofpmf_32, read_mhpmeventh,
write_mhpmeventh,
.min_priv_ver = PRIV_VERSION_1_12_0},
-[CSR_MHPMEVENT10H] = { "mhpmevent10h",sscofpmf, read_mhpmeventh,
+[CSR_MHPMEVENT10H] = { "mhpmevent10h",sscofpmf_32, read_mhpmeventh,
write_mhpmeventh,
.min_priv_ver = PRIV_VERSION_1_12_0},
-[CSR_MHPMEVENT11H] = { "mhpmevent11h",sscofpmf, read_mhpmeventh,
+[CSR_MHPMEVENT11H] = { "mhpmevent11h",sscofpmf_32, read_mhpmeventh,
write_mhpmeventh,
.min_priv_ver = PRIV_VERSION_1_12_0},
-[CSR_MHPMEVENT12H] = { "mhpmevent12h",sscofpmf, read_mhpmeventh,
+[CSR_MHPMEVENT12H] = { "mhpmevent12h",sscofpmf_32, read_mhpmeventh,
write_mhpmeventh,
.min_priv_ver = PRIV_VERSION_1_12_0},
-[CSR_MHPMEVENT13H] = { "mhpmevent13h",sscofpmf, read_mhpmeventh,
+[CSR_MHPMEVENT13H] = { "mhpmevent13h",sscofpmf_32, read_mhpmeventh,
write_mhpmeventh,
.min_priv_ver = PRIV_VERSION_1_12_0},
-[CSR_MHPMEVENT14H] = { "mhpmevent14h",sscofpmf, read_mhpmeventh,
+[CSR_MHPMEVENT14H] = { "mhpmevent14h",sscofpmf_32, read_mhpmeventh,
write_mhpmeventh,
.min_priv_ver = PRIV_VERSION_1_12_0},
-[CSR_MHPMEVENT15H] = { "mhpmevent15h",sscofpmf, read_mhpmeventh,
+[CSR_MHPMEVENT15H] = { "mhpmevent15h",sscofpmf_32, read_mhpmeventh,
write_mhpmeventh,
.min_priv_ver = PRIV_VERSION_1_12_0},
-[CSR_MHPMEVENT16H] = { "mhpmevent16h",sscofpmf, read_mhpmeventh,
+[CSR_MHPMEVENT16H] = { "mhpmevent16h",sscofpmf_32, read_mhpmeventh,
Re: [PATCH v9 0/9] Unified CPU type check
Hi Gavin, On 13/12/23 11:54, Gavin Shan wrote: On 12/13/23 20:08, Philippe Mathieu-Daudé wrote: On 12/12/23 05:55, Gavin Shan wrote: On 12/4/23 10:47, Gavin Shan wrote: This series bases on Phil's repository because the prepatory commits have been queued to the branch. https://gitlab.com/philmd/qemu.git (branch: cpus-next) There are two places where the user specified CPU type is checked to see if it's supported or allowed by the board: machine_run_board_init() and mc->init(). We don't have to maintain two duplicate sets of logic. This series intends to move the check to machine_run_board_init() so that we have unified CPU type check. This series can be checked out from: g...@github.com:gwshan/qemu.git (branch: kvm/cpu-type) PATCH[1-4] refactors and improves the logic to validate CPU type in machine_run_board_init() PATCH[5-9] validates the CPU type in machine_run_board_init() for the individual boards v6: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg00768.html v7: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01045.html v8: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01168.html Ping to see if there is a chance to queue it up before the Chrismas? :) Series queued. "Before" Christmas will depend on the final release tag. Thanks for the various iterations, Phil, thank you for you continuous reviews and valuable comments. Yes, the final merge to master branch depends on the release plan. 'queue' meant to merge the series to your 'cpus-next' branch ;-) I had to fix 3 different issues caught by our CI. Next time please run your series on GitLab CI, you just have to push your branch and wait for the result :) Now merged as 445946f4dd..cd75cc6337. Happy new year!
Re: [PATCH v9 3/9] machine: Improve is_cpu_type_supported()
On 4/12/23 01:47, Gavin Shan wrote:
It's no sense to check the CPU type when mc->valid_cpu_types[0] is
NULL, which is a program error. Raise an assert on this.
A precise hint for the error message is given when mc->valid_cpu_types[0]
is the only valid entry. Besides, enumeration on mc->valid_cpu_types[0]
when we have mutiple valid entries there is avoided to increase the code
readability, as suggested by Philippe Mathieu-Daudé.
Besides, @cc comes from machine->cpu_type or mc->default_cpu_type. For
the later case, it can be NULL and it's also a program error. We should
use assert() in this case.
Signed-off-by: Gavin Shan
v9: assert(mc->valid_cpu_types[0] != NULL) (Phil)
assert(cc != NULL); (Phil)
---
hw/core/machine.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 1797e002f9..4ae9aaee8e 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1400,6 +1400,7 @@ static bool is_cpu_type_supported(const MachineState
*machine, Error **errp)
* type is provided through '-cpu' option.
*/
if (mc->valid_cpu_types && machine->cpu_type) {
+assert(mc->valid_cpu_types[0] != NULL);
for (i = 0; mc->valid_cpu_types[i]; i++) {
if (object_class_dynamic_cast(oc, mc->valid_cpu_types[i])) {
break;
@@ -1409,20 +1410,27 @@ static bool is_cpu_type_supported(const MachineState
*machine, Error **errp)
/* The user specified CPU type isn't valid */
if (!mc->valid_cpu_types[i]) {
error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
-error_append_hint(errp, "The valid types are: %s",
- mc->valid_cpu_types[0]);
-for (i = 1; mc->valid_cpu_types[i]; i++) {
-error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
+if (!mc->valid_cpu_types[1]) {
+error_append_hint(errp, "The only valid type is: %s\n",
+ mc->valid_cpu_types[0]);
+} else {
+error_append_hint(errp, "The valid types are: ");
+for (i = 0; mc->valid_cpu_types[i]; i++) {
+error_append_hint(errp, "%s%s",
+ mc->valid_cpu_types[i],
+ mc->valid_cpu_types[i + 1] ? ", " : "");
+}
+error_append_hint(errp, "\n");
}
-error_append_hint(errp, "\n");
return false;
}
}
/* Check if CPU type is deprecated and warn if so */
cc = CPU_CLASS(oc);
-if (cc && cc->deprecation_note) {
+assert(cc != NULL);
+if (cc->deprecation_note) {
warn_report("CPU model %s is deprecated -- %s",
machine->cpu_type, cc->deprecation_note);
}
Since we were getting:
$ qemu-system-s390x -M none
QEMU 8.2.50 monitor - type 'help' for more information
qemu-system-s390x: ../../hw/core/machine.c:1444: _Bool
is_cpu_type_supported(const MachineState *, Error **): Assertion `cc !=
NULL' failed.
Aborted (core dumped)
I added a check on mc->valid_cpu_types before calling
is_cpu_type_supported() in the previous patch. See commit acbadc5a29.
Re: [PATCH] hw/audio/sb16: Do not migrate qdev properties
On 5/1/24 02:59, Peter Xu wrote: On Thu, Jan 04, 2024 at 05:48:18PM +0100, Philippe Mathieu-Daudé wrote: If there are no objections I'll queue this patch (fixing the typo reported by Zoltan). Yes feel free to. Thanks, Thanks, merged as commit fa293f8125.
Re: [PATCH] hw/m68k/mcf5206: Embed m5206_timer_state in m5206_mbar_state
On 21/12/23 13:29, Thomas Huth wrote: There's no need to explicitely allocate the memory here, we can simply embed it into the m5206_mbar_state instead. Signed-off-by: Thomas Huth --- hw/m68k/mcf5206.c | 20 1 file changed, 8 insertions(+), 12 deletions(-) Patch merged as commit 5e077a7767.
Re: [PATCH v2] hw/net/can/sja1000: fix bug for single acceptance filter and standard frame
On 4/1/24 00:14, Pavel Pisa wrote:
From: Pavel Pisa
A CAN sja1000 standard frame filter mask has been computed and applied
incorrectly for standard frames when single Acceptance Filter Mode
(MOD_AFM = 1) has been selected. The problem has not been found
by Linux kernel testing because it uses dual filter mode (MOD_AFM = 0)
and leaves falters fully open.
The problem has been noticed by Grant Ramsay when testing with Zephyr
RTOS which uses single filter mode.
Signed-off-by: Pavel Pisa
Reported-by: Grant Ramsay
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2028
Fixes: 733210e754 ("hw/net/can: SJA1000 chip register level emulation")
---
hw/net/can/can_sja1000.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Patch merged as commit 25145a7d77.
Re: [PATCH 3/5] migration: Introduce unimplemented 'qatzip' compression method
Bryan Zhang writes:
+cc Yuan Liu, Daniel Berrangé
> Adds support for 'qatzip' as an option for the multifd compression
> method parameter, but copy-pastes the no-op logic to leave the actual
> methods effectively unimplemented. This is in preparation of a
> subsequent commit that will implement actually using QAT for compression
> and decompression.
>
> Signed-off-by: Bryan Zhang
> Signed-off-by: Hao Xiang
> ---
> hw/core/qdev-properties-system.c | 6 ++-
> migration/meson.build| 1 +
> migration/multifd-qatzip.c | 81
> migration/multifd.h | 1 +
> qapi/migration.json | 5 +-
> 5 files changed, 92 insertions(+), 2 deletions(-)
> create mode 100644 migration/multifd-qatzip.c
>
> diff --git a/hw/core/qdev-properties-system.c
> b/hw/core/qdev-properties-system.c
> index 1a396521d5..d8e48dcb0e 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -658,7 +658,11 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
> const PropertyInfo qdev_prop_multifd_compression = {
> .name = "MultiFDCompression",
> .description = "multifd_compression values, "
> - "none/zlib/zstd",
> + "none/zlib/zstd"
> +#ifdef CONFIG_QATZIP
> + "/qatzip"
> +#endif
> + ,
> .enum_table = _lookup,
> .get = qdev_propinfo_get_enum,
> .set = qdev_propinfo_set_enum,
> diff --git a/migration/meson.build b/migration/meson.build
> index 92b1cc4297..e20f318379 100644
> --- a/migration/meson.build
> +++ b/migration/meson.build
> @@ -40,6 +40,7 @@ if get_option('live_block_migration').allowed()
>system_ss.add(files('block.c'))
> endif
> system_ss.add(when: zstd, if_true: files('multifd-zstd.c'))
> +system_ss.add(when: qatzip, if_true: files('multifd-qatzip.c'))
>
> specific_ss.add(when: 'CONFIG_SYSTEM_ONLY',
> if_true: files('ram.c',
> diff --git a/migration/multifd-qatzip.c b/migration/multifd-qatzip.c
> new file mode 100644
> index 00..1733bbddb7
> --- /dev/null
> +++ b/migration/multifd-qatzip.c
> @@ -0,0 +1,81 @@
> +/*
> + * Multifd QATzip compression implementation
> + *
> + * Copyright (c) Bytedance
> + *
> + * Authors:
> + * Bryan Zhang
> + * Hao Xiang
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "exec/ramblock.h"
> +#include "exec/target_page.h"
> +#include "qapi/error.h"
> +#include "migration.h"
> +#include "options.h"
> +#include "multifd.h"
> +
> +static int qatzip_send_setup(MultiFDSendParams *p, Error **errp)
> +{
> +return 0;
> +}
> +
> +static void qatzip_send_cleanup(MultiFDSendParams *p, Error **errp) {};
> +
> +static int qatzip_send_prepare(MultiFDSendParams *p, Error **errp)
> +{
> +MultiFDPages_t *pages = p->pages;
> +
> +for (int i = 0; i < p->normal_num; i++) {
> +p->iov[p->iovs_num].iov_base = pages->block->host + p->normal[i];
> +p->iov[p->iovs_num].iov_len = p->page_size;
> +p->iovs_num++;
> +}
> +
> +p->next_packet_size = p->normal_num * p->page_size;
> +p->flags |= MULTIFD_FLAG_NOCOMP;
> +return 0;
> +}
> +
> +static int qatzip_recv_setup(MultiFDRecvParams *p, Error **errp)
> +{
> +return 0;
> +}
> +
> +static void qatzip_recv_cleanup(MultiFDRecvParams *p) {};
> +
> +static int qatzip_recv_pages(MultiFDRecvParams *p, Error **errp)
> +{
> +uint32_t flags = p->flags & MULTIFD_FLAG_COMPRESSION_MASK;
> +
> +if (flags != MULTIFD_FLAG_NOCOMP) {
> +error_setg(errp, "multifd %u: flags received %x flags expected %x",
> + p->id, flags, MULTIFD_FLAG_NOCOMP);
> +return -1;
> +}
> +for (int i = 0; i < p->normal_num; i++) {
> +p->iov[i].iov_base = p->host + p->normal[i];
> +p->iov[i].iov_len = p->page_size;
> +}
> +return qio_channel_readv_all(p->c, p->iov, p->normal_num, errp);
> +}
> +
> +static MultiFDMethods multifd_qatzip_ops = {
> +.send_setup = qatzip_send_setup,
> +.send_cleanup = qatzip_send_cleanup,
> +.send_prepare = qatzip_send_prepare,
> +.recv_setup = qatzip_recv_setup,
> +.recv_cleanup = qatzip_recv_cleanup,
> +.recv_pages = qatzip_recv_pages
> +};
> +
> +static void multifd_qatzip_register(void)
> +{
> +multifd_register_ops(MULTIFD_COMPRESSION_QATZIP, _qatzip_ops);
> +}
> +
> +migration_init(multifd_qatzip_register);
> diff --git a/migration/multifd.h b/migration/multifd.h
> index a835643b48..5600f7fc82 100644
> --- a/migration/multifd.h
> +++ b/migration/multifd.h
> @@ -33,6 +33,7 @@ int multifd_queue_page(QEMUFile *f, RAMBlock *block,
> ram_addr_t offset);
> #define MULTIFD_FLAG_NOCOMP (0 << 1)
> #define MULTIFD_FLAG_ZLIB (1 << 1)
> #define MULTIFD_FLAG_ZSTD (2 << 1)
> +#define MULTIFD_FLAG_QATZIP (3 << 1)
>
> /* This value needs to be a multiple of
[PULL 2/6] chardev/char.c: fix "abstract device type" error message
Current error message:
qemu-system-x86_64: -chardev spice,id=foo: Parameter 'driver' expects an
abstract device type
while in fact the meaning is in reverse, -chardev expects
a non-abstract device type.
Fixes: 777357d758d9 ("chardev: qom-ify" 2016-12-07)
Signed-off-by: Michael Tokarev
Reviewed-by: Zhao Liu
---
chardev/char.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/chardev/char.c b/chardev/char.c
index 996a024c7a..119b548784 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -518,7 +518,7 @@ static const ChardevClass *char_get_class(const char
*driver, Error **errp)
if (object_class_is_abstract(oc)) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "driver",
- "an abstract device type");
+ "a non-abstract device type");
return NULL;
}
--
2.39.2
[PULL 1/6] target/riscv: Fix mcycle/minstret increment behavior
From: Xu Lu
The mcycle/minstret counter's stop flag is mistakenly updated on a copy
on stack. Thus the counter increments even when the CY/IR bit in the
mcountinhibit register is set. This commit corrects its behavior.
Fixes: 3780e33732f88 (target/riscv: Support mcycle/minstret write operation)
Signed-off-by: Xu Lu
Reviewed-by: Daniel Henrique Barboza
Signed-off-by: Michael Tokarev
---
target/riscv/csr.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index fde7ce1a53..c50a33397c 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -907,11 +907,11 @@ static int write_mhpmcounterh(CPURISCVState *env, int
csrno, target_ulong val)
static RISCVException riscv_pmu_read_ctr(CPURISCVState *env, target_ulong *val,
bool upper_half, uint32_t ctr_idx)
{
-PMUCTRState counter = env->pmu_ctrs[ctr_idx];
-target_ulong ctr_prev = upper_half ? counter.mhpmcounterh_prev :
- counter.mhpmcounter_prev;
-target_ulong ctr_val = upper_half ? counter.mhpmcounterh_val :
-counter.mhpmcounter_val;
+PMUCTRState *counter = >pmu_ctrs[ctr_idx];
+target_ulong ctr_prev = upper_half ? counter->mhpmcounterh_prev :
+ counter->mhpmcounter_prev;
+target_ulong ctr_val = upper_half ? counter->mhpmcounterh_val :
+counter->mhpmcounter_val;
if (get_field(env->mcountinhibit, BIT(ctr_idx))) {
/*
@@ -919,12 +919,12 @@ static RISCVException riscv_pmu_read_ctr(CPURISCVState
*env, target_ulong *val,
* stop the icount counting. Just return the counter value written by
* the supervisor to indicate that counter was not incremented.
*/
-if (!counter.started) {
+if (!counter->started) {
*val = ctr_val;
return RISCV_EXCP_NONE;
} else {
/* Mark that the counter has been stopped */
-counter.started = false;
+counter->started = false;
}
}
--
2.39.2
[PULL 6/6] docs: use "buses" rather than "busses"
From: Samuel Tardieu If "busses" might be encountered as a plural of "bus" (5 instances), the correct spelling is "buses" (26 instances). Fixing those 5 instances makes the doc more consistent. Signed-off-by: Samuel Tardieu Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Pavel Pisa Reviewed-by: Zhao Liu Signed-off-by: Michael Tokarev --- docs/system/arm/palm.rst| 2 +- docs/system/arm/xscale.rst | 2 +- docs/system/devices/can.rst | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/system/arm/palm.rst b/docs/system/arm/palm.rst index 47ff9b36d4..61bc8d34f4 100644 --- a/docs/system/arm/palm.rst +++ b/docs/system/arm/palm.rst @@ -14,7 +14,7 @@ following elements: - On-chip Real Time Clock - TI TSC2102i touchscreen controller / analog-digital converter / - Audio CODEC, connected through MicroWire and |I2S| busses + Audio CODEC, connected through MicroWire and |I2S| buses - GPIO-connected matrix keypad diff --git a/docs/system/arm/xscale.rst b/docs/system/arm/xscale.rst index d2d5949e10..e239136c3c 100644 --- a/docs/system/arm/xscale.rst +++ b/docs/system/arm/xscale.rst @@ -32,4 +32,4 @@ The clamshell PDA models emulation includes the following peripherals: - Three on-chip UARTs -- WM8750 audio CODEC on |I2C| and |I2S| busses +- WM8750 audio CODEC on |I2C| and |I2S| buses diff --git a/docs/system/devices/can.rst b/docs/system/devices/can.rst index 0af3d9912a..09121836fd 100644 --- a/docs/system/devices/can.rst +++ b/docs/system/devices/can.rst @@ -1,12 +1,12 @@ CAN Bus Emulation Support = The CAN bus emulation provides mechanism to connect multiple -emulated CAN controller chips together by one or multiple CAN busses -(the controller device "canbus" parameter). The individual busses +emulated CAN controller chips together by one or multiple CAN buses +(the controller device "canbus" parameter). The individual buses can be connected to host system CAN API (at this time only Linux SocketCAN is supported). -The concept of busses is generic and different CAN controllers +The concept of buses is generic and different CAN controllers can be implemented. The initial submission implemented SJA1000 controller which -- 2.39.2
[PULL 0/6] Trivial patches for 2024-01-05
The following changes since commit 0c1eccd368af8805ec0fb11e6cf25d0684d37328: Merge tag 'hw-cpus-20240105' of https://github.com/philmd/qemu into staging (2024-01-05 16:08:58 +) are available in the Git repository at: https://gitlab.com/mjt0k/qemu.git tags/pull-trivial-patches for you to fetch changes up to 8a780cd212647a6013c8ea59e0929dad996e2c54: docs: use "buses" rather than "busses" (2024-01-05 22:28:54 +0300) trivial patches for 2024-01-05 Random stuff here and there, plus a riscv bugfix. Bin Meng (1): hw/net: cadence_gem: Fix MDIO_OP_xxx values Max Erenberg (1): edu: fix DMA range upper bound check Michael Tokarev (2): chardev/char.c: fix "abstract device type" error message audio/audio.c: remove trailing newline in error_setg Samuel Tardieu (1): docs: use "buses" rather than "busses" Xu Lu (1): target/riscv: Fix mcycle/minstret increment behavior audio/audio.c | 2 +- chardev/char.c | 2 +- docs/system/arm/palm.rst| 2 +- docs/system/arm/xscale.rst | 2 +- docs/system/devices/can.rst | 6 +++--- hw/misc/edu.c | 2 +- hw/net/cadence_gem.c| 4 ++-- target/riscv/csr.c | 14 +++--- 8 files changed, 17 insertions(+), 17 deletions(-)
[PULL 5/6] edu: fix DMA range upper bound check
From: Max Erenberg
The edu_check_range function checks that start <= end1 < end2, where
end1 is the upper bound (exclusive) of the guest-supplied DMA range and
end2 is the upper bound (exclusive) of the device's allowed DMA range.
When the guest tries to transfer exactly DMA_SIZE (4096) bytes, end1
will be equal to end2, so the check fails and QEMU aborts with this
puzzling error message (newlines added for formatting):
qemu: hardware error: EDU: DMA range
0x0004-0x00040fff out of bounds
(0x0004-0x00040fff)!
By checking end1 <= end2 instead, guests will be allowed to transfer
exactly 4096 bytes. It is not necessary to explicitly check for
start <= end1 because the previous two checks (within(addr, start, end2)
and end1 > addr) imply start < end1.
Fixes: b30934cb52a7 ("hw: misc, add educational driver", 2015-01-21)
Signed-off-by: Max Erenberg
Signed-off-by: Michael Tokarev
---
hw/misc/edu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index a1f8bc77e7..e64a246d3f 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -115,7 +115,7 @@ static void edu_check_range(uint64_t addr, uint64_t size1,
uint64_t start,
uint64_t end2 = start + size2;
if (within(addr, start, end2) &&
-end1 > addr && within(end1, start, end2)) {
+end1 > addr && end1 <= end2) {
return;
}
--
2.39.2
[PULL 4/6] hw/net: cadence_gem: Fix MDIO_OP_xxx values
From: Bin Meng
Testing upstream U-Boot with 'sifive_u' machine we see:
=> dhcp
ethernet@1009: PHY present at 0
Could not get PHY for ethernet@1009: addr 0
phy_connect failed
This has been working till QEMU 8.1 but broken since QEMU 8.2.
Fixes: 1b09eeb122aa ("hw/net/cadence_gem: use FIELD to describe PHYMNTNC
register fields")
Reported-by: Heinrich Schuchardt
Signed-off-by: Bin Meng
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Heinrich Schuchardt
Signed-off-by: Michael Tokarev
---
hw/net/cadence_gem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index d7b7b134b0..ec7bf562e5 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -199,8 +199,8 @@ REG32(PHYMNTNC, 0x34) /* Phy Maintenance reg */
FIELD(PHYMNTNC, PHY_ADDR, 23, 5)
FIELD(PHYMNTNC, OP, 28, 2)
FIELD(PHYMNTNC, ST, 30, 2)
-#define MDIO_OP_READ0x3
-#define MDIO_OP_WRITE 0x2
+#define MDIO_OP_READ0x2
+#define MDIO_OP_WRITE 0x1
REG32(RXPAUSE, 0x38) /* RX Pause Time reg */
REG32(TXPAUSE, 0x3c) /* TX Pause Time reg */
--
2.39.2
[PULL 3/6] audio/audio.c: remove trailing newline in error_setg
error_setg() appends newline to the formatted message.
Fixes: cb94ff5f80c5 ("audio: propagate Error * out of audio_init")
Signed-off-by: Michael Tokarev
Reviewed-by: Philippe Mathieu-Daudé
---
audio/audio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/audio/audio.c b/audio/audio.c
index a1097bb016..af0ae33fed 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1744,7 +1744,7 @@ static AudioState *audio_init(Audiodev *dev, Error **errp)
if (driver) {
done = !audio_driver_init(s, driver, dev, errp);
} else {
-error_setg(errp, "Unknown audio driver `%s'\n", drvname);
+error_setg(errp, "Unknown audio driver `%s'", drvname);
}
if (!done) {
goto out;
--
2.39.2
CI "pages" job failing with incomprehensible error message from htags
https://gitlab.com/qemu-project/qemu/-/jobs/5871592479 failed with $ htags -anT --tree-view=filetree -m qemu_init -t "Welcome to the QEMU sourcecode" htags: Negative exec line limit = -371 Does anybody have any idea what this is about ? thanks -- PMM
Re: [PULL 00/71] HW core patches for 2024-01-05
On Fri, 5 Jan 2024 at 17:05, Philippe Mathieu-Daudé wrote: > > On 5/1/24 16:41, Philippe Mathieu-Daudé wrote: > > The following changes since commit 05470c3979d5485003e129ff4b0c2ef98af91d86: > > > >Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging > > (2024-01-04 19:55:20 +) > > > > are available in the Git repository at: > > > >https://github.com/philmd/qemu.git tags/hw-cpus-20240105 > > > > for you to fetch changes up to a318da6b3f6a88e6cfd6953c519def9457e8962f: > > > >target/sparc: Simplify qemu_irq_ack (2024-01-05 16:20:15 +0100) > > > > > > HW core patch queue > > > > - Unify CPU QOM type checks (Gavin) > > - Simplify uses of some CPU related property (Philippe) > >(start-powered-off, ARM reset-cbar and mp-affinity) > > - Header and documentation cleanups (Zhao, Philippe) > > - Have Memory API return boolean indicating possible error > > - Fix frame filter mask in CAN sja1000 model (Pavel) > > - QOM embed MCF5206 timer into SoC (Thomas) > > - Simplify LEON3 qemu_irq_ack handler (Clément) > > > > > > > > Philippe Mathieu-Daudé (37): > > >backends: Simplify host_memory_backend_memory_complete() > > I neglected to run checkpatch.pl on this patch, so it lacks: > > -- >8 -- > diff --git a/backends/hostmem.c b/backends/hostmem.c > index 30f69b2cb5..987f6f591e 100644 > --- a/backends/hostmem.c > +++ b/backends/hostmem.c > @@ -346,5 +346,7 @@ host_memory_backend_memory_complete(UserCreatable > *uc, Error **errp) > unsigned long maxnode = (lastbit + 1) % (MAX_NODES + 1); > -/* ensure policy won't be ignored in case memory is preallocated > +/* > + * Ensure policy won't be ignored in case memory is preallocated >* before mbind(). note: MPOL_MF_STRICT is ignored on hugepages so > - * this doesn't catch hugepage case. */ > + * this doesn't catch hugepage case. > + */ > unsigned flags = MPOL_MF_STRICT | MPOL_MF_MOVE; > @@ -365,3 +367,4 @@ host_memory_backend_memory_complete(UserCreatable > *uc, Error **errp) > > -/* We can have up to MAX_NODES nodes, but we need to pass maxnode+1 > +/* > + * We can have up to MAX_NODES nodes, but we need to pass maxnode+1 >* as argument to mbind() due to an old Linux bug (feature?) which > @@ -393,3 +396,4 @@ host_memory_backend_memory_complete(UserCreatable > *uc, Error **errp) > #endif > -/* Preallocate memory after the NUMA policy has been instantiated. > +/* > + * Preallocate memory after the NUMA policy has been instantiated. >* This is necessary to guarantee memory is allocated with > --- > > Since the PR is already in the testing pipeline: > - if it get merged, I'll send a cleanup patch > - otherwise if it fails I'll fix it. It just passed the last CI job on a retry, so please send a followup patch. Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/9.0 for any user-visible changes. -- PMM
acpiBitsTest.test_acpi_smbios_bits test intermittently times out
The avocado test acpiBitsTest.test_acpi_smbios_bits seems to be flaky in CI -- sometimes it appears to time out. https://gitlab.com/qemu-project/qemu/-/issues/2077 has the details (including links to jobs etc). As far as I can see, the test is still running when after about a minute it gets timed out. (Though the python tracebacks in the logs are not easy for me to interpret, so I might be wrong). This I find a bit confusing, because tests/avocado/acpi-bits.py sets "timeout = 200". So maybe that isn't taking effect properly? Does anybody have time to investigate this? If not, we can disable the test as flaky until somebody does have the time. thanks -- PMM
KVM/QEMU Community Call 9th Jan Agenda Items
Hi, The KVM/QEMU community call is at: https://meet.jit.si/kvmcallmeeting @ 9/1/2024 14:00 UTC If anyone has any agenda items please reply to this thread. -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v3 3/6] target/riscv: Add helper functions to calculate current number of masked bits for pointer masking
On Fri, Jan 05, 2024 at 10:33:40AM +0300, Alexey Baturo wrote:
I think you're right, thanks.
I'll add a check for M-mode as well and I guess I'll have to rename the
function.
Any ideas on the proper and self-describing name?
Since all we care for is whether virtual memory is enabled and in effect or not.
Some suggestions below
`
bool riscv_cpu_mmu_enabled
bool riscv_cpu_paging_enabled
bool riscv_cpu_virt_mem_enabled
Thanks
пт, 5 янв. 2024 г. в 03:46, Deepak Gupta :
On Wed, Jan 3, 2024 at 10:59 AM Alexey Baturo
wrote:
> +
> +bool riscv_cpu_bare_mode(CPURISCVState *env)
> +{
> +int satp_mode = 0;
> +#ifndef CONFIG_USER_ONLY
> +if (riscv_cpu_mxl(env) == MXL_RV32) {
> +satp_mode = get_field(env->satp, SATP32_MODE);
> +} else {
> +satp_mode = get_field(env->satp, SATP64_MODE);
> +}
> +#endif
> +return (satp_mode == VM_1_10_MBARE);
> +}
> +
Assume the CPU was in S or U with satp = non-bare mode but then a
transfer to M-mode happened.
In that case, even though the CPU is in M mode, the above function
will return non-bare mode and enforce
signed extension on M mode pointer masking (if enabled).
right or am I missing something here?
Re: [PATCH v3 5/6] target/riscv: Update address modify functions to take into account pointer masking
On Fri, Jan 05, 2024 at 10:29:35AM +0300, Alexey Baturo wrote:
+addr = addr << pmlen;
+if (signext) {
+addr = (target_long)addr >> pmlen;
+} else {
+addr = addr >> pmlen;
Could you please elaborate a bit more on your concern here?
I believe this code works as intended: https://godbolt.org/z/b9c7na13a
Nevermind I missed this above in code.
addr = addr << pmlen;
You're good. Sorry about that.
Thanks
пт, 5 янв. 2024 г. в 04:02, Deepak Gupta :
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -94,6 +94,18 @@ static inline uint32_t vext_max_elems(uint32_t desc,
uint32_t log2_esz)
>
> static inline target_ulong adjust_addr(CPURISCVState *env, target_ulong
addr)
> {
> +RISCVPmPmm pmm = riscv_pm_get_pmm(env);
> +if (pmm == PMM_FIELD_DISABLED)
> +return addr;
> +int pmlen = riscv_pm_get_pmlen(pmm);
> +bool signext = !riscv_cpu_bare_mode(env);
> +addr = addr << pmlen;
> +/* sign/zero extend masked address by N-1 bit */
> +if (signext) {
> +addr = (target_long)addr >> pmlen;
These look like right shift operations and not sign extensions of N-1 bit
> +} else {
> +addr = addr >> pmlen;
Same here.
> +}
> return addr;
> }
>
> --
> 2.34.1
>
>
[PATCH v3 2/4] tests/qtest/migration: Add infrastructure to skip tests on older QEMUs
We can run the migration tests with two different QEMU binaries to
test migration compatibility between QEMU versions. This means we'll
be running the tests with an older QEMU in either source or
destination.
We need to avoid trying to test functionality that is unknown to the
older QEMU. This could mean new features, bug fixes, error message
changes, QEMU command line changes, migration API changes, etc.
Add a 'since' argument to the tests that inform when the functionality
that is being test has been added to QEMU so we can skip the test on
older versions.
Also add a version comparison function so we can adapt test code
depending on the QEMU binary version being used.
Signed-off-by: Fabiano Rosas
---
tests/qtest/migration-helpers.h | 1 +
tests/qtest/migration-helpers.c | 48 +
tests/qtest/migration-test.c| 29
3 files changed, 78 insertions(+)
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index e31dc85cc7..7b4f8e851e 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -47,4 +47,5 @@ char *find_common_machine_version(const char *mtype, const
char *var1,
const char *var2);
char *resolve_machine_version(const char *alias, const char *var1,
const char *var2);
+int migration_vercmp(QTestState *who, const char *tgt_version);
#endif /* MIGRATION_HELPERS_H */
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 24fb7b3525..bc4fd93e8c 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -292,3 +292,51 @@ char *resolve_machine_version(const char *alias, const
char *var1,
return find_common_machine_version(machine_name, var1, var2);
}
+
+int migration_vercmp(QTestState *who, const char *tgt_version)
+{
+g_autofree char *version = g_strdup(tgt_version);
+int major = 0, minor = 0, micro = 0;
+int tgt_major = 0, tgt_minor = 0, tgt_micro = 0;
+const char *delim = ".";
+char *tok;
+
+qtest_query_version(who, , , );
+
+tok = strtok(version, delim);
+assert(tok);
+tgt_major = atoi(tok);
+assert(tgt_major);
+
+if (major > tgt_major) {
+return -1;
+}
+if (major < tgt_major) {
+return 1;
+}
+
+tok = strtok(NULL, delim);
+assert(tok);
+tgt_minor = atoi(tok);
+
+if (minor > tgt_minor) {
+return -1;
+}
+if (minor < tgt_minor) {
+return 1;
+}
+
+tok = strtok(NULL, delim);
+if (tok) {
+tgt_micro = atoi(tok);
+}
+
+if (micro > tgt_micro) {
+return -1;
+}
+if (micro < tgt_micro) {
+return 1;
+}
+
+return 0;
+}
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index d520c587f7..001470238b 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -637,6 +637,12 @@ typedef struct {
bool use_dirty_ring;
const char *opts_source;
const char *opts_target;
+/*
+ * If a test checks against new functionality that might not be
+ * present in older QEMUs this needs to be set so we can skip
+ * running it when doing compatibility testing.
+ */
+const char *since;
} MigrateStart;
/*
@@ -850,6 +856,17 @@ static int test_migrate_start(QTestState **from,
QTestState **to,
qtest_qmp_set_event_callback(*from,
migrate_watch_for_stop,
_src_stop);
+
+if (args->since && migration_vercmp(*from, args->since) < 0) {
+g_autofree char *msg = NULL;
+
+msg = g_strdup_printf("Test requires at least QEMU version %s",
+ args->since);
+g_test_skip(msg);
+qtest_quit(*from);
+
+return -1;
+}
}
cmd_target = g_strdup_printf("-accel kvm%s -accel tcg "
@@ -872,6 +889,18 @@ static int test_migrate_start(QTestState **from,
QTestState **to,
migrate_watch_for_resume,
_dst_resume);
+if (args->since && migration_vercmp(*to, args->since) < 0) {
+g_autofree char *msg = NULL;
+
+msg = g_strdup_printf("Test requires at least QEMU version %s",
+ args->since);
+g_test_skip(msg);
+qtest_quit(*to);
+qtest_quit(*from);
+
+return -1;
+}
+
/*
* Remove shmem file immediately to avoid memory leak in test failed case.
* It's valid because QEMU has already opened this file
--
2.35.3
[PATCH v3 0/4] migration & CI: Add a CI job for migration compat testing
Here's the second half of adding a migration compatibility test to CI.
We've already added support for running the full set of migration
tests with two QEMU binaries since commit 5050ad2a380
("tests/qtest/migration: Support more than one QEMU binary"), now
what's left is adding it to the CI.
I included patches that solve the problem of testing older QEMUs with
new test code. The old QEMU might lack features, bug fixes, etc. that
the tests expect to be present. After this series we can specify a
minimal QEMU version that a specific test supports.
changes since v2:
- fixed version comparison once again
- removed the 8.2 fixes. We don't need them anymore because we're now
testing two 8.2 streams (8.2.0 vs. 8.2.50).
CI run: https://gitlab.com/farosas/qemu/-/pipelines/1128168149
v2:
https://lore.kernel.org/r/20240104171857.20108-1-faro...@suse.de
v1:
https://lore.kernel.org/r/20231207155809.25673-1-faro...@suse.de
Fabiano Rosas (4):
tests/qtest: Add a helper to query the QEMU version
tests/qtest/migration: Add infrastructure to skip tests on older QEMUs
ci: Add a migration compatibility test job
[NOT FOR MERGE] tests/qtest/migration: Adapt tests to use older QEMUs
tests/qtest/libqtest.h | 10 ++
tests/qtest/migration-helpers.h | 1 +
tests/qtest/libqtest.c | 24 +
tests/qtest/migration-helpers.c | 48 +
tests/qtest/migration-test.c| 63 +
.gitlab-ci.d/buildtest.yml | 53 +++
6 files changed, 192 insertions(+), 7 deletions(-)
--
2.35.3
[PATCH v3 3/4] ci: Add a migration compatibility test job
The migration tests have support for being passed two QEMU binaries to
test migration compatibility.
Add a CI job that builds the lastest release of QEMU and another job
that uses that version plus an already present build of the current
version and run the migration tests with the two, both as source and
destination. I.e.:
old QEMU (n-1) -> current QEMU (development tree)
current QEMU (development tree) -> old QEMU (n-1)
The purpose of this CI job is to ensure the code we're about to merge
will not cause a migration compatibility problem when migrating the
next release (which will contain that code) to/from the previous
release.
I'm leaving the jobs as manual for now because using an older QEMU in
tests could hit bugs that were already fixed in the current
development tree and we need to handle those case-by-case.
Note: for user forks, the version tags need to be pushed to gitlab
otherwise it won't be able to checkout a different version.
Signed-off-by: Fabiano Rosas
---
.gitlab-ci.d/buildtest.yml | 53 ++
1 file changed, 53 insertions(+)
diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 91663946de..81163a3f6a 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -167,6 +167,59 @@ build-system-centos:
x86_64-softmmu rx-softmmu sh4-softmmu nios2-softmmu
MAKE_CHECK_ARGS: check-build
+build-previous-qemu:
+ extends: .native_build_job_template
+ artifacts:
+when: on_success
+expire_in: 2 days
+paths:
+ - build-previous
+exclude:
+ - build-previous/**/*.p
+ - build-previous/**/*.a.p
+ - build-previous/**/*.fa.p
+ - build-previous/**/*.c.o
+ - build-previous/**/*.c.o.d
+ - build-previous/**/*.fa
+ needs:
+job: amd64-opensuse-leap-container
+ variables:
+QEMU_JOB_OPTIONAL: 1
+IMAGE: opensuse-leap
+TARGETS: x86_64-softmmu aarch64-softmmu
+ before_script:
+- export QEMU_PREV_VERSION="$(sed 's/\([0-9.]*\)\.[0-9]*/v\1.0/' VERSION)"
+- git checkout $QEMU_PREV_VERSION
+ after_script:
+- mv build build-previous
+
+.migration-compat-common:
+ extends: .common_test_job_template
+ needs:
+- job: build-previous-qemu
+- job: build-system-opensuse
+ allow_failure: true
+ variables:
+QEMU_JOB_OPTIONAL: 1
+IMAGE: opensuse-leap
+MAKE_CHECK_ARGS: check-build
+ script:
+- cd build
+- QTEST_QEMU_BINARY_SRC=../build-previous/qemu-system-${TARGET}
+ QTEST_QEMU_BINARY=./qemu-system-${TARGET}
./tests/qtest/migration-test
+- QTEST_QEMU_BINARY_DST=../build-previous/qemu-system-${TARGET}
+ QTEST_QEMU_BINARY=./qemu-system-${TARGET}
./tests/qtest/migration-test
+
+migration-compat-aarch64:
+ extends: .migration-compat-common
+ variables:
+TARGET: aarch64
+
+migration-compat-x86_64:
+ extends: .migration-compat-common
+ variables:
+TARGET: x86_64
+
check-system-centos:
extends: .native_test_job_template
needs:
--
2.35.3
[PATCH v3 4/4] [NOT FOR MERGE] tests/qtest/migration: Adapt tests to use older QEMUs
[This patch is not necessary anymore after 8.2 has been released]
Add the 'since' annotations to recently added tests and adapt the
postcopy test to use the older "uri" API when needed.
Signed-off-by: Fabiano Rosas
---
tests/qtest/migration-test.c | 34 +++---
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 001470238b..599f51f978 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1338,14 +1338,21 @@ static int migrate_postcopy_prepare(QTestState
**from_ptr,
migrate_ensure_non_converge(from);
migrate_prepare_for_dirty_mem(from);
-qtest_qmp_assert_success(to, "{ 'execute': 'migrate-incoming',"
- " 'arguments': { "
- " 'channels': [ { 'channel-type': 'main',"
- " 'addr': { 'transport': 'socket',"
- "'type': 'inet',"
- "'host': '127.0.0.1',"
- "'port': '0' } } ] } }");
+/* New syntax was introduced in 8.2 */
+if (migration_vercmp(to, "8.2") < 0) {
+qtest_qmp_assert_success(to, "{ 'execute': 'migrate-incoming',"
+ " 'arguments': { "
+ " 'uri': 'tcp:127.0.0.1:0' } }");
+} else {
+qtest_qmp_assert_success(to, "{ 'execute': 'migrate-incoming',"
+ " 'arguments': { "
+ " 'channels': [ { 'channel-type':
'main',"
+ " 'addr': { 'transport': 'socket',"
+ "'type': 'inet',"
+ "'host': '127.0.0.1',"
+ "'port': '0' } } ] } }");
+}
/* Wait for the first serial output from the source */
wait_for_serial("src_serial");
@@ -1603,6 +1610,9 @@ static void test_postcopy_recovery_double_fail(void)
{
MigrateCommon args = {
.postcopy_recovery_test_fail = true,
+.start = {
+.since = "8.2",
+},
};
test_postcopy_recovery_common();
@@ -1665,6 +1675,7 @@ static void test_analyze_script(void)
{
MigrateStart args = {
.opts_source = "-uuid ----",
+.since = "8.2",
};
QTestState *from, *to;
g_autofree char *uri = NULL;
@@ -2090,6 +2101,9 @@ static void test_precopy_file(void)
MigrateCommon args = {
.connect_uri = uri,
.listen_uri = "defer",
+.start = {
+.since = "8.2"
+},
};
test_file_common(, true);
@@ -2134,6 +2148,9 @@ static void test_precopy_file_offset(void)
.connect_uri = uri,
.listen_uri = "defer",
.finish_hook = file_offset_finish_hook,
+.start = {
+.since = "8.2"
+},
};
test_file_common(, false);
@@ -2148,6 +2165,9 @@ static void test_precopy_file_offset_bad(void)
.connect_uri = uri,
.listen_uri = "defer",
.result = MIG_TEST_QMP_ERROR,
+.start = {
+.since = "8.2"
+},
};
test_file_common(, false);
--
2.35.3
[PATCH v3 1/4] tests/qtest: Add a helper to query the QEMU version
Reviewed-by: Thomas Huth
Signed-off-by: Fabiano Rosas
---
tests/qtest/libqtest.h | 10 ++
tests/qtest/libqtest.c | 24
2 files changed, 34 insertions(+)
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 6e3d3525bf..5ec758242b 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -1085,4 +1085,14 @@ bool have_qemu_img(void);
*/
bool mkimg(const char *file, const char *fmt, unsigned size_mb);
+/**
+ * qtest_query_version:
+ * @qts: QTestState instance to operate on
+ * @major: Pointer to where to store the major version number
+ * @minor: Pointer to where to store the minor version number
+ * @micro: Pointer to where to store the micro version number
+ *
+ */
+void qtest_query_version(QTestState *qts, int *major, int *minor, int *micro);
+
#endif
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index f33a210861..af779a3248 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -337,6 +337,30 @@ void qtest_remove_abrt_handler(void *data)
}
}
+void qtest_query_version(QTestState *qts, int *major, int *minor, int *micro)
+{
+QDict *rsp, *version;
+
+rsp = qtest_qmp_assert_success_ref(qts, "{ 'execute': 'query-version' }");
+g_assert(rsp);
+
+version = qdict_get_qdict(rsp, "qemu");
+
+if (major) {
+*major = qdict_get_int(version, "major");
+}
+
+if (minor) {
+*minor = qdict_get_int(version, "minor");
+}
+
+if (micro) {
+*micro = qdict_get_int(version, "micro");
+}
+
+qobject_unref(rsp);
+}
+
static const char *qtest_qemu_binary(const char *var)
{
const char *qemu_bin;
--
2.35.3
Re: [PATCH v2 2/3] hw/arm/armv7m: alias the NVIC "num-prio-bits" property
Peter Maydell writes: There's a comment in include/hw/arm/armv7m.h which documents all the GPIO inputs, QOM properties, etc, that this device has -- that also needs a line adding to it for this property. Thanks Peter for your review. I'll send a v3 containing the requested change after Inès has submitted an updated version of her "Add minimal support for the B-L475E-IOT01A board" serie on which this one is based. Best. Sam -- Samuel Tardieu
Re: [PATCH v2 3/3] hw/arm/socs: configure priority bits for existing SOCs
On Wed, 3 Jan 2024 at 15:53, Samuel Tardieu wrote: > > Update the number of priority bits for a number of existing > SoCs according to their technical documentation: > > - STM32F100/F205/F405/L4x5: 4 bits > - Stellaris (Sandstorm/Fury): 3 bits > > Signed-off-by: Samuel Tardieu Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH] docs: use "buses" rather than "busses"
On Wed, 3 Jan 2024 at 17:28, Samuel Tardieu wrote: > > If "busses" might be encountered as a plural of "bus" (5 instances), > the correct spelling is "buses" (26 instances). Fixing those 5 > instances makes the doc more consistent. > > Signed-off-by: Samuel Tardieu Applied to target-arm.next, thanks (since 2 out of 3 of these are arm related). -- PMM
Re: [PATCH v2 2/3] hw/arm/armv7m: alias the NVIC "num-prio-bits" property
On Wed, 3 Jan 2024 at 15:53, Samuel Tardieu wrote: > > A SoC will not have a direct access to the NVIC embedded in its ARM > core. By aliasing the "num-prio-bits" property similarly to what is > done for the "num-irq" one, a SoC can easily configure it on its > armv7m instance. > > Signed-off-by: Samuel Tardieu > --- > hw/arm/armv7m.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c > index d10abb36a8..4fda2d1d47 100644 > --- a/hw/arm/armv7m.c > +++ b/hw/arm/armv7m.c > @@ -256,6 +256,8 @@ static void armv7m_instance_init(Object *obj) > object_initialize_child(obj, "nvic", >nvic, TYPE_NVIC); > object_property_add_alias(obj, "num-irq", >OBJECT(>nvic), "num-irq"); > +object_property_add_alias(obj, "num-prio-bits", > + OBJECT(>nvic), "num-prio-bits"); > > object_initialize_child(obj, "systick-reg-ns", >systick[M_REG_NS], > TYPE_SYSTICK); There's a comment in include/hw/arm/armv7m.h which documents all the GPIO inputs, QOM properties, etc, that this device has -- that also needs a line adding to it for this property. Otherwise Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 1/3] hw/intc/armv7m_nvic: add "num-prio-bits" property
On Wed, 3 Jan 2024 at 15:53, Samuel Tardieu wrote: > > Cortex-M NVIC can have a different number of priority bits. > Cortex-M0/M0+/M1 devices must use 2 or more bits, while devices based > on ARMv7m and up must use 3 or more bits. > > This adds a "num-prio-bits" property which will get sensible default > values if unset (2 or 8 depending on the device). Unless a SOC > specifies the number of bits to use, the previous behavior is > maintained for backward compatibility. > > Signed-off-by: Samuel Tardieu > Suggested-by: Anton Kochkov > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1122 > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH] arm_gic: Implement GICC_AIAR, GICC_AEOIR and GICC_AHPPIR
On Wed, 20 Dec 2023 at 06:47, Andrei Homescu wrote:
>
> From: Arve Hjønnevåg
>
> Implement aliased registers so group 1 interrupts can be used in secure
> mode.
Hi; thanks for this patch.
> GICC_AEOIR is only implemented as a direct alias to GICC_EOIR for now as
> gic_complete_irq does not currently check if the cpu is in secure mode.
I'm not really sure what this comment has in mind, but I think
perhaps it is alluding to the issue that https://r.android.com/859189
fixes ? In any case, we should either expand it to be clearer about
what problem it's referring to or else delete it as no longer
relevant.
> Upstreamed from https://r.android.com/705890 and
> https://r.android.com/859189.
>
> Signed-off-by: Arve Hjønnevåg
> Signed-off-by: Matthew Maurer
Hmm, I'm not entirely sure if we should be creating signed-off-by
lines if they've not been provided by the original authors.
(We can take the code because it's from a GPL2'd fork, and we
should credit the original authors, but a signed-off-by tag
says a bit more than that.)
> Signed-off-by: Andrei Homescu
> ---
> hw/intc/arm_gic.c | 31 ---
> 1 file changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index 074cf50af2..d0e267a4b2 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -854,7 +854,8 @@ static void gic_deactivate_irq(GICState *s, int cpu, int
> irq, MemTxAttrs attrs)
> gic_clear_active(s, irq, cpu);
> }
>
> -static void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
> +static void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs,
> + bool ns_irq)
I think this argument should be called group1_only (see later).
> {
> int cm = 1 << cpu;
> int group;
There's a codepath here for "gic_is_vcpu())"; we should
add a comment somewhere there that says something like:
/*
* Acknowledging a group 0 interrupt via GICV_AEOIR is
* UNPREDICTABLE; we choose to treat it as if the guest
* had acknowledged it via GICV_EOIR, i.e. we ignore
* the group1_only flag.
*/
> @@ -922,7 +923,7 @@ static void gic_complete_irq(GICState *s, int cpu, int
> irq, MemTxAttrs attrs)
>
> group = gic_has_groups(s) && gic_test_group(s, irq, cpu);
>
> -if (gic_cpu_ns_access(s, cpu, attrs) && !group) {
> +if (ns_irq && !group) {
> DPRINTF("Non-secure EOI for Group0 interrupt %d ignored\n", irq);
> return;
> }
> @@ -1647,6 +1648,22 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu,
> int offset,
> *data = s->abpr[cpu];
> }
> break;
> +case 0x20: /* Aliased Interrupt Acknowledge */
> +if (!gic_has_groups(s) || (s->security_extn && !attrs.secure)) {
This doesn't look like the right condition -- the spec
says GICC_AIAR is only present in GICv2. gic_has_groups()
will return true for GICv2 or for a GICv1 with the security
extensions. (That's the right check for GICC_ABPR, but not
for GICC_AIAR or GICC_AHPPIR or GIC_AEOIR.)
It will also mean we incorrectly return 0 for the case
of the GICV_AIAR vcpu interface register. (This is why the
existing code for the GICC_ABPR uses gic_cpu_ns_access() in
this part of its check.)
> +*data = 0;
> +} else {
> +attrs.secure = false;
> +*data = gic_acknowledge_irq(s, cpu, attrs);
This doesn't do the right thing I think for the GICV_AIAR
vcpu interface, or for a GICv2 which doesn't have the security
extensions, because gic_cpu_ns_access() will return false
for both of those cases even if attrs.secure is false, and
so when gic_acknowledge_irq() calls gic_get_current_pending_irq()
it won't get the group 1 IRQ it's supposed to.
> +}
> +break;
> +case 0x28: /* Aliased Highest Priority Pending Interrupt */
> +if (!gic_has_groups(s) || (s->security_extn && !attrs.secure)) {
> +*data = 0;
> +} else {
> +attrs.secure = false;
> +*data = gic_get_current_pending_irq(s, cpu, attrs);
> +}
Similarly this isn't correct for the GICv2-no-security-interface
and the GICV-vcpu-interface cases.
I think we probably need to plumb through more of the "what
behaviour do we need" rather than trying to do it by setting
attrs.secure. My first thought is a 'bool group1_only' argument
to gic_get_current_pending_irq() and gic_acknowledge_irq(),
so that those functions can give the behaviour we want for
the alias registers.
> +break;
> case 0xd0: case 0xd4: case 0xd8: case 0xdc:
> {
> int regno = (offset - 0xd0) / 4;
> @@ -1724,7 +1741,15 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu,
> int offset,
> }
> break;
> case 0x10: /* End Of Interrupt */
> -gic_complete_irq(s, cpu, value & 0x3ff, attrs);
> +gic_complete_irq(s, cpu, value & 0x3ff, attrs,
> + gic_cpu_ns_access(s, cpu, attrs));
> +
Re: [PATCH-for-9.0 03/25] memory: Have memory_region_init_rom_nomigrate() handler return a boolean
Hi Peter,
On 5/1/24 15:57, Peter Maydell wrote:
On Fri, 5 Jan 2024 at 14:46, Philippe Mathieu-Daudé wrote:
On 21/11/23 13:10, Manos Pitsidianakis wrote:
On Mon, 20 Nov 2023 23:32, Philippe Mathieu-Daudé
wrote:
Following the example documented since commit e3fe3988d7 ("error:
Document Error API usage rules"), have cpu_exec_realizefn()
return a boolean indicating whether an error is set or not.
Signed-off-by: Philippe Mathieu-Daudé
---
include/exec/memory.h | 4 +++-
system/memory.c | 8 ++--
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/system/memory.c b/system/memory.c
index 337b12a674..bfe0b62d59 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1729,14 +1729,18 @@ void memory_region_init_alias(MemoryRegion *mr,
mr->alias_offset = offset;
}
-void memory_region_init_rom_nomigrate(MemoryRegion *mr,
+bool memory_region_init_rom_nomigrate(MemoryRegion *mr,
Object *owner,
const char *name,
uint64_t size,
Error **errp)
{
-memory_region_init_ram_flags_nomigrate(mr, owner, name, size, 0,
errp);
+bool rv;
+
+rv = memory_region_init_ram_flags_nomigrate(mr, owner, name,
size, 0, errp);
mr->readonly = true;
+
By the way, do we want to set mr->readonly on failure? Should there be
modifications if an error is propagated upwards?
Good point
I don't think it matters much. If the init function fails,
then the MemoryRegion is not initialized, and there's
nothing you can do with the struct except free it (if it
was in allocated memory). Whether the readonly field is
true or false doesn't matter, because conceptually it's
all undefined-values. And memory_region_init_ram_flags_nomigrate()
has already written to some fields, so avoiding changing
mr->readonly specifically doesn't seem worthwhile.
I concur with your analysis. QEMU Error* type is helpful to propagate
errors, but the cleanup path is rarely well implemented. See for
example the many returns in DeviceRealize handlers without releasing
previously allocated mem.
In this particular patch case, I find Manos suggestion useful, the
code now uses a simpler pattern and avoid having to look at the
callee implementation.
The updated patch is already in a PR I posted before reading your
comment. The changes seem innocuous to me, so not worthwhile to
restore the previous patch content. But if you object, I don't mind
reposting the PR.
Regards,
Phil.
Re: [PULL 00/71] HW core patches for 2024-01-05
On 5/1/24 16:41, Philippe Mathieu-Daudé wrote: The following changes since commit 05470c3979d5485003e129ff4b0c2ef98af91d86: Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging (2024-01-04 19:55:20 +) are available in the Git repository at: https://github.com/philmd/qemu.git tags/hw-cpus-20240105 for you to fetch changes up to a318da6b3f6a88e6cfd6953c519def9457e8962f: target/sparc: Simplify qemu_irq_ack (2024-01-05 16:20:15 +0100) HW core patch queue - Unify CPU QOM type checks (Gavin) - Simplify uses of some CPU related property (Philippe) (start-powered-off, ARM reset-cbar and mp-affinity) - Header and documentation cleanups (Zhao, Philippe) - Have Memory API return boolean indicating possible error - Fix frame filter mask in CAN sja1000 model (Pavel) - QOM embed MCF5206 timer into SoC (Thomas) - Simplify LEON3 qemu_irq_ack handler (Clément) Philippe Mathieu-Daudé (37): backends: Simplify host_memory_backend_memory_complete() I neglected to run checkpatch.pl on this patch, so it lacks: -- >8 -- diff --git a/backends/hostmem.c b/backends/hostmem.c index 30f69b2cb5..987f6f591e 100644 --- a/backends/hostmem.c +++ b/backends/hostmem.c @@ -346,5 +346,7 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp) unsigned long maxnode = (lastbit + 1) % (MAX_NODES + 1); -/* ensure policy won't be ignored in case memory is preallocated +/* + * Ensure policy won't be ignored in case memory is preallocated * before mbind(). note: MPOL_MF_STRICT is ignored on hugepages so - * this doesn't catch hugepage case. */ + * this doesn't catch hugepage case. + */ unsigned flags = MPOL_MF_STRICT | MPOL_MF_MOVE; @@ -365,3 +367,4 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp) -/* We can have up to MAX_NODES nodes, but we need to pass maxnode+1 +/* + * We can have up to MAX_NODES nodes, but we need to pass maxnode+1 * as argument to mbind() due to an old Linux bug (feature?) which @@ -393,3 +396,4 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp) #endif -/* Preallocate memory after the NUMA policy has been instantiated. +/* + * Preallocate memory after the NUMA policy has been instantiated. * This is necessary to guarantee memory is allocated with --- Since the PR is already in the testing pipeline: - if it get merged, I'll send a cleanup patch - otherwise if it fails I'll fix it. Sorry for missing that. Phil.
Re: Question about CXL emulation in QEMU
"周童" <273415...@qq.com> writes: > Dear Experts, (add maintainers to CC) > > I am writing to seek your assistance about CXL emulation in QEMU. I am Zhou > Tong and I am researching how to use QEMU > to simulate CXL over Ethernet。 > > I want to implement remote registration of CXL.mem devices based on the QOM > model. The general idea is: the CXL slave > side notifies the master side of the size of the CXL memory and other control > information through Ethernet, and the master > side registers the CXL device locally based on the control information. When > the master accesses the CXL device, KVM is > responsible for intercepting the action of accessing the memory, and > encapsulates the CXL message and forwards it to the > slave through Ethernet，ultimately achieving remote CXL memory access.. Ask > the experts how to register the CXL device > locally based on the control information without occupying the HVA resources > of the master host (OR display the CXL > device). > > Thank you in advance for your attention to this email, and I eagerly look > forward to any insights or advice you may be able to > provide. If there is a more convenient time for us to discuss this matter > further, please let me know, and I will be more than > happy to accommodate your schedule. > > Once again, thank you for your impactful contributions to the open-source > community, and I greatly appreciate your time and > consideration. > > Regard, > > Zhou Tong. > > - > > * 周童 >273415...@qq.com > * -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH] i386/sev: Nitpick at the error message's output
Hyman Huang writes: > The incorrect error message was produced as a result of > the return number being disregarded on the sev_kvm_init > failure path. > > For instance, when a user's failure to launch a SEV guest > is caused by an incorrect IOCTL, the following message is > reported: > > kvm: sev_kvm_init: failed to initialize ret=-25 fw_error=0 > kvm: failed to initialize kvm: Operation not permitted > > While the error message's accurate output should be: > > kvm: sev_kvm_init: failed to initialize ret=-25 fw_error=0 > kvm: failed to initialize kvm: Inappropriate ioctl for device > > Fix this by returning the return number directly on the > failure path. > > Signed-off-by: Hyman Huang > --- > target/i386/sev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/i386/sev.c b/target/i386/sev.c > index 9a71246682..4a69ca457c 100644 > --- a/target/i386/sev.c > +++ b/target/i386/sev.c > @@ -1019,7 +1019,7 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error > **errp) > err: > sev_guest = NULL; > ram_block_discard_disable(false); > -return -1; > +return ret; I don't think this will be correct in all cases because there are other legs (e.g. if (host_cbitpos != sev->cbitpos)) where ret may be the successful setting of ram_block_discard_disable(true). You might want to explore if the function can be re-written with explicit return's and utilise autofree to do the clean-up of dynamic objects. I think this entails setting sev_guest at the end of the function just before the return 0. I'm not sure if there is a clean way to handle ram_block_discard_disable(false); cleanly for all the failure legs though. Maybe someone with more familiarity with the code has some ideas? > } > > int -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH] hw/arm: add cache controller for Freescale i.MX6
On Tue, 19 Dec 2023 at 10:55, Nikita Ostrenkov wrote: > > Signed-off-by: Nikita Ostrenkov > --- > hw/arm/Kconfig| 1 + > hw/arm/fsl-imx6.c | 3 +++ > 2 files changed, 4 insertions(+) > Applied to target-arm.next, thanks. -- PMM
Re: [PATCH] i386/sev: Nitpick at the error message's output
On Sat, Jan 06, 2024 at 12:09:55AM +0800, Hyman Huang wrote: > The incorrect error message was produced as a result of > the return number being disregarded on the sev_kvm_init > failure path. > > For instance, when a user's failure to launch a SEV guest > is caused by an incorrect IOCTL, the following message is > reported: > > kvm: sev_kvm_init: failed to initialize ret=-25 fw_error=0 > kvm: failed to initialize kvm: Operation not permitted > > While the error message's accurate output should be: > > kvm: sev_kvm_init: failed to initialize ret=-25 fw_error=0 > kvm: failed to initialize kvm: Inappropriate ioctl for device > > Fix this by returning the return number directly on the > failure path. > > Signed-off-by: Hyman Huang > --- > target/i386/sev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Daniel P. Berrangé > > diff --git a/target/i386/sev.c b/target/i386/sev.c > index 9a71246682..4a69ca457c 100644 > --- a/target/i386/sev.c > +++ b/target/i386/sev.c > @@ -1019,7 +1019,7 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error > **errp) > err: > sev_guest = NULL; > ram_block_discard_disable(false); > -return -1; > +return ret; > } With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v6 02/11] virtio-gpu: Configure new feature flag context_create_with_flags for virglrenderer
Huang Rui writes: > Configure a new feature flag (context_create_with_flags) for > virglrenderer. > > Originally-by: Antonio Caggiano > Signed-off-by: Huang Rui Reviewed-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
[PATCH] i386/sev: Nitpick at the error message's output
The incorrect error message was produced as a result of the return number being disregarded on the sev_kvm_init failure path. For instance, when a user's failure to launch a SEV guest is caused by an incorrect IOCTL, the following message is reported: kvm: sev_kvm_init: failed to initialize ret=-25 fw_error=0 kvm: failed to initialize kvm: Operation not permitted While the error message's accurate output should be: kvm: sev_kvm_init: failed to initialize ret=-25 fw_error=0 kvm: failed to initialize kvm: Inappropriate ioctl for device Fix this by returning the return number directly on the failure path. Signed-off-by: Hyman Huang --- target/i386/sev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/i386/sev.c b/target/i386/sev.c index 9a71246682..4a69ca457c 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -1019,7 +1019,7 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp) err: sev_guest = NULL; ram_block_discard_disable(false); -return -1; +return ret; } int -- 2.39.1
Re: [PATCH v6 11/11] virtio-gpu: make blob scanout use dmabuf fd
Alex Bennée writes:
> Huang Rui writes:
>
>> From: Robert Beckett
>>
>> This relies on a virglrenderer change to include the dmabuf fd when
>> returning resource info.
>>
>
>> +static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
>> + struct virtio_gpu_ctrl_command *cmd)
>> +{
>> +struct virgl_gpu_resource *vres;
>> +struct virtio_gpu_framebuffer fb = { 0 };
>> +struct virtio_gpu_set_scanout_blob ss;
>> +struct virgl_renderer_resource_info info;
>> +uint64_t fbend;
>> +
>> +VIRTIO_GPU_FILL_CMD(ss);
>> +virtio_gpu_scanout_blob_bswap();
>> +trace_virtio_gpu_cmd_set_scanout_blob(ss.scanout_id, ss.resource_id,
>> + ss.r.width, ss.r.height, ss.r.x,
>> + ss.r.y);
>> +
>> +if (ss.scanout_id >= g->parent_obj.conf.max_outputs) {
>> +qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout id specified
>> %d",
>> + __func__, ss.scanout_id);
>> +cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
>> +return;
>> +}
>> +
>> +if (ss.resource_id == 0) {
>> +virtio_gpu_disable_scanout(g, ss.scanout_id);
>> +return;
>> +}
>> +
>> +if (ss.width < 16 ||
>> +ss.height < 16 ||
>> +ss.r.x + ss.r.width > ss.width ||
>> +ss.r.y + ss.r.height > ss.height) {
>> +qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout %d bounds for"
>> + " resource %d, rect (%d,%d)+%d,%d, fb %d %d\n",
>> + __func__, ss.scanout_id, ss.resource_id,
>> + ss.r.x, ss.r.y, ss.r.width, ss.r.height,
>> + ss.width, ss.height);
>> +cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
>> +return;
>> +}
>> +
>> +if (!console_has_gl(g->parent_obj.scanout[ss.scanout_id].con)) {
>> +qemu_log_mask(LOG_GUEST_ERROR, "%s: unable to scanout blot without
>> GL!\n", __func__);
>> +return;
>> +}
>> +
>> +vres = virgl_gpu_find_resource(g, ss.resource_id);
>> +if (!vres) {
>> +qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: illegal resource specified %d\n",
>> + __func__, ss.resource_id);
>> +cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
>> +return;
>> +}
>> +if (virgl_renderer_resource_get_info(ss.resource_id, )) {
>> +qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: illegal virgl resource specified %d\n",
>> + __func__, ss.resource_id);
>> +cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
>> +return;
>> +}
>
> Minor nit, the format of the following needs to include braces.
>
>> +if (!vres->res.dmabuf_fd && info.fd)
>> +vres->res.dmabuf_fd = info.fd;
>
> However I'm seeing:
>
> cc -m64 -mcx16 -Ilibcommon.fa.p -I../../common-user/host/x86_64
> -I../../linux-user/include/host/x86_64 -I../../linux-user/include -Iui
> -I../../ui -I/usr/include/capstone -I/usr/include/p11-kit-1
> -I/usr/include/pixman-1 -I/usr/include/libpng16 -I/usr/include/spice-server
> -I/usr/include/spice-1 -I/usr/include/libusb-1.0 -I/usr/include/SDL2
> -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include
> -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/gio-unix-2.0
> -I/usr/include/slirp -I/usr/include/gtk-3.0 -I/usr/include/pango-1.0
> -I/usr/include/harfbuzz -I/usr/include/freetype2 -I/usr/include/fribidi
> -I/usr/include/cairo -I/usr/include/gdk-pixbuf-2.0
> -I/usr/include/x86_64-linux-gnu -I/usr/include/atk-1.0
> -I/usr/include/at-spi2-atk/2.0 -I/usr/include/at-spi-2.0
> -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include
> -I/usr/include/vte-2.91 -I/usr/include/virgl
> -I/home/alex/lsrc/qemu.git/builds/extra.libs/install/include
> -I/usr/include/cacard -I/usr/include/nss -I/usr/include/nspr
> -I/usr/include/PCSC -I/usr/include/pipewire-0.3 -I/usr/include/spa-0.2
> -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g
> -fstack-protector-strong -Wundef -Wwrite-strings -Wmissing-prototypes
> -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration
> -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k
> -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels
> -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute
> -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -Wshadow=local
> -isystem /home/alex/lsrc/qemu.git/linux-headers -isystem linux-headers
> -iquote . -iquote /home/alex/lsrc/qemu.git -iquote
> /home/alex/lsrc/qemu.git/include -iquote
> /home/alex/lsrc/qemu.git/host/include/x86_64 -iquote
> /home/alex/lsrc/qemu.git/host/include/generic -iquote
> /home/alex/lsrc/qemu.git/tcg/i386 -pthread -D_GNU_SOURCE
> -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common
>
Re: [PULL 00/26] Migration 20240104 patches
On Thu, 4 Jan 2024 at 04:33, wrote: > > From: Peter Xu > > The following changes since commit 7425b6277f12e82952cede1f531bfc689bf77fb1: > > Merge tag 'tracing-pull-request' of https://gitlab.com/stefanha/qemu into > staging (2023-12-27 05:15:32 -0500) > > are available in the Git repository at: > > https://gitlab.com/peterx/qemu.git tags/migration-20240104-pull-request > > for you to fetch changes up to b12635ff08ab2e5a6a955c6866ef4525fb3deb5d: > > migration: fix coverity migrate_mode finding (2024-01-04 09:52:42 +0800) > > > migration 1st pull for 9.0 > > - We lost Juan and Leo in the maintainers file > - Steven's suspend state fix > - Steven's fix for coverity on migrate_mode > - Avihai's migration cleanup series > > I notice that your gpg key doesn't seem to be signed by anybody else; you might look at whether it's easy to get it signed by somebody else (eg some of your redhat colleagues). Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/9.0 for any user-visible changes. -- PMM
[PULL 63/71] hw: Simplify memory_region_init_ram() calls
Mechanical change using the following coccinelle script:
@@
expression mr, owner, arg3, arg4, errp;
@@
- memory_region_init_ram(mr, owner, arg3, arg4, );
if (
- errp
+ !memory_region_init_ram(mr, owner, arg3, arg4, )
) {
...
return;
}
and removing the local Error variable.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Manos Pitsidianakis
Reviewed-by: Andrew Jeffery # aspeed
Reviewed-by: Gavin Shan
Message-Id: <20231120213301.24349-21-phi...@linaro.org>
---
hw/arm/aspeed_ast2400.c | 6 ++
hw/arm/aspeed_ast2600.c | 6 ++
hw/arm/fsl-imx25.c | 6 ++
hw/arm/fsl-imx31.c | 6 ++
hw/arm/fsl-imx6.c | 6 ++
hw/arm/integratorcp.c | 7 ++-
hw/arm/nrf51_soc.c | 7 ++-
hw/ppc/rs6000_mc.c | 7 ++-
8 files changed, 16 insertions(+), 35 deletions(-)
diff --git a/hw/arm/aspeed_ast2400.c b/hw/arm/aspeed_ast2400.c
index a4334c81b8..0baa2ff96e 100644
--- a/hw/arm/aspeed_ast2400.c
+++ b/hw/arm/aspeed_ast2400.c
@@ -247,7 +247,6 @@ static void aspeed_ast2400_soc_realize(DeviceState *dev,
Error **errp)
Aspeed2400SoCState *a = ASPEED2400_SOC(dev);
AspeedSoCState *s = ASPEED_SOC(dev);
AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
-Error *err = NULL;
g_autofree char *sram_name = NULL;
/* Default boot region (SPI memory or ROMs) */
@@ -276,9 +275,8 @@ static void aspeed_ast2400_soc_realize(DeviceState *dev,
Error **errp)
/* SRAM */
sram_name = g_strdup_printf("aspeed.sram.%d", CPU(>cpu[0])->cpu_index);
-memory_region_init_ram(>sram, OBJECT(s), sram_name, sc->sram_size,
);
-if (err) {
-error_propagate(errp, err);
+if (!memory_region_init_ram(>sram, OBJECT(s), sram_name, sc->sram_size,
+errp)) {
return;
}
memory_region_add_subregion(s->memory,
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index b965fbab5e..3a9a303ab8 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -282,7 +282,6 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev,
Error **errp)
Aspeed2600SoCState *a = ASPEED2600_SOC(dev);
AspeedSoCState *s = ASPEED_SOC(dev);
AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
-Error *err = NULL;
qemu_irq irq;
g_autofree char *sram_name = NULL;
@@ -355,9 +354,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev,
Error **errp)
/* SRAM */
sram_name = g_strdup_printf("aspeed.sram.%d", CPU(>cpu[0])->cpu_index);
-memory_region_init_ram(>sram, OBJECT(s), sram_name, sc->sram_size,
);
-if (err) {
-error_propagate(errp, err);
+if (!memory_region_init_ram(>sram, OBJECT(s), sram_name, sc->sram_size,
+errp)) {
return;
}
memory_region_add_subregion(s->memory,
diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
index 9aabbf7f58..b15435ccaf 100644
--- a/hw/arm/fsl-imx25.c
+++ b/hw/arm/fsl-imx25.c
@@ -299,10 +299,8 @@ static void fsl_imx25_realize(DeviceState *dev, Error
**errp)
>rom[1]);
/* initialize internal RAM (128 KB) */
-memory_region_init_ram(>iram, NULL, "imx25.iram", FSL_IMX25_IRAM_SIZE,
- );
-if (err) {
-error_propagate(errp, err);
+if (!memory_region_init_ram(>iram, NULL, "imx25.iram",
+FSL_IMX25_IRAM_SIZE, errp)) {
return;
}
memory_region_add_subregion(get_system_memory(), FSL_IMX25_IRAM_ADDR,
diff --git a/hw/arm/fsl-imx31.c b/hw/arm/fsl-imx31.c
index def27bb913..1d5dcd51e8 100644
--- a/hw/arm/fsl-imx31.c
+++ b/hw/arm/fsl-imx31.c
@@ -208,10 +208,8 @@ static void fsl_imx31_realize(DeviceState *dev, Error
**errp)
>rom);
/* initialize internal RAM (16 KB) */
-memory_region_init_ram(>iram, NULL, "imx31.iram", FSL_IMX31_IRAM_SIZE,
- );
-if (err) {
-error_propagate(errp, err);
+if (!memory_region_init_ram(>iram, NULL, "imx31.iram",
+FSL_IMX31_IRAM_SIZE, errp)) {
return;
}
memory_region_add_subregion(get_system_memory(), FSL_IMX31_IRAM_ADDR,
diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index 7dc42cbfe6..58f37e7c11 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -443,10 +443,8 @@ static void fsl_imx6_realize(DeviceState *dev, Error
**errp)
>caam);
/* OCRAM memory */
-memory_region_init_ram(>ocram, NULL, "imx6.ocram", FSL_IMX6_OCRAM_SIZE,
- );
-if (err) {
-error_propagate(errp, err);
+if (!memory_region_init_ram(>ocram, NULL, "imx6.ocram",
+FSL_IMX6_OCRAM_SIZE, errp)) {
return;
}
memory_region_add_subregion(get_system_memory(), FSL_IMX6_OCRAM_ADDR,
diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index e602ca5e14..1830e1d785
[PULL 45/71] memory: Have memory_region_init_ram_nomigrate() handler return a boolean
Following the example documented since commit e3fe3988d7 ("error:
Document Error API usage rules"), have memory_region_init_ram_nomigrate
return a boolean indicating whether an error is set or not.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Manos Pitsidianakis
Reviewed-by: Peter Xu
Reviewed-by: Gavin Shan
Message-Id: <20231120213301.24349-3-phi...@linaro.org>
---
include/exec/memory.h | 4 +++-
system/memory.c | 5 +++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1df7fb0fed..413fbda089 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1288,8 +1288,10 @@ void memory_region_init_io(MemoryRegion *mr,
*
* Note that this function does not do anything to cause the data in the
* RAM memory region to be migrated; that is the responsibility of the caller.
+ *
+ * Return: true on success, else false setting @errp with error.
*/
-void memory_region_init_ram_nomigrate(MemoryRegion *mr,
+bool memory_region_init_ram_nomigrate(MemoryRegion *mr,
Object *owner,
const char *name,
uint64_t size,
diff --git a/system/memory.c b/system/memory.c
index 6110e851f1..f9e5ae22d5 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1546,13 +1546,14 @@ void memory_region_init_io(MemoryRegion *mr,
mr->terminates = true;
}
-void memory_region_init_ram_nomigrate(MemoryRegion *mr,
+bool memory_region_init_ram_nomigrate(MemoryRegion *mr,
Object *owner,
const char *name,
uint64_t size,
Error **errp)
{
-memory_region_init_ram_flags_nomigrate(mr, owner, name, size, 0, errp);
+return memory_region_init_ram_flags_nomigrate(mr, owner, name,
+ size, 0, errp);
}
bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
--
2.41.0
[PULL 62/71] misc: Simplify qemu_prealloc_mem() calls
Since qemu_prealloc_mem() returns whether or not an error
occured, we don't need to check the @errp pointer. Remove
local_err uses when we can return directly.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Manos Pitsidianakis
Reviewed-by: Gavin Shan
Message-Id: <20231120213301.24349-20-phi...@linaro.org>
---
backends/hostmem.c | 22 +++---
hw/virtio/virtio-mem.c | 6 ++
2 files changed, 9 insertions(+), 19 deletions(-)
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 1b0043a0d9..30f69b2cb5 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -219,7 +219,6 @@ static bool host_memory_backend_get_prealloc(Object *obj,
Error **errp)
static void host_memory_backend_set_prealloc(Object *obj, bool value,
Error **errp)
{
-Error *local_err = NULL;
HostMemoryBackend *backend = MEMORY_BACKEND(obj);
if (!backend->reserve && value) {
@@ -237,10 +236,8 @@ static void host_memory_backend_set_prealloc(Object *obj,
bool value,
void *ptr = memory_region_get_ram_ptr(>mr);
uint64_t sz = memory_region_size(>mr);
-qemu_prealloc_mem(fd, ptr, sz, backend->prealloc_threads,
- backend->prealloc_context, _err);
-if (local_err) {
-error_propagate(errp, local_err);
+if (!qemu_prealloc_mem(fd, ptr, sz, backend->prealloc_threads,
+ backend->prealloc_context, errp)) {
return;
}
backend->prealloc = true;
@@ -398,16 +395,11 @@ host_memory_backend_memory_complete(UserCreatable *uc,
Error **errp)
* This is necessary to guarantee memory is allocated with
* specified NUMA policy in place.
*/
-if (backend->prealloc) {
-Error *local_err = NULL;
-
-qemu_prealloc_mem(memory_region_get_fd(>mr), ptr, sz,
- backend->prealloc_threads,
- backend->prealloc_context, _err);
-if (local_err) {
-error_propagate(errp, local_err);
-return;
-}
+if (backend->prealloc &&
!qemu_prealloc_mem(memory_region_get_fd(>mr),
+ptr, sz,
+backend->prealloc_threads,
+backend->prealloc_context,
errp)) {
+return;
}
}
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index dc4709790f..99ab989852 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -605,8 +605,7 @@ static int virtio_mem_set_block_state(VirtIOMEM *vmem,
uint64_t start_gpa,
int fd = memory_region_get_fd(>memdev->mr);
Error *local_err = NULL;
-qemu_prealloc_mem(fd, area, size, 1, NULL, _err);
-if (local_err) {
+if (!qemu_prealloc_mem(fd, area, size, 1, NULL, _err)) {
static bool warned;
/*
@@ -1249,8 +1248,7 @@ static int virtio_mem_prealloc_range_cb(VirtIOMEM *vmem,
void *arg,
int fd = memory_region_get_fd(>memdev->mr);
Error *local_err = NULL;
-qemu_prealloc_mem(fd, area, size, 1, NULL, _err);
-if (local_err) {
+if (!qemu_prealloc_mem(fd, area, size, 1, NULL, _err)) {
error_report_err(local_err);
return -ENOMEM;
}
--
2.41.0
[PULL 52/71] memory: Simplify memory_region_init_rom_device_nomigrate() calls
Mechanical change using the following coccinelle script:
@@
expression mr, owner, arg3, arg4, arg5, arg6, errp;
@@
- memory_region_init_rom_device_nomigrate(mr, owner, arg3, arg4, arg5, arg6,
);
if (
- errp
+ !memory_region_init_rom_device_nomigrate(mr, owner, arg3, arg4, arg5,
arg6, )
) {
...
return;
}
and removing the local Error variable.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Peter Xu
Reviewed-by: Gavin Shan
Message-Id: <20231120213301.24349-10-phi...@linaro.org>
---
system/memory.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/system/memory.c b/system/memory.c
index 8db271c19e..c6562f5e86 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -3627,12 +3627,9 @@ void memory_region_init_rom_device(MemoryRegion *mr,
Error **errp)
{
DeviceState *owner_dev;
-Error *err = NULL;
-memory_region_init_rom_device_nomigrate(mr, owner, ops, opaque,
-name, size, );
-if (err) {
-error_propagate(errp, err);
+if (!memory_region_init_rom_device_nomigrate(mr, owner, ops, opaque,
+ name, size, errp)) {
return;
}
/* This will assert if owner is neither NULL nor a DeviceState.
--
2.41.0
[PULL 16/71] target/openrisc: Use generic cpu_list()
From: Gavin Shan
Before it's applied:
[gshan@gshan q]$ ./build/qemu-or1k -cpu ?
Available CPUs:
or1200
any
After it's applied:
[gshan@gshan q]$ ./build/qemu-or1k -cpu ?
Available CPUs:
any
or1200
Signed-off-by: Gavin Shan
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-ID: <20231114235628.534334-17-gs...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé
---
target/openrisc/cpu.h | 3 ---
target/openrisc/cpu.c | 42 --
2 files changed, 45 deletions(-)
diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
index dedeb89f8e..b454014ddd 100644
--- a/target/openrisc/cpu.h
+++ b/target/openrisc/cpu.h
@@ -299,15 +299,12 @@ struct ArchCPU {
CPUOpenRISCState env;
};
-void cpu_openrisc_list(void);
void openrisc_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
int openrisc_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
int openrisc_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
void openrisc_translate_init(void);
int print_insn_or1k(bfd_vma addr, disassemble_info *info);
-#define cpu_list cpu_openrisc_list
-
#ifndef CONFIG_USER_ONLY
hwaddr openrisc_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index f7d53c592a..381ebe00d3 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -253,48 +253,6 @@ static void openrisc_cpu_class_init(ObjectClass *oc, void
*data)
cc->tcg_ops = _tcg_ops;
}
-/* Sort alphabetically by type name, except for "any". */
-static gint openrisc_cpu_list_compare(gconstpointer a, gconstpointer b)
-{
-ObjectClass *class_a = (ObjectClass *)a;
-ObjectClass *class_b = (ObjectClass *)b;
-const char *name_a, *name_b;
-
-name_a = object_class_get_name(class_a);
-name_b = object_class_get_name(class_b);
-if (strcmp(name_a, "any-" TYPE_OPENRISC_CPU) == 0) {
-return 1;
-} else if (strcmp(name_b, "any-" TYPE_OPENRISC_CPU) == 0) {
-return -1;
-} else {
-return strcmp(name_a, name_b);
-}
-}
-
-static void openrisc_cpu_list_entry(gpointer data, gpointer user_data)
-{
-ObjectClass *oc = data;
-const char *typename;
-char *name;
-
-typename = object_class_get_name(oc);
-name = g_strndup(typename,
- strlen(typename) - strlen("-" TYPE_OPENRISC_CPU));
-qemu_printf(" %s\n", name);
-g_free(name);
-}
-
-void cpu_openrisc_list(void)
-{
-GSList *list;
-
-list = object_class_get_list(TYPE_OPENRISC_CPU, false);
-list = g_slist_sort(list, openrisc_cpu_list_compare);
-qemu_printf("Available CPUs:\n");
-g_slist_foreach(list, openrisc_cpu_list_entry, NULL);
-g_slist_free(list);
-}
-
#define DEFINE_OPENRISC_CPU_TYPE(cpu_model, initfn) \
{ \
.parent = TYPE_OPENRISC_CPU,\
--
2.41.0
[PULL 61/71] util/oslib: Have qemu_prealloc_mem() handler return a boolean
Following the example documented since commit e3fe3988d7 ("error:
Document Error API usage rules"), have qemu_prealloc_mem()
return a boolean indicating whether an error is set or not.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Peter Xu
Reviewed-by: Gavin Shan
Message-Id: <20231120213301.24349-19-phi...@linaro.org>
---
include/qemu/osdep.h | 4 +++-
util/oslib-posix.c | 7 +--
util/oslib-win32.c | 4 +++-
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index d30ba73eda..db366d6796 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -678,8 +678,10 @@ typedef struct ThreadContext ThreadContext;
* memory area starting at @area with the size of @sz. After a successful call,
* each page in the area was faulted in writable at least once, for example,
* after allocating file blocks for mapped files.
+ *
+ * Return: true on success, else false setting @errp with error.
*/
-void qemu_prealloc_mem(int fd, char *area, size_t sz, int max_threads,
+bool qemu_prealloc_mem(int fd, char *area, size_t sz, int max_threads,
ThreadContext *tc, Error **errp);
/**
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index e86fd64e09..7c297003b9 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -497,7 +497,7 @@ static bool madv_populate_write_possible(char *area, size_t
pagesize)
errno != EINVAL;
}
-void qemu_prealloc_mem(int fd, char *area, size_t sz, int max_threads,
+bool qemu_prealloc_mem(int fd, char *area, size_t sz, int max_threads,
ThreadContext *tc, Error **errp)
{
static gsize initialized;
@@ -506,6 +506,7 @@ void qemu_prealloc_mem(int fd, char *area, size_t sz, int
max_threads,
size_t numpages = DIV_ROUND_UP(sz, hpagesize);
bool use_madv_populate_write;
struct sigaction act;
+bool rv = true;
/*
* Sense on every invocation, as MADV_POPULATE_WRITE cannot be used for
@@ -534,7 +535,7 @@ void qemu_prealloc_mem(int fd, char *area, size_t sz, int
max_threads,
qemu_mutex_unlock(_mutex);
error_setg_errno(errp, errno,
"qemu_prealloc_mem: failed to install signal handler");
-return;
+return false;
}
}
@@ -544,6 +545,7 @@ void qemu_prealloc_mem(int fd, char *area, size_t sz, int
max_threads,
if (ret) {
error_setg_errno(errp, -ret,
"qemu_prealloc_mem: preallocating memory failed");
+rv = false;
}
if (!use_madv_populate_write) {
@@ -555,6 +557,7 @@ void qemu_prealloc_mem(int fd, char *area, size_t sz, int
max_threads,
}
qemu_mutex_unlock(_mutex);
}
+return rv;
}
char *qemu_get_pid_name(pid_t pid)
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 55b0189dc3..c4a5f05a49 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -264,7 +264,7 @@ int getpagesize(void)
return system_info.dwPageSize;
}
-void qemu_prealloc_mem(int fd, char *area, size_t sz, int max_threads,
+bool qemu_prealloc_mem(int fd, char *area, size_t sz, int max_threads,
ThreadContext *tc, Error **errp)
{
int i;
@@ -274,6 +274,8 @@ void qemu_prealloc_mem(int fd, char *area, size_t sz, int
max_threads,
for (i = 0; i < sz / pagesize; i++) {
memset(area + pagesize * i, 0, 1);
}
+
+return true;
}
char *qemu_get_pid_name(pid_t pid)
--
2.41.0
[PULL 67/71] hw/nvram: Simplify memory_region_init_rom_device() calls
Mechanical change using the following coccinelle script:
@@
expression mr, owner, arg3, arg4, arg5, arg6, errp;
@@
- memory_region_init_rom_device(mr, owner, arg3, arg4, arg5, arg6, );
if (
- errp
+ !memory_region_init_rom_device(mr, owner, arg3, arg4, arg5, arg6, )
) {
...
return;
}
and removing the local Error variable.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Manos Pitsidianakis
Reviewed-by: Gavin Shan
Message-Id: <20231120213301.24349-25-phi...@linaro.org>
---
hw/nvram/nrf51_nvm.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/hw/nvram/nrf51_nvm.c b/hw/nvram/nrf51_nvm.c
index ed8b836074..73564f7e6e 100644
--- a/hw/nvram/nrf51_nvm.c
+++ b/hw/nvram/nrf51_nvm.c
@@ -336,12 +336,9 @@ static void nrf51_nvm_init(Object *obj)
static void nrf51_nvm_realize(DeviceState *dev, Error **errp)
{
NRF51NVMState *s = NRF51_NVM(dev);
-Error *err = NULL;
-memory_region_init_rom_device(>flash, OBJECT(dev), _ops, s,
-"nrf51_soc.flash", s->flash_size, );
-if (err) {
-error_propagate(errp, err);
+if (!memory_region_init_rom_device(>flash, OBJECT(dev), _ops, s,
+ "nrf51_soc.flash", s->flash_size,
errp)) {
return;
}
--
2.41.0
[PULL 60/71] backends: Reduce variable scope in host_memory_backend_memory_complete
Reduce the _err variable use and remove the 'out:' label.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Manos Pitsidianakis
Reviewed-by: Gavin Shan
Message-Id: <20231120213301.24349-18-phi...@linaro.org>
---
backends/hostmem.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 3f8eb936d7..1b0043a0d9 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -324,7 +324,6 @@ host_memory_backend_memory_complete(UserCreatable *uc,
Error **errp)
{
HostMemoryBackend *backend = MEMORY_BACKEND(uc);
HostMemoryBackendClass *bc = MEMORY_BACKEND_GET_CLASS(uc);
-Error *local_err = NULL;
void *ptr;
uint64_t sz;
@@ -400,15 +399,16 @@ host_memory_backend_memory_complete(UserCreatable *uc,
Error **errp)
* specified NUMA policy in place.
*/
if (backend->prealloc) {
+Error *local_err = NULL;
+
qemu_prealloc_mem(memory_region_get_fd(>mr), ptr, sz,
backend->prealloc_threads,
backend->prealloc_context, _err);
if (local_err) {
-goto out;
+error_propagate(errp, local_err);
+return;
}
}
-out:
-error_propagate(errp, local_err);
}
static bool
--
2.41.0
[PULL 39/71] hw/arm/bcm2836: Use ARM_CPU 'mp-affinity' property
The 'mp-affinity' property is present since commit 15a21fe028
("target-arm: Add mp-affinity property for ARM CPU class").
Use it and remove a /* TODO */ comment. Since all ARM CPUs
have this property, use _abort, because this call can
not fail.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Reviewed-by: Peter Maydell
Message-Id: <20231123143813.42632-4-phi...@linaro.org>
---
hw/arm/bcm2836.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index a1bd1406e1..289c30e6b6 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -127,8 +127,8 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
qdev_get_gpio_in_named(DEVICE(>control), "gpu-fiq", 0));
for (n = 0; n < BCM283X_NCPUS; n++) {
-/* TODO: this should be converted to a property of ARM_CPU */
-s->cpu[n].core.mp_affinity = (bc->clusterid << 8) | n;
+object_property_set_int(OBJECT(>cpu[n].core), "mp-affinity",
+(bc->clusterid << 8) | n, _abort);
/* set periphbase/CBAR value for CPU-local registers */
object_property_set_int(OBJECT(>cpu[n].core), "reset-cbar",
--
2.41.0
[PULL 38/71] hw/arm/bcm2836: Simplify use of 'reset-cbar' property
bcm2836_realize() is called by
- bcm2836_class_init() which sets:
bc->cpu_type = ARM_CPU_TYPE_NAME("cortex-a7")
- bcm2837_class_init() which sets:
bc->cpu_type = ARM_CPU_TYPE_NAME("cortex-a53")
Both Cortex-A7 / A53 have the ARM_FEATURE_CBAR set. If it isn't,
then this is a programming error: use _abort.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Reviewed-by: Peter Maydell
Message-Id: <20231123143813.42632-3-phi...@linaro.org>
---
hw/arm/bcm2836.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 166dc896c0..a1bd1406e1 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -131,10 +131,8 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
s->cpu[n].core.mp_affinity = (bc->clusterid << 8) | n;
/* set periphbase/CBAR value for CPU-local registers */
-if (!object_property_set_int(OBJECT(>cpu[n].core), "reset-cbar",
- bc->peri_base, errp)) {
-return;
-}
+object_property_set_int(OBJECT(>cpu[n].core), "reset-cbar",
+bc->peri_base, _abort);
/* start powered off if not enabled */
if (!object_property_set_bool(OBJECT(>cpu[n].core),
--
2.41.0
[PULL 70/71] hw/net/can/sja1000: fix bug for single acceptance filter and standard frame
From: Pavel Pisa
A CAN sja1000 standard frame filter mask has been computed and applied
incorrectly for standard frames when single Acceptance Filter Mode
(MOD_AFM = 1) has been selected. The problem has not been found
by Linux kernel testing because it uses dual filter mode (MOD_AFM = 0)
and leaves falters fully open.
The problem has been noticed by Grant Ramsay when testing with Zephyr
RTOS which uses single filter mode.
Signed-off-by: Pavel Pisa
Reported-by: Grant Ramsay
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2028
Fixes: 733210e754 ("hw/net/can: SJA1000 chip register level emulation")
Message-ID: <20240103231426.5685-1-p...@fel.cvut.cz>
---
hw/net/can/can_sja1000.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
index 1165d59824..6694d7bfd8 100644
--- a/hw/net/can/can_sja1000.c
+++ b/hw/net/can/can_sja1000.c
@@ -108,7 +108,7 @@ void can_sja_single_filter(struct qemu_can_filter *filter,
}
filter->can_mask = (uint32_t)amr[0] << 3;
-filter->can_mask |= (uint32_t)amr[1] << 5;
+filter->can_mask |= (uint32_t)amr[1] >> 5;
filter->can_mask = ~filter->can_mask & QEMU_CAN_SFF_MASK;
if (!(amr[1] & 0x10)) {
filter->can_mask |= QEMU_CAN_RTR_FLAG;
--
2.41.0
[PULL 51/71] memory: Have memory_region_init_rom_device_nomigrate() return a boolean
Following the example documented since commit e3fe3988d7
("error: Document Error API usage rules"), have
memory_region_init_rom_device_nomigrate() return a boolean
indicating whether an error is set or not.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Peter Xu
Reviewed-by: Gavin Shan
Message-Id: <20231120213301.24349-9-phi...@linaro.org>
---
include/exec/memory.h | 4 +++-
system/memory.c | 4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 9f54bc4af8..6d7b49b735 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1523,8 +1523,10 @@ bool memory_region_init_rom_nomigrate(MemoryRegion *mr,
*must be unique within any device
* @size: size of the region.
* @errp: pointer to Error*, to store an error if it happens.
+ *
+ * Return: true on success, else false setting @errp with error.
*/
-void memory_region_init_rom_device_nomigrate(MemoryRegion *mr,
+bool memory_region_init_rom_device_nomigrate(MemoryRegion *mr,
Object *owner,
const MemoryRegionOps *ops,
void *opaque,
diff --git a/system/memory.c b/system/memory.c
index 069aa5ee08..8db271c19e 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1716,7 +1716,7 @@ bool memory_region_init_rom_nomigrate(MemoryRegion *mr,
return true;
}
-void memory_region_init_rom_device_nomigrate(MemoryRegion *mr,
+bool memory_region_init_rom_device_nomigrate(MemoryRegion *mr,
Object *owner,
const MemoryRegionOps *ops,
void *opaque,
@@ -1737,7 +1737,9 @@ void memory_region_init_rom_device_nomigrate(MemoryRegion
*mr,
mr->size = int128_zero();
object_unparent(OBJECT(mr));
error_propagate(errp, err);
+return false;
}
+return true;
}
void memory_region_init_iommu(void *_iommu_mr,
--
2.41.0
[PULL 65/71] hw/sparc: Simplify memory_region_init_ram_nomigrate() calls
Mechanical change using the following coccinelle script:
@@
expression mr, owner, arg3, arg4, errp;
@@
- memory_region_init_ram_nomigrate(mr, owner, arg3, arg4, );
if (
- errp
+ !memory_region_init_ram_nomigrate(mr, owner, arg3, arg4, )
) {
...
return;
}
and removing the local Error variable.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Gavin Shan
Message-Id: <20231120213301.24349-23-phi...@linaro.org>
---
hw/sparc/sun4m.c | 21 ++---
hw/sparc64/sun4u.c | 7 ++-
2 files changed, 8 insertions(+), 20 deletions(-)
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 64895aebe3..550af01690 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -577,12 +577,9 @@ static void idreg_realize(DeviceState *ds, Error **errp)
{
IDRegState *s = MACIO_ID_REGISTER(ds);
SysBusDevice *dev = SYS_BUS_DEVICE(ds);
-Error *local_err = NULL;
-memory_region_init_ram_nomigrate(>mem, OBJECT(ds), "sun4m.idreg",
- sizeof(idreg_data), _err);
-if (local_err) {
-error_propagate(errp, local_err);
+if (!memory_region_init_ram_nomigrate(>mem, OBJECT(ds), "sun4m.idreg",
+ sizeof(idreg_data), errp)) {
return;
}
@@ -631,12 +628,9 @@ static void afx_realize(DeviceState *ds, Error **errp)
{
AFXState *s = TCX_AFX(ds);
SysBusDevice *dev = SYS_BUS_DEVICE(ds);
-Error *local_err = NULL;
-memory_region_init_ram_nomigrate(>mem, OBJECT(ds), "sun4m.afx", 4,
- _err);
-if (local_err) {
-error_propagate(errp, local_err);
+if (!memory_region_init_ram_nomigrate(>mem, OBJECT(ds), "sun4m.afx",
+ 4, errp)) {
return;
}
@@ -715,12 +709,9 @@ static void prom_realize(DeviceState *ds, Error **errp)
{
PROMState *s = OPENPROM(ds);
SysBusDevice *dev = SYS_BUS_DEVICE(ds);
-Error *local_err = NULL;
-memory_region_init_ram_nomigrate(>prom, OBJECT(ds), "sun4m.prom",
- PROM_SIZE_MAX, _err);
-if (local_err) {
-error_propagate(errp, local_err);
+if (!memory_region_init_ram_nomigrate(>prom, OBJECT(ds), "sun4m.prom",
+ PROM_SIZE_MAX, errp)) {
return;
}
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index c871170378..24d53bf5fd 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -454,12 +454,9 @@ static void prom_realize(DeviceState *ds, Error **errp)
{
PROMState *s = OPENPROM(ds);
SysBusDevice *dev = SYS_BUS_DEVICE(ds);
-Error *local_err = NULL;
-memory_region_init_ram_nomigrate(>prom, OBJECT(ds), "sun4u.prom",
- PROM_SIZE_MAX, _err);
-if (local_err) {
-error_propagate(errp, local_err);
+if (!memory_region_init_ram_nomigrate(>prom, OBJECT(ds), "sun4u.prom",
+ PROM_SIZE_MAX, errp)) {
return;
}
--
2.41.0
[PULL 66/71] hw/misc: Simplify memory_region_init_ram_from_fd() calls
Mechanical change using the following coccinelle script:
@@
expression mr, owner, arg3, arg4, arg5, arg6, arg7, errp;
@@
- memory_region_init_ram_from_fd(mr, owner, arg3, arg4, arg5, arg6, arg7,
);
if (
- errp
+ !memory_region_init_ram_from_fd(mr, owner, arg3, arg4, arg5, arg6,
arg7, )
) {
...
return;
}
and removing the local Error variable.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Manos Pitsidianakis
Reviewed-by: Gavin Shan
Message-Id: <20231120213301.24349-24-phi...@linaro.org>
---
hw/misc/ivshmem.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 62af75e862..a2fd0bc365 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -476,7 +476,6 @@ static void setup_interrupt(IVShmemState *s, int vector,
Error **errp)
static void process_msg_shmem(IVShmemState *s, int fd, Error **errp)
{
-Error *local_err = NULL;
struct stat buf;
size_t size;
@@ -496,10 +495,9 @@ static void process_msg_shmem(IVShmemState *s, int fd,
Error **errp)
size = buf.st_size;
/* mmap the region and map into the BAR2 */
-memory_region_init_ram_from_fd(>server_bar2, OBJECT(s), "ivshmem.bar2",
- size, RAM_SHARED, fd, 0, _err);
-if (local_err) {
-error_propagate(errp, local_err);
+if (!memory_region_init_ram_from_fd(>server_bar2, OBJECT(s),
+"ivshmem.bar2", size, RAM_SHARED,
+fd, 0, errp)) {
return;
}
--
2.41.0
[PULL 14/71] target/m68k: Use generic cpu_list()
From: Gavin Shan
Before it's applied:
[gshan@gshan q]$ ./build/qemu-system-m68k -cpu ?
cfv4e
m5206
m5208
m68000
m68010
m68020
m68030
m68040
m68060
any
After it's applied:
[gshan@gshan q]$ ./build/qemu-system-m68k -cpu ?
Available CPUs:
any
cfv4e
m5206
m5208
m68000
m68010
m68020
m68030
m68040
m68060
Signed-off-by: Gavin Shan
Reviewed-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Message-ID: <20231114235628.534334-15-gs...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé
---
target/m68k/cpu.h| 4
target/m68k/helper.c | 40
2 files changed, 44 deletions(-)
diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index 6cfc696d2b..d13427b0fe 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -556,8 +556,6 @@ static inline bool m68k_feature(CPUM68KState *env, int
feature)
return (env->features & BIT_ULL(feature)) != 0;
}
-void m68k_cpu_list(void);
-
void register_m68k_insns (CPUM68KState *env);
enum {
@@ -576,8 +574,6 @@ enum {
#define CPU_RESOLVING_TYPE TYPE_M68K_CPU
-#define cpu_list m68k_cpu_list
-
/* MMU modes definitions */
#define MMU_KERNEL_IDX 0
#define MMU_USER_IDX 1
diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 0a1544cd68..14508dfa11 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -29,46 +29,6 @@
#define SIGNBIT (1u << 31)
-/* Sort alphabetically, except for "any". */
-static gint m68k_cpu_list_compare(gconstpointer a, gconstpointer b)
-{
-ObjectClass *class_a = (ObjectClass *)a;
-ObjectClass *class_b = (ObjectClass *)b;
-const char *name_a, *name_b;
-
-name_a = object_class_get_name(class_a);
-name_b = object_class_get_name(class_b);
-if (strcmp(name_a, "any-" TYPE_M68K_CPU) == 0) {
-return 1;
-} else if (strcmp(name_b, "any-" TYPE_M68K_CPU) == 0) {
-return -1;
-} else {
-return strcasecmp(name_a, name_b);
-}
-}
-
-static void m68k_cpu_list_entry(gpointer data, gpointer user_data)
-{
-ObjectClass *c = data;
-const char *typename;
-char *name;
-
-typename = object_class_get_name(c);
-name = g_strndup(typename, strlen(typename) - strlen("-" TYPE_M68K_CPU));
-qemu_printf("%s\n", name);
-g_free(name);
-}
-
-void m68k_cpu_list(void)
-{
-GSList *list;
-
-list = object_class_get_list(TYPE_M68K_CPU, false);
-list = g_slist_sort(list, m68k_cpu_list_compare);
-g_slist_foreach(list, m68k_cpu_list_entry, NULL);
-g_slist_free(list);
-}
-
static int cf_fpu_gdb_get_reg(CPUM68KState *env, GByteArray *mem_buf, int n)
{
if (n < 8) {
--
2.41.0
[PULL 23/71] hw/core: Add machine_class_default_cpu_type()
Add a helper to return a machine default CPU type.
If this machine is restricted to a single CPU type,
use it as default, obviously.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-Id: <20231116163726.28952-1-phi...@linaro.org>
---
include/hw/boards.h | 6 ++
hw/core/machine.c | 8
system/vl.c | 2 +-
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 8af165f4dc..bcfde8a84d 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -24,6 +24,12 @@ OBJECT_DECLARE_TYPE(MachineState, MachineClass, MACHINE)
extern MachineState *current_machine;
+/**
+ * machine_class_default_cpu_type: Return the machine default CPU type.
+ * @mc: Machine class
+ */
+const char *machine_class_default_cpu_type(MachineClass *mc);
+
void machine_add_audiodev_property(MachineClass *mc);
void machine_run_board_init(MachineState *machine, const char *mem_path, Error
**errp);
bool machine_usb(MachineState *machine);
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2699bcba53..0198b54b39 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1390,6 +1390,14 @@ out:
return r;
}
+const char *machine_class_default_cpu_type(MachineClass *mc)
+{
+if (mc->valid_cpu_types && !mc->valid_cpu_types[1]) {
+/* Only a single CPU type allowed: use it as default. */
+return mc->valid_cpu_types[0];
+}
+return mc->default_cpu_type;
+}
void machine_run_board_init(MachineState *machine, const char *mem_path, Error
**errp)
{
diff --git a/system/vl.c b/system/vl.c
index 6b87bfa32c..fbdf8bd55a 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -3737,7 +3737,7 @@ void qemu_init(int argc, char **argv)
migration_object_init();
/* parse features once if machine provides default cpu_type */
-current_machine->cpu_type = machine_class->default_cpu_type;
+current_machine->cpu_type = machine_class_default_cpu_type(machine_class);
if (cpu_option) {
current_machine->cpu_type = parse_cpu_option(cpu_option);
}
--
2.41.0
[PULL 44/71] memory: Have memory_region_init_ram_flags_nomigrate() return a boolean
Following the example documented since commit e3fe3988d7 ("error:
Document Error API usage rules"), have memory_region_init_ram_nomigrate
return a boolean indicating whether an error is set or not.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Manos Pitsidianakis
Reviewed-by: Peter Xu
Reviewed-by: Gavin Shan
Message-Id: <20231120213301.24349-2-phi...@linaro.org>
---
include/exec/memory.h | 4 +++-
system/memory.c | 4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index f172e82ac9..1df7fb0fed 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1310,8 +1310,10 @@ void memory_region_init_ram_nomigrate(MemoryRegion *mr,
*
* Note that this function does not do anything to cause the data in the
* RAM memory region to be migrated; that is the responsibility of the caller.
+ *
+ * Return: true on success, else false setting @errp with error.
*/
-void memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
+bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
Object *owner,
const char *name,
uint64_t size,
diff --git a/system/memory.c b/system/memory.c
index 798b6c0a17..6110e851f1 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1555,7 +1555,7 @@ void memory_region_init_ram_nomigrate(MemoryRegion *mr,
memory_region_init_ram_flags_nomigrate(mr, owner, name, size, 0, errp);
}
-void memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
+bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
Object *owner,
const char *name,
uint64_t size,
@@ -1572,7 +1572,9 @@ void memory_region_init_ram_flags_nomigrate(MemoryRegion
*mr,
mr->size = int128_zero();
object_unparent(OBJECT(mr));
error_propagate(errp, err);
+return false;
}
+return true;
}
void memory_region_init_resizeable_ram(MemoryRegion *mr,
--
2.41.0
[PULL 25/71] machine: Introduce helper is_cpu_type_supported()
From: Gavin Shan
The logic, to check if the specified CPU type is supported in
machine_run_board_init(), is independent enough. Factor it out into
helper is_cpu_type_supported(). machine_run_board_init() looks a bit
clean with this. Since we're here, @machine_class is renamed to @mc to
avoid multiple line spanning of code. The comments are tweaked a bit
either.
No functional change intended.
Signed-off-by: Gavin Shan
Reviewed-by: Philippe Mathieu-Daudé
Message-ID: <20231204004726.483558-3-gs...@redhat.com>
[PMD: Only call new helper if machine->cpu_type is not NULL]
Signed-off-by: Philippe Mathieu-Daudé
---
hw/core/machine.c | 84 ++-
1 file changed, 46 insertions(+), 38 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 1898d1d1d7..0119b11fc8 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1399,12 +1399,53 @@ const char *machine_class_default_cpu_type(MachineClass
*mc)
return mc->default_cpu_type;
}
+static bool is_cpu_type_supported(const MachineState *machine, Error **errp)
+{
+MachineClass *mc = MACHINE_GET_CLASS(machine);
+ObjectClass *oc = object_class_by_name(machine->cpu_type);
+CPUClass *cc;
+int i;
+
+/*
+ * Check if the user specified CPU type is supported when the valid
+ * CPU types have been determined. Note that the user specified CPU
+ * type is provided through '-cpu' option.
+ */
+if (mc->valid_cpu_types) {
+for (i = 0; mc->valid_cpu_types[i]; i++) {
+if (object_class_dynamic_cast(oc, mc->valid_cpu_types[i])) {
+break;
+}
+}
+
+/* The user specified CPU type isn't valid */
+if (!mc->valid_cpu_types[i]) {
+error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
+error_append_hint(errp, "The valid types are: %s",
+ mc->valid_cpu_types[0]);
+for (i = 1; mc->valid_cpu_types[i]; i++) {
+error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
+}
+
+error_append_hint(errp, "\n");
+return false;
+}
+}
+
+/* Check if CPU type is deprecated and warn if so */
+cc = CPU_CLASS(oc);
+if (cc && cc->deprecation_note) {
+warn_report("CPU model %s is deprecated -- %s",
+machine->cpu_type, cc->deprecation_note);
+}
+
+return true;
+}
+
void machine_run_board_init(MachineState *machine, const char *mem_path, Error
**errp)
{
ERRP_GUARD();
MachineClass *machine_class = MACHINE_GET_CLASS(machine);
-ObjectClass *oc = object_class_by_name(machine->cpu_type);
-CPUClass *cc;
/* This checkpoint is required by replay to separate prior clock
reading from the other reads, because timer polling functions query
@@ -1459,42 +1500,9 @@ void machine_run_board_init(MachineState *machine, const
char *mem_path, Error *
machine->ram = machine_consume_memdev(machine, machine->memdev);
}
-/* If the machine supports the valid_cpu_types check and the user
- * specified a CPU with -cpu check here that the user CPU is supported.
- */
-if (machine_class->valid_cpu_types && machine->cpu_type) {
-int i;
-
-for (i = 0; machine_class->valid_cpu_types[i]; i++) {
-if (object_class_dynamic_cast(oc,
- machine_class->valid_cpu_types[i])) {
-/* The user specified CPU is in the valid field, we are
- * good to go.
- */
-break;
-}
-}
-
-if (!machine_class->valid_cpu_types[i]) {
-/* The user specified CPU is not valid */
-error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
-error_append_hint(errp, "The valid types are: %s",
- machine_class->valid_cpu_types[0]);
-for (i = 1; machine_class->valid_cpu_types[i]; i++) {
-error_append_hint(errp, ", %s",
- machine_class->valid_cpu_types[i]);
-}
-
-error_append_hint(errp, "\n");
-return;
-}
-}
-
-/* Check if CPU type is deprecated and warn if so */
-cc = CPU_CLASS(oc);
-if (cc && cc->deprecation_note) {
-warn_report("CPU model %s is deprecated -- %s", machine->cpu_type,
-cc->deprecation_note);
+/* Check if the CPU type is supported */
+if (machine->cpu_type && !is_cpu_type_supported(machine, errp)) {
+return;
}
if (machine->cgs) {
--
2.41.0
[PULL 50/71] memory: Have memory_region_init_rom() handler return a boolean
Following the example documented since commit e3fe3988d7 ("error:
Document Error API usage rules"), have memory_region_init_rom()
return a boolean indicating whether an error is set or not.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Manos Pitsidianakis
Reviewed-by: Gavin Shan
Message-Id: <20231120213301.24349-8-phi...@linaro.org>
---
include/exec/memory.h | 4 +++-
system/memory.c | 6 --
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index b2dce73e7f..9f54bc4af8 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1611,8 +1611,10 @@ bool memory_region_init_ram(MemoryRegion *mr,
*must be unique within any device
* @size: size of the region.
* @errp: pointer to Error*, to store an error if it happens.
+ *
+ * Return: true on success, else false setting @errp with error.
*/
-void memory_region_init_rom(MemoryRegion *mr,
+bool memory_region_init_rom(MemoryRegion *mr,
Object *owner,
const char *name,
uint64_t size,
diff --git a/system/memory.c b/system/memory.c
index 45ce6fd6c1..069aa5ee08 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -3593,7 +3593,7 @@ bool memory_region_init_ram(MemoryRegion *mr,
return true;
}
-void memory_region_init_rom(MemoryRegion *mr,
+bool memory_region_init_rom(MemoryRegion *mr,
Object *owner,
const char *name,
uint64_t size,
@@ -3602,7 +3602,7 @@ void memory_region_init_rom(MemoryRegion *mr,
DeviceState *owner_dev;
if (!memory_region_init_rom_nomigrate(mr, owner, name, size, errp)) {
-return;
+return false;
}
/* This will assert if owner is neither NULL nor a DeviceState.
* We only want the owner here for the purposes of defining a
@@ -3612,6 +3612,8 @@ void memory_region_init_rom(MemoryRegion *mr,
*/
owner_dev = DEVICE(owner);
vmstate_register_ram(mr, owner_dev);
+
+return true;
}
void memory_region_init_rom_device(MemoryRegion *mr,
--
2.41.0
[PULL 68/71] hw/pci-host/raven: Propagate error in raven_realize()
When an Error** reference is available, it is better to
propagate local errors, rather then using generic ones,
which might terminate the whole QEMU process.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Manos Pitsidianakis
Reviewed-by: Gavin Shan
Message-Id: <20231120213301.24349-26-phi...@linaro.org>
---
hw/pci-host/raven.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
index f71d4872c8..c7a0a2878a 100644
--- a/hw/pci-host/raven.c
+++ b/hw/pci-host/raven.c
@@ -345,8 +345,10 @@ static void raven_realize(PCIDevice *d, Error **errp)
d->config[PCI_LATENCY_TIMER] = 0x10;
d->config[PCI_CAPABILITY_LIST] = 0x00;
-memory_region_init_rom_nomigrate(>bios, OBJECT(s), "bios", BIOS_SIZE,
- _fatal);
+if (!memory_region_init_rom_nomigrate(>bios, OBJECT(s), "bios",
+ BIOS_SIZE, errp)) {
+return;
+}
memory_region_add_subregion(get_system_memory(), (uint32_t)(-BIOS_SIZE),
>bios);
if (s->bios_name) {
--
2.41.0
[PULL 17/71] target/riscv: Use generic cpu_list()
From: Gavin Shan
Before it's applied:
[gshan@gshan q]$ ./build/qemu-system-riscv64 -cpu ?
any
max
rv64
shakti-c
sifive-e51
sifive-u54
thead-c906
veyron-v1
x-rv128
After it's applied:
[gshan@gshan q]$ ./build/qemu-system-riscv64 -cpu ?
Available CPUs:
any
max
rv64
shakti-c
sifive-e51
sifive-u54
thead-c906
veyron-v1
x-rv128
Signed-off-by: Gavin Shan
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-ID: <20231114235628.534334-18-gs...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé
---
target/riscv/cpu.h | 2 --
target/riscv/cpu.c | 29 -
2 files changed, 31 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index d74b361be6..2725528bb5 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -490,9 +490,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
MMUAccessType access_type, int mmu_idx,
bool probe, uintptr_t retaddr);
char *riscv_isa_string(RISCVCPU *cpu);
-void riscv_cpu_list(void);
-#define cpu_list riscv_cpu_list
#define cpu_mmu_index riscv_cpu_mmu_index
#ifndef CONFIG_USER_ONLY
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 523e9a16ea..22d7422c89 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1733,35 +1733,6 @@ char *riscv_isa_string(RISCVCPU *cpu)
return isa_str;
}
-static gint riscv_cpu_list_compare(gconstpointer a, gconstpointer b)
-{
-ObjectClass *class_a = (ObjectClass *)a;
-ObjectClass *class_b = (ObjectClass *)b;
-const char *name_a, *name_b;
-
-name_a = object_class_get_name(class_a);
-name_b = object_class_get_name(class_b);
-return strcmp(name_a, name_b);
-}
-
-static void riscv_cpu_list_entry(gpointer data, gpointer user_data)
-{
-const char *typename = object_class_get_name(OBJECT_CLASS(data));
-int len = strlen(typename) - strlen(RISCV_CPU_TYPE_SUFFIX);
-
-qemu_printf("%.*s\n", len, typename);
-}
-
-void riscv_cpu_list(void)
-{
-GSList *list;
-
-list = object_class_get_list(TYPE_RISCV_CPU, false);
-list = g_slist_sort(list, riscv_cpu_list_compare);
-g_slist_foreach(list, riscv_cpu_list_entry, NULL);
-g_slist_free(list);
-}
-
#define DEFINE_CPU(type_name, initfn) \
{ \
.name = type_name, \
--
2.41.0
[PULL 41/71] hw: Simplify accesses to the CPUState::'start-powered-off' property
The 'start-powered-off' property has been added to ARM CPUs in
commit 5de164304a ("arm: Allow secondary KVM CPUs to be booted
via PSCI"), then eventually got generalized to all CPUs in commit
c1b701587e ("target/arm: Move start-powered-off property to generic
CPUState"). Since all CPUs have it, no need to check whether it is
available. Updating this property can't fail, so use _abort.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Reviewed-by: Peter Maydell
Message-Id: <20231123143813.42632-5-phi...@linaro.org>
---
hw/arm/armsse.c | 6 ++
hw/arm/armv7m.c | 8 ++--
hw/arm/bcm2836.c | 8 ++--
hw/mips/cps.c| 7 +++
hw/ppc/e500.c| 2 +-
hw/sparc/sun4m.c | 2 +-
6 files changed, 11 insertions(+), 22 deletions(-)
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index 02b4f6596f..91502d157a 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -1022,10 +1022,8 @@ static void armsse_realize(DeviceState *dev, Error
**errp)
* later if necessary.
*/
if (extract32(info->cpuwait_rst, i, 1)) {
-if (!object_property_set_bool(cpuobj, "start-powered-off", true,
- errp)) {
-return;
-}
+object_property_set_bool(cpuobj, "start-powered-off", true,
+ _abort);
}
if (!s->cpu_fpu[i]) {
if (!object_property_set_bool(cpuobj, "vfp", false, errp)) {
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 375a40962f..e39b61bc1a 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -318,12 +318,6 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
return;
}
}
-if (object_property_find(OBJECT(s->cpu), "start-powered-off")) {
-if (!object_property_set_bool(OBJECT(s->cpu), "start-powered-off",
- s->start_powered_off, errp)) {
-return;
-}
-}
if (object_property_find(OBJECT(s->cpu), "vfp")) {
if (!object_property_set_bool(OBJECT(s->cpu), "vfp", s->vfp, errp)) {
return;
@@ -334,6 +328,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
return;
}
}
+object_property_set_bool(OBJECT(s->cpu), "start-powered-off",
+ s->start_powered_off, _abort);
/*
* Real M-profile hardware can be configured with a different number of
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 289c30e6b6..b0674a22a6 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -135,12 +135,8 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
bc->peri_base, _abort);
/* start powered off if not enabled */
-if (!object_property_set_bool(OBJECT(>cpu[n].core),
- "start-powered-off",
- n >= s->enabled_cpus,
- errp)) {
-return;
-}
+object_property_set_bool(OBJECT(>cpu[n].core), "start-powered-off",
+ n >= s->enabled_cpus, _abort);
if (!qdev_realize(DEVICE(>cpu[n].core), NULL, errp)) {
return;
diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index b6612c1762..4f12e23ab5 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -78,10 +78,9 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
CPUMIPSState *env = >env;
/* All VPs are halted on reset. Leave powering up to CPC. */
-if (!object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
- errp)) {
-return;
-}
+object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
+ _abort);
+
/* All cores use the same clock tree */
qdev_connect_clock_in(DEVICE(cpu), "clk-in", s->clock);
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 384226296b..566f1200dd 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -955,7 +955,7 @@ void ppce500_init(MachineState *machine)
* when implementing non-kernel boot.
*/
object_property_set_bool(OBJECT(cs), "start-powered-off", i != 0,
- _fatal);
+ _abort);
qdev_realize_and_unref(DEVICE(cs), NULL, _fatal);
if (!firstenv) {
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 17bf5f2879..64895aebe3 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -804,7 +804,7 @@ static void cpu_devinit(const char *cpu_type, unsigned int
id,
qemu_register_reset(sun4m_cpu_reset, cpu);
object_property_set_bool(OBJECT(cpu), "start-powered-off", id != 0,
- _fatal);
+ _abort);
qdev_realize_and_unref(DEVICE(cpu), NULL, _fatal);
cpu_sparc_set_id(env, id);
*cpu_irqs =
[PULL 35/71] hw/cpu/core: Cleanup unused included header in core.c
From: Zhao Liu Remove unused header (qemu/module.h and sysemu/cpus.h) in core.c, and reorder the remaining header files (except qemu/osdep.h) in alphabetical order. Tested by "./configure" and then "make". Signed-off-by: Zhao Liu Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20231127145611.925817-2-zhao1@linux.intel.com> --- hw/cpu/core.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/hw/cpu/core.c b/hw/cpu/core.c index 9876075155..495a5c30ff 100644 --- a/hw/cpu/core.c +++ b/hw/cpu/core.c @@ -8,12 +8,11 @@ */ #include "qemu/osdep.h" -#include "hw/cpu/core.h" -#include "qapi/visitor.h" -#include "qemu/module.h" -#include "qapi/error.h" -#include "sysemu/cpus.h" + #include "hw/boards.h" +#include "hw/cpu/core.h" +#include "qapi/error.h" +#include "qapi/visitor.h" static void core_prop_get_core_id(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) -- 2.41.0
[PULL 48/71] memory: Simplify memory_region_init_ram_from_fd() calls
Mechanical change using the following coccinelle script:
@@
expression mr, owner, arg3, arg4, arg5, arg6, arg7, errp;
@@
- memory_region_init_ram_from_fd(mr, owner, arg3, arg4, arg5, arg6, arg7,
);
if (
- errp
+ !memory_region_init_ram_from_fd(mr, owner, arg3, arg4, arg5, arg6,
arg7, )
) {
...
return;
}
and removing the local Error variable.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Manos Pitsidianakis
Reviewed-by: Gavin Shan
Message-Id: <20231120213301.24349-6-phi...@linaro.org>
---
system/memory.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/system/memory.c b/system/memory.c
index 3997ac5928..c1374e9968 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -3577,11 +3577,8 @@ void memory_region_init_ram(MemoryRegion *mr,
Error **errp)
{
DeviceState *owner_dev;
-Error *err = NULL;
-memory_region_init_ram_nomigrate(mr, owner, name, size, );
-if (err) {
-error_propagate(errp, err);
+if (!memory_region_init_ram_nomigrate(mr, owner, name, size, errp)) {
return;
}
/* This will assert if owner is neither NULL nor a DeviceState.
--
2.41.0
