qemu-img: very bad performance with the default number of coroutines (8) in presence of compression. Fixed adding "-m 1" to the cmdline.
Hello, pretty much the $Subject. We had reports of very bad performance with the default options when running: $ time qemu-img convert -c -p -O qcow2 file.qcow2 file.disk real7m13.220s user6m56.682s sys 0m13.038s I studied this and noticed that for every coroutine a thread was created. But at no point ever were two coroutines making progress at the same time. On the countrary, they constantly competed to run, leading to very slow task progress. The workaround we found here was to just add "-m 1" to the command line, leading to a halving of the time taken for running the command: $ time qemu-img convert -c --m 1 p -O qcow2 file.qcow2 file.disk real3m22.212s user3m13.744s sys 0m7.881s We repeated this test on a variety of hardware configurations, and the relative results are always the same. Have you witnessed the same situation? Should we change the default in qemu-img from 8 coroutines to 1, at least for the "-c" case? In case I can submit a simple patch that does that, but looking forward for your thoughts. It is possible that coroutines were added in an attempt to improve performance, so maybe there is more here to fix, although in my view already fixing the existing bad default would already be a big improvement in the meantime. Thoughts? Thanks, Claudio
Re: [PATCH v2 00/10] block: Make raw_co_get_allocated_file_size asynchronous
Hi all, we currently have to maintain something downstream for this, since the current behavior can compound problems on top of existing bad NFS latency, could someone continue to help reviewing this work? Thanks, Claudio On 6/9/23 22:19, Fabiano Rosas wrote: > Hi, > > The major change from the last version is that this time I'm moving > all of the callers of bdrv_get_allocated_file_size() into > coroutines. I had to make some temporary changes to avoid asserts > while not all the callers were converted. > > I tried my best to explain why I think the changes are safe. To avoid > changing too much of the code I added a change that removes the > dependency of qmp_query_block from hmp_nbd_server_start, that way I > don't need to move all of the nbd code into a coroutine as well. > > Based on: > [PATCH v2 00/11] block: Re-enable the graph lock > https://lore.kernel.org/r/20230605085711.21261-1-kw...@redhat.com > > changes: > > - fixed duplicated commit message [Lin] > - clarified why we need to convert info-block [Claudio] > - added my rationale of why the changes are safe [Eric] > - converted all callers to coroutines [Kevin] > - made hmp_nbd_server_start don't depend on qmp_query_block > > CI run: https://gitlab.com/farosas/qemu/-/pipelines/895525156 > === > v1: > https://lore.kernel.org/r/20230523213903.18418-1-faro...@suse.de > > As discussed in another thread [1], here's an RFC addressing a VCPU > softlockup encountered when issuing QMP commands that target a disk > placed on NFS. > > Since QMP commands happen with the qemu_global_mutex locked, any > command that takes too long to finish will block other threads waiting > to take the global mutex. One such thread could be a VCPU thread going > out of the guest to handle IO. > > This is the case when issuing the QMP command query-block, which > eventually calls raw_co_get_allocated_file_size(). This function makes > an 'fstat' call that has been observed to take a long time (seconds) > over NFS. > > NFS latency issues aside, we can improve the situation by not blocking > VCPU threads while the command is running. > > Move the 'fstat' call into the thread-pool and make the necessary > adaptations to ensure raw_co_get_allocated_file_size runs in a > coroutine in the block driver aio_context. > > 1- Question about QMP and BQL > https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg03141.html > > Fabiano Rosas (8): > block: Remove bdrv_query_block_node_info > block: Remove unnecessary variable in bdrv_block_device_info > block: Allow the wrapper script to see functions declared in qapi.h > block: Temporarily mark bdrv_co_get_allocated_file_size as mixed > block: Convert bdrv_query_block_graph_info to coroutine > block: Convert bdrv_block_device_info into co_wrapper > block: Don't query all block devices at hmp_nbd_server_start > block: Convert qmp_query_block() to coroutine_fn > > João Silva (1): > block: Add a thread-pool version of fstat > > Lin Ma (1): > block: Convert qmp_query_named_block_nodes to coroutine > > block/file-posix.c | 40 +-- > block/meson.build | 1 + > block/monitor/block-hmp-cmds.c | 22 ++- > block/qapi.c | 62 +- > blockdev.c | 6 +-- > hmp-commands-info.hx | 1 + > include/block/block-hmp-cmds.h | 2 +- > include/block/qapi.h | 17 > include/block/raw-aio.h| 4 +- > qapi/block-core.json | 5 ++- > qemu-img.c | 2 - > scripts/block-coroutine-wrapper.py | 1 + > 12 files changed, 90 insertions(+), 73 deletions(-) >
Re: [PATCH] block-migration: Ensure we don't crash during migration cleanup
On 8/8/23 19:08, Stefan Hajnoczi wrote:
> On Mon, Jul 31, 2023 at 05:33:38PM -0300, Fabiano Rosas wrote:
>> We can fail the blk_insert_bs() at init_blk_migration(), leaving the
>> BlkMigDevState without a dirty_bitmap and BlockDriverState. Account
>> for the possibly missing elements when doing cleanup.
>>
>> Fix the following crashes:
>>
>> Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
>> 0x55ec83ef in bdrv_release_dirty_bitmap (bitmap=0x0) at
>> ../block/dirty-bitmap.c:359
>> 359 BlockDriverState *bs = bitmap->bs;
>> #0 0x55ec83ef in bdrv_release_dirty_bitmap (bitmap=0x0) at
>> ../block/dirty-bitmap.c:359
>> #1 0x55bba331 in unset_dirty_tracking () at
>> ../migration/block.c:371
>> #2 0x55bbad98 in block_migration_cleanup_bmds () at
>> ../migration/block.c:681
>>
>> Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
>> 0x55e971ff in bdrv_op_unblock (bs=0x0,
>> op=BLOCK_OP_TYPE_BACKUP_SOURCE, reason=0x0) at ../block.c:7073
>> 7073QLIST_FOREACH_SAFE(blocker, >op_blockers[op], list, next) {
>> #0 0x55e971ff in bdrv_op_unblock (bs=0x0,
>> op=BLOCK_OP_TYPE_BACKUP_SOURCE, reason=0x0) at ../block.c:7073
>> #1 0x55e9734a in bdrv_op_unblock_all (bs=0x0, reason=0x0) at
>> ../block.c:7095
>> #2 0x55bbae13 in block_migration_cleanup_bmds () at
>> ../migration/block.c:690
>>
>> Signed-off-by: Fabiano Rosas
>> ---
>> migration/block.c | 11 +--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> Sorry, I missed this patch!
>
> If this needs to be in QEMU 8.1 (-rc3 is being tagged today), please
> reply and provide a justification. At this point only security fixes and
> showstoppers will be merged. Thanks!
>
> Applied to my block-next tree for QEMU 8.2:
> https://gitlab.com/stefanha/qemu/commits/block-next
>
> Stefan
Thanks, and in my personal view I think it's ok for 8.2, IIUC it happens during
the migration to file work which is not in 8.1 anyway,
Fabiano correct me here if I am wrong,
Ciao,
Claudio
Re: [PATCH] block-migration: Ensure we don't crash during migration cleanup
added Kevin and Hanna for block, since this seems still untouched?
Thanks,
Claudio
On 7/31/23 22:33, Fabiano Rosas wrote:
> We can fail the blk_insert_bs() at init_blk_migration(), leaving the
> BlkMigDevState without a dirty_bitmap and BlockDriverState. Account
> for the possibly missing elements when doing cleanup.
>
> Fix the following crashes:
>
> Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
> 0x55ec83ef in bdrv_release_dirty_bitmap (bitmap=0x0) at
> ../block/dirty-bitmap.c:359
> 359 BlockDriverState *bs = bitmap->bs;
> #0 0x55ec83ef in bdrv_release_dirty_bitmap (bitmap=0x0) at
> ../block/dirty-bitmap.c:359
> #1 0x55bba331 in unset_dirty_tracking () at ../migration/block.c:371
> #2 0x55bbad98 in block_migration_cleanup_bmds () at
> ../migration/block.c:681
>
> Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
> 0x55e971ff in bdrv_op_unblock (bs=0x0,
> op=BLOCK_OP_TYPE_BACKUP_SOURCE, reason=0x0) at ../block.c:7073
> 7073QLIST_FOREACH_SAFE(blocker, >op_blockers[op], list, next) {
> #0 0x55e971ff in bdrv_op_unblock (bs=0x0,
> op=BLOCK_OP_TYPE_BACKUP_SOURCE, reason=0x0) at ../block.c:7073
> #1 0x55e9734a in bdrv_op_unblock_all (bs=0x0, reason=0x0) at
> ../block.c:7095
> #2 0x55bbae13 in block_migration_cleanup_bmds () at
> ../migration/block.c:690
>
> Signed-off-by: Fabiano Rosas
> ---
> migration/block.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/migration/block.c b/migration/block.c
> index b9580a6c7e..86c2256a2b 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -368,7 +368,9 @@ static void unset_dirty_tracking(void)
> BlkMigDevState *bmds;
>
> QSIMPLEQ_FOREACH(bmds, _mig_state.bmds_list, entry) {
> -bdrv_release_dirty_bitmap(bmds->dirty_bitmap);
> +if (bmds->dirty_bitmap) {
> +bdrv_release_dirty_bitmap(bmds->dirty_bitmap);
> +}
> }
> }
>
> @@ -676,13 +678,18 @@ static int64_t get_remaining_dirty(void)
> static void block_migration_cleanup_bmds(void)
> {
> BlkMigDevState *bmds;
> +BlockDriverState *bs;
> AioContext *ctx;
>
> unset_dirty_tracking();
>
> while ((bmds = QSIMPLEQ_FIRST(_mig_state.bmds_list)) != NULL) {
> QSIMPLEQ_REMOVE_HEAD(_mig_state.bmds_list, entry);
> -bdrv_op_unblock_all(blk_bs(bmds->blk), bmds->blocker);
> +
> +bs = blk_bs(bmds->blk);
> +if (bs) {
> +bdrv_op_unblock_all(bs, bmds->blocker);
> +}
> error_free(bmds->blocker);
>
> /* Save ctx, because bmds->blk can disappear during blk_unref. */
Re: [RFC PATCH 4/6] Convert query-block/info_block to coroutine
On 5/24/23 11:24, Lin Ma wrote: > The query-named-block-nodes is only availabe for qmp, not support hmp yet. > > Lin Ok, makes sense.
Re: [RFC PATCH 6/6] block: Add a thread-pool version of fstat
On 5/23/23 23:39, Fabiano Rosas wrote:
> From: João Silva
>
> The fstat call can take a long time to finish when running over
> NFS. Add a version of it that runs in the thread pool.
>
> Adapt one of its users, raw_co_get_allocated_file size to use the new
> version. That function is called via QMP under the qemu_global_mutex
> so it has a large chance of blocking VCPU threads in case it takes too
> long to finish.
>
> Signed-off-by: João Silva
> Signed-off-by: Fabiano Rosas
> ---
> block/file-posix.c | 40 +---
> include/block/raw-aio.h | 4 +++-
> 2 files changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 0ab158efba..0a29a6cc43 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -227,6 +227,9 @@ typedef struct RawPosixAIOData {
> struct {
> unsigned long op;
> } zone_mgmt;
> +struct {
> +struct stat *st;
> +} fstat;
> };
> } RawPosixAIOData;
>
> @@ -2644,6 +2647,34 @@ static void raw_close(BlockDriverState *bs)
> }
> }
>
> +static int handle_aiocb_fstat(void *opaque)
> +{
> +RawPosixAIOData *aiocb = opaque;
> +
> +if (fstat(aiocb->aio_fildes, aiocb->fstat.st) < 0) {
> +return -errno;
> +}
> +
> +return 0;
> +}
> +
> +static int coroutine_fn raw_co_fstat(BlockDriverState *bs, struct stat *st)
> +{
> +BDRVRawState *s = bs->opaque;
> +RawPosixAIOData acb;
> +
> +acb = (RawPosixAIOData) {
> +.bs = bs,
> +.aio_fildes = s->fd,
> +.aio_type = QEMU_AIO_FSTAT,
> +.fstat = {
> +.st = st,
> +},
> +};
> +
> +return raw_thread_pool_submit(handle_aiocb_fstat, );
> +}
> +
> /**
> * Truncates the given regular file @fd to @offset and, when growing, fills
> the
> * new space according to @prealloc.
> @@ -2883,11 +2914,14 @@ static int64_t coroutine_fn
> raw_co_getlength(BlockDriverState *bs)
> static int64_t coroutine_fn raw_co_get_allocated_file_size(BlockDriverState
> *bs)
> {
> struct stat st;
> -BDRVRawState *s = bs->opaque;
> +int ret;
>
> -if (fstat(s->fd, ) < 0) {
> -return -errno;
> +ret = raw_co_fstat(bs, );
> +
> +if (ret) {
> +return ret;
> }
> +
> return (int64_t)st.st_blocks * 512;
> }
>
> diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
> index 0fe85ade77..7dc6d24e21 100644
> --- a/include/block/raw-aio.h
> +++ b/include/block/raw-aio.h
> @@ -31,6 +31,7 @@
> #define QEMU_AIO_ZONE_REPORT 0x0100
> #define QEMU_AIO_ZONE_MGMT0x0200
> #define QEMU_AIO_ZONE_APPEND 0x0400
> +#define QEMU_AIO_FSTAT0x0800
> #define QEMU_AIO_TYPE_MASK \
> (QEMU_AIO_READ | \
> QEMU_AIO_WRITE | \
> @@ -42,7 +43,8 @@
> QEMU_AIO_TRUNCATE | \
> QEMU_AIO_ZONE_REPORT | \
> QEMU_AIO_ZONE_MGMT | \
> - QEMU_AIO_ZONE_APPEND)
> + QEMU_AIO_ZONE_APPEND | \
> + QEMU_AIO_FSTAT)
>
> /* AIO flags */
> #define QEMU_AIO_MISALIGNED 0x1000
Reviewed-by: Claudio Fontana
Re: [RFC PATCH 4/6] Convert query-block/info_block to coroutine
On 5/23/23 23:39, Fabiano Rosas wrote:
> From: Lin Ma
>
> Sometimes the query-block performs time-consuming I/O(say waiting for
> the fstat of NFS complete), So let's make this QMP handler runs in a
> coroutine.
>
> The following patch moves the fstat() into a thread pool.
>
> Signed-off-by: Lin Ma
> Signed-off-by: Fabiano Rosas
Apart from the wrong subject,
why is this change not including the update to:
hmp-commands-info.hx
like the previous one?
Thanks,
C
> ---
> blockdev.c | 6 +++---
> qapi/block-core.json | 3 ++-
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 5d56b79df4..6412548662 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2804,9 +2804,9 @@ void qmp_drive_backup(DriveBackup *backup, Error **errp)
> blockdev_do_action(, errp);
> }
>
> -BlockDeviceInfoList *qmp_query_named_block_nodes(bool has_flat,
> - bool flat,
> - Error **errp)
> +BlockDeviceInfoList *coroutine_fn qmp_query_named_block_nodes(bool has_flat,
> + bool flat,
> + Error **errp)
> {
> bool return_flat = has_flat && flat;
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index da95fe456c..0559c38412 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1972,7 +1972,8 @@
> { 'command': 'query-named-block-nodes',
>'returns': [ 'BlockDeviceInfo' ],
>'data': { '*flat': 'bool' },
> - 'allow-preconfig': true }
> + 'allow-preconfig': true,
> + 'coroutine': true}
>
> ##
> # @XDbgBlockGraphNodeType:
Re: [RFC PATCH 3/6] Convert query-block/info_block to coroutine
On 5/23/23 23:39, Fabiano Rosas wrote:
> From: Lin Ma
>
> Sometimes the query-block performs time-consuming I/O(say waiting for
> the fstat of NFS complete), So let's make this QMP handler runs in a
> coroutine.
>
> The following patch moves the fstat() into a thread pool.
Hi, this message talks about QMP and query-block only and the subject talks
about info_block as well.
We need to clarify this.
Also, lets make it clear that one is a QMP command, and the other is an HMP
command.
In any case I would say:
"Convert QMP command query-block to use a coroutine", and in case we also need
info_block, an additional patch could take care of the HMP command with a
subject like:
"Convert HMP command info_block to use a coroutine".
Ciao,
Claudio
>
> Signed-off-by: Lin Ma
> Signed-off-by: Fabiano Rosas
> ---
> block/monitor/block-hmp-cmds.c | 2 +-
> block/qapi.c | 2 +-
> hmp-commands-info.hx | 1 +
> include/block/block-hmp-cmds.h | 2 +-
> qapi/block-core.json | 2 +-
> 5 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
> index ca2599de44..4b704010bc 100644
> --- a/block/monitor/block-hmp-cmds.c
> +++ b/block/monitor/block-hmp-cmds.c
> @@ -738,7 +738,7 @@ static void print_block_info(Monitor *mon, BlockInfo
> *info,
> }
> }
>
> -void hmp_info_block(Monitor *mon, const QDict *qdict)
> +void coroutine_fn hmp_info_block(Monitor *mon, const QDict *qdict)
> {
> BlockInfoList *block_list, *info;
> BlockDeviceInfoList *blockdev_list, *blockdev;
> diff --git a/block/qapi.c b/block/qapi.c
> index 79bf80c503..ae6cd1c2ff 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -667,7 +667,7 @@ bdrv_query_bds_stats(BlockDriverState *bs, bool blk_level)
> return s;
> }
>
> -BlockInfoList *qmp_query_block(Error **errp)
> +BlockInfoList *coroutine_fn qmp_query_block(Error **errp)
> {
> BlockInfoList *head = NULL, **p_next =
> BlockBackend *blk;
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 47d63d26db..996895f417 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -65,6 +65,7 @@ ERST
> .help = "show info of one block device or all block devices "
>"(-n: show named nodes; -v: show details)",
> .cmd= hmp_info_block,
> +.coroutine = true,
> },
>
> SRST
> diff --git a/include/block/block-hmp-cmds.h b/include/block/block-hmp-cmds.h
> index 71113cd7ef..6d9152318f 100644
> --- a/include/block/block-hmp-cmds.h
> +++ b/include/block/block-hmp-cmds.h
> @@ -48,7 +48,7 @@ void hmp_eject(Monitor *mon, const QDict *qdict);
>
> void hmp_qemu_io(Monitor *mon, const QDict *qdict);
>
> -void hmp_info_block(Monitor *mon, const QDict *qdict);
> +void coroutine_fn hmp_info_block(Monitor *mon, const QDict *qdict);
> void hmp_info_blockstats(Monitor *mon, const QDict *qdict);
> void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
> void hmp_info_snapshots(Monitor *mon, const QDict *qdict);
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 98d9116dae..da95fe456c 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -838,7 +838,7 @@
> #}
> ##
> { 'command': 'query-block', 'returns': ['BlockInfo'],
> - 'allow-preconfig': true }
> + 'allow-preconfig': true, 'coroutine': true }
>
> ##
> # @BlockDeviceTimedStats:
Re: [RFC PATCH 1/6] block: Remove bdrv_query_block_node_info
On 5/23/23 23:38, Fabiano Rosas wrote:
> The last call site of this function has been removed by commit
> c04d0ab026 ("qemu-img: Let info print block graph").
>
> Signed-off-by: Fabiano Rosas
> ---
> block/qapi.c | 27 ---
> include/block/qapi.h | 3 ---
> 2 files changed, 30 deletions(-)
>
> diff --git a/block/qapi.c b/block/qapi.c
> index f34f95e0ef..79bf80c503 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -309,33 +309,6 @@ out:
> aio_context_release(bdrv_get_aio_context(bs));
> }
>
> -/**
> - * bdrv_query_block_node_info:
> - * @bs: block node to examine
> - * @p_info: location to store node information
> - * @errp: location to store error information
> - *
> - * Store image information about @bs in @p_info.
> - *
> - * @p_info will be set only on success. On error, store error in @errp.
> - */
> -void bdrv_query_block_node_info(BlockDriverState *bs,
> -BlockNodeInfo **p_info,
> -Error **errp)
> -{
> -BlockNodeInfo *info;
> -ERRP_GUARD();
> -
> -info = g_new0(BlockNodeInfo, 1);
> -bdrv_do_query_node_info(bs, info, errp);
> -if (*errp) {
> -qapi_free_BlockNodeInfo(info);
> -return;
> -}
> -
> -*p_info = info;
> -}
> -
> /**
> * bdrv_query_image_info:
> * @bs: block node to examine
> diff --git a/include/block/qapi.h b/include/block/qapi.h
> index 18d48ddb70..8663971c58 100644
> --- a/include/block/qapi.h
> +++ b/include/block/qapi.h
> @@ -36,9 +36,6 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
> int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>SnapshotInfoList **p_list,
>Error **errp);
> -void bdrv_query_block_node_info(BlockDriverState *bs,
> -BlockNodeInfo **p_info,
> - Error **errp);
> void bdrv_query_image_info(BlockDriverState *bs,
> ImageInfo **p_info,
> bool flat,
Reviewed-by: Claudio Fontana
Re: intermittent failures in iotest 108
On 11/10/22 14:51, Claudio Fontana wrote: > On 11/10/22 14:50, Kevin Wolf wrote: >> Am 07.11.2022 um 14:36 hat Claudio Fontana geschrieben: >>> Hello Kevin and all, >>> >>> I seem to be getting intermittent failures with mainline iotest 108. Is >>> this already known? >>> >>> ../configure --enable-tcg --enable-kvm --enable-modules --enable-debug >>> make -j >>> make -j check >> >> At least I haven't hit it yet. Running just the one test case in a loop >> doesn't seem to trigger it either. Maybe it only fails under load? btw are you running tests in parallel make -j check ?
Re: intermittent failures in iotest 108
On 11/10/22 14:50, Kevin Wolf wrote: > Am 07.11.2022 um 14:36 hat Claudio Fontana geschrieben: >> Hello Kevin and all, >> >> I seem to be getting intermittent failures with mainline iotest 108. Is this >> already known? >> >> ../configure --enable-tcg --enable-kvm --enable-modules --enable-debug >> make -j >> make -j check > > At least I haven't hit it yet. Running just the one test case in a loop > doesn't seem to trigger it either. Maybe it only fails under load? > > Kevin > I'll give it a few more spins, see how often I hit it. Ciao, C
intermittent failures in iotest 108
Hello Kevin and all,
I seem to be getting intermittent failures with mainline iotest 108. Is this
already known?
../configure --enable-tcg --enable-kvm --enable-modules --enable-debug
make -j
make -j check
▶ 614/634 qcow2 108
FAIL
614/634 qemu:block / qemu-iotests qcow2
ERROR 155.16s exit status 1
>>> MALLOC_PERTURB_=33 PYTHON=/usr/bin/python3 /usr/bin/sh
>>> /home/claudio/git/qemu/build-all/../tests/qemu-iotests/../check-block.sh
>>> qcow2
――― ✀
―――
stderr:
--- /home/claudio/git/qemu/tests/qemu-iotests/108.out
+++ /home/claudio/git/qemu/build-all/tests/qemu-iotests/scratch/108.out.bad
@@ -152,40 +152,4 @@
--- Rebuilding refcount structures on block devices ---
-{ "execute": "qmp_capabilities" }
-{"return": {}}
-{ "execute": "blockdev-create",
- "arguments": {
- "job-id": "create",
- "options": {
- "driver": "IMGFMT",
- "file": "file",
- "size": 67108864,
- "cluster-size": 512
- } } }
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "created", "id": "create"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "running", "id": "create"}}
-{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "create"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "create"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "create"}}
-{ "execute": "job-dismiss", "arguments": { "id": "create" } }
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "null", "id": "create"}}
-{"return": {}}
-{ "execute": "quit" }
-{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
-
-wrote 65536/65536 bytes at offset 0
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-ERROR cluster 0 refcount=0 reference=1
-Rebuilding refcount structure
-The following inconsistencies were found and repaired:
-
-0 leaked clusters
-1 corruptions
-
-Double checking the fixed image now...
-No errors were found on the image.
-*** done
+Timeout waiting for capabilities on handle 0
(test program exited with status code 1)
――
615/634 qemu:softfloat+softfloat-conv / fp-test-float-to-float
OK 0.18s
616/634 qemu:softfloat+softfloat-conv / fp-test-int-to-float
OK 0.18s
617/634 qemu:softfloat+softfloat-conv / fp-test-uint-to-float
OK 0.17s
618/634 qemu:softfloat+softfloat-conv / fp-test-float-to-int
OK 0.17s
619/634 qemu:softfloat+softfloat-conv / fp-test-float-to-uint
OK 0.16s
620/634 qemu:softfloat+softfloat-conv / fp-test-round-to-integer
OK 0.15s
621/634 qemu:softfloat+softfloat-compare / fp-test-lt_quiet
OK 0.18s
622/634 qemu:softfloat+softfloat-compare / fp-test-le_quiet
OK 0.19s
623/634 qemu:softfloat+softfloat-compare / fp-test-eq_signaling
OK 0.24s
624/634 qemu:softfloat+softfloat-compare / fp-test-le
OK 0.24s
625/634 qemu:qapi-schema+qapi-doc / QAPI rST doc
OK 0.02s
626/634 qemu:softfloat+softfloat-ops / fp-test-sqrt
OK 0.15s
627/634 qemu:softfloat+softfloat-ops / fp-test-log2
OK 0.28s
628/634 qemu:qapi-schema+qapi-frontend / QAPI schema regression tests
OK 0.33s
629/634 qemu:softfloat+softfloat-ops / fp-test-sub
OK 1.90s
630/634 qemu:softfloat+softfloat-ops / fp-test-add
OK 2.05s
631/634 qemu:decodetree / decodetree
OK 2.57s
632/634 qemu:softfloat+softfloat-ops /
Re: [PATCH v7 00/22] host: Support macOS 12
On 5/9/22 2:31 PM, Philippe Mathieu-Daudé wrote: > On 3/5/22 11:40, Claudio Fontana wrote: >> On 3/7/22 12:17 AM, Philippe Mathieu-Daudé wrote: >>> From: Philippe Mathieu-Daudé >>> >>> Few patches to be able to build QEMU on macOS 12 (Monterey). >>> >>> Missing review: >>> 0006-hvf-Fix-OOB-write-in-RDTSCP-instruction-decode.patch >>> 0013-osdep-Avoid-using-Clang-specific-__builtin_available.patch >>> 0014-meson-Resolve-the-entitlement.sh-script-once-for-goo.patch >>> 0015-meson-Log-QEMU_CXXFLAGS-content-in-summary.patch >>> 0016-configure-Pass-filtered-QEMU_OBJCFLAGS-to-meson.patch >>> 0017-ui-cocoa-Constify-qkeycode-translation-arrays.patch >>> 0020-ui-cocoa-capture-all-keys-and-combos-when-mouse-is-g.patch >>> 0021-ui-cocoa-add-option-to-swap-Option-and-Command.patch >>> 0022-gitlab-ci-Support-macOS-12-via-cirrus-run.patch >>> >>> Since v6: >>> - Dropped merged patches >>> - Addressed Akihiko Odaki comments (squashed 2 patches, added R/T-b) >>> - Dropped 'configure: Disable out-of-line atomic operations on Aarch64' >>> - Add few macos patches on the list pending for 7.0 so tested by CI >> >> >> Hi Philippe, I did not find v6 somehow, >> >> and I was looking for patch: >> >> "[PATCH v5 06/16] hvf: Enable RDTSCP support" >> >> was it dropped / merged with something else? I do not see it in latest git, >> nor in this respin, >> maybe it is in your tree somewhere? > > The patch stayed unreviewed during 2 months, so I dropped it. > > Now it got at least a Tested-by tag from Silvio, I'll include it in the > next PR. > > Regards, > > Phil. > Awesome, thanks for the update! Claudio
Re: [PATCH v7 00/22] host: Support macOS 12
On 3/7/22 12:17 AM, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé
>
> Few patches to be able to build QEMU on macOS 12 (Monterey).
>
> Missing review:
> 0006-hvf-Fix-OOB-write-in-RDTSCP-instruction-decode.patch
> 0013-osdep-Avoid-using-Clang-specific-__builtin_available.patch
> 0014-meson-Resolve-the-entitlement.sh-script-once-for-goo.patch
> 0015-meson-Log-QEMU_CXXFLAGS-content-in-summary.patch
> 0016-configure-Pass-filtered-QEMU_OBJCFLAGS-to-meson.patch
> 0017-ui-cocoa-Constify-qkeycode-translation-arrays.patch
> 0020-ui-cocoa-capture-all-keys-and-combos-when-mouse-is-g.patch
> 0021-ui-cocoa-add-option-to-swap-Option-and-Command.patch
> 0022-gitlab-ci-Support-macOS-12-via-cirrus-run.patch
>
> Since v6:
> - Dropped merged patches
> - Addressed Akihiko Odaki comments (squashed 2 patches, added R/T-b)
> - Dropped 'configure: Disable out-of-line atomic operations on Aarch64'
> - Add few macos patches on the list pending for 7.0 so tested by CI
Hi Philippe, I did not find v6 somehow,
and I was looking for patch:
"[PATCH v5 06/16] hvf: Enable RDTSCP support"
was it dropped / merged with something else? I do not see it in latest git, nor
in this respin,
maybe it is in your tree somewhere?
Thanks,
Claudio
>
> Since v5:
> - Fixed failed rebase between patches 10 and 16 (Akihiko)
> - Include "ui/cocoa: Fix the leak of qemu_console_get_label"
>
> Since v4:
> - Use MAC_OS_X_VERSION_MIN_REQUIRED definition (Akihiko)
> - Include patches from Akihiko
>
> Since v3:
> - Fix --enable-modules
> - Ignore #pragma on softfloat3 tests
> - Addressed Akihiko Odaki comments
> - Include Cameron Esfahani patches
>
> Since v2:
> - Addressed Akihiko Odaki comments:
> . use __is_identifier(),
> . remove cocoa setAllowedFileTypes()
> - Addressed Daniel Berrangé comment:
> . rebased on testing/next, update libvirt-ci/lcitool
>
> Akihiko Odaki (2):
> audio: Log context for audio bug
> coreaudio: Always return 0 in handle_voice_change
>
> Cameron Esfahani (2):
> hvf: Use standard CR0 and CR4 register definitions
> hvf: Fix OOB write in RDTSCP instruction decode
>
> Carwyn Ellis (2):
> ui/cocoa: add option to disable left-command forwarding to guest
> ui/cocoa: release mouse when user switches away from QEMU window
>
> Gustavo Noronha Silva (2):
> ui/cocoa: capture all keys and combos when mouse is grabbed
> ui/cocoa: add option to swap Option and Command
>
> Philippe Mathieu-Daudé (14):
> configure: Allow passing extra Objective C compiler flags
> tests/fp/berkeley-testfloat-3: Ignore ignored #pragma directives
> hvf: Make hvf_get_segments() / hvf_put_segments() local
> hvf: Remove deprecated hv_vcpu_flush() calls
> block/file-posix: Remove a deprecation warning on macOS 12
> audio/coreaudio: Remove a deprecation warning on macOS 12
> audio/dbus: Fix building with modules on macOS
> audio: Rename coreaudio extension to use Objective-C compiler
> osdep: Avoid using Clang-specific __builtin_available()
> meson: Resolve the entitlement.sh script once for good
> meson: Log QEMU_CXXFLAGS content in summary
> configure: Pass filtered QEMU_OBJCFLAGS to meson
> ui/cocoa: Constify qkeycode translation arrays
> gitlab-ci: Support macOS 12 via cirrus-run
>
> .gitlab-ci.d/cirrus.yml| 16
> .gitlab-ci.d/cirrus/macos-12.vars | 16
> audio/audio.c | 25 +++---
> audio/audio_template.h | 27 +++
> audio/{coreaudio.c => coreaudio.m} | 23 +++---
> audio/meson.build | 4 +-
> block/file-posix.c | 14 +++-
> configure | 31
> include/qemu/osdep.h | 10 +--
> meson.build| 17 +++-
> qapi/ui.json | 29 +++
> qemu-options.hx| 15
> target/i386/hvf/vmx.h | 19 +++--
> target/i386/hvf/x86.c | 6 +-
> target/i386/hvf/x86.h | 34
> target/i386/hvf/x86_decode.c | 12 ++-
> target/i386/hvf/x86_mmu.c | 2 +-
> target/i386/hvf/x86_task.c | 4 +-
> target/i386/hvf/x86hvf.c | 14 +++-
> target/i386/hvf/x86hvf.h | 3 +-
> tests/fp/meson.build | 5 ++
> tests/lcitool/refresh | 1 +
> ui/cocoa.m | 122 ++---
> 23 files changed, 327 insertions(+), 122 deletions(-)
> create mode 100644 .gitlab-ci.d/cirrus/macos-12.vars
> rename audio/{coreaudio.c => coreaudio.m} (97%)
>
Re: [PATCH v2 00/18] modules: add metadata database
On 6/10/21 11:54 AM, Gerd Hoffmann wrote:
> On Thu, Jun 10, 2021 at 10:32:39AM +0200, Claudio Fontana wrote:
>> On 6/10/21 7:57 AM, Gerd Hoffmann wrote:
>>> This patch series adds support for module metadata. Here are the pieces
>>> of the puzzle:
>>>
>>> (1) Macros are added to store metadata in a .modinfo elf section
>>> (idea stolen from the linux kernel).
>>> (2) A utility to scan modules, collect metadata from the .modinfo
>>> sections, store it in a file (modinfo.json) for later consumption
>>> by qemu. Can also be easily inspected using 'jq'.
>>> (3) Adding annotations to the modules we have.
>>> (4) Drop hard-coded lists from utils/module.c
>>>
>>> take care,
>>> Gerd
>>
>> The background has disappeared compared with V1.
>>
>> V1 says:
>>
>> "Background is that the hard-coded lists in util/module.c are somewhat
>> ugly and also wouldn't work very well with a large number of modules,
>> so I'm looking for something else."
>
> Well, it's point (4) now (a bit short indeed ...).
>
>> Can you write more about what the actual high level goals of this series are?
>
> Right now we have information about modules hard-coded in various places
> in qemu. Most obvious ones are module_deps[] and qom_modules[] (both in
> util/module.c), but also qemu_load_module_for_opts() (in softmmu/vl.c)
> and maybe more I missed.
Maybe a good idea to find out.
>
> So, when you go build some qom object modular today you'll have to go
> add that to the qom_modules[] list. With this patch series applied
> you'll go add a 'module_obj("typename");' to your source file instead.
>
> Same goes for other metadata, see the "add $foo module annotations"
> patches for examples.
>
>> We are in general making QEMU more and more difficult to get into,
>> requiring more and more investment for new contributors to get
>> productive.
>>
>> Is the additional complexity justified? What is the benefit, and is
>> simplification a goal of the series as well?
>
> IMHO it is a simplification for developers. Modules are more
So the whole endevour here is to remove the need to update modules in two/three
places?
It might simplify life for developers adding a module,
but it is at a cost for the developers trying to keep the plumbing to work in
my opinion.
Based on the information you provided, the reason this whole series exists
seems to be to remove the need to update modules in multiple places.
Should the design focus be on that and that only?
Is there a real need to copy over the mechanism from the kernel, or are the
requirements actually different/simpler here?
> self-contained with this in place. You just add the annotation macros
> and you are done. No need to edit manually maintained lists at some
> non-obvious place elsewhere in the tree. Also no patch conflicts in
> those lists. We have type_init() + friends for simliar reasons.
>
> The price for that simplification is the new utility needed which
> collects and stores the metadata. But that is something you only need
> to worry about when actually working on module support. The build
> system keeps the database automatically up-to-date and most developers
> shouldn't even notice that it is there.
>
> take care,
> Gerd
>
>
Re: [PATCH v2 17/18] modules: check arch and block load on mismatch
On 6/10/21 4:03 PM, Gerd Hoffmann wrote: > Hi, > >> Is the JSON file completely static, listing all modules that were built >> regardless of whether they are currently installed, or would it need to >> be refreshed when installing/uninstalling RPMs with modules ? I would >> think we can do the former and simply handle missing modules on disk >> fairly easily > > We can do both. The file is generated and installed as part of the > build/install process, and it can be simply used as-is even if some of > the modules are missing. > > It's also possible to update the modinfo.json file in postinstall / > postuninstall by simply running qemu-modinfo, so only the modules > actually installed are listed there. > > take care, > Gerd > > I fail to see why that extra complication is needed at all. Why don't we just build the modules for the targets we intend to build, and install them as .so files in a target arch directory? What problem is the json solving? Sorry if obvious. Thanks, Claudio
Re: [PATCH v2 17/18] modules: check arch and block load on mismatch
On 6/10/21 2:35 PM, Daniel P. Berrangé wrote:
> On Thu, Jun 10, 2021 at 07:57:54AM +0200, Gerd Hoffmann wrote:
>> Add module_allow_arch() to set the target architecture.
>> In case a module is limited to some arch verify arches
>> match and ignore the module if not.
>>
>> Signed-off-by: Gerd Hoffmann
>> ---
>> include/qemu/module.h | 1 +
>> softmmu/vl.c | 3 +++
>> util/module.c | 15 +++
>> 3 files changed, 19 insertions(+)
>>
>> diff --git a/include/qemu/module.h b/include/qemu/module.h
>> index d3cab3c25a2f..7825f6d8c847 100644
>> --- a/include/qemu/module.h
>> +++ b/include/qemu/module.h
>> @@ -72,6 +72,7 @@ void module_call_init(module_init_type type);
>> bool module_load_one(const char *prefix, const char *lib_name, bool
>> mayfail);
>> void module_load_qom_one(const char *type);
>> void module_load_qom_all(void);
>> +void module_allow_arch(const char *arch);
>>
>> /*
>> * macros to store module metadata in a .modinfo section.
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index ba26a042b284..96316774fcc9 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -126,6 +126,8 @@
>> #include "sysemu/iothread.h"
>> #include "qemu/guest-random.h"
>>
>> +#include "config-host.h"
>> +
>> #define MAX_VIRTIO_CONSOLES 1
>>
>> typedef struct BlockdevOptionsQueueEntry {
>> @@ -2723,6 +2725,7 @@ void qemu_init(int argc, char **argv, char **envp)
>> error_init(argv[0]);
>> qemu_init_exec_dir(argv[0]);
>>
>> +module_allow_arch(TARGET_NAME);
>> qemu_init_subsystems();
>>
>> /* first pass of option parsing */
>> diff --git a/util/module.c b/util/module.c
>> index 6e4199169c41..564b8e3da760 100644
>> --- a/util/module.c
>> +++ b/util/module.c
>> @@ -122,6 +122,12 @@ void module_call_init(module_init_type type)
>> static Modules *modinfo;
>> static char *module_dirs[5];
>> static int module_ndirs;
>> +static const char *module_arch;
>> +
>> +void module_allow_arch(const char *arch)
>> +{
>> +module_arch = arch;
>> +}
>>
>> static void module_load_path_init(void)
>> {
>> @@ -295,6 +301,14 @@ bool module_load_one(const char *prefix, const char
>> *lib_name, bool mayfail)
>> module_load_modinfo();
>>
>> for (modlist = modinfo->list; modlist != NULL; modlist = modlist->next)
>> {
>> +if (modlist->value->has_arch) {
>> +if (strcmp(modlist->value->name, module_name) == 0) {
>> +if (!module_arch ||
>> +strcmp(modlist->value->arch, module_arch) != 0) {
>> +return false;
>> +}
>> +}
>> +}
>
> I have a little hard time following the code paths, but IIUC, with this
> change, instead of "module.so" we would have multiple copies of something
> like "module-$arch.so" ? Then we load them all, read their modinfo section
> and discard the ones with non-matching arch ?
>
> If that is a correct understanding, then I wonder about the scalability of
> it. We have 30 system emulator targets right now, so if we get even a few
> arch specific modues, that's going to be alot of modules we're loading,
> checking and discarding.
>
> Wouldn't it be simpler if we just made use of the directory layout
> /usr/lib64/qemu/$mod.so for common modules and /usr/lib64/qemu/$arch/$mod.so
> for arch specific modules. That would let us load only the modules we know
> are applicable for the system target arch and not need this post-load
> filtering from metadata.
>
> Regards,
> Daniel
>
agreed.
Claudio
Re: [PATCH v2 00/18] modules: add metadata database
On 6/10/21 7:57 AM, Gerd Hoffmann wrote: > This patch series adds support for module metadata. Here are the pieces > of the puzzle: > > (1) Macros are added to store metadata in a .modinfo elf section > (idea stolen from the linux kernel). > (2) A utility to scan modules, collect metadata from the .modinfo > sections, store it in a file (modinfo.json) for later consumption > by qemu. Can also be easily inspected using 'jq'. > (3) Adding annotations to the modules we have. > (4) Drop hard-coded lists from utils/module.c > > take care, > Gerd The background has disappeared compared with V1. V1 says: "Background is that the hard-coded lists in util/module.c are somewhat ugly and also wouldn't work very well with a large number of modules, so I'm looking for something else." Can you write more about what the actual high level goals of this series are? We are in general making QEMU more and more difficult to get into, requiring more and more investment for new contributors to get productive. Is the additional complexity justified? What is the benefit, and is simplification a goal of the series as well? > > Gerd Hoffmann (18): > modules: add metadata macros, add qxl module annotations > qapi: add ModuleInfo schema > modules: add qemu-modinfo utility > modules: add virtio-gpu module annotations > modules: add chardev module annotations > modules: add audio module annotations > modules: add usb-redir module annotations > modules: add ccid module annotations > modules: add ui module annotations > modules: add s390x module annotations > modules: add block module annotations > modules: add module_load_path_init helper > modules: load modinfo.json > modules: use modinfo for dependencies > modules: use modinfo for qom load > modules: use modinfo for qemu opts load > modules: check arch and block load on mismatch > [fixup] module_load_modinfo > > include/qemu/module.h | 23 +++ > audio/spiceaudio.c | 2 + > block/iscsi-opts.c | 1 + > chardev/baum.c | 1 + > chardev/spice.c | 4 + > hw/display/qxl.c| 4 + > hw/display/vhost-user-gpu-pci.c | 1 + > hw/display/vhost-user-gpu.c | 1 + > hw/display/vhost-user-vga.c | 1 + > hw/display/virtio-gpu-base.c| 1 + > hw/display/virtio-gpu-gl.c | 3 + > hw/display/virtio-gpu-pci-gl.c | 3 + > hw/display/virtio-gpu-pci.c | 2 + > hw/display/virtio-gpu.c | 1 + > hw/display/virtio-vga-gl.c | 3 + > hw/display/virtio-vga.c | 2 + > hw/s390x/virtio-ccw-gpu.c | 3 + > hw/usb/ccid-card-emulated.c | 1 + > hw/usb/ccid-card-passthru.c | 1 + > hw/usb/redirect.c | 1 + > qemu-modinfo.c | 270 ++ > softmmu/vl.c| 20 +-- > stubs/module-opts.c | 4 - > ui/egl-headless.c | 4 + > ui/gtk.c| 4 + > ui/sdl2.c | 4 + > ui/spice-app.c | 3 + > ui/spice-core.c | 5 + > util/module.c | 282 +++- > meson.build | 11 ++ > qapi/meson.build| 1 + > qapi/modules.json | 36 > qapi/qapi-schema.json | 1 + > util/trace-events | 3 + > 34 files changed, 576 insertions(+), 131 deletions(-) > create mode 100644 qemu-modinfo.c > create mode 100644 qapi/modules.json >
Re: "make check" broken with everything but tools disabled
On 3/18/21 11:03 AM, Paolo Bonzini wrote: > On 18/03/21 10:16, Claudio Fontana wrote: >> my experience with the new build system (meson-based), is that I have to do: >> >> make >> >> first, and then >> >> make check >> >> later, or bugs start happening > > This shouldn't be needed. > > Paolo > I am pretty sure it didn't work in some cases, maybe with check-tcg only.. will keep an eye on this. Ciao, Claudio
Re: "make check" broken with everything but tools disabled
On 3/16/21 2:28 PM, Markus Armbruster wrote: > Watch this: > > $ mkdir bld-tools > $ cd bld-tools > $ ../configure --disable-system --disable-user --enable-tools > $ make check > [...] > make: *** No rule to make target 'tests/qemu-iotests/socket_scm_helper', > needed by 'check-block'. Stop. > > Hi Markus, I can reproduce this error too. When looking at the sequence of commands I was almost sure it was due to a missing "make" before the "make check", as my experience with the new build system (meson-based), is that I have to do: make first, and then make check later, or bugs start happening, build can end up doing the wrong things. I thought this was a known limitation and I am currently just dealing with it. But then I tried to apply this to your specific case, and I get the same error nevertheless. make: *** No rule to make target 'tests/qemu-iotests/socket_scm_helper', needed by 'check-block'. Stop. Ciao, CLaudio
Re: [PATCH 00/10] target: Provide target-specific Kconfig
Hi all, where are we with this series? I am trying to figure out how to apply the whole thing, in order to build only compatible ARM boards and devices for KVM-only builds. Ie, I would like to combine this with: "arm cleanup experiment for kvm-only build" https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg02790.html My series there gets us to a buildable and working KVM-only ARM target, with all make check tests passing. However, in order to be able to remove more code, remove stubs etc, I found the presence of incompatible ARM boards and devices as blocking additional cleanups. Therefore, in order to proceed with additional cleanup in arm, and adding the accel-specific extensions to the cpu class, I see being able to disable incompatible boards for KVM as necessary. IIUC there are: * This series "target: Provide target-specific Kconfig" * Support disabling TCG on ARM ? * Support disabling TCG on ARM (part 2) ? What is the current state here? Thanks, Claudio On 1/31/21 12:13 PM, Philippe Mathieu-Daudé wrote: > Hi, > > This series add a Kconfig file to each target, allowing > to select target-specific features there, instead of from > the hardware Kconfig. > > This simplifies managing multi-arch features such semihosting. > > Series organization: > > 1/ Some targets use the architecture symbol to select boards and > peripherals (SH4 and LM32), we need to clean that first. > > 2/ Introduce empty target Kconfig, update meson. > > 3/ Move architectural features out of hardware: >- x86 SEV >- ARM v7m >- generic semihosting > > [following only important to patchew, unrelated to this series] > Based-on: <20210131105918.228787-1-f4...@amsat.org> > > Philippe Mathieu-Daudé (10): > hw/sh4/Kconfig: Rename CONFIG_SH4 -> CONFIG_SH4_PERIPHERALS > hw/lm32/Kconfig: Introduce CONFIG_LM32_EVR for lm32-evr/uclinux boards > hw/sh4/Kconfig: Rename CONFIG_LM32 -> CONFIG_LM32_PERIPHERALS > hw/lm32/Kconfig: Have MILKYMIST select LM32_PERIPHERALS > meson: Introduce target-specific Kconfig > target/i386: Move SEV feature to target Kconfig > target/arm: Move V7M feature to target Kconfig > default-configs: Remove unnecessary SEMIHOSTING selection > target: Move ARM_COMPATIBLE_SEMIHOSTING feature to target Kconfig > target: Move SEMIHOSTING feature to target Kconfig > > default-configs/devices/arm-softmmu.mak | 2 -- > default-configs/devices/lm32-softmmu.mak | 4 +--- > default-configs/devices/m68k-softmmu.mak | 2 -- > .../devices/mips-softmmu-common.mak | 3 --- > default-configs/devices/nios2-softmmu.mak | 2 -- > default-configs/devices/riscv32-softmmu.mak | 2 -- > default-configs/devices/riscv64-softmmu.mak | 2 -- > default-configs/devices/unicore32-softmmu.mak | 1 - > default-configs/devices/xtensa-softmmu.mak| 2 -- > meson.build | 3 ++- > Kconfig | 1 + > hw/arm/Kconfig| 4 > hw/block/meson.build | 2 +- > hw/char/meson.build | 6 ++--- > hw/i386/Kconfig | 4 > hw/intc/meson.build | 4 ++-- > hw/lm32/Kconfig | 10 +--- > hw/lm32/meson.build | 2 +- > hw/sh4/Kconfig| 6 ++--- > hw/timer/meson.build | 4 ++-- > target/Kconfig| 23 +++ > target/alpha/Kconfig | 2 ++ > target/arm/Kconfig| 11 + > target/avr/Kconfig| 2 ++ > target/cris/Kconfig | 2 ++ > target/hppa/Kconfig | 2 ++ > target/i386/Kconfig | 9 > target/lm32/Kconfig | 3 +++ > target/m68k/Kconfig | 3 +++ > target/microblaze/Kconfig | 2 ++ > target/mips/Kconfig | 7 ++ > target/moxie/Kconfig | 2 ++ > target/nios2/Kconfig | 3 +++ > target/openrisc/Kconfig | 2 ++ > target/ppc/Kconfig| 5 > target/riscv/Kconfig | 7 ++ > target/rx/Kconfig | 2 ++ > target/s390x/Kconfig | 2 ++ > target/sh4/Kconfig| 2 ++ > target/sparc/Kconfig | 5 > target/tilegx/Kconfig | 2 ++ > target/tricore/Kconfig| 2 ++ > target/unicore32/Kconfig | 3 +++ > target/xtensa/Kconfig | 3 +++ > 44 files changed, 129 insertions(+), 43 deletions(-) > create mode
Re: [PATCH v6 03/11] target/arm: Restrict ARMv4 cpus to TCG accel
Hi, I am trying to take these patches, in the hope that they help with some of the test issues I am having with the kvm-only build, but they fail with: target/arm/Kconfig: does not exist in index so I guess I need the "target/arm/Kconfig" series right, how can I find that one? Thanks, Claudio On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote: > KVM requires the target cpu to be at least ARMv8 architecture > (support on ARMv7 has been dropped in commit 82bf7ae84ce: > "target/arm: Remove KVM support for 32-bit Arm hosts"). > > Only enable the following ARMv4 CPUs when TCG is available: > > - StrongARM (SA1100/1110) > - OMAP1510 (TI925T) > > The following machines are no more built when TCG is disabled: > > - cheetah Palm Tungsten|E aka. Cheetah PDA (OMAP310) > - sx1 Siemens SX1 (OMAP310) V2 > - sx1-v1 Siemens SX1 (OMAP310) V1 > > Signed-off-by: Philippe Mathieu-Daudé > --- > default-configs/devices/arm-softmmu.mak | 2 -- > hw/arm/Kconfig | 4 > target/arm/Kconfig | 4 > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/default-configs/devices/arm-softmmu.mak > b/default-configs/devices/arm-softmmu.mak > index 0824e9be795..6ae964c14fd 100644 > --- a/default-configs/devices/arm-softmmu.mak > +++ b/default-configs/devices/arm-softmmu.mak > @@ -14,8 +14,6 @@ CONFIG_INTEGRATOR=y > CONFIG_FSL_IMX31=y > CONFIG_MUSICPAL=y > CONFIG_MUSCA=y > -CONFIG_CHEETAH=y > -CONFIG_SX1=y > CONFIG_NSERIES=y > CONFIG_STELLARIS=y > CONFIG_REALVIEW=y > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig > index f3ecb73a3d8..f2957b33bee 100644 > --- a/hw/arm/Kconfig > +++ b/hw/arm/Kconfig > @@ -31,6 +31,8 @@ config ARM_VIRT > > config CHEETAH > bool > +default y if TCG && ARM > +select ARM_V4 > select OMAP > select TSC210X > > @@ -249,6 +251,8 @@ config COLLIE > > config SX1 > bool > +default y if TCG && ARM > +select ARM_V4 > select OMAP > > config VERSATILE > diff --git a/target/arm/Kconfig b/target/arm/Kconfig > index ae89d05c7e5..811e1e81652 100644 > --- a/target/arm/Kconfig > +++ b/target/arm/Kconfig > @@ -6,6 +6,10 @@ config AARCH64 > bool > select ARM > > +config ARM_V4 > +bool > +depends on TCG && ARM > + > config ARM_V7M > bool > select PTIMER >
Re: [PATCH 8/9] hw/arm/virt: Restrict 32-bit CPUs to TCG
Hi Peter,
what do you think of the following patch? We messaged yesterday about
cortex-a15 being the default cpu for virt,
this patch would need also changing the default CPU for virt under KVM I would
think.
Or, we could change the virt default cpu to "max"?
Thanks,
Claudio
On 2/5/21 4:19 PM, Andrew Jones wrote:
> On Fri, Feb 05, 2021 at 03:43:44PM +0100, Philippe Mathieu-Daudé wrote:
>> Support for ARMv7 has been dropped in commit 82bf7ae84ce
>> ("target/arm: Remove KVM support for 32-bit Arm hosts").
>> Restrict the 32-bit CPUs to --enable-tcg builds.
>>
>> Signed-off-by: Philippe Mathieu-Daudé
>> ---
>> hw/arm/virt.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index f5e4a6ec914..ab6300650f9 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -197,8 +197,10 @@ static const int a15irqmap[] = {
>> };
>>
>> static const char *valid_cpus[] = {
>> +#ifdef CONFIG_TCG
>> ARM_CPU_TYPE_NAME("cortex-a7"),
>> ARM_CPU_TYPE_NAME("cortex-a15"),
>> +#endif /* CONFIG_TCG */
>> #ifdef TARGET_AARCH64
>> ARM_CPU_TYPE_NAME("cortex-a53"),
>> ARM_CPU_TYPE_NAME("cortex-a57"),
>> --
>> 2.26.2
>>
>
> So this filters the cpus out of KVM only builds, which seems
> reasonable to do. Of course, if the build is for both KVM and
> TCG, then the cpus won't be filtered out and we'll have to rely
> on the runtime checks to error out if one where to try a 32-bit
> cpu with KVM. But that's fine too, so
>
> Reviewed-by: Andrew Jones
>
> Thanks,
> drew
>
>
Re: [PATCH 0/9] hw/arm/virt: Improve CPU help and fix testing under KVM
On 2/5/21 3:43 PM, Philippe Mathieu-Daudé wrote: > Yet again bugfixes and cleanup patches noticed while > rebasing my "Support disabling TCG on ARM (part 2)" series. > > Sending them independently as they aren't directly dependent > of it so don't have to be delayed by other unanswered questions. > > Please review, > > Phil. > > Philippe Mathieu-Daudé (9): > tests/qtest/arm-cpu-features: Remove Cortex-A15 check > tests/qtest: Restrict xlnx-can-test to TCG builds > tests/qtest/boot-serial-test: Test Virt machine with 'max' > tests/qtest/cdrom-test: Only allow the Virt machine under KVM > hw/arm/virt: Improve CPU name in help message > hw/arm/virt: Display list of valid CPUs for the Virt machine > hw/arm/virt: Do not include 64-bit CPUs in 32-bit build > hw/arm/virt: Restrict 32-bit CPUs to TCG > tests/qtest/arm-cpu-features: Restrict TCG-only tests > > hw/arm/virt.c | 20 +- > tests/qtest/arm-cpu-features.c | 37 ++ > tests/qtest/boot-serial-test.c | 2 +- > tests/qtest/cdrom-test.c | 5 - > tests/qtest/meson.build| 2 +- > 5 files changed, 54 insertions(+), 12 deletions(-) > Hi Philippe, Markus, I encountered an issue where device-introspect-test.c gets me a qemu-system-aarch64 launched as: ./qemu-system-aarch64 -qtest ... -display none -nodefaults -machine none -accel qtest and results in an attempt to create a device "ast2400-a1", which tries to create a arm926-arm-cpu, which fails because it is a "TCG" cpu, that is not built in my kvm-only build. Any ideas why this happens? Thanks, Claudio
Re: [RFC PATCH 9/9] tests/qtest/arm-cpu-features: Restrict TCG-only tests
On 2/5/21 4:30 PM, Philippe Mathieu-Daudé wrote:
> On 2/5/21 4:20 PM, Claudio Fontana wrote:
>> On 2/5/21 3:43 PM, Philippe Mathieu-Daudé wrote:
>>> Some tests explicitly request the TCG accelerator. As these
>>> tests will obviously fails if TCG is not present, disable
>>> them in such case.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé
>>> ---
>>> Cc: Roman Bolshakov
>>> Cc: Claudio Fontana
>>>
>>> RFC because of the TODO.
>>>
>>> Roman posted a series to have a QMP command to query enabled
>>> accelerators.
>>> ---
>>> tests/qtest/arm-cpu-features.c | 33 +
>>> 1 file changed, 29 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
>>> index c59c3cb002b..c6e86282b66 100644
>>> --- a/tests/qtest/arm-cpu-features.c
>>> +++ b/tests/qtest/arm-cpu-features.c
>>> @@ -20,7 +20,7 @@
>>> */
>>> #define SVE_MAX_VQ 16
>>>
>>> -#define MACHINE "-machine virt,gic-version=max -accel tcg "
>>> +#define MACHINE_TCG "-machine virt,gic-version=max -accel tcg "
>>> #define MACHINE_KVM "-machine virt,gic-version=max -accel kvm -accel tcg "
>>> #define QUERY_HEAD "{ 'execute': 'query-cpu-model-expansion', " \
>>> " 'arguments': { 'type': 'full', "
>>> @@ -41,6 +41,16 @@ static bool kvm_enabled(QTestState *qts)
>>> return enabled;
>>> }
>>>
>>> +static bool tcg_enabled(QTestState *qts)
>>> +{
>>> +/* TODO: Implement QMP query-accel? */
>>> +#ifdef CONFIG_TCG
>>> +return true;
>>> +#else
>>> +return false;
>>> +#endif /* CONFIG_TCG */
>>
>>
>> I would not use the same name as the existing tcg_enabled(), which has
>> different semantics, even in test code;
>>
>> what you mean here is tcg_available() right?
>
> No, I meant static tcg_enabled as the kvm_enabled() earlier method:
Aha, so it's the other way around, we are actually testing if the TCG
accelerator is currently selected in QEMU,
and the patch implements it using CONFIG_TCG as a placeholder for it, since we
do not have query-accel yet, got it.
>
> static bool kvm_enabled(QTestState *qts)
> {
> QDict *resp, *qdict;
> bool enabled;
>
> resp = qtest_qmp(qts, "{ 'execute': 'query-kvm' }");
> g_assert(qdict_haskey(resp, "return"));
> qdict = qdict_get_qdict(resp, "return");
> g_assert(qdict_haskey(qdict, "enabled"));
> enabled = qdict_get_bool(qdict, "enabled");
> qobject_unref(resp);
>
> return enabled;
> }
>
> This should be moved to something generic to QTest IMO,
> and we need some runtime qtest_is_accelerator_enabled().
>
Agreed,
thanks,
Claudio
Re: [RFC PATCH 9/9] tests/qtest/arm-cpu-features: Restrict TCG-only tests
On 2/5/21 3:43 PM, Philippe Mathieu-Daudé wrote:
> Some tests explicitly request the TCG accelerator. As these
> tests will obviously fails if TCG is not present, disable
> them in such case.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> Cc: Roman Bolshakov
> Cc: Claudio Fontana
>
> RFC because of the TODO.
>
> Roman posted a series to have a QMP command to query enabled
> accelerators.
> ---
> tests/qtest/arm-cpu-features.c | 33 +
> 1 file changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
> index c59c3cb002b..c6e86282b66 100644
> --- a/tests/qtest/arm-cpu-features.c
> +++ b/tests/qtest/arm-cpu-features.c
> @@ -20,7 +20,7 @@
> */
> #define SVE_MAX_VQ 16
>
> -#define MACHINE "-machine virt,gic-version=max -accel tcg "
> +#define MACHINE_TCG "-machine virt,gic-version=max -accel tcg "
> #define MACHINE_KVM "-machine virt,gic-version=max -accel kvm -accel tcg "
> #define QUERY_HEAD "{ 'execute': 'query-cpu-model-expansion', " \
> " 'arguments': { 'type': 'full', "
> @@ -41,6 +41,16 @@ static bool kvm_enabled(QTestState *qts)
> return enabled;
> }
>
> +static bool tcg_enabled(QTestState *qts)
> +{
> +/* TODO: Implement QMP query-accel? */
> +#ifdef CONFIG_TCG
> +return true;
> +#else
> +return false;
> +#endif /* CONFIG_TCG */
I would not use the same name as the existing tcg_enabled(), which has
different semantics, even in test code;
what you mean here is tcg_available() right?
> +}
> +
> static QDict *do_query_no_props(QTestState *qts, const char *cpu_type)
> {
> return qtest_qmp(qts, QUERY_HEAD "'model': { 'name': %s }"
> @@ -352,7 +362,12 @@ static void sve_tests_sve_max_vq_8(const void *data)
> {
> QTestState *qts;
>
> -qts = qtest_init(MACHINE "-cpu max,sve-max-vq=8");
> +qts = qtest_init(MACHINE_TCG "-cpu max,sve-max-vq=8");
> +
> +if (!tcg_enabled(qts)) {
> +qtest_quit(qts);
> +return;
> +}
>
> assert_sve_vls(qts, "max", BIT_ULL(8) - 1, NULL);
>
> @@ -387,7 +402,12 @@ static void sve_tests_sve_off(const void *data)
> {
> QTestState *qts;
>
> -qts = qtest_init(MACHINE "-cpu max,sve=off");
> +qts = qtest_init(MACHINE_TCG "-cpu max,sve=off");
> +
> +if (!tcg_enabled(qts)) {
> +qtest_quit(qts);
> +return;
> +}
>
> /* SVE is off, so the map should be empty. */
> assert_sve_vls(qts, "max", 0, NULL);
> @@ -443,7 +463,12 @@ static void test_query_cpu_model_expansion(const void
> *data)
> {
> QTestState *qts;
>
> -qts = qtest_init(MACHINE "-cpu max");
> +qts = qtest_init(MACHINE_TCG "-cpu max");
> +
> +if (!tcg_enabled(qts)) {
> +qtest_quit(qts);
> +return;
> +}
>
> /* Test common query-cpu-model-expansion input validation */
> assert_type_full(qts);
>
Re: [PATCH] iotests: 30: drop from auto group (and effectively from make check)
Just in case it helps,
I started getting this error only when I rebased to latest master a couple days
ago, after regular rebasing and testing with full make check every 1 or 2 days.
Ciao,
Claudio
On 2/5/21 12:10 PM, Vladimir Sementsov-Ogievskiy wrote:
> I reproduced the following crash fast enough:
>
> 0 raise () at /lib64/libc.so.6
> 1 abort () at /lib64/libc.so.6
> 2 _nl_load_domain.cold () at /lib64/libc.so.6
> 3 annobin_assert.c_end () at /lib64/libc.so.6
> 4 bdrv_reopen_multiple (bs_queue=0x55de75fa9b70, errp=0x0)
>at ../block.c:3820
> 5 bdrv_reopen_set_read_only (bs=0x55de760fc020, read_only=true,
>errp=0x0) at ../block.c:3870
> 6 stream_clean (job=0x55de75fa9410) at ../block/stream.c:99
> 7 job_clean (job=0x55de75fa9410) at ../job.c:680
> 8 job_finalize_single (job=0x55de75fa9410) at ../job.c:696
> 9 job_txn_apply (job=0x55de75fa9410,
>fn=0x55de741eee27 ) at ../job.c:158
> 10 job_do_finalize (job=0x55de75fa9410) at ../job.c:805
> 11 job_completed_txn_success (job=0x55de75fa9410) at ../job.c:855
> 12 job_completed (job=0x55de75fa9410) at ../job.c:868
> 13 job_exit (opaque=0x55de75fa9410) at ../job.c:888
> 14 aio_bh_call (bh=0x55de76b9b4e0) at ../util/async.c:136
> 15 aio_bh_poll (ctx=0x55de75bc5300) at ../util/async.c:164
> 16 aio_dispatch (ctx=0x55de75bc5300) at ../util/aio-posix.c:381
> 17 aio_ctx_dispatch (source=0x55de75bc5300, callback=0x0,
>user_data=0x0) at ../util/async.c:306
> 18 g_main_context_dispatch () at /lib64/libglib-2.0.so.0
> 19 glib_pollfds_poll () at ../util/main-loop.c:232
> 20 os_host_main_loop_wait (timeout=0) at ../util/main-loop.c:255
> 21 main_loop_wait (nonblocking=0) at ../util/main-loop.c:531
> 22 qemu_main_loop () at ../softmmu/runstate.c:722
> 23 main (argc=20, argv=0x7ffe218f0268, envp=0x7ffe218f0310) at
>../softmmu/main.c:50
>
> (gdb) fr 4
> 4 bdrv_reopen_multiple (bs_queue=0x55de75fa9b70, errp=0x0) at
> ../block.c:3820
> 3820assert(perm == state->perm);
> (gdb) list
> 3815
> 3816if (ret == 0) {
> 3817uint64_t perm, shared;
> 3818
> 3819bdrv_get_cumulative_perm(state->bs, ,
> );
> 3820assert(perm == state->perm);
> 3821assert(shared == state->shared_perm);
> 3822
> 3823bdrv_set_perm(state->bs);
> 3824} else {
> (gdb) p perm
> $1 = 1
> (gdb) p state->perm
> $2 = 0
>
> Then I had 38 successful iterations and another crash:
> 0 bdrv_check_update_perm (bs=0x5631ac97bc50, q=0x0, new_used_perm=1,
>new_shared_perm=31, ignore_children=0x0, errp=0x7ffd9d477cf8) at
>../block.c:2197
> 1 bdrv_root_attach_child
> (child_bs=0x5631ac97bc50, child_name=0x5631aaf6b1f9 "backing",
> child_class=0x5631ab280ca0 , child_role=8,
> ctx=0x5631ab757300, perm=1, shared_perm=31, opaque=0x5631abb8c020,
> errp=0x7ffd9d477cf8)
> at ../block.c:2642
> 2 bdrv_attach_child (parent_bs=0x5631abb8c020,
>child_bs=0x5631ac97bc50, child_name=0x5631aaf6b1f9 "backing",
>child_class=0x5631ab280ca0 , child_role=8,
>errp=0x7ffd9d477cf8)
> at ../block.c:2719
> 3 bdrv_set_backing_hd (bs=0x5631abb8c020, backing_hd=0x5631ac97bc50,
>errp=0x7ffd9d477cf8) at ../block.c:2854
> 4 stream_prepare (job=0x5631ac751eb0) at ../block/stream.c:74
> 5 job_prepare (job=0x5631ac751eb0) at ../job.c:784
> 6 job_txn_apply (job=0x5631ac751eb0, fn=0x5631aacb1156 )
>at ../job.c:158
> 7 job_do_finalize (job=0x5631ac751eb0) at ../job.c:801
> 8 job_completed_txn_success (job=0x5631ac751eb0) at ../job.c:855
> 9 job_completed (job=0x5631ac751eb0) at ../job.c:868
> 10 job_exit (opaque=0x5631ac751eb0) at ../job.c:888
> 11 aio_bh_call (bh=0x7f3d9c007680) at ../util/async.c:136
> 12 aio_bh_poll (ctx=0x5631ab757300) at ../util/async.c:164
> 13 aio_dispatch (ctx=0x5631ab757300) at ../util/aio-posix.c:381
> 14 aio_ctx_dispatch (source=0x5631ab757300, callback=0x0,
>user_data=0x0) at ../util/async.c:306
> 15 g_main_context_dispatch () at /lib64/libglib-2.0.so.0
> 16 glib_pollfds_poll () at ../util/main-loop.c:232
> 17 os_host_main_loop_wait (timeout=0) at ../util/main-loop.c:255
> 18 main_loop_wait (nonblocking=0) at ../util/main-loop.c:531
> 19 qemu_main_loop () at ../softmmu/runstate.c:722
> 20 main (argc=20, argv=0x7ffd9d478198, envp=0x7ffd9d478240) at
>../softmmu/main.c:50
> (gdb) list
> 2192QLIST_FOREACH(c, >parents, next_parent) {
> 2193if (g_slist_find(ignore_children, c)) {
> 2194continue;
> 2195}
> 2196
> 2197if ((new_used_perm & c->shared_perm) != new_used_perm)
> {
> 2198char *user = bdrv_child_user_desc(c);
> 2199char *perm_names = bdrv_perm_names(new_used_perm &
> ~c->shared_perm);
> 2200
> 2201error_setg(errp, "Conflicts with use by %s as '%s',
> which does not "
> (gdb) p c
> $1 = (BdrvChild *) 0x8585858585858585
>
> and
Re: iotest 30 failing
On 2/4/21 11:38 PM, Vladimir Sementsov-Ogievskiy wrote: > 04.02.2021 20:51, Peter Maydell wrote: >> On Thu, 4 Feb 2021 at 17:48, Philippe Mathieu-Daudé >> wrote: >>> >>> Hi, >>> >>> Based on commit 1ed9228f63e (ericb/tags/pull-nbd-2021-02-02-v2) >>> I got: >>> >>>TEST iotest-qcow2: 030 [fail] >> >> Yes; see also this thread: >> https://lore.kernel.org/qemu-devel/9e71568c-ce4a-f844-fbd3-a4a59f850...@redhat.com/ >> >> Can somebody write a simple patch to disable the test entirely, >> please? It's too unreliable to be in our CI set. >> > > I can.. > > Still, maybe I'll try tomorrow to make v2 from my "[PATCH RFC 0/5] Fix > accidental crash in iotest 30"? > > It would be very interesting will it fail after that fix. My experiments > shows that my patches helps. But the question is do we really have the same > crash in all these reports or not. So I think it worth taking my fix (even > being incomplete solution) to understand do we ignore some unknown bug. > Today's master passes the test for me, yesterday's failed. Was something changed or is it just flicky.. Ciao, Claudio
Re: [PATCH v6 01/11] sysemu/tcg: Introduce tcg_builtin() helper
On 1/31/21 4:23 PM, Philippe Mathieu-Daudé wrote:
> On 1/31/21 3:18 PM, Claudio Fontana wrote:
>> On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote:
>>> Modules are registered early with type_register_static().
>>>
>>> We would like to call tcg_enabled() when registering QOM types,
>>
>>
>> Hi Philippe,
>>
>> could this not be controlled by meson at this stage?
>> On X86, I register the tcg-specific types in tcg/* in modules that are only
>> built for TCG.
>>
>> Maybe tcg_builtin() is useful anyway, thinking long term at loadable modules,
>> but there we are interested in whether tcg code is available or not,
>> regardless of whether it's builtin,
>> or needs to be loaded via a .so plugin..
>>
>> maybe tcg_available()?
>
> The alternatives I found:
>
> - reorder things in vl.c?
>
> - use ugly #ifdef'ry, see this patch:
> https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg08037.html
Not sure it's that ugly,
if it is followed (or replaced by) exporting those pieces to separate files,
which are only built by meson on CONFIG_TCG.
I did not try to do it, so you know best of course.
Ciao,
Claudio
>
> - this earlier approach I previously discarded:
>
> -- >8 --
> diff --git a/include/qom/object.h b/include/qom/object.h
> index d378f13a116..30590c6fac3 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -403,9 +403,12 @@ struct Object
> * parent class initialization has occurred, but before the class itself
> * is initialized. This is the function to use to undo the effects of
> * memcpy from the parent class to the descendants.
> - * @class_data: Data to pass to the @class_init,
> + * @class_data: Data to pass to the @class_registerable, @class_init,
> * @class_base_init. This can be useful when building dynamic
> * classes.
> + * @registerable: This function is called when modules are registered,
> + * prior to any class initialization. When present and returning %false,
> + * the type is not registered, the class is not present (not usable).
> * @interfaces: The list of interfaces associated with this type. This
> * should point to a static array that's terminated with a zero filled
> * element.
> @@ -428,6 +431,7 @@ struct TypeInfo
> void (*class_base_init)(ObjectClass *klass, void *data);
> void *class_data;
>
> +bool (*registerable)(void *data);
> InterfaceInfo *interfaces;
> };
>
> diff --git a/qom/object.c b/qom/object.c
> index 2fa0119647c..0febaffa12e 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -138,6 +138,10 @@ static TypeImpl *type_new(const TypeInfo *info)
> static TypeImpl *type_register_internal(const TypeInfo *info)
> {
> TypeImpl *ti;
> +
> +if (info->registerable && !info->registerable(info->class_data)) {
> +return NULL;
> +}
> ti = type_new(info);
>
> type_table_add(ti);
>
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 990509d3852..1a2b1889da4 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -24,6 +24,7 @@
> #include "hw/loader.h"
> #include "hw/arm/boot.h"
> #include "sysemu/sysemu.h"
> +#include "sysemu/tcg.h"
> #include "qom/object.h"
>
> #define SMPBOOT_ADDR0x300 /* this should leave enough space for
> ATAGS */
> @@ -368,18 +369,26 @@ static void raspi3b_machine_class_init(ObjectClass
> *oc, void *data)
> };
> #endif /* TARGET_AARCH64 */
>
> +static bool raspi_machine_requiring_tcg_accel(void *data)
> +{
> +return tcg_builtin();
> +}
> +
> static const TypeInfo raspi_machine_types[] = {
> {
> .name = MACHINE_TYPE_NAME("raspi0"),
> .parent = TYPE_RASPI_MACHINE,
> +.registerable = raspi_machine_requiring_tcg_accel,
> .class_init = raspi0_machine_class_init,
> }, {
> .name = MACHINE_TYPE_NAME("raspi1ap"),
> .parent = TYPE_RASPI_MACHINE,
> +.registerable = raspi_machine_requiring_tcg_accel,
> .class_init = raspi1ap_machine_class_init,
> }, {
> .name = MACHINE_TYPE_NAME("raspi2b"),
> .parent = TYPE_RASPI_MACHINE,
> +.registerable = raspi_machine_requiring_tcg_accel,
> .class_init = raspi2b_machine_class_init,
> #ifdef TARGET_AARCH64
> }, {
> ---
>
>>
>> Ciao,
>>
>> Claudio
>>
>>> but tcg_enabled() returns tcg_allowed which is a runtime property
>>> initialized later (See commit
Re: [PATCH v6 00/11] Support disabling TCG on ARM (part 2)
On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote: > Cover from Samuel Ortiz from (part 1) [1]: > > This patchset allows for building and running ARM targets with TCG > disabled. [...] > > The rationale behind this work comes from the NEMU project where > we're trying to only support x86 and ARM 64-bit architectures, > without including the TCG code base. We can only do so if we can > build and run ARM binaries with TCG disabled. > > Peter mentioned in v5 [6] that since 32-bit host has been removed, > we have to remove v7 targets. This is not done in this series, as > linking succeeds, and there is enough material to review (no need > to spend time on that extra patch if the current approach is not > accepted). > > CI: https://gitlab.com/philmd/qemu/-/pipelines/249272441 > > v6: > - rebased on "target/arm/Kconfig" series > - introduce/use tcg_builtin() for realview machines > > v5: > - addressed Paolo/Richard/Thomas review comments from v4 [5]. > > v4 almost 2 years later... [2]: > - Rebased on Meson > - Addressed Richard review comments > - Addressed Claudio review comments > > v3 almost 18 months later [3]: > - Rebased > - Addressed Thomas review comments > - Added Travis-CI job to keep building --disable-tcg on ARM > > v2 [4]: > - Addressed review comments from Richard and Thomas from v1 [1] > > Regards, > > Phil. > > [1]: https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg02451.html > [2]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg689168.html > [3]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg641796.html > [4]: https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05003.html > [5]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg746041.html > [6]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg777669.html > > Based-on: <2021013316.232778-1-f4...@amsat.org> > "target: Provide target-specific Kconfig" > > Philippe Mathieu-Daudé (9): > sysemu/tcg: Introduce tcg_builtin() helper > exec: Restrict TCG specific headers > target/arm: Restrict ARMv4 cpus to TCG accel > target/arm: Restrict ARMv5 cpus to TCG accel > target/arm: Restrict ARMv6 cpus to TCG accel > target/arm: Restrict ARMv7 R-profile cpus to TCG accel > target/arm: Restrict ARMv7 M-profile cpus to TCG accel > target/arm: Reorder meson.build rules > .travis.yml: Add a KVM-only Aarch64 job > > Samuel Ortiz (1): > target/arm: Do not build TCG objects when TCG is off > > Thomas Huth (1): > target/arm: Make m_helper.c optional via CONFIG_ARM_V7M > > default-configs/devices/aarch64-softmmu.mak | 1 - > default-configs/devices/arm-softmmu.mak | 27 > include/exec/helper-proto.h | 2 + > include/sysemu/tcg.h| 2 + > target/arm/cpu.h| 12 > hw/arm/realview.c | 7 +- > target/arm/cpu_tcg.c| 4 +- > target/arm/helper.c | 7 -- > target/arm/m_helper-stub.c | 73 + > tests/qtest/cdrom-test.c| 6 +- > .travis.yml | 32 + > hw/arm/Kconfig | 38 +++ > target/arm/Kconfig | 17 + > target/arm/meson.build | 28 +--- > 14 files changed, 196 insertions(+), 60 deletions(-) > create mode 100644 target/arm/m_helper-stub.c > Looking at this series, just my 2 cents on how I would suggest to go forward: I could again split my series in two parts, with only the TCG Ops in the first part. Then this series could be merged, enabling --disable-tcg for ARM, then I could extend the second part of my series to include ARM as well. Wdyt? (Probably Richard?) Thanks, Claudio
Re: [PATCH v6 07/11] target/arm: Restrict ARMv7 M-profile cpus to TCG accel
On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote: > KVM requires the target cpu to be at least ARMv8 architecture > (support on ARMv7 has been dropped in commit 82bf7ae84ce: > "target/arm: Remove KVM support for 32-bit Arm hosts"). > > Beside, KVM only supports A-profile, thus won't be able to run > M-profile cpus. > > Only enable the following ARMv7 M-Profile CPUs when TCG is available: > > - Cortex-M0 > - Cortex-M3 > - Cortex-M4 > - Cortex-M33 > > The following machines are no more built when TCG is disabled: > > - emcraft-sf2 SmartFusion2 SOM kit from Emcraft (M2S010) > - highbank Calxeda Highbank (ECX-1000) > - lm3s6965evb Stellaris LM3S6965EVB (Cortex-M3) > - lm3s811evb Stellaris LM3S811EVB (Cortex-M3) > - midway Calxeda Midway (ECX-2000) > - mps2-an385 ARM MPS2 with AN385 FPGA image for Cortex-M3 > - mps2-an386 ARM MPS2 with AN386 FPGA image for Cortex-M4 > - mps2-an500 ARM MPS2 with AN500 FPGA image for Cortex-M7 > - mps2-an505 ARM MPS2 with AN505 FPGA image for Cortex-M33 > - mps2-an511 ARM MPS2 with AN511 DesignStart FPGA image for > Cortex-M3 > - mps2-an521 ARM MPS2 with AN521 FPGA image for dual Cortex-M33 > - musca-a ARM Musca-A board (dual Cortex-M33) > - musca-b1 ARM Musca-B1 board (dual Cortex-M33) > - netduino2Netduino 2 Machine (Cortex-M3) > - netduinoplus2Netduino Plus 2 Machine(Cortex-M4) > > We don't need to enforce CONFIG_ARM_V7M in default-configs anymore. > > Signed-off-by: Philippe Mathieu-Daudé > --- > default-configs/devices/arm-softmmu.mak | 11 --- > hw/arm/Kconfig | 7 +++ > target/arm/Kconfig | 1 + > 3 files changed, 8 insertions(+), 11 deletions(-) > > diff --git a/default-configs/devices/arm-softmmu.mak > b/default-configs/devices/arm-softmmu.mak > index 175530595ce..0fc80d7d6df 100644 > --- a/default-configs/devices/arm-softmmu.mak > +++ b/default-configs/devices/arm-softmmu.mak > @@ -1,28 +1,17 @@ > # Default configuration for arm-softmmu > > -# TODO: ARM_V7M is currently always required - make this more flexible! > -CONFIG_ARM_V7M=y > - > # CONFIG_PCI_DEVICES=n > # CONFIG_TEST_DEVICES=n > > CONFIG_ARM_VIRT=y > CONFIG_CUBIEBOARD=y > CONFIG_EXYNOS4=y > -CONFIG_HIGHBANK=y > -CONFIG_MUSCA=y > -CONFIG_STELLARIS=y > CONFIG_REALVIEW=y > CONFIG_VEXPRESS=y > CONFIG_ZYNQ=y > CONFIG_NPCM7XX=y > -CONFIG_NETDUINO2=y > -CONFIG_NETDUINOPLUS2=y > -CONFIG_MPS2=y > CONFIG_RASPI=y > CONFIG_SABRELITE=y > -CONFIG_EMCRAFT_SF2=y > -CONFIG_MICROBIT=y > CONFIG_FSL_IMX7=y > CONFIG_FSL_IMX6UL=y > CONFIG_ALLWINNER_H3=y > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig > index 4baf1f97694..62f8b0d24e7 100644 > --- a/hw/arm/Kconfig > +++ b/hw/arm/Kconfig > @@ -60,6 +60,7 @@ config EXYNOS4 > > config HIGHBANK > bool > +default y if TCG && ARM > select A9MPCORE > select A15MPCORE > select AHCI > @@ -95,6 +96,7 @@ config MAINSTONE > > config MUSCA > bool > +default y if TCG && ARM > select ARMSSE > select PL011 > select PL031 > @@ -115,10 +117,12 @@ config MUSICPAL > > config NETDUINO2 > bool > +default y if TCG && ARM > select STM32F205_SOC > > config NETDUINOPLUS2 > bool > +default y if TCG && ARM > select STM32F405_SOC > > config NSERIES > @@ -240,6 +244,7 @@ config SABRELITE > > config STELLARIS > bool > +default y if TCG && ARM > select ARM_V7M > select CMSDK_APB_WATCHDOG > select I2C > @@ -443,6 +448,7 @@ config ASPEED_SOC > > config MPS2 > bool > +default y if TCG && ARM > select ARMSSE > select LAN9118 > select MPS2_FPGAIO > @@ -496,6 +502,7 @@ config NRF51_SOC > > config EMCRAFT_SF2 > bool > +default y if TCG && ARM > select MSF2 > select SSI_M25P80 > > diff --git a/target/arm/Kconfig b/target/arm/Kconfig > index 4dc96c46520..07a2fad7a2b 100644 > --- a/target/arm/Kconfig > +++ b/target/arm/Kconfig > @@ -24,4 +24,5 @@ config ARM_V7R > > config ARM_V7M > bool > +depends on TCG && ARM > select PTIMER > Acked-by: Claudio Fontana
Re: [PATCH v6 06/11] target/arm: Restrict ARMv7 R-profile cpus to TCG accel
On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote: > KVM requires the target cpu to be at least ARMv8 architecture > (support on ARMv7 has been dropped in commit 82bf7ae84ce: > "target/arm: Remove KVM support for 32-bit Arm hosts"). > > Beside, KVM only supports A-profile, thus won't be able to run > R-profile cpus. > > Only enable the following ARMv7 R-Profile CPUs when TCG is available: > > - Cortex-R5 > - Cortex-R5F > > The following machine is no more built when TCG is disabled: > > - xlnx-zcu102 Xilinx ZynqMP ZCU102 board with 4xA53s and 2xR5Fs > > Signed-off-by: Philippe Mathieu-Daudé > --- > default-configs/devices/aarch64-softmmu.mak | 1 - > hw/arm/Kconfig | 2 ++ > target/arm/Kconfig | 4 > 3 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/default-configs/devices/aarch64-softmmu.mak > b/default-configs/devices/aarch64-softmmu.mak > index 958b1e08e40..a4202f56817 100644 > --- a/default-configs/devices/aarch64-softmmu.mak > +++ b/default-configs/devices/aarch64-softmmu.mak > @@ -3,6 +3,5 @@ > # We support all the 32 bit boards so need all their config > include arm-softmmu.mak > > -CONFIG_XLNX_ZYNQMP_ARM=y > CONFIG_XLNX_VERSAL=y > CONFIG_SBSA_REF=y > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig > index 6c4bce4d637..4baf1f97694 100644 > --- a/hw/arm/Kconfig > +++ b/hw/arm/Kconfig > @@ -360,8 +360,10 @@ config STM32F405_SOC > > config XLNX_ZYNQMP_ARM > bool > +default y if TCG && ARM > select AHCI > select ARM_GIC > +select ARM_V7R > select CADENCE > select DDC > select DPCD > diff --git a/target/arm/Kconfig b/target/arm/Kconfig > index fbb7bba9018..4dc96c46520 100644 > --- a/target/arm/Kconfig > +++ b/target/arm/Kconfig > @@ -18,6 +18,10 @@ config ARM_V6 > bool > depends on TCG && ARM > > +config ARM_V7R > +bool > +depends on TCG && ARM > + > config ARM_V7M > bool > select PTIMER > Acked-by: Claudio Fontana
Re: [PATCH v6 05/11] target/arm: Restrict ARMv6 cpus to TCG accel
On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote:
> KVM requires the target cpu to be at least ARMv8 architecture
> (support on ARMv7 has been dropped in commit 82bf7ae84ce:
> "target/arm: Remove KVM support for 32-bit Arm hosts").
>
> Only enable the following ARMv6 CPUs when TCG is available:
>
> - ARM1136
> - ARM1176
> - ARM11MPCore
> - Cortex-M0
>
> The following machines are no more built when TCG is disabled:
>
> - kzm ARM KZM Emulation Baseboard (ARM1136)
> - microbit BBC micro:bit (Cortex-M0)
> - n800 Nokia N800 tablet aka. RX-34 (OMAP2420)
> - n810 Nokia N810 tablet aka. RX-44 (OMAP2420)
> - realview-eb-mpcore ARM RealView Emulation Baseboard (ARM11MPCore)
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> default-configs/devices/arm-softmmu.mak | 2 --
> hw/arm/realview.c | 2 +-
> tests/qtest/cdrom-test.c| 2 +-
> hw/arm/Kconfig | 6 ++
> target/arm/Kconfig | 4
> 5 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/default-configs/devices/arm-softmmu.mak
> b/default-configs/devices/arm-softmmu.mak
> index 0aad35da0c4..175530595ce 100644
> --- a/default-configs/devices/arm-softmmu.mak
> +++ b/default-configs/devices/arm-softmmu.mak
> @@ -10,9 +10,7 @@ CONFIG_ARM_VIRT=y
> CONFIG_CUBIEBOARD=y
> CONFIG_EXYNOS4=y
> CONFIG_HIGHBANK=y
> -CONFIG_FSL_IMX31=y
> CONFIG_MUSCA=y
> -CONFIG_NSERIES=y
> CONFIG_STELLARIS=y
> CONFIG_REALVIEW=y
> CONFIG_VEXPRESS=y
> diff --git a/hw/arm/realview.c b/hw/arm/realview.c
> index 2dcf0a4c23e..0606d22da14 100644
> --- a/hw/arm/realview.c
> +++ b/hw/arm/realview.c
> @@ -463,8 +463,8 @@ static void realview_machine_init(void)
> {
> if (tcg_builtin()) {
> type_register_static(_eb_type);
> +type_register_static(_eb_mpcore_type);
> }
> -type_register_static(_eb_mpcore_type);
> type_register_static(_pb_a8_type);
> type_register_static(_pbx_a9_type);
> }
> diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
> index 1f1bc26fa7a..cb0409c5a11 100644
> --- a/tests/qtest/cdrom-test.c
> +++ b/tests/qtest/cdrom-test.c
> @@ -224,8 +224,8 @@ int main(int argc, char **argv)
> const char *armmachines[] = {
> #ifdef CONFIG_TCG
> "realview-eb",
> -#endif /* CONFIG_TCG */
> "realview-eb-mpcore",
> +#endif /* CONFIG_TCG */
> "realview-pb-a8",
> "realview-pbx-a9", "versatileab", "versatilepb", "vexpress-a15",
> "vexpress-a9", "virt", NULL
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 560442bfc5c..6c4bce4d637 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -123,6 +123,8 @@ config NETDUINOPLUS2
>
> config NSERIES
> bool
> +default y if TCG && ARM
> +select ARM_V6
> select OMAP
> select TMP105 # tempature sensor
> select BLIZZARD # LCD/TV controller
> @@ -401,6 +403,8 @@ config FSL_IMX25
>
> config FSL_IMX31
> bool
> +default y if TCG && ARM
> +select ARM_V6
> select SERIAL
> select IMX
> select IMX_I2C
> @@ -478,11 +482,13 @@ config FSL_IMX6UL
>
> config MICROBIT
> bool
> +default y if TCG && ARM
> select NRF51_SOC
>
> config NRF51_SOC
> bool
> select I2C
> +select ARM_V6
> select ARM_V7M
> select UNIMP
>
> diff --git a/target/arm/Kconfig b/target/arm/Kconfig
> index 9b3635617dc..fbb7bba9018 100644
> --- a/target/arm/Kconfig
> +++ b/target/arm/Kconfig
> @@ -14,6 +14,10 @@ config ARM_V5
> bool
> depends on TCG && ARM
>
> +config ARM_V6
> +bool
> +depends on TCG && ARM
> +
> config ARM_V7M
> bool
> select PTIMER
>
Added Cc: Eduardo,
Looks good to me in general,
Acked-by: Claudio Fontana
I am just wondering about that if (tcg_builtin()) / (or _available() as I
suggest elsewhere),
should we instead use the build system already at this stage, so no such check
is necessary?
It could be a successive change, but then tcg_builtin() would be introduced,
only to become useless after the proper refactoring is done,
and the build system is used to select the right modules to compile, which
would do the registration.
Ciao,
Claudio
Re: [PATCH v6 04/11] target/arm: Restrict ARMv5 cpus to TCG accel
= {
> -"realview-eb", "realview-eb-mpcore", "realview-pb-a8",
> +#ifdef CONFIG_TCG
> +"realview-eb",
> +#endif /* CONFIG_TCG */
> +"realview-eb-mpcore",
> +"realview-pb-a8",
> "realview-pbx-a9", "versatileab", "versatilepb", "vexpress-a15",
> "vexpress-a9", "virt", NULL
> };
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index f2957b33bee..560442bfc5c 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -42,6 +42,8 @@ config CUBIEBOARD
>
> config DIGIC
> bool
> +default y if TCG && ARM
> +select ARM_V5
> select PTIMER
> select PFLASH_CFI02
>
> @@ -72,6 +74,8 @@ config HIGHBANK
>
> config INTEGRATOR
> bool
> +default y if TCG && ARM
> +select ARM_V5
> select ARM_TIMER
> select INTEGRATOR_DEBUG
> select PL011 # UART
> @@ -84,6 +88,7 @@ config INTEGRATOR
>
> config MAINSTONE
> bool
> +default y if TCG && ARM
> select PXA2XX
> select PFLASH_CFI01
> select SMC91C111
> @@ -98,6 +103,8 @@ config MUSCA
>
> config MUSICPAL
> bool
> +default y if TCG && ARM
> +select ARM_V5
> select OR_IRQ
> select BITBANG_I2C
> select MARVELL_88W8618
> @@ -138,6 +145,7 @@ config OMAP
>
> config PXA2XX
> bool
> +select ARM_V5
> select FRAMEBUFFER
> select I2C
> select SERIAL
> @@ -147,12 +155,14 @@ config PXA2XX
>
> config GUMSTIX
> bool
> +default y if TCG && ARM
> select PFLASH_CFI01
> select SMC91C111
> select PXA2XX
>
> config TOSA
> bool
> +default y if TCG && ARM
> select ZAURUS # scoop
> select MICRODRIVE
> select PXA2XX
> @@ -160,6 +170,7 @@ config TOSA
>
> config SPITZ
> bool
> +default y if TCG && ARM
> select ADS7846 # touch-screen controller
> select MAX111X # A/D converter
> select WM8750 # audio codec
> @@ -172,6 +183,7 @@ config SPITZ
>
> config Z2
> bool
> +default y if TCG && ARM
> select PFLASH_CFI01
> select WM8750
> select PL011 # UART
> @@ -245,6 +257,7 @@ config STRONGARM
>
> config COLLIE
> bool
> +default y if TCG && ARM
> select PFLASH_CFI01
> select ZAURUS # scoop
> select STRONGARM
> @@ -257,6 +270,8 @@ config SX1
>
> config VERSATILE
> bool
> +default y if TCG && ARM
> +select ARM_V5
> select ARM_TIMER # sp804
> select PFLASH_CFI01
> select LSI_SCSI_PCI
> @@ -376,6 +391,8 @@ config NPCM7XX
>
> config FSL_IMX25
> bool
> +default y if TCG && ARM
> +select ARM_V5
> select IMX
> select IMX_FEC
> select IMX_I2C
> @@ -402,6 +419,8 @@ config FSL_IMX6
>
> config ASPEED_SOC
> bool
> +default y if TCG && ARM
> +select ARM_V5
> select DS1338
> select FTGMAC100
> select I2C
> diff --git a/target/arm/Kconfig b/target/arm/Kconfig
> index 811e1e81652..9b3635617dc 100644
> --- a/target/arm/Kconfig
> +++ b/target/arm/Kconfig
> @@ -10,6 +10,10 @@ config ARM_V4
> bool
> depends on TCG && ARM
>
> +config ARM_V5
> +bool
> +depends on TCG && ARM
> +
> config ARM_V7M
> bool
> select PTIMER
>
Looks good to me
Acked-by: Claudio Fontana
Re: [PATCH v6 03/11] target/arm: Restrict ARMv4 cpus to TCG accel
On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote: > KVM requires the target cpu to be at least ARMv8 architecture > (support on ARMv7 has been dropped in commit 82bf7ae84ce: > "target/arm: Remove KVM support for 32-bit Arm hosts"). > > Only enable the following ARMv4 CPUs when TCG is available: > > - StrongARM (SA1100/1110) > - OMAP1510 (TI925T) > > The following machines are no more built when TCG is disabled: > > - cheetah Palm Tungsten|E aka. Cheetah PDA (OMAP310) > - sx1 Siemens SX1 (OMAP310) V2 > - sx1-v1 Siemens SX1 (OMAP310) V1 > > Signed-off-by: Philippe Mathieu-Daudé > --- > default-configs/devices/arm-softmmu.mak | 2 -- > hw/arm/Kconfig | 4 > target/arm/Kconfig | 4 > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/default-configs/devices/arm-softmmu.mak > b/default-configs/devices/arm-softmmu.mak > index 0824e9be795..6ae964c14fd 100644 > --- a/default-configs/devices/arm-softmmu.mak > +++ b/default-configs/devices/arm-softmmu.mak > @@ -14,8 +14,6 @@ CONFIG_INTEGRATOR=y > CONFIG_FSL_IMX31=y > CONFIG_MUSICPAL=y > CONFIG_MUSCA=y > -CONFIG_CHEETAH=y > -CONFIG_SX1=y > CONFIG_NSERIES=y > CONFIG_STELLARIS=y > CONFIG_REALVIEW=y > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig > index f3ecb73a3d8..f2957b33bee 100644 > --- a/hw/arm/Kconfig > +++ b/hw/arm/Kconfig > @@ -31,6 +31,8 @@ config ARM_VIRT > > config CHEETAH > bool > +default y if TCG && ARM > +select ARM_V4 > select OMAP > select TSC210X > > @@ -249,6 +251,8 @@ config COLLIE > > config SX1 > bool > +default y if TCG && ARM > +select ARM_V4 > select OMAP > > config VERSATILE > diff --git a/target/arm/Kconfig b/target/arm/Kconfig > index ae89d05c7e5..811e1e81652 100644 > --- a/target/arm/Kconfig > +++ b/target/arm/Kconfig > @@ -6,6 +6,10 @@ config AARCH64 > bool > select ARM > > +config ARM_V4 > +bool > +depends on TCG && ARM > + > config ARM_V7M > bool > select PTIMER > Looks good to me Acked-by: Claudio Fontana
Re: [PATCH v6 02/11] exec: Restrict TCG specific headers
On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote: > Fixes when building with --disable-tcg on ARM: > > In file included from target/arm/helper.c:16: > include/exec/helper-proto.h:42:10: fatal error: tcg-runtime.h: No such file > or directory > 42 | #include "tcg-runtime.h" > | ^~~ > > Signed-off-by: Philippe Mathieu-Daudé > --- > include/exec/helper-proto.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/exec/helper-proto.h b/include/exec/helper-proto.h > index 659f9298e8f..740bff3bb4d 100644 > --- a/include/exec/helper-proto.h > +++ b/include/exec/helper-proto.h > @@ -39,8 +39,10 @@ dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), > dh_ctype(t3), \ > > #include "helper.h" > #include "trace/generated-helpers.h" > +#ifdef CONFIG_TCG > #include "tcg-runtime.h" > #include "plugin-helpers.h" > +#endif /* CONFIG_TCG */ > > #undef IN_HELPER_PROTO > > Ok, this would go away when applying the refactoring to ARM though right? Ie the file should not need including at all later on right? Anyway: Reviewed-by: Claudio Fontana
Re: [PATCH v6 01/11] sysemu/tcg: Introduce tcg_builtin() helper
On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote: > Modules are registered early with type_register_static(). > > We would like to call tcg_enabled() when registering QOM types, Hi Philippe, could this not be controlled by meson at this stage? On X86, I register the tcg-specific types in tcg/* in modules that are only built for TCG. Maybe tcg_builtin() is useful anyway, thinking long term at loadable modules, but there we are interested in whether tcg code is available or not, regardless of whether it's builtin, or needs to be loaded via a .so plugin.. maybe tcg_available()? Ciao, Claudio > but tcg_enabled() returns tcg_allowed which is a runtime property > initialized later (See commit 2f181fbd5a9 which introduced the > MachineInitPhase in "hw/qdev-core.h" representing the different > phases of machine initialization and commit 0427b6257e2 which > document the initialization order). > > As we are only interested if the TCG accelerator is builtin, > regardless of being enabled, introduce the tcg_builtin() helper. > > Signed-off-by: Philippe Mathieu-Daudé > --- > Cc: Markus Armbruster > --- > include/sysemu/tcg.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/sysemu/tcg.h b/include/sysemu/tcg.h > index 00349fb18a7..6ac5c2ca89d 100644 > --- a/include/sysemu/tcg.h > +++ b/include/sysemu/tcg.h > @@ -13,8 +13,10 @@ void tcg_exec_init(unsigned long tb_size, int splitwx); > #ifdef CONFIG_TCG > extern bool tcg_allowed; > #define tcg_enabled() (tcg_allowed) > +#define tcg_builtin() 1 > #else > #define tcg_enabled() 0 > +#define tcg_builtin() 0 > #endif > > #endif >
Re: [PATCH] iotests: Allow running from different directory
On 9/3/20 7:21 PM, Kevin Wolf wrote: > Am 03.09.2020 um 14:54 hat Max Reitz geschrieben: >> On 02.09.20 13:03, Kevin Wolf wrote: >>> It is convenient to be able to edit the tests and run them without >>> changing the current working directory back and forth. Instead of >>> assuming that $PWD is the qemu-iotests build directory, derive the build >>> directory from the executed script. >>> >>> This allows 'check' to find the required files even when called from >>> another directory. The scratch directory will still be in the current >>> working directory. >>> >>> Signed-off-by: Kevin Wolf >>> --- >>> tests/qemu-iotests/check | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check >>> index 3ab859ac1a..22ada6a549 100755 >>> --- a/tests/qemu-iotests/check >>> +++ b/tests/qemu-iotests/check >>> @@ -44,7 +44,7 @@ then >>> _init_error "failed to obtain source tree name from check symlink" >>> fi >>> source_iotests=$(cd "$source_iotests"; pwd) || _init_error "failed to >>> enter source tree" >>> -build_iotests=$PWD >>> +build_iotests=$(dirname "$0") >> >> This breaks running check from the build tree. >> (i.e. cd $build/tests/qemu-iotests; ./check) >> >> The problem is that to run the test, we do cd to the source directory >> ($source_iotests), and so $build_iotests then becomes invalid if it’s >> just a relative path. In my case, this leads to the following error: >> >> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 >> +./common.rc: line 139: $QEMU/tests/qemu-iotests/../../qemu-img: No such >> file or directory > > Ah, my symlinks in the source tree made it work for me. > >> I think this could be resolved by wrapping the $(dirname) in >> $(realpath), i.e. >> >> build_iotests=$(realpath "$(dirname "$0")") > > Sounds good, I'll update it in my tree. > > Kevin > Hello Kevin, the committed patch in master is now (not sure where it changed): diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index 3ab859ac1a..e14a1f354d 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -44,7 +44,7 @@ then _init_error "failed to obtain source tree name from check symlink" fi source_iotests=$(cd "$source_iotests"; pwd) || _init_error "failed to enter source tree" -build_iotests=$PWD +build_iotests=$(readlink -f $(dirname "$0")) else # called from the source tree source_iotests=$PWD - This seems to break the MacOS build (Cirrus-ci) though: readlink: illegal option -- f usage: readlink [-n] [file ...] ./check: line 60: /common.env: No such file or directory check: failed to source common.env (make sure the qemu-iotests are run from tests/qemu-iotests in the build tree) gmake: *** [/private/var/folders/3y/l0z1x3693dl_8n0qybp4dqwhgn/T/cirrus-ci-build/tests/Makefile.include:144: check-block] Error 1 Exit status: 2 Ciao, Claudio
Re: [PATCH] iotests: Allow running from different directory
On 9/2/20 1:03 PM, Kevin Wolf wrote: > It is convenient to be able to edit the tests and run them without > changing the current working directory back and forth. Instead of > assuming that $PWD is the qemu-iotests build directory, derive the build > directory from the executed script. > > This allows 'check' to find the required files even when called from > another directory. The scratch directory will still be in the current > working directory. > > Signed-off-by: Kevin Wolf > --- > tests/qemu-iotests/check | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check > index 3ab859ac1a..22ada6a549 100755 > --- a/tests/qemu-iotests/check > +++ b/tests/qemu-iotests/check > @@ -44,7 +44,7 @@ then > _init_error "failed to obtain source tree name from check symlink" > fi > source_iotests=$(cd "$source_iotests"; pwd) || _init_error "failed to > enter source tree" > -build_iotests=$PWD > +build_iotests=$(dirname "$0") > else > # called from the source tree > source_iotests=$PWD > Like it, thanks! Reviewed-by: Claudio Fontana
Re: [PATCH for-5.1 1/2] block: Fix bdrv_aligned_p*v() for qiov_offset != 0
Tested-by: Claudio Fontana
On 7/28/20 2:08 PM, Max Reitz wrote:
> Since these functions take a @qiov_offset, they must always take it into
> account when working with @qiov. There are a couple of places where
> they do not, but they should.
>
> Fixes: 65cd4424b9df03bb5195351c33e04cbbecc0705c
> Fixes: 28c4da28695bdbe04b336b2c9c463876cc3aaa6d
> Reported-by: Claudio Fontana
> Reported-by: Bruce Rogers
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Max Reitz
> ---
> block/io.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index b6564e34c5..ad3a51ed53 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1524,12 +1524,13 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild
> *child,
> assert(num);
>
> ret = bdrv_driver_preadv(bs, offset + bytes - bytes_remaining,
> - num, qiov, bytes - bytes_remaining, 0);
> + num, qiov,
> + qiov_offset + bytes - bytes_remaining,
> 0);
> max_bytes -= num;
> } else {
> num = bytes_remaining;
> -ret = qemu_iovec_memset(qiov, bytes - bytes_remaining, 0,
> -bytes_remaining);
> +ret = qemu_iovec_memset(qiov, qiov_offset + bytes -
> bytes_remaining,
> +0, bytes_remaining);
> }
> if (ret < 0) {
> goto out;
> @@ -2032,7 +2033,8 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild
> *child,
> }
>
> ret = bdrv_driver_pwritev(bs, offset + bytes - bytes_remaining,
> - num, qiov, bytes - bytes_remaining,
> + num, qiov,
> + qiov_offset + bytes - bytes_remaining,
>local_flags);
> if (ret < 0) {
> break;
>
Re: [PATCH for-5.1 2/2] iotests/028: Add test for cross-base-EOF reads
Tested-by: Claudio Fontana
On 7/28/20 2:08 PM, Max Reitz wrote:
> Signed-off-by: Max Reitz
> ---
> tests/qemu-iotests/028 | 19 +++
> tests/qemu-iotests/028.out | 11 +++
> 2 files changed, 30 insertions(+)
>
> diff --git a/tests/qemu-iotests/028 b/tests/qemu-iotests/028
> index 5d043cef92..6dd3ae09a3 100755
> --- a/tests/qemu-iotests/028
> +++ b/tests/qemu-iotests/028
> @@ -142,6 +142,25 @@ TEST_IMG="${TEST_IMG}.copy" io_zero readv $(( offset +
> 32 * 1024 )) 512 1024 32
>
> _check_test_img
>
> +echo
> +echo '=== Reading across backing EOF in one operation ==='
> +echo
> +
> +# Use a cluster boundary as the base end here
> +base_size=$((3 * 1024 * 1024 * 1024))
> +
> +TEST_IMG="$TEST_IMG.base" _make_test_img $base_size
> +_make_test_img -b "$TEST_IMG.base" -F $IMGFMT $image_size
> +
> +# Write 16 times 42 at the end of the base image
> +$QEMU_IO -c "write -P 42 $((base_size - 16)) 16" "$TEST_IMG.base" \
> +| _filter_qemu_io
> +
> +# Read 32 bytes across the base EOF from the top;
> +# should be 16 times 0x2a, then 16 times 0x00
> +$QEMU_IO -c "read -v $((base_size - 16)) 32" "$TEST_IMG" \
> +| _filter_qemu_io
> +
> # success, all done
> echo "*** done"
> rm -f $seq.full
> diff --git a/tests/qemu-iotests/028.out b/tests/qemu-iotests/028.out
> index 12f82c6a6c..5a68de5c46 100644
> --- a/tests/qemu-iotests/028.out
> +++ b/tests/qemu-iotests/028.out
> @@ -730,4 +730,15 @@ read 512/512 bytes at offset 3221257728
> read 512/512 bytes at offset 3221258752
> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> No errors were found on the image.
> +
> +=== Reading across backing EOF in one operation ===
> +
> +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=3221225472
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4294968832
> backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
> +wrote 16/16 bytes at offset 3221225456
> +16 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +bff0: 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a
> +c000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +read 32/32 bytes at offset 3221225456
> +32 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> *** done
>
