Re: [PATCH v3 3/3] cpu, softmmu/vl.c: Change parsing of -cpu argument to allow -cpu cpu, help to print options for the CPU type similar to how the '-device' option works.
Hi, Due to extracting CPU features via a qmp command, it only works on qemu-system-* builds. Building qmp for non system builds strikes me as extreme overkill so I need a way to exclude this from non system builds. Or maybe there's a way to disentangle querying CPU features independent from the qom or qmp based data structures it's currently intertwined with. Thanks, -Dinah On Tue, Nov 14, 2023 at 12:44 PM Markus Armbruster wrote: > Dinah B writes: > > > Hi, > > > > Is there a way to distinguish between qemu-system-* vs qemu-* builds? > > At first I thought #CONFIG_LINUX_USER might be it but not all non-mmu > > builds set this. > > What are you trying to accomplish? > >
Re: [PATCH v3 3/3] cpu, softmmu/vl.c: Change parsing of -cpu argument to allow -cpu cpu, help to print options for the CPU type similar to how the '-device' option works.
Hi, Is there a way to distinguish between qemu-system-* vs qemu-* builds? At first I thought #CONFIG_LINUX_USER might be it but not all non-mmu builds set this. Thanks, -Dinah On Wed, Aug 2, 2023 at 1:36 AM Markus Armbruster wrote: > Dinah B writes: > > > Thanks, I will fix this. I somehow didn't catch that you had replied to > the > > old one. > > Happens :) > >
Re: [PATCH v3 1/3] qapi: Moved architecture agnostic data types to `machine`
Hi,
I noticed that qapi now has a machine-common category - do you think these
changes would be more appropriate in that file
rather than "machine" for the revision?
Thanks and sorry for the delay,
-Dinah
On Tue, Aug 1, 2023 at 9:09 AM Markus Armbruster wrote:
> Dinah Baum writes:
>
> > Signed-off-by: Dinah Baum
> > ---
> > qapi/machine-target.json | 78 +---
> > qapi/machine.json| 77 +++
> > 2 files changed, 78 insertions(+), 77 deletions(-)
> >
> > diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> > index f0a6b72414..3ee2f7ca6b 100644
> > --- a/qapi/machine-target.json
> > +++ b/qapi/machine-target.json
> > @@ -4,83 +4,7 @@
> > # This work is licensed under the terms of the GNU GPL, version 2 or
> later.
> > # See the COPYING file in the top-level directory.
> >
> > -##
> > -# @CpuModelInfo:
> > -#
> > -# Virtual CPU model.
> > -#
> > -# A CPU model consists of the name of a CPU definition, to which delta
> > -# changes are applied (e.g. features added/removed). Most magic values
> > -# that an architecture might require should be hidden behind the name.
> > -# However, if required, architectures can expose relevant properties.
> > -#
> > -# @name: the name of the CPU definition the model is based on
> > -#
> > -# @props: a dictionary of QOM properties to be applied
> > -#
> > -# Since: 2.8
> > -##
> > -{ 'struct': 'CpuModelInfo',
> > - 'data': { 'name': 'str',
> > -'*props': 'any' } }
> > -
> > -##
> > -# @CpuModelExpansionType:
> > -#
> > -# An enumeration of CPU model expansion types.
> > -#
> > -# @static: Expand to a static CPU model, a combination of a static
> > -# base model name and property delta changes. As the static base
> > -# model will never change, the expanded CPU model will be the
> > -# same, independent of QEMU version, machine type, machine
> > -# options, and accelerator options. Therefore, the resulting
> > -# model can be used by tooling without having to specify a
> > -# compatibility machine - e.g. when displaying the "host" model.
> > -# The @static CPU models are migration-safe.
> > -#
> > -# @full: Expand all properties. The produced model is not guaranteed
> > -# to be migration-safe, but allows tooling to get an insight and
> > -# work with model details.
> > -#
> > -# Note: When a non-migration-safe CPU model is expanded in static
> > -# mode, some features enabled by the CPU model may be omitted,
> > -# because they can't be implemented by a static CPU model
> > -# definition (e.g. cache info passthrough and PMU passthrough in
> > -# x86). If you need an accurate representation of the features
> > -# enabled by a non-migration-safe CPU model, use @full. If you
> > -# need a static representation that will keep ABI compatibility
> > -# even when changing QEMU version or machine-type, use @static
> > -# (but keep in mind that some features may be omitted).
> > -#
> > -# Since: 2.8
> > -##
> > -{ 'enum': 'CpuModelExpansionType',
> > - 'data': [ 'static', 'full' ] }
> > -
> > -##
> > -# @CpuModelCompareResult:
> > -#
> > -# An enumeration of CPU model comparison results. The result is
> > -# usually calculated using e.g. CPU features or CPU generations.
> > -#
> > -# @incompatible: If model A is incompatible to model B, model A is not
> > -# guaranteed to run where model B runs and the other way around.
> > -#
> > -# @identical: If model A is identical to model B, model A is
> > -# guaranteed to run where model B runs and the other way around.
> > -#
> > -# @superset: If model A is a superset of model B, model B is
> > -# guaranteed to run where model A runs. There are no guarantees
> > -# about the other way.
> > -#
> > -# @subset: If model A is a subset of model B, model A is guaranteed to
> > -# run where model B runs. There are no guarantees about the other
> > -# way.
> > -#
> > -# Since: 2.8
> > -##
> > -{ 'enum': 'CpuModelCompareResult',
> > - 'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }
> > +{ 'include': 'machine.json' }
> >
> > ##
> > # @CpuModelBaselineInfo:
> > diff --git a/qapi/machine.json b/qapi/machine.json
> > index a08b6576ca..192c781310 100644
> > --- a/qapi/machine.json
> > +++ b/qapi/machine.json
> > @@ -1691,3 +1691,80 @@
> > { 'command': 'dumpdtb',
> >'data': { 'filename': 'str' },
> >'if': 'CONFIG_FDT' }
> > +
> > +##
> > +# @CpuModelInfo:
> > +#
> > +# Virtual CPU model.
> > +#
> > +# A CPU model consists of the name of a CPU definition, to which delta
> > +# changes are applied (e.g. features added/removed). Most magic values
> > +# that an architecture might require should be hidden behind the name.
> > +# However, if required, architectures can expose relevant properties.
> > +#
> > +# @name: the name of the CPU definition the model is based on
> > +#
> > +# @props: a dictionary of QOM properties to b
Re: [PATCH v3 3/3] cpu, softmmu/vl.c: Change parsing of -cpu argument to allow -cpu cpu, help to print options for the CPU type similar to how the '-device' option works.
Thanks, I will fix this. I somehow didn't catch that you had replied to the old one. -Dinah On Tue, Aug 1, 2023 at 10:10 AM Markus Armbruster wrote: > Dinah Baum writes: > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1480 > > Signed-off-by: Dinah Baum > > > > Signed-off-by: Dinah Baum > > Looks basically the same as v2, which means my review still applies. > > Message-ID: <878rdbfww1@pond.sub.org> > https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg06699.html > > If you need further assistance, just ask. > >
Re: [PATCH v3 3/3] cpu, softmmu/vl.c: Change parsing of -cpu argument to allow -cpu cpu, help to print options for the CPU type similar to how the '-device' option works.
Just realized that the commit message on this one got a little mangled. I'm
happy to revise it but I'd prefer to get the code reviewed first before
doing a purely commit message change.
-Dinah
On Sun, Jul 30, 2023 at 2:41 AM Dinah Baum wrote:
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1480
> Signed-off-by: Dinah Baum
>
> Signed-off-by: Dinah Baum
> ---
> cpu.c| 41
> include/qapi/qmp/qdict.h | 1 +
> qemu-options.hx | 7 ---
> qobject/qdict.c | 5 +
> softmmu/vl.c | 35 +-
> 5 files changed, 85 insertions(+), 4 deletions(-)
>
> diff --git a/cpu.c b/cpu.c
> index a99d09cd47..9971ffeeba 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -43,6 +43,10 @@
> #include "trace/trace-root.h"
> #include "qemu/accel.h"
> #include "qemu/plugin.h"
> +#include "qemu/cutils.h"
> +#include "qemu/qemu-print.h"
> +#include "qapi/qmp/qdict.h"
> +#include "qapi/qmp/qobject.h"
>
> uintptr_t qemu_host_page_size;
> intptr_t qemu_host_page_mask;
> @@ -312,6 +316,43 @@ CpuModelExpansionInfo
> *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
> return get_cpu_model_expansion_info(type, model, errp);
> }
>
> +void list_cpu_model_expansion(CpuModelExpansionType type,
> + CpuModelInfo *model,
> + Error **errp)
> +{
> +CpuModelExpansionInfo *expansion_info;
> +QDict *qdict;
> +QDictEntry *qdict_entry;
> +const char *key;
> +QObject *obj;
> +QType q_type;
> +GPtrArray *array;
> +int i;
> +const char *type_name;
> +
> +expansion_info = get_cpu_model_expansion_info(type, model, errp);
> +if (expansion_info) {
> +qdict = qobject_to(QDict, expansion_info->model->props);
> +if (qdict) {
> +qemu_printf("%s features:\n", model->name);
> +array = g_ptr_array_new();
> +for (qdict_entry = (QDictEntry *)qdict_first(qdict);
> qdict_entry;
> + qdict_entry = (QDictEntry *)qdict_next(qdict,
> qdict_entry)) {
> +g_ptr_array_add(array, qdict_entry);
> +}
> +g_ptr_array_sort(array, (GCompareFunc)dict_key_compare);
> +for (i = 0; i < array->len; i++) {
> +qdict_entry = array->pdata[i];
> +key = qdict_entry_key(qdict_entry);
> +obj = qdict_get(qdict, key);
> +q_type = qobject_type(obj);
> +type_name = QType_str(q_type);
> +qemu_printf(" %s=<%s>\n", key, type_name);
> +}
> +}
> +}
> +}
> +
> #if defined(CONFIG_USER_ONLY)
> void tb_invalidate_phys_addr(hwaddr addr)
> {
> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
> index 82e90fc072..d0b6c3d358 100644
> --- a/include/qapi/qmp/qdict.h
> +++ b/include/qapi/qmp/qdict.h
> @@ -67,5 +67,6 @@ bool qdict_get_try_bool(const QDict *qdict, const char
> *key, bool def_value);
> const char *qdict_get_try_str(const QDict *qdict, const char *key);
>
> QDict *qdict_clone_shallow(const QDict *src);
> +int dict_key_compare(QDictEntry **entry1, QDictEntry **entry2);
>
> #endif /* QDICT_H */
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 29b98c3d4c..e0f0284927 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -169,11 +169,12 @@ SRST
> ERST
>
> DEF("cpu", HAS_ARG, QEMU_OPTION_cpu,
> -"-cpu cpuselect CPU ('-cpu help' for list)\n", QEMU_ARCH_ALL)
> +"-cpu cpuselect CPU ('-cpu help' for list)\n"
> +"use '-cpu cpu,help' to print possible properties\n",
> QEMU_ARCH_ALL)
> SRST
> ``-cpu model``
> -Select CPU model (``-cpu help`` for list and additional feature
> -selection)
> +Select CPU model (``-cpu help`` and ``-cpu cpu,help``) for list and
> additional feature
> +selection
> ERST
>
> DEF("accel", HAS_ARG, QEMU_OPTION_accel,
> diff --git a/qobject/qdict.c b/qobject/qdict.c
> index 8faff230d3..31407e62f6 100644
> --- a/qobject/qdict.c
> +++ b/qobject/qdict.c
> @@ -447,3 +447,8 @@ void qdict_unref(QDict *q)
> {
> qobject_unref(q);
> }
> +
> +int dict_key_compare(QDictEntry **entry1, QDictEntry **entry2)
> +{
> +return g_strcmp0(qdict_entry_key(*entry1), qdict_entry_key(*entry2));
> +}
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index b0b96f67fa..1fd87f2c06 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -501,6 +501,15 @@ static QemuOptsList qemu_action_opts = {
> },
> };
>
> +static QemuOptsList qemu_cpu_opts = {
> +.name = "cpu",
> +.implied_opt_name = "cpu",
> +.head = QTAILQ_HEAD_INITIALIZER(qemu_cpu_opts.head),
> +.desc = {
> +{ /* end of list */ }
> +},
> +};
> +
> const char *qemu_get_vm_name(void)
> {
> return qemu_name;
> @@ -1159,6 +1168,26 @@ static int device_init_func(void *opaque, QemuOpts
> *opts, Error **errp)
> return 0;
> }
>
> +static int cpu_h
Re: [PATCH v2 0/3] Enable -cpu ,help
Friendly ping for code review on this patch series. Full series: https://lore.kernel.org/qemu-devel/20230314100026.536079-1-dinahbaum...@gmail.com/ Thanks, -DInah On Tue, Mar 14, 2023 at 6:00 AM Dinah Baum wrote: > Part 1 is a refactor/code motion patch for > qapi/machine target required for setup of > > Part 2 which enables query-cpu-model-expansion > on all architectures > > Part 3 implements the ',help' feature > > Limitations: > Currently only 'FULL' expansion queries are implemented since > that's the only type enabled on the architectures that > allow feature probing > > Unlike the 'device,help' command, default values aren't > printed > > Dinah Baum (3): > qapi/machine-target: refactor machine-target > cpu, qapi, target/arm, i386, s390x: Generalize > query-cpu-model-expansion > cpu, qdict, vl: Enable printing options for CPU type > > MAINTAINERS | 1 + > cpu.c| 61 +++ > include/exec/cpu-common.h| 10 +++ > include/qapi/qmp/qdict.h | 2 + > qapi/machine-target-common.json | 130 +++ > qapi/machine-target.json | 129 +- > qapi/meson.build | 1 + > qemu-options.hx | 7 +- > qobject/qdict.c | 5 ++ > softmmu/vl.c | 36 - > target/arm/arm-qmp-cmds.c| 7 +- > target/arm/cpu.h | 7 +- > target/i386/cpu-sysemu.c | 7 +- > target/i386/cpu.h| 6 ++ > target/s390x/cpu.h | 7 ++ > target/s390x/cpu_models_sysemu.c | 6 +- > 16 files changed, 278 insertions(+), 144 deletions(-) > create mode 100644 qapi/machine-target-common.json > > -- > 2.30.2 > >
Re: Adopting abandoned patch?
It looks like the author didn't include a "Signed off" in their patch draft and it doesn't look like Debian qemu-kvm maintainers ever merged it. Does this change the patch adoption process? Thanks, -Dinah On Mon, Feb 27, 2023 at 4:23 PM Dinah B wrote: > Thanks, here's the original patch: > https://bugs.debian.org/cgi-bin/bugreport.cgi?att=2;bug=621529;filename=multiboot2.patch;msg=15 > > On Mon, Feb 27, 2023 at 4:59 AM Alex Bennée > wrote: > >> >> Dinah B writes: >> >> > Hi, >> > >> > I'm looking to get more involved in contributing to QEMU. I noticed >> that there are some issues in the tracker >> > where a sample patch has been contributed but never got merged, like a >> proposal to add multiboot2 support: >> > https://gitlab.com/qemu-project/qemu/-/issues/389 >> >> I couldn't see a patch attached to the bug report. Is it elsewhere? >> >> > >> > Is another dev allowed to "adopt" the patch as-is, with proper >> attribution to the original dev and drive it to >> > completion/merging (there are some features missing)? Or is "starting >> from scratch" required for legal >> > reasons? >> >> It's certainly possible to pick up a patch from someone else and take it >> forward. Aside from addressing any review comments I think the minimum >> requirement is the authors original Signed-off-by is intact which >> asserts they could contribute code to the project. >> >> -- >> Alex Bennée >> Virtualisation Tech Lead @ Linaro >> >
Re: Adopting abandoned patch?
Thanks, here's the original patch: https://bugs.debian.org/cgi-bin/bugreport.cgi?att=2;bug=621529;filename=multiboot2.patch;msg=15 On Mon, Feb 27, 2023 at 4:59 AM Alex Bennée wrote: > > Dinah B writes: > > > Hi, > > > > I'm looking to get more involved in contributing to QEMU. I noticed that > there are some issues in the tracker > > where a sample patch has been contributed but never got merged, like a > proposal to add multiboot2 support: > > https://gitlab.com/qemu-project/qemu/-/issues/389 > > I couldn't see a patch attached to the bug report. Is it elsewhere? > > > > > Is another dev allowed to "adopt" the patch as-is, with proper > attribution to the original dev and drive it to > > completion/merging (there are some features missing)? Or is "starting > from scratch" required for legal > > reasons? > > It's certainly possible to pick up a patch from someone else and take it > forward. Aside from addressing any review comments I think the minimum > requirement is the authors original Signed-off-by is intact which > asserts they could contribute code to the project. > > -- > Alex Bennée > Virtualisation Tech Lead @ Linaro >
Adopting abandoned patch?
Hi, I'm looking to get more involved in contributing to QEMU. I noticed that there are some issues in the tracker where a sample patch has been contributed but never got merged, like a proposal to add multiboot2 support: https://gitlab.com/qemu-project/qemu/-/issues/389 Is another dev allowed to "adopt" the patch as-is, with proper attribution to the original dev and drive it to completion/merging (there are some features missing)? Or is "starting from scratch" required for legal reasons? Thanks, -Dinah
Re: [PATCH 1/2] configure: Add 'mkdir build' check
*ping* Patch series: https://lore.kernel.org/qemu-devel/20230208233111.398577-1-dinahbaum...@gmail.com/ -Dinah On Wed, Feb 8, 2023 at 6:31 PM Dinah Baum wrote: > QEMU configure script goes into an infinite error printing loop > when in read only directory due to 'build' dir never being created. > > Checking if 'mkdir dir' succeeds prevents this error. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/321 > --- > configure | 15 ++- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/configure b/configure > index 64960c6000..3b384914ce 100755 > --- a/configure > +++ b/configure > @@ -31,10 +31,11 @@ then > fi > fi > > -mkdir build > -touch $MARKER > +if mkdir build > +then > +touch $MARKER > > -cat > GNUmakefile <<'EOF' > +cat > GNUmakefile <<'EOF' > # This file is auto-generated by configure to support in-source tree > # 'make' command invocation > > @@ -56,8 +57,12 @@ force: ; > GNUmakefile: ; > > EOF > -cd build > -exec "$source_path/configure" "$@" > +cd build > +exec "$source_path/configure" "$@" > +else > +echo "ERROR: Unable to use ./build dir, try using a > ../qemu/configure build" > +exit 1 > +fi > fi > > # Temporary directory used for files created while > -- > 2.30.2 > >
Re: [PATCH] configure: Add 'mkdir build' check
Hi, thanks for the feedback - I'll revise it. Small question - Paolo
Bonzini specified that 'configure --help' should work even if the build
doesn't.
Currently the script functions that handle argument reading aren't
initialized or run until after the build is done, so if the build fails, so
do they.
I see 2 paths forward:
1. Code motion them to be initialized and run before we check for the build
directory
2. Break them into a helper script and load them in the main configure
script before we check for the build directory
Is one of these options preferable to the other?
On Mon, Feb 6, 2023 at 9:42 AM Peter Maydell
wrote:
> On Sun, 5 Feb 2023 at 07:44, Dinah Baum wrote:
> >
> > QEMU configure script goes into an infinite error printing loop
> > when in read only directory due to 'build' dir never being created.
> >
> > Checking if 'mkdir dir' succeeds and if the directory is
> > writeable prevents this error.
> >
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/321
> >
> > Signed-off-by: Dinah Baum
>
> Hi; thanks for sending this patch.
>
> > ---
> > configure | 37 ++---
> > 1 file changed, 30 insertions(+), 7 deletions(-)
> >
> > diff --git a/configure b/configure
> > index 64960c6000..fe9028991f 100755
> > --- a/configure
> > +++ b/configure
> > @@ -32,9 +32,11 @@ then
> > fi
> >
> > mkdir build
> > -touch $MARKER
> > +if [ -d build ] && [ -w build ]
> > +then
> > +touch $MARKER
>
> It would be more straightforward to check whether
> the 'mkdir' and 'touch' commands succeed, I think.
>
> >
> > -cat > GNUmakefile <<'EOF'
> > +cat > GNUmakefile <<'EOF'
> > # This file is auto-generated by configure to support in-source tree
> > # 'make' command invocation
> >
> > @@ -56,8 +58,15 @@ force: ;
> > GNUmakefile: ;
> >
> > EOF
> > -cd build
> > -exec "$source_path/configure" "$@"
> > +cd build
> > +exec "$source_path/configure" "$@"
> > +elif ! [ -d build ]
> > +then
> > +echo "ERROR: Unable to create ./build dir, try using a
> ../qemu/configure build"
> > +elif ! [ -w build ]
> > +then
> > +echo "ERROR: ./build dir not writeable, try using a
> ../qemu/configure build"
> > +fi
>
> If these are errors, we should exit immediately, not
> continue further trying to run code.
>
> > fi
> >
> > # Temporary directory used for files created while
> > @@ -181,9 +190,12 @@ compile_prog() {
> >
> > # symbolically link $1 to $2. Portable version of "ln -sf".
> > symlink() {
> > - rm -rf "$2"
> > - mkdir -p "$(dirname "$2")"
> > - ln -s "$1" "$2"
> > + if [ -d $source_path/build ] && [ -w $source_path/build ]
> > + then
> > + rm -rf "$2"
> > + mkdir -p "$(dirname "$2")"
> > + ln -s "$1" "$2"
> > + fi
>
> The symlink function is a utility one used in various
> places in the code. It may be used for other directories
> than $source_path/build. If we need to better handle
> errors here then we should do that by checking the
> exit status of the commands (and probably passing the
> return status back up for the caller to look at).
>
> But there's a lot of code in configure that assumes it
> can write to the destination directory elsewhere too,
> so why change this function specifically ?
>
> > }
> >
> > # check whether a command is available to this shell (may be either an
> > @@ -2287,7 +2299,18 @@ fi
> > ###
> > # generate config-host.mak
> >
> > +if ! [ -d $source_path/build ] || ! [ -w $source_path/build ]
>
> You can't assume that the build dir is always $source_path/build
> -- that's just the default if the user ran configure from
> the source directory.
>
> > +then
> > +echo "ERROR: ./build dir unusable, exiting"
> > +# cleanup
> > +rm -f config.log
> > +rm -f Makefile.prereqs
> > +rm -r "$TMPDIR1"
> > +exit 1
>
> Most of these haven't been created at this point, so don't
> need to be deleted. (If you do the error-exit earlier,
> as I suggest, then this is clearer.)
>
> > +fi
> > +
> > if ! (GIT="$git" "$source_path/scripts/git-submodule.sh"
> "$git_submodules_action" "$git_submodules"); then
> > +echo "BAD"
> > exit 1
> > fi
>
> thanks
> -- PMM
>
