Re: [PATCH 1/2] hw/dma: sifive_pdma: Improve code readability for "!!foo & bar"
Bin Meng writes:
> Hi Markus,
>
> On Mon, Sep 27, 2021 at 2:51 PM Markus Armbruster wrote:
>>
>> Bin Meng writes:
>>
>> > GCC seems to be strict about processing pattern like "!!for & bar".
>> > When 'bar' is not 0 or 1, it complains with -Werror=parentheses:
>> >
>> > suggest parentheses around operand of ‘!’ or change ‘&’ to ‘&&’ or ‘!’
>> > to ‘~’ [-Werror=parentheses]
>> >
>> > Add a () around "foo && bar", which also improves code readability.
>> >
>> > Signed-off-by: Bin Meng
>> > ---
>> >
>> > hw/dma/sifive_pdma.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
>> > index b4fd40573a..b8ec7621f3 100644
>> > --- a/hw/dma/sifive_pdma.c
>> > +++ b/hw/dma/sifive_pdma.c
>> > @@ -243,7 +243,7 @@ static void sifive_pdma_write(void *opaque, hwaddr
>> > offset,
>> > offset &= 0xfff;
>> > switch (offset) {
>> > case DMA_CONTROL:
>> > -claimed = !!s->chan[ch].control & CONTROL_CLAIM;
>> > +claimed = !!(s->chan[ch].control & CONTROL_CLAIM);
>> >
>> > if (!claimed && (value & CONTROL_CLAIM)) {
>> > /* reset Next* registers */
>>
>> Old code
>>
>> first double-negate, mapping zero to zero, non-zero to one
>> then mask, which does nothing, because CONTROL_CLAIM is 1
>>
>> New code:
>>
>> first mask, yielding 0 or 1
>> then double-negate, which does nothing
>>
>> Looks like a bug fix to me. If I'm right, the commit message is wrong,
>> and the double negate is redundant.
>>
>
> Thanks for the review. The double negate is not needed with
> CONTROL_CLAIM which is 1, but is needed if the bit is in another
> position.
It's not needed even then: conversion from integer type to bool takes
care of it. It's not wrong, though.
However, the commit message does look wrong to me.
[PATCH v2 1/2] hw/dma: sifive_pdma: Fix Control.claim bit detection
At present the codes detect whether the DMA channel is claimed by:
claimed = !!s->chan[ch].control & CONTROL_CLAIM;
As ! has higher precedence over & (bitwise and), this is essentially
claimed = (!!s->chan[ch].control) & CONTROL_CLAIM;
which is wrong, as any non-zero bit set in the control register will
produce a result of a claimed channel.
Fixes: de7c7988d25d ("hw/dma: sifive_pdma: reset Next* registers when
Control.claim is set")
Signed-off-by: Bin Meng
---
Changes in v2:
- reword the commit message
hw/dma/sifive_pdma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
index b4fd40573a..b8ec7621f3 100644
--- a/hw/dma/sifive_pdma.c
+++ b/hw/dma/sifive_pdma.c
@@ -243,7 +243,7 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
offset &= 0xfff;
switch (offset) {
case DMA_CONTROL:
-claimed = !!s->chan[ch].control & CONTROL_CLAIM;
+claimed = !!(s->chan[ch].control & CONTROL_CLAIM);
if (!claimed && (value & CONTROL_CLAIM)) {
/* reset Next* registers */
--
2.25.1
[PATCH v2 2/2] hw/dma: sifive_pdma: Don't run DMA when channel is disclaimed
If Control.run bit is set while not preserving the Control.claim
bit, the DMA transfer shall not be started.
The following result is PDMA tested in U-Boot on Unleashed board:
=> mw.l 0x300 0x0 <= Disclaim channel 0
=> mw.l 0x300 0x1 <= Claim channel 0
=> mw.l 0x304 0x5500 <= wsize = rsize = 5 (2^5 = 32 bytes)
=> mw.q 0x308 0x2 <= NextBytes = 2
=> mw.q 0x310 0x8400 <= NextDestination = 0x8400
=> mw.q 0x318 0x84001000 <= NextSource = 0x84001000
=> mw.l 0x8400 0x87654321 <= Fill test data to dst
=> mw.l 0x84001000 0x12345678 <= Fill test data to src
=> md.l 0x8400 1; md.l 0x84001000 1<= Dump src/dst memory contents
8400: 87654321 !Ce.
84001000: 12345678 xV4.
=> md.l 0x300 8<= Dump PDMA status
0300: 0001 5500 0002 ...U
0310: 8400 84001000
=> mw.l 0x300 0x2 <= Set channel 0 run bit only
=> md.l 0x300 8<= Dump PDMA status
0300: 5500 0002 ...U
0310: 8400 84001000
=> md.l 0x8400 1; md.l 0x84001000 1<= Dump src/dst memory contents
8400: 87654321 !Ce.
84001000: 12345678 xV4.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/dma/sifive_pdma.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
index b8ec7621f3..85fe34f5f3 100644
--- a/hw/dma/sifive_pdma.c
+++ b/hw/dma/sifive_pdma.c
@@ -232,7 +232,7 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
{
SiFivePDMAState *s = opaque;
int ch = SIFIVE_PDMA_CHAN_NO(offset);
-bool claimed;
+bool claimed, run;
if (ch >= SIFIVE_PDMA_CHANS) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid channel no %d\n",
@@ -244,6 +244,7 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
switch (offset) {
case DMA_CONTROL:
claimed = !!(s->chan[ch].control & CONTROL_CLAIM);
+run = !!(s->chan[ch].control & CONTROL_RUN);
if (!claimed && (value & CONTROL_CLAIM)) {
/* reset Next* registers */
@@ -254,13 +255,19 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
s->chan[ch].next_src = 0;
}
+/* claim bit can only be cleared when run is low */
+if (run && !(value & CONTROL_CLAIM)) {
+value |= CONTROL_CLAIM;
+}
+
s->chan[ch].control = value;
/*
* If channel was not claimed before run bit is set,
+ * or if the channel is disclaimed when run was low,
* DMA won't run.
*/
-if (!claimed) {
+if (!claimed || (!run && !(value & CONTROL_CLAIM))) {
s->chan[ch].control &= ~CONTROL_RUN;
return;
}
--
2.25.1
Re: gitlab-ci: amd64-opensuse-leap-container job failing
On Sun, Sep 26, 2021 at 07:23:56PM +0200, Philippe Mathieu-Daudé wrote: > Hi, > > FYI the OpenSUSE job is failing since few days, i.e.: > https://gitlab.com/qemu-project/qemu/-/jobs/1622345026 > > Retrieving repository 'Main Repository' metadata > [..error] > Repository 'Main Repository' is invalid. > > [repo-oss|http://download.opensuse.org/distribution/leap/15.2/repo/oss/] > Valid metadata not found at specified URL > History: >- Download (curl) error for > 'http://download.opensuse.org/distribution/leap/15.2/repo/oss/repodata/repomd.xml': > Error code: Curl error 56 > Error message: Recv failure: Connection reset by peer >- Can't provide /repodata/repomd.xml > Please check if the URIs defined for this repository are pointing to a > valid repository. > Warning: Skipping repository 'Main Repository' because of the above error. > > I tried to run 'zypper ref' with: It isn't confined to only SuSE. In libvirt we've had similar problems with several other jobs, though are suse jobs are the worst affected. GitLab have finally acknowledged it is an general infra issue affecting many things: https://status.gitlab.com/ https://gitlab.com/gitlab-com/gl-infra/production/-/issues/5590 > -- >8 -- > --- a/tests/docker/dockerfiles/opensuse-leap.docker > +++ b/tests/docker/dockerfiles/opensuse-leap.docker > @@ -109,5 +109,7 @@ ENV PACKAGES \ > zlib-devel > ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3.6 > > -RUN zypper update -y && zypper --non-interactive install -y $PACKAGES > +RUN zypper refresh && \ > +zypper update -y && \ > +zypper --non-interactive install -y $PACKAGES > RUN rpm -q $PACKAGES | sort > /packages.txt > --- > > but no luck: https://gitlab.com/philmd/qemu/-/jobs/1623554962 > > Should we temporarily disable to job and its dependencies? Given it is believed to be a gitlab infra issue, rather than a problem of ours, or something we're using, I think best to wait a little longer to see if they get fix the infra. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PULL 3/5] hmp: Unbreak "change vnc"
From: Markus Armbruster
HMP command "change vnc" can take the password as argument, or prompt
for it:
(qemu) change vnc password 123
(qemu) change vnc password
Password: ***
(qemu)
This regressed in commit cfb5387a1d "hmp: remove "change vnc TARGET"
command", v6.0.0.
(qemu) change vnc passwd 123
Password: ***
(qemu) change vnc passwd
(qemu)
The latter passes NULL to qmp_change_vnc_password(), which is a no-no.
Looks like it puts the display into "password required, but none set"
state.
The logic error is easy to miss in review, but testing should've
caught it.
Fix the obvious way.
Fixes: cfb5387a1de2acda23fb5c97d2378b9e7ddf8025
Cc: qemu-sta...@nongnu.org
Signed-off-by: Markus Armbruster
Reviewed-by: Daniel P. Berrangé
Reviewed-by: Gerd Hoffmann
Message-Id: <20210909081219.308065-2-arm...@redhat.com>
Signed-off-by: Laurent Vivier
---
monitor/hmp-cmds.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index e00255f7ee70..a7e197a90bf7 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1496,7 +1496,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
}
if (strcmp(target, "passwd") == 0 ||
strcmp(target, "password") == 0) {
-if (arg) {
+if (!arg) {
MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
monitor_read_password(hmp_mon, hmp_change_read_arg, NULL);
return;
--
2.31.1
Re: [PATCH v2] target/i386: Use assert() to sanity-check b1 in SSE decode
Ping^2 !
thanks
-- PMM
On Mon, 13 Sept 2021 at 13:34, Peter Maydell wrote:
>
> Ping? (this has been reviewed)
>
> thanks
> -- PMM
>
> On Wed, 1 Sept 2021 at 15:10, Peter Maydell wrote:
> >
> > In the SSE decode function gen_sse(), we combine a byte
> > 'b' and a value 'b1' which can be [0..3], and switch on them:
> >b |= (b1 << 8);
> >switch (b) {
> >...
> >default:
> >unknown_op:
> >gen_unknown_opcode(env, s);
> >return;
> >}
> >
> > In three cases inside this switch, we were then also checking for
> > "if (b1 >= 2) { goto unknown_op; }".
> > However, this can never happen, because the 'case' values in each place
> > are 0x0nn or 0x1nn and the switch will have directed the b1 == (2, 3)
> > cases to the default already.
> >
> > This check was added in commit c045af25a52e9 in 2010; the added code
> > was unnecessary then as well, and was apparently intended only to
> > ensure that we never accidentally ended up indexing off the end
> > of an sse_op_table with only 2 entries as a result of future bugs
> > in the decode logic.
> >
> > Change the checks to assert() instead, and make sure they're always
> > immediately before the array access they are protecting.
> >
> > Fixes: Coverity CID 1460207
> > Signed-off-by: Peter Maydell
> > ---
> > v1->v2: use assert() rather than just deleting the if()s
> >
> > target/i386/tcg/translate.c | 12 +++-
> > 1 file changed, 3 insertions(+), 9 deletions(-)
> >
> > diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> > index aacb605eee4..a4fee5e445d 100644
> > --- a/target/i386/tcg/translate.c
> > +++ b/target/i386/tcg/translate.c
> > @@ -3521,9 +3521,6 @@ static void gen_sse(CPUX86State *env, DisasContext
> > *s, int b,
> > case 0x171: /* shift xmm, im */
> > case 0x172:
> > case 0x173:
> > -if (b1 >= 2) {
> > -goto unknown_op;
> > -}
> > val = x86_ldub_code(env, s);
> > if (is_xmm) {
> > tcg_gen_movi_tl(s->T0, val);
> > @@ -3542,6 +3539,7 @@ static void gen_sse(CPUX86State *env, DisasContext
> > *s, int b,
> > offsetof(CPUX86State, mmx_t0.MMX_L(1)));
> > op1_offset = offsetof(CPUX86State,mmx_t0);
> > }
> > +assert(b1 < 2);
> > sse_fn_epp = sse_op_table2[((b - 1) & 3) * 8 +
> > (((modrm >> 3)) & 7)][b1];
> > if (!sse_fn_epp) {
> > @@ -3772,10 +3770,8 @@ static void gen_sse(CPUX86State *env, DisasContext
> > *s, int b,
> > rm = modrm & 7;
> > reg = ((modrm >> 3) & 7) | REX_R(s);
> > mod = (modrm >> 6) & 3;
> > -if (b1 >= 2) {
> > -goto unknown_op;
> > -}
> >
> > +assert(b1 < 2);
> > sse_fn_epp = sse_op_table6[b].op[b1];
> > if (!sse_fn_epp) {
> > goto unknown_op;
> > @@ -4202,10 +4198,8 @@ static void gen_sse(CPUX86State *env, DisasContext
> > *s, int b,
> > rm = modrm & 7;
> > reg = ((modrm >> 3) & 7) | REX_R(s);
> > mod = (modrm >> 6) & 3;
> > -if (b1 >= 2) {
> > -goto unknown_op;
> > -}
> >
> > +assert(b1 < 2);
> > sse_fn_eppi = sse_op_table7[b].op[b1];
> > if (!sse_fn_eppi) {
> > goto unknown_op;
> > --
> > 2.20.1
Re: [PATCH 06/11] qdev: Add Error parameter to qdev_set_id()
Hi Kevin,
I proposed a very similar patch in our rfc series because we needed some
of the cleaning you do here.
https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg05679.html
I've added a bit of doc for the function, feel free to take it if you want.
On 9/24/21 16:09, Vladimir Sementsov-Ogievskiy wrote:
24.09.2021 12:04, Kevin Wolf wrote:
object_property_add_child() fails (with _abort) if an object with
the same name already exists. As long as QemuOpts is in use for -device
and device_add, it catches duplicate IDs before qdev_set_id() is even
called. However, for enabling non-QemuOpts code paths, we need to make
sure that the condition doesn't cause a crash, but fails gracefully.
Signed-off-by: Kevin Wolf
---
include/monitor/qdev.h | 2 +-
hw/xen/xen-legacy-backend.c | 3 ++-
softmmu/qdev-monitor.c | 16 ++--
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/include/monitor/qdev.h b/include/monitor/qdev.h
index 389287eb44..7961308c75 100644
--- a/include/monitor/qdev.h
+++ b/include/monitor/qdev.h
@@ -9,6 +9,6 @@ void qmp_device_add(QDict *qdict, QObject **ret_data,
Error **errp);
int qdev_device_help(QemuOpts *opts);
DeviceState *qdev_device_add(QemuOpts *opts, Error **errp);
-void qdev_set_id(DeviceState *dev, char *id);
+void qdev_set_id(DeviceState *dev, char *id, Error **errp);
#endif
diff --git a/hw/xen/xen-legacy-backend.c b/hw/xen/xen-legacy-backend.c
index dd8ae1452d..17aca85ddc 100644
--- a/hw/xen/xen-legacy-backend.c
+++ b/hw/xen/xen-legacy-backend.c
@@ -276,7 +276,8 @@ static struct XenLegacyDevice
*xen_be_get_xendev(const char *type, int dom,
xendev = g_malloc0(ops->size);
object_initialize(>qdev, ops->size, TYPE_XENBACKEND);
OBJECT(xendev)->free = g_free;
- qdev_set_id(DEVICE(xendev), g_strdup_printf("xen-%s-%d", type,
dev));
+ qdev_set_id(DEVICE(xendev), g_strdup_printf("xen-%s-%d", type, dev),
+ _abort);
qdev_realize(DEVICE(xendev), xen_sysbus, _fatal);
object_unref(OBJECT(xendev));
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 1207e57a46..c2af906df0 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -593,26 +593,27 @@ static BusState *qbus_find(const char *path,
Error **errp)
}
/* Takes ownership of @id, will be freed when deleting the device */
-void qdev_set_id(DeviceState *dev, char *id)
+void qdev_set_id(DeviceState *dev, char *id, Error **errp)
According to recommendations in error.h, worth adding also return value
(for example true=success false=failure), so [..]
{
if (id) {
dev->id = id;
}
Unrelated but.. What's the strange logic?
Is it intended that with passed id=NULL we don't update dev->id variable
but try to do following logic with old dev->id?
dev->id is expected to be NULL. The object is created just before
calling this function so it is always the case. We could probably assert
this.
if (dev->id) {
- object_property_add_child(qdev_get_peripheral(), dev->id,
- OBJECT(dev));
+ object_property_try_add_child(qdev_get_peripheral(), dev->id,
+ OBJECT(dev), errp);
} else {
static int anon_count;
gchar *name = g_strdup_printf("device[%d]", anon_count++);
- object_property_add_child(qdev_get_peripheral_anon(), name,
- OBJECT(dev));
+ object_property_try_add_child(qdev_get_peripheral_anon(), name,
+ OBJECT(dev), errp);
g_free(name);
}
}
DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
{
+ ERRP_GUARD();
DeviceClass *dc;
const char *driver, *path;
DeviceState *dev = NULL;
@@ -691,7 +692,10 @@ DeviceState *qdev_device_add(QemuOpts *opts,
Error **errp)
}
}
- qdev_set_id(dev, g_strdup(qemu_opts_id(opts)));
+ qdev_set_id(dev, g_strdup(qemu_opts_id(opts)), errp);
+ if (*errp) {
+ goto err_del_dev;
+ }
[..] here we'll have
if (!qdev_set_id(...)) {
goto err_del_dev;
}
and no need for ERRP_GUARD.
/* set properties */
if (qemu_opt_foreach(opts, set_property, dev, errp)) {
Re: [PATCH] allwinner-h3: Switch to SMC as PSCI conduit
On 9/20/21 22:39, Alexander Graf wrote:
> The Allwinner H3 SoC uses Cortex-A7 cores which support virtualization.
> However, today we are configuring QEMU to use HVC as PSCI conduit.
>
> That means HVC calls get trapped into QEMU instead of the guest's own
> emulated CPU and thus break the guest's ability to execute virtualization.
>
> Fix this by moving to SMC as conduit, freeing up HYP completely to the VM.
>
> Signed-off-by: Alexander Graf
> Fixes: 740dafc0ba0 ("hw/arm: add Allwinner H3 System-on-Chip")
> ---
> hw/arm/allwinner-h3.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 05/27] linux-user/arm: Implement setup_sigtramp
On Fri, 24 Sept 2021 at 17:59, Richard Henderson
wrote:
>
> Update the trampoline code to match the kernel: this uses
> sp-relative accesses rather than pc-relative.
>
> Signed-off-by: Richard Henderson
These functions must write at most 8 bytes:
> +static void write_arm_sigreturn(uint32_t *rc, int syscall)
> +{
> +__put_user(ARM_MOV_R7_IMM(syscall), rc);
> +__put_user(ARM_SWI_SYS(syscall), rc + 1);
> +}
> +
> +static void write_thumb_sigreturn(uint32_t *rc, int syscall)
> +{
> +__put_user(THUMB_SWI_SYS << 16 | THUMB_MOVS_R7_IMM(syscall), rc);
> +}
>
> /*
> - * Stub needed to make sure the FD register (r9) contains the right
> - * value.
> + * Stub needed to make sure the FD register (r9) contains the right value.
> + * Use the same instruction sequence as the kernel.
> */
> -static const unsigned long sigreturn_fdpic_codes[3] = {
> -0xe59fc004, /* ldr r12, [pc, #4] to read function descriptor */
> -0xe59c9004, /* ldr r9, [r12, #4] to setup GOT */
> -0xe59cf000 /* ldr pc, [r12] to jump into restorer */
> -};
...and these must write at most 12 bytes. But nothing states
or asserts that.
> +static void write_arm_fdpic_sigreturn(uint32_t *rc, int ofs)
> +{
> +assert(ofs <= 0xfff);
> +__put_user(0xe59d3000 | ofs, rc + 0); /* ldr r3, [sp, #ofs] */
> +__put_user(0xe8930908, rc + 1); /* ldm r3, { r3, r9 } */
> +__put_user(0xe12fff13, rc + 2); /* bx r3 */
> +}
>
> -static const unsigned long sigreturn_fdpic_thumb_codes[3] = {
> -0xc008f8df, /* ldr r12, [pc, #8] to read function descriptor */
> -0x9004f8dc, /* ldr r9, [r12, #4] to setup GOT */
> -0xf000f8dc /* ldr pc, [r12] to jump into restorer */
> -};
> +static void write_thumb_fdpic_sigreturn(void *vrc, int ofs)
> +{
> +uint16_t *rc = vrc;
> +
> +assert((ofs & ~0x3fc) == 0);
> +__put_user(0x9b00 | (ofs >> 2), rc + 0); /* ldr r3, [sp, #ofs] */
> +__put_user(0xcb0c, rc + 1); /* ldm r3, { r2, r3 } */
> +__put_user(0x4699, rc + 2); /* mov r9, r3 */
> +__put_user(0x4710, rc + 3); /* bx r2 */
> +}
>
> -retcode = rc_addr + thumb;
> +/* Each trampoline variant consumes a 12-byte slot. */
> +retcode = sigreturn_fdpic_tramp + retcode_idx * 12 + thumb;
> } else {
> retcode = ka->sa_restorer;
> }
> } else {
> -
> -retcode = rc_addr + thumb;
> +/* Each trampoline variant consumes 8-byte slot. */
> +retcode = default_sigreturn + retcode_idx * 8 + thumb;
These 12 and 8 magic numbers correspond to the maximum sequence sizes
above...
> +void setup_sigtramp(abi_ulong sigtramp_page)
> +{
> +enum {
> +SIGFRAME_FDPIC_OFS = offsetof(struct sigframe, retcode[3]),
> +RT_SIGFRAME_FDPIC_OFS = offsetof(struct rt_sigframe, retcode[3]),
> +};
> +
> +uint32_t total_size = 4 * 8 + 4 * 12;
> +uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, total_size, 0);
> +uint32_t i = 0;
> +
> +assert(tramp != NULL);
> +
> +default_sigreturn = sigtramp_page;
> +write_arm_sigreturn([i], TARGET_NR_sigreturn);
> +i += 2;
> +write_thumb_sigreturn([i], TARGET_NR_sigreturn);
> +i += 2;
> +write_arm_sigreturn([i], TARGET_NR_rt_sigreturn);
> +i += 2;
> +write_thumb_sigreturn([i], TARGET_NR_rt_sigreturn);
> +i += 2;
...and these "+=2" and the "+=3" later do as well, but with
a count of 32-bit words rather than bytes. I think it would be
useful to at least have some defined constants for the lengths
rather than hard-coded 8,12,2,3, and comments that the write_
functions must not write more than however-many bytes.
> +
> +/*
> + * FDPIC require trampolines to call sa_restorer, and different
> + * from the pc-relative versions we write to the stack.
> + *
> + * ARM versions use:
> + *ldr r3, [sp, #ofs]
> + *ldr r9, [r3, #4]
> + *ldr pc, [r3, #0]
This comment doesn't match the code that write_arm_fdpic_sigreturn()
now generates. The "different from the pc-relative versions we
write from the stack" bit doesn't seem to be right either, given
we call the same functions in both places to write the code.
> + *
> + * Thumb versions use:
> + *ldr r3, [sp, #ofs]
> + *ldmia r3, {r2, r3}
> + *mov r9, r3
> + *bxr2
> + */
> +sigreturn_fdpic_tramp = sigtramp_page + i * 4;
> +
> +/* ARM sigframe */
> +write_arm_fdpic_sigreturn(tramp + i,
> + offsetof(struct sigframe, retcode[3]));
> +i += 3;
> +
> +/* Thumb sigframe */
> +write_thumb_fdpic_sigreturn(tramp + i,
> +offsetof(struct sigframe, retcode[3]));
> +i += 3;
> +
> +/* ARM rt_sigframe */
> +write_arm_fdpic_sigreturn(tramp + i,
> + offsetof(struct rt_sigframe, retcode[3]));
> +i += 3;
> +
> +/* Thumb
Re: [PATCH v3 0/9] hw/nvram: hw/arm: Introduce Xilinx eFUSE and BBRAM
On Fri, 17 Sept 2021 at 06:24, Tong Ho wrote: > > This series implements the Xilinx eFUSE and BBRAM devices for > the Versal and ZynqMP product families. > > Furthermore, both new devices are connected to the xlnx-versal-virt > board and the xlnx-zcu102 board. > > See changes in docs/system/arm/xlnx-versal-virt.rst for detail. > > --- Applied to target-arm.next, thanks. -- PMM
Re: [PATCH v3 1/3] hw/virtio: Comment virtqueue_flush() must be called with RCU read lock
On Mon, Sep 27 2021, Philippe Mathieu-Daudé wrote: > On 9/27/21 12:18, Cornelia Huck wrote: >> On Mon, Sep 06 2021, Philippe Mathieu-Daudé wrote: >> >>> Reported-by: Stefano Garzarella >>> Suggested-by: Stefan Hajnoczi >>> Signed-off-by: Philippe Mathieu-Daudé >>> --- >>> include/hw/virtio/virtio.h | 7 +++ >>> hw/virtio/virtio.c | 1 + >>> 2 files changed, 8 insertions(+) >>> >>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h >>> index 8bab9cfb750..c1c5f6e53c8 100644 >>> --- a/include/hw/virtio/virtio.h >>> +++ b/include/hw/virtio/virtio.h >>> @@ -186,6 +186,13 @@ void virtio_delete_queue(VirtQueue *vq); >>> >>> void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem, >>> unsigned int len); >>> +/** >>> + * virtqueue_flush: >>> + * @vq: The #VirtQueue >>> + * @count: Number of elements to flush >>> + * >>> + * Must be called within RCU critical section. >>> + */ >> >> Hm... do these doc comments belong into .h files, or rather into .c files? > > Maybe we should restart this old thread, vote(?) and settle on a style? > > https://lore.kernel.org/qemu-devel/349cd87b-0526-30b8-d9cd-0eee537ab...@redhat.com/ My vote would still go to putting this into .c files :) Not sure if we want to go through the hassle of a wholesale cleanup; but if others agree, we could at least start with putting new doc comments next to the implementation. Do we actually consume these comments in an automated way somewhere?
Re: [PATCH 11/11] Deprecate stable non-JSON -device and -object
On Mon, Sep 27, 2021 at 12:17:03PM +0200, Kevin Wolf wrote: > Am 27.09.2021 um 10:21 hat Daniel P. Berrangé geschrieben: > > On Mon, Sep 27, 2021 at 10:15:43AM +0200, Paolo Bonzini wrote: > > > On 24/09/21 11:04, Kevin Wolf wrote: > > > > We want to switch both from QemuOpts to the keyval parser in the future, > > > > which results in some incompatibilities, mainly around list handling. > > > > Mark the non-JSON version of both as unstable syntax so that management > > > > tools switch to JSON and we can later make the change without breaking > > > > things. > > > > > > Maybe we need a different section for unstable syntaxes, rather than > > > overloading deprecated.rst? > > > > This case feels like it hits two scenarios - we want to declare it > > unstable, which is something we should document in qemu-options.hx. > > Actually, I think a section for unstable syntaxes or generally > compatibility promises wouldn't hurt. When I checked, I couldn't find > any documentation about the support status of HMP either. > > Basically, I imagine HMP and non-JSON -device/-object would be on the > same level: We don't change things without a reason, but if we do want > to change things, compatibility isn't an argument against making the > change. > > > We want to also to warn of specific breakage when the impl changes > > which is something suitable for deprecations. > > We don't do this for HMP either for individual changes. Well HMP as a whole is considered non-stable, so we don't need to call out individual things. We've got a simple story that QMP == stable, HMP == unstable. The comparison here would be if we declared the entire QEMU CLI to be unstable, except for JSON syntax args. > Basically this deprecation notice was meant to make people aware that > we're lowering the support status from a long-term stable interface to > HMP-like. Bearing in mind our previous discussions it feels like our goal is that we're tending towards a world where we are only wanting to consider JSON based configuration to be stable, and everything else non-stable. I think that's a good long term plan, but if we're really doing that then I think we need to big picture explain it in our docs rather than mention it in passing against individual args. BTW I'm also not a fan of deprecating stuff when our documentation is still using the deprecated syntax and nothing shows the new preferred syntax. We've got alot of results for $ git grep -- ' -object' Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
On Mon, Sep 27, 2021 at 4:08 AM Peter Maydell wrote: > On Mon, 27 Sept 2021 at 10:59, Daniel P. Berrangé > wrote: > > If we look at linux-user/meson.build though things are more complex. > > There we have alot of sub-dirs, and meson.biuld in those dirs adds > > generators for various files. So conceivably skipping linux-user > > will mean we won't auto-generate files we don't need on non-Linux. > > The top level meson.build doesn't call process on the > syscall_nr_generators[] list unless we're building linux-user > targets, so I don't think we will auto-generate anything we > don't need. > The problem is that I'm trying to add some os-specific files to the bsd-user in a patch set I sent off. bsd-user compiles for different BSDs, and I'd like to start pulling in per-bsd files as I'm upstreaming. To do that, I have this construct in the bsd-user/meson.build file: # Pull in the OS-specific build glue, if any if fs.exists(targetos) subdir(targetos) endif primarily because the builds failed on Linux when it tried to find the non-existent bsd-user/linunx directory. The question came up on how to cope with this situation, and Philippe sent off this series as an alternative to that construct. The whole reason why we descend into the linux-user directory on BSD and into the bsd-user directory on linux is unclear to me as well. So this is preparing the ground for my future work of upstreaming. I'm agnostic as to how it's resolved, but it needs to be resolved. Warner
Re: [PATCH 09/11] qdev: Avoid QemuOpts in QMP device_add
On 9/24/21 11:04, Kevin Wolf wrote:
Directly call qdev_device_add_from_qdict() for QMP device_add instead of
first going through QemuOpts and converting back to QDict.
Note that this changes the behaviour of device_add, though in ways that
should be considered bug fixes:
QemuOpts ignores differences between data types, so you could
successfully pass a string "123" for an integer property, or a string
"on" for a boolean property (and vice versa). After this change, the
correct data type for the property must be used in the JSON input.
qemu_opts_from_qdict() also silently ignores any options whose value is
a QDict, QList or QNull.
To illustrate, the following QMP command was accepted before and is now
rejected for both reasons:
{ "execute": "device_add",
"arguments": { "driver": "scsi-cd",
"drive": { "completely": "invalid" },
"physical_block_size": "4096" } }
Signed-off-by: Kevin Wolf
---
softmmu/qdev-monitor.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index c09b7430eb..8622ccade6 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -812,7 +812,8 @@ void hmp_info_qdm(Monitor *mon, const QDict *qdict)
qdev_print_devinfos(true);
}
-void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
+static void monitor_device_add(QDict *qdict, QObject **ret_data,
+ bool from_json, Error **errp)
{
QemuOpts *opts;
DeviceState *dev;
@@ -825,7 +826,9 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error
**errp)
qemu_opts_del(opts);
return;
}
-dev = qdev_device_add(opts, errp);
+qemu_opts_del(opts);
+
+dev = qdev_device_add_from_qdict(qdict, from_json, errp);
Hi Kevin,
I'm wandering if deleting the opts (which remove it from the "device"
opts list) is really a no-op ?
The opts list is, eg, traversed in hw/net/virtio-net.c in the function
failover_find_primary_device_id() which may be called during the
virtio_net_set_features() (a TYPE_VIRTIO_NET method).
I do not have the knowledge to tell when this method is called. But If
this is after we create the devices. Then the list will be empty at this
point now.
It seems, there are 2 other calling sites of
"qemu_opts_foreach(qemu_find_opts("device"), [...]" in net/vhost-user.c
and net/vhost-vdpa.c
--
Damien
Re: [PATCH v3 1/3] hw/virtio: Comment virtqueue_flush() must be called with RCU read lock
On 9/27/21 12:18, Cornelia Huck wrote:
> On Mon, Sep 06 2021, Philippe Mathieu-Daudé wrote:
>
>> Reported-by: Stefano Garzarella
>> Suggested-by: Stefan Hajnoczi
>> Signed-off-by: Philippe Mathieu-Daudé
>> ---
>> include/hw/virtio/virtio.h | 7 +++
>> hw/virtio/virtio.c | 1 +
>> 2 files changed, 8 insertions(+)
>>
>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>> index 8bab9cfb750..c1c5f6e53c8 100644
>> --- a/include/hw/virtio/virtio.h
>> +++ b/include/hw/virtio/virtio.h
>> @@ -186,6 +186,13 @@ void virtio_delete_queue(VirtQueue *vq);
>>
>> void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
>> unsigned int len);
>> +/**
>> + * virtqueue_flush:
>> + * @vq: The #VirtQueue
>> + * @count: Number of elements to flush
>> + *
>> + * Must be called within RCU critical section.
>> + */
>
> Hm... do these doc comments belong into .h files, or rather into .c files?
Maybe we should restart this old thread, vote(?) and settle on a style?
https://lore.kernel.org/qemu-devel/349cd87b-0526-30b8-d9cd-0eee537ab...@redhat.com/
>> void virtqueue_flush(VirtQueue *vq, unsigned int count);
>> void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
>>unsigned int len);
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index 3a1f6c520cb..97f60017466 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -896,6 +896,7 @@ static void virtqueue_packed_flush(VirtQueue *vq,
>> unsigned int count)
>> }
>> }
>>
>> +/* Called within rcu_read_lock(). */
>> void virtqueue_flush(VirtQueue *vq, unsigned int count)
>> {
>> if (virtio_device_disabled(vq->vdev)) {
>
> The content of the change looks good to me, I'm only wondering about
> the style...
>
Re: [PATCH v3 2/9] qapi: make blockdev-add a coroutine command
07.09.2021 09:14, Markus Armbruster wrote:
Vladimir Sementsov-Ogievskiy writes:
06.09.2021 22:28, Markus Armbruster wrote:
Vladimir Sementsov-Ogievskiy writes:
We are going to support nbd reconnect on open in a next commit. This
means that we want to do several connection attempts during some time.
And this should be done in a coroutine, otherwise we'll stuck.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
qapi/block-core.json | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 06674c25c9..6e4042530a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4219,7 +4219,8 @@
# <- { "return": {} }
#
##
-{ 'command': 'blockdev-add', 'data': 'BlockdevOptions', 'boxed': true }
+{ 'command': 'blockdev-add', 'data': 'BlockdevOptions', 'boxed': true,
+ 'coroutine': true }
##
# @blockdev-reopen:
Why is this safe?
Prior discusson:
Message-ID: <87lfq0yp9v@dusky.pond.sub.org>
https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg04921.html
Hmm.. I'm afraid, that I can't prove that it's safe. At least it will mean to
audit .bdrv_open() of all block drivers.. And nothing prevents creating new
incompatible drivers in future..
Breaking existing block drivers is more serious than adding new drivers
broken.
On the other hand, looking at qmp_blockdev_add, bdrv_open() is the only thing
of interest.
And theoretically, bdrv_open() should work in coroutine context. We do call
this function from coroutine_fn functions sometimes. So, maybe, if in some
circumstances, bdrv_open() is not compatible with coroutine context, we can
consider it as a bug? And fix it later, if it happen?
If it's already used in ways that require coroutine-compatibility, we'd
merely add another way for existing bugs to bite. Kevin, is it?
In general, the less coroutine-incompatibility we have, the better.
From the thread I quoted:
Kevin Wolf writes:
> AM 22.01.2020 um 13:15 hat Markus Armbruster geschrieben:
[...]
>> Is coroutine-incompatible command handler code necessary or accidental?
>>
>> By "necessary" I mean there are (and likely always will be) commands
>> that need to do stuff that cannot or should not be done on coroutine
>> context.
>>
>> "Accidental" is the opposite: coroutine-incompatibility can be regarded
>> as a fixable flaw.
>
> I expect it's mostly accidental, but maybe not all of it.
I'm inclinded to regard accidental coroutine-incompatibility as a bug.
As long as the command doesn't have the coroutine flag set, it's a
latent bug.
Setting the coroutine flag without auditing the code risks making such
latent bugs actual bugs. A typical outcome is deadlock.
Whether we're willing to accept such risk is not for me to decide.
We weren't when we merged the coroutine work, at least not wholesale.
The risk is perhaps less scary for a single command. On the other hand,
code audit is less daunting, too.
Kevin, what do you think?
Any thoughts?
I think, we can simply proceed without this patch. That just means that
blockdev-add remains blocking, and using it to add nbd node with long
open-timeout when guest is running [*] will be inconvenient (we don't want to
block the running guest). Still commandline interface, and running blockdev-add
when guest is paused is OK.
Anyway, this case [*] is not super useful: OK, guest isn't blocked, but you
can't issue more qmp commands until open-timeout passed. That's not very
convenient for running vm.
Side thought: don't we have/plan async qmp commands or something like this? So,
that command is started in a coroutine, but user don't have to wait for finish
to run more QMP commands? It should be more useful for command that may take
long time. We have jobs, but implementing new job for such command seems too
heavy.
--
Best regards,
Vladimir
Re: [question] VFIO Device Migration: The vCPU may be paused during vfio device DMA in iommu nested stage mode && vSVA
Hi Kevin: On 2021/9/24 14:47, Tian, Kevin wrote: From: Kunkun Jiang Sent: Friday, September 24, 2021 2:19 PM Hi all, I encountered a problem in vfio device migration test. The vCPU may be paused during vfio-pci DMA in iommu nested stage mode && vSVA. This may lead to migration fail and other problems related to device hardware and driver implementation. It may be a bit early to discuss this issue, after all, the iommu nested stage mode and vSVA are not yet mature. But judging from the current implementation, we will definitely encounter this problem in the future. Yes, this is a known limitation to support migration with vSVA. This is the current process of vSVA processing translation fault in iommu nested stage mode (take SMMU as an example): guest os 4.handle translation fault 5.send CMD_RESUME to vSMMU qemu 3.inject fault into guest os 6.deliver response to host os (vfio/vsmmu) host os 2.notify the qemu 7.send CMD_RESUME to SMMU (vfio/smmu) SMMU 1.address translation fault 8.retry or terminate The order is 1--->8. Currently, qemu may pause vCPU at any step. It is possible to pause vCPU at step 1-5, that is, in a DMA. This may lead to migration fail and other problems related to device hardware and driver implementation. For example, the device status cannot be changed from RUNNING && SAVING to SAVING, because the device DMA is not over. As far as i can see, vCPU should not be paused during a device IO process, such as DMA. However, currently live migration does not pay attention to the state of vfio device when pausing the vCPU. And if the vCPU is not paused, the vfio device is always running. This looks like a *deadlock*. Basically this requires: 1) stopping vCPU after stopping device (could selectively enable this sequence for vSVA); How to tell if vSVA is open? In fact, as long as it is in iommu nested stage mode, there will be such a problem, whether it is vSVA or no-vSVA. In no-vSVA mode, a fault can also be generated by modifying the guest device driver. 2) when stopping device, the driver should block new requests from vCPU (queued to a pending list) and then drain all in-fly requests including faults; * to block this further requires switching from fast-path to slow trap-emulation path for the cmd portal before stopping the device; 3) save the pending requests in the vm image and replay them after the vm is resumed; * finally disable blocking by switching back to the fast-path for the cmd portal; Is there any related patch sent out and discussed? I might have overlooked that. We may be able to discuss and finalize a specification for this problem. Thanks, Kunkun Jiang Do you have any ideas to solve this problem? Looking forward to your replay. We verified above flow can work in our internal POC. Thanks Kevin
Re: [PULL 00/25] QAPI patches patches for 2021-09-25
Peter Maydell writes: > On Sat, 25 Sept 2021 at 07:25, Markus Armbruster wrote: >> >> The following changes since commit 11a11998460ed84d9a127c025f50f7234e5a483f: >> >> Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20210921' into >> staging (2021-09-24 13:21:18 -0400) >> >> are available in the Git repository at: >> >> git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2021-09-25 >> >> for you to fetch changes up to 5757c0904e2e8a7f5d9ff359b30542cfcb70556b: >> >> tests/qapi-schema: Make test-qapi.py -u work when files are absent >> (2021-09-25 07:00:50 +0200) >> >> >> QAPI patches patches for 2021-09-25 >> > > Fails to build, all hosts except x86-64 Linux: > > In file included from qapi/qapi-visit-char.h:17, > from qapi/qapi-commands-char.c:19: > qapi/qapi-types-char.h:500:5: error: unknown type name 'ChardevSpiceChannel' > 500 | ChardevSpiceChannel *data; > | ^~~ > qapi/qapi-types-char.h:507:5: error: unknown type name 'ChardevSpicePort' > 507 | ChardevSpicePort *data; > | ^~~~ > qapi/qapi-types-char.h:514:5: error: unknown type name 'ChardevQemuVDAgent' > 514 | ChardevQemuVDAgent *data; > | ^~ Sorry about that. Respin on the way.
Re: [PATCH v1 1/1] hw/riscv: shakti_c: Mark as not user creatable
On Mon, Sep 27, 2021 at 3:13 PM Alistair Francis wrote: > > From: Alistair Francis > > Mark the shakti_c machine as not user creatable as it uses serial_hd. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/639 > Signed-off-by: Alistair Francis > --- > hw/riscv/shakti_c.c | 2 ++ > 1 file changed, 2 insertions(+) > Reviewed-by: Bin Meng
Re: [PATCH] hw/misc: Add a virtual pci device to dynamically attach memory to QEMU
On Sun, Sep 26, 2021 at 10:16:14AM +0800, David Dai wrote:
> Add a virtual pci to QEMU, the pci device is used to dynamically attach memory
> to VM, so driver in guest can apply host memory in fly without virtualization
> management software's help, such as libvirt/manager. The attached memory is
> isolated from System RAM, it can be used in heterogeneous memory management
> for
> virtualization. Multiple VMs dynamically share same computing device memory
> without memory overcommit.
>
> Signed-off-by: David Dai
CCing David Hildenbrand (virtio-balloon and virtio-mem) and Igor
Mammedov (host memory backend).
> ---
> docs/devel/dynamic_mdev.rst | 122 ++
> hw/misc/Kconfig | 5 +
> hw/misc/dynamic_mdev.c | 456
> hw/misc/meson.build | 1 +
> 4 files changed, 584 insertions(+)
> create mode 100644 docs/devel/dynamic_mdev.rst
> create mode 100644 hw/misc/dynamic_mdev.c
>
> diff --git a/docs/devel/dynamic_mdev.rst b/docs/devel/dynamic_mdev.rst
> new file mode 100644
> index 00..8e2edb6600
> --- /dev/null
> +++ b/docs/devel/dynamic_mdev.rst
> @@ -0,0 +1,122 @@
> +Motivation:
> +In heterogeneous computing system, accelorator generally exposes its device
s/accelorator/accelerator/
(There are missing articles and small grammar tweaks that could be made,
but I'm skipping the English language stuff for now.)
> +memory to host via PCIe and CXL.mem(Compute Express Link) to share memory
> +between host and device, and these memory generally are uniformly managed by
> +host, they are called HDM (host managed device memory), further SVA (share
> +virtual address) can be achieved on this base. One computing device may be
> used
Is this Shared Virtual Addressing (SVA) (also known as Shared Virtual
Memory)? If yes, please use the exact name ("Shared Virtual Addressing",
not "share virtual address") so that's clear and the reader can easily
find out more information through a web search.
> +by multiple virtual machines if it supports SRIOV, to efficiently use device
> +memory in virtualization, each VM allocates device memory on-demand without
> +overcommit, but how to dynamically attach host memory resource to VM. A
> virtual
I cannot parse this sentence. Can you rephrase it and/or split it into
multiple sentences?
> +PCI device, dynamic_mdev, is introduced to achieve this target. dynamic_mdev
I suggest calling it "memdev" instead of "mdev" to prevent confusion
with VFIO mdev.
> +has a big bar space which size can be assigned by user when creating VM, the
> +bar doesn't have backend memory at initialization stage, later driver in
> guest
> +triggers QEMU to map host memory to the bar space. how much memory, when and
> +where memory will be mapped to are determined by guest driver, after device
> +memory has been attached to the virtual PCI bar, application in guest can
> +access device memory by the virtual PCI bar. Memory allocation and
> negotiation
> +are left to guest driver and memory backend implementation. dynamic_mdev is a
> +mechanism which provides significant benefits to heterogeneous memory
> +virtualization.
David and Igor: please review this design. I'm not familiar enough with
the various memory hotplug and ballooning mechanisms to give feedback on
this.
> +Implementation:
> +dynamic_mdev device has two bars, bar0 and bar2. bar0 is a 32-bit register
> bar
> +which used to host control register for control and message communication,
> Bar2
> +is a 64-bit mmio bar, which is used to attach host memory to, the bar size
> can
> +be assigned via parameter when creating VM. Host memory is attached to this
> bar
> +via mmap API.
> +
> +
> + VM1 VM2
> + -----
> +| application | | application |
> +| | | |
> +|---| |--|
> +| guest driver | | guest driver |
> +| |--|| | | -| |
> +| | pci mem bar || | | pci mem bar | |
> + ---|--|- ---|--|---
> + --- -- --
> +|| | | | | | |
> + --- -- --
> +\ /
> + \/
> + \ /
> + \/
> +| |
> +V V
> + --- /dev/mdev.mmap
> +| -- -- -- -- -- -- |
> +|| | | | | | | | | | | | <-free_mem_list |
> +| -- -- -- -- -- -- |
> +| |
> +| HDM(host managed device memory )|
> +
Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
On Mon, Sep 27, 2021 at 11:15 AM Peter Maydell wrote:
> On Sun, 26 Sept 2021 at 23:04, Philippe Mathieu-Daudé wrote:
> > Reported-by: Warner Losh
> > Signed-off-by: Philippe Mathieu-Daudé
> > ---
> > bsd-user/meson.build | 4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/bsd-user/meson.build b/bsd-user/meson.build
> > index 03695493408..a7607e1c884 100644
> > --- a/bsd-user/meson.build
> > +++ b/bsd-user/meson.build
> > @@ -1,3 +1,7 @@
> > +if not config_host.has_key('CONFIG_BSD')
> > + subdir_done()
> > +endif
> > +
> > bsd_user_ss.add(files(
> >'bsdload.c',
> >'elfload.c',
>
>
> So, what's the reason for this change?
https://lore.kernel.org/qemu-devel/canczdfprc16ezjqcwjmyeapx6eym9nxsoqatbagr+czis4r...@mail.gmail.com/
linux-user/meson.build is evaluated on bsd, and bsd-user/meson.build on Linux.
> The commit messages and
> the cover letter don't really explain it. Is this fixing a bug
> (if so what?), a precaution to avoid possible future bugs,
> fixing a performance issue with how long meson takes to run (if
> so, how much effect does this have), or something else?
I'll wait for feedback from Paolo, then work on the explanation.
Regards,
Phil.
Re: gitlab-ci: amd64-opensuse-leap-container job failing
On 9/27/21 10:35, Daniel P. Berrangé wrote: > On Sun, Sep 26, 2021 at 07:23:56PM +0200, Philippe Mathieu-Daudé wrote: >> Hi, >> >> FYI the OpenSUSE job is failing since few days, i.e.: >> https://gitlab.com/qemu-project/qemu/-/jobs/1622345026 > It isn't confined to only SuSE. In libvirt we've had similar problems > with several other jobs, though are suse jobs are the worst affected. > > GitLab have finally acknowledged it is an general infra issue affecting > many things: > >https://status.gitlab.com/ >https://gitlab.com/gitlab-com/gl-infra/production/-/issues/5590 >> Should we temporarily disable to job and its dependencies? > > Given it is believed to be a gitlab infra issue, rather than a problem > of ours, or something we're using, I think best to wait a little longer > to see if they get fix the infra. OK (I checked the status page during Saturday and Sunday morning and it was all green). Thanks for investigating, Phil.
Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
On Mon, 27 Sept 2021 at 10:59, Daniel P. Berrangé wrote: > If we look at linux-user/meson.build though things are more complex. > There we have alot of sub-dirs, and meson.biuld in those dirs adds > generators for various files. So conceivably skipping linux-user > will mean we won't auto-generate files we don't need on non-Linux. The top level meson.build doesn't call process on the syscall_nr_generators[] list unless we're building linux-user targets, so I don't think we will auto-generate anything we don't need. -- PMM
Re: [PATCH v3 1/3] hw/virtio: Comment virtqueue_flush() must be called with RCU read lock
On Mon, Sep 06 2021, Philippe Mathieu-Daudé wrote:
> Reported-by: Stefano Garzarella
> Suggested-by: Stefan Hajnoczi
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> include/hw/virtio/virtio.h | 7 +++
> hw/virtio/virtio.c | 1 +
> 2 files changed, 8 insertions(+)
>
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 8bab9cfb750..c1c5f6e53c8 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -186,6 +186,13 @@ void virtio_delete_queue(VirtQueue *vq);
>
> void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
> unsigned int len);
> +/**
> + * virtqueue_flush:
> + * @vq: The #VirtQueue
> + * @count: Number of elements to flush
> + *
> + * Must be called within RCU critical section.
> + */
Hm... do these doc comments belong into .h files, or rather into .c files?
> void virtqueue_flush(VirtQueue *vq, unsigned int count);
> void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
>unsigned int len);
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 3a1f6c520cb..97f60017466 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -896,6 +896,7 @@ static void virtqueue_packed_flush(VirtQueue *vq,
> unsigned int count)
> }
> }
>
> +/* Called within rcu_read_lock(). */
> void virtqueue_flush(VirtQueue *vq, unsigned int count)
> {
> if (virtio_device_disabled(vq->vdev)) {
The content of the change looks good to me, I'm only wondering about
the style...
Re: [PATCH v3 0/5] introduce QArray
On Mon, 27 Sep 2021 12:35:16 +0200 Christian Schoenebeck wrote: > On Dienstag, 31. August 2021 14:25:04 CEST Christian Schoenebeck wrote: > > On Dienstag, 31. August 2021 13:58:02 CEST Greg Kurz wrote: > > > On Thu, 26 Aug 2021 14:47:26 +0200 > > > > > > Christian Schoenebeck wrote: > > > > Patches 1 and 2 introduce include/qemu/qarray.h which implements a deep > > > > auto free mechanism for arrays. See commit log of patch 1 for a detailed > > > > explanation and motivation for introducing QArray. > > > > > > > > Patches 3..5 are provided (e.g. as example) for 9p being the first user > > > > of > > > > this new QArray API. These particular patches 3..5 are rebased on my > > > > current 9p queue: https://github.com/cschoenebeck/qemu/commits/9p.next > > > > > > > which are basically just the following two queued patches: > > > This looks nice indeed but I have the impression the same could be > > > achieved using glib's g_autoptr framework with less code being added > > > to QEMU (at the cost of being less generic maybe). > > > > I haven't seen a way doing this with glib, except of GArray which has some > > disadvantages. But who knows, maybe I was missing something. > > Ping > > Let's do this? > Hi Christian, Sorry I don't have enough bandwidth to review or to look for an alternate way... :-\ So I suggest you just go forward with this series. Hopefully you can get some reviews from Markus and/or Richard. Cheers, -- Greg > > > Anyway, we should likely sort out the SEGV issue you're hitting > > > before going forward with supplementary changes in v9fs_walk(). > > > > Yeah, let's postpone this series here. I'll look into the Twalk crash issue > > more closely today and will get back on that issue first. > > > > Best regards, > > Christian Schoenebeck > >
Re: [PATCH 11/11] Deprecate stable non-JSON -device and -object
Am 27.09.2021 um 11:00 hat Peter Maydell geschrieben: > On Fri, 24 Sept 2021 at 10:14, Kevin Wolf wrote: > > > > We want to switch both from QemuOpts to the keyval parser in the future, > > which results in some incompatibilities, mainly around list handling. > > Mark the non-JSON version of both as unstable syntax so that management > > tools switch to JSON and we can later make the change without breaking > > things. > > > > Signed-off-by: Kevin Wolf > > > +Stable non-JSON ``-device`` and ``-object`` syntax (since 6.2) > > +'' > > + > > +If you rely on a stable interface for ``-device`` and ``-object`` that > > doesn't > > +change incompatibly between QEMU versions (e.g. because you are using the > > QEMU > > +command line as a machine interface in scripts rather than interactively), > > use > > +JSON syntax for these options instead. > > + > > +There is no intention to remove support for non-JSON syntax entirely, but > > +future versions may change the way to spell some options. > > As it stands, this is basically saying "pretty much anybody > using the command line, your stuff may break in future, instead > use some other interface you've never heard of, which doesn't > appear to be documented in the manual and which none of the > documentation's examples use". The documentation is a valid criticism. We need to document the JSON interfaces properly (which will really mostly be a pointer to the existing QMP documentation at least for -object, but it's important to tell people where to look for the details). > Is there some more limited deprecation we can do rather than "the > entire commandline for almost all users" ? I don't think "almost all" users is true. I see three groups of users: 1. Using a management tool that is probably using libvirt. This is likely the vast majority of users. They won't notice a difference because libvirt abstracts it away. libvirt developers are actively asking us for JSON (and QAPI) based interfaces because using the same representation to describe configurations in QMP and on the CLI makes their life easier. 2. People starting QEMU on the command line manually. This is essentially the same situation as HMP: If something changes, you get an error message, you look up the new syntax, done. Small inconvenience, but that's it. This includes simple scripts that just start QEMU and are only used to store a long command line somewhere. 3. People writing more complex scripts or applications that invoke QEMU manually instead of using libvirt. They may actually be hurt by such changes. They should probably be using a proper machine interface, i.e. JSON mode, so the deprecation notice is for them to change their code. This is probably a small minority and not "almost all users". Yes, we could in theory do a more limited deprecation. The planned change from my side is just going from QemuOpts to the keyval parser, which doesn't change anything in the vast majority of cases. But we have the separation in the monitor between QMP and HMP for a good reason: Requirements for a good machine interface are different from a good human interface. The same is true for the command line. So it seems to make a lot of sense to me to have both a machine interface (JSON based) and a human interface (non-JSON) on the command line, too, and take the same liberties for evolving the human interface as we already do in HMP - which means that it's technically an unstable interface where compatibility doesn't prevent improvements, but not that it looks completely different in every QEMU version. Kevin
Re: [PATCH 09/11] qdev: Avoid QemuOpts in QMP device_add
Am 27.09.2021 um 13:06 hat Damien Hedde geschrieben:
> On 9/24/21 11:04, Kevin Wolf wrote:
> > Directly call qdev_device_add_from_qdict() for QMP device_add instead of
> > first going through QemuOpts and converting back to QDict.
> >
> > Note that this changes the behaviour of device_add, though in ways that
> > should be considered bug fixes:
> >
> > QemuOpts ignores differences between data types, so you could
> > successfully pass a string "123" for an integer property, or a string
> > "on" for a boolean property (and vice versa). After this change, the
> > correct data type for the property must be used in the JSON input.
> >
> > qemu_opts_from_qdict() also silently ignores any options whose value is
> > a QDict, QList or QNull.
> >
> > To illustrate, the following QMP command was accepted before and is now
> > rejected for both reasons:
> >
> > { "execute": "device_add",
> >"arguments": { "driver": "scsi-cd",
> > "drive": { "completely": "invalid" },
> > "physical_block_size": "4096" } }
> >
> > Signed-off-by: Kevin Wolf
> > ---
> > softmmu/qdev-monitor.c | 18 +++---
> > 1 file changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> > index c09b7430eb..8622ccade6 100644
> > --- a/softmmu/qdev-monitor.c
> > +++ b/softmmu/qdev-monitor.c
> > @@ -812,7 +812,8 @@ void hmp_info_qdm(Monitor *mon, const QDict *qdict)
> > qdev_print_devinfos(true);
> > }
> > -void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
> > +static void monitor_device_add(QDict *qdict, QObject **ret_data,
> > + bool from_json, Error **errp)
> > {
> > QemuOpts *opts;
> > DeviceState *dev;
> > @@ -825,7 +826,9 @@ void qmp_device_add(QDict *qdict, QObject **ret_data,
> > Error **errp)
> > qemu_opts_del(opts);
> > return;
> > }
> > -dev = qdev_device_add(opts, errp);
> > +qemu_opts_del(opts);
> > +
> > +dev = qdev_device_add_from_qdict(qdict, from_json, errp);
>
> Hi Kevin,
>
> I'm wandering if deleting the opts (which remove it from the "device" opts
> list) is really a no-op ?
It's not exactly a no-op. Previously, the QemuOpts would only be freed
when the device is destroying, now we delete it immediately after
creating the device. This could matter in some cases.
The one case I was aware of is that QemuOpts used to be responsible for
checking for duplicate IDs. Obviously, it can't do this job any more
when we call qemu_opts_del() right after creating the device. This is
the reason for patch 6.
> The opts list is, eg, traversed in hw/net/virtio-net.c in the function
> failover_find_primary_device_id() which may be called during the
> virtio_net_set_features() (a TYPE_VIRTIO_NET method).
> I do not have the knowledge to tell when this method is called. But If this
> is after we create the devices. Then the list will be empty at this point
> now.
>
> It seems, there are 2 other calling sites of
> "qemu_opts_foreach(qemu_find_opts("device"), [...]" in net/vhost-user.c and
> net/vhost-vdpa.c
Yes, you are right. These callers probably need to be changed. Going
through the command line options rather than looking at the actual
device objects that exist doesn't feel entirely clean anyway.
Kevin
Re: [PATCH] hw/misc: Add a virtual pci device to dynamically attach memory to QEMU
On Mon, Sep 27, 2021 at 10:27:06AM +0200, Stefan Hajnoczi (stefa...@redhat.com)
wrote:
> On Sun, Sep 26, 2021 at 10:16:14AM +0800, David Dai wrote:
> > Add a virtual pci to QEMU, the pci device is used to dynamically attach
> > memory
> > to VM, so driver in guest can apply host memory in fly without
> > virtualization
> > management software's help, such as libvirt/manager. The attached memory is
> > isolated from System RAM, it can be used in heterogeneous memory management
> > for
> > virtualization. Multiple VMs dynamically share same computing device memory
> > without memory overcommit.
> >
> > Signed-off-by: David Dai
>
> CCing David Hildenbrand (virtio-balloon and virtio-mem) and Igor
> Mammedov (host memory backend).
>
> > ---
> > docs/devel/dynamic_mdev.rst | 122 ++
> > hw/misc/Kconfig | 5 +
> > hw/misc/dynamic_mdev.c | 456
> > hw/misc/meson.build | 1 +
> > 4 files changed, 584 insertions(+)
> > create mode 100644 docs/devel/dynamic_mdev.rst
> > create mode 100644 hw/misc/dynamic_mdev.c
> >
> > diff --git a/docs/devel/dynamic_mdev.rst b/docs/devel/dynamic_mdev.rst
> > new file mode 100644
> > index 00..8e2edb6600
> > --- /dev/null
> > +++ b/docs/devel/dynamic_mdev.rst
> > @@ -0,0 +1,122 @@
> > +Motivation:
> > +In heterogeneous computing system, accelorator generally exposes its device
>
> s/accelorator/accelerator/
>
> (There are missing articles and small grammar tweaks that could be made,
> but I'm skipping the English language stuff for now.)
>
Thank you for your review.
> > +memory to host via PCIe and CXL.mem(Compute Express Link) to share memory
> > +between host and device, and these memory generally are uniformly managed
> > by
> > +host, they are called HDM (host managed device memory), further SVA (share
> > +virtual address) can be achieved on this base. One computing device may be
> > used
>
> Is this Shared Virtual Addressing (SVA) (also known as Shared Virtual
> Memory)? If yes, please use the exact name ("Shared Virtual Addressing",
> not "share virtual address") so that's clear and the reader can easily
> find out more information through a web search.
>
Yes, you are right.
> > +by multiple virtual machines if it supports SRIOV, to efficiently use
> > device
> > +memory in virtualization, each VM allocates device memory on-demand without
> > +overcommit, but how to dynamically attach host memory resource to VM. A
> > virtual
>
> I cannot parse this sentence. Can you rephrase it and/or split it into
> multiple sentences?
>
> > +PCI device, dynamic_mdev, is introduced to achieve this target.
> > dynamic_mdev
>
> I suggest calling it "memdev" instead of "mdev" to prevent confusion
> with VFIO mdev.
>
I agree your suggestion.
I will make changes according to your comments at new patch.
> > +has a big bar space which size can be assigned by user when creating VM,
> > the
> > +bar doesn't have backend memory at initialization stage, later driver in
> > guest
> > +triggers QEMU to map host memory to the bar space. how much memory, when
> > and
> > +where memory will be mapped to are determined by guest driver, after device
> > +memory has been attached to the virtual PCI bar, application in guest can
> > +access device memory by the virtual PCI bar. Memory allocation and
> > negotiation
> > +are left to guest driver and memory backend implementation. dynamic_mdev
> > is a
> > +mechanism which provides significant benefits to heterogeneous memory
> > +virtualization.
>
> David and Igor: please review this design. I'm not familiar enough with
> the various memory hotplug and ballooning mechanisms to give feedback on
> this.
>
> > +Implementation:
> > +dynamic_mdev device has two bars, bar0 and bar2. bar0 is a 32-bit register
> > bar
> > +which used to host control register for control and message communication,
> > Bar2
> > +is a 64-bit mmio bar, which is used to attach host memory to, the bar size
> > can
> > +be assigned via parameter when creating VM. Host memory is attached to
> > this bar
> > +via mmap API.
> > +
> > +
> > + VM1 VM2
> > + -----
> > +| application | | application |
> > +| | | |
> > +|---| |--|
> > +| guest driver | | guest driver |
> > +| |--|| | | -| |
> > +| | pci mem bar || | | pci mem bar | |
> > + ---|--|- ---|--|---
> > + --- -- --
> > +|| | | | | | |
> > + --- -- --
> > +\ /
> > + \/
> > + \ /
> > + \
Re: [PATCH v3 0/5] introduce QArray
On Dienstag, 31. August 2021 14:25:04 CEST Christian Schoenebeck wrote: > On Dienstag, 31. August 2021 13:58:02 CEST Greg Kurz wrote: > > On Thu, 26 Aug 2021 14:47:26 +0200 > > > > Christian Schoenebeck wrote: > > > Patches 1 and 2 introduce include/qemu/qarray.h which implements a deep > > > auto free mechanism for arrays. See commit log of patch 1 for a detailed > > > explanation and motivation for introducing QArray. > > > > > > Patches 3..5 are provided (e.g. as example) for 9p being the first user > > > of > > > this new QArray API. These particular patches 3..5 are rebased on my > > > current 9p queue: https://github.com/cschoenebeck/qemu/commits/9p.next > > > > > which are basically just the following two queued patches: > > This looks nice indeed but I have the impression the same could be > > achieved using glib's g_autoptr framework with less code being added > > to QEMU (at the cost of being less generic maybe). > > I haven't seen a way doing this with glib, except of GArray which has some > disadvantages. But who knows, maybe I was missing something. Ping Let's do this? > > Anyway, we should likely sort out the SEGV issue you're hitting > > before going forward with supplementary changes in v9fs_walk(). > > Yeah, let's postpone this series here. I'll look into the Twalk crash issue > more closely today and will get back on that issue first. > > Best regards, > Christian Schoenebeck
Re: [PATCH v12 10/10] arm: tcg: Adhere to SMCCC 1.3 section 5.2
On Thu, 16 Sept 2021 at 16:54, Alexander Graf wrote: > > The SMCCC 1.3 spec section 5.2 says > > The Unknown SMC Function Identifier is a sign-extended value of (-1) > that is returned in the R0, W0 or X0 registers. An implementation must > return this error code when it receives: > > * An SMC or HVC call with an unknown Function Identifier > * An SMC or HVC call for a removed Function Identifier > * An SMC64/HVC64 call from AArch32 state > > To comply with these statements, let's always return -1 when we encounter > an unknown HVC or SMC call. > > Signed-off-by: Alexander Graf > Reviewed-by: Peter Maydell Applied this final patch to target-arm.next now we've sorted out the problem with the orangepi, thanks. -- PMM
Re: [PATCH] hw: Add a 'Sensor devices' qdev category
On Mon, Sep 27, 2021 at 12:15:18AM +0200, Philippe Mathieu-Daudé wrote:
> Sensors models are listed in the 'Misc devices' category.
> Move them to their own category.
>
> For the devices in the hw/sensor/ directory, the category
> is obvious.
>
> hw/arm/z2.c models the AER915 model which is described
> on [*] as:
>
> The 14-pin chip marked AER915 just below the expansion
> port is a 80C51-type microcontroller, similar to Philips
> P89LPC915. It has an 8-bit A/D which is used to determine
> which of six buttons are pressed on the resistor-network
> wired remote. It communicates with the main cpu via I2C.
>
> It was introduced in commit 3bf11207c06 ("Add support for
> Zipit Z2 machine") with this comment:
>
> 248 static uint8_t aer915_recv(I2CSlave *slave)
> 249 {
> ...
> 253 switch (s->buf[0]) {
> 254 /* Return hardcoded battery voltage,
> 255 * 0xf0 means ~4.1V
> 256 */
> 257 case 0x02:
> 258 retval = 0xf0;
> 259 break;
>
> For QEMU the AER915 is a very simple sensor model.
>
> [*] https://www.bealecorner.org/best/measure/z2/index.html
>
> Signed-off-by: Philippe Mathieu-Daudé
This makes sense to me. I'd like to hear from others on this.
-corey
> ---
> include/hw/qdev-core.h | 1 +
> hw/arm/z2.c| 1 +
> hw/sensor/adm1272.c| 1 +
> hw/sensor/dps310.c | 1 +
> hw/sensor/emc141x.c| 1 +
> hw/sensor/max34451.c | 2 ++
> hw/sensor/tmp105.c | 1 +
> hw/sensor/tmp421.c | 1 +
> softmmu/qdev-monitor.c | 1 +
> 9 files changed, 10 insertions(+)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 34c8a7506a1..f6241212247 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -26,6 +26,7 @@ typedef enum DeviceCategory {
> DEVICE_CATEGORY_SOUND,
> DEVICE_CATEGORY_MISC,
> DEVICE_CATEGORY_CPU,
> +DEVICE_CATEGORY_SENSOR,
> DEVICE_CATEGORY_MAX
> } DeviceCategory;
>
> diff --git a/hw/arm/z2.c b/hw/arm/z2.c
> index 9c1e876207b..62db9741106 100644
> --- a/hw/arm/z2.c
> +++ b/hw/arm/z2.c
> @@ -288,6 +288,7 @@ static void aer915_class_init(ObjectClass *klass, void
> *data)
> k->recv = aer915_recv;
> k->send = aer915_send;
> dc->vmsd = _aer915_state;
> +set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
> }
>
> static const TypeInfo aer915_info = {
> diff --git a/hw/sensor/adm1272.c b/hw/sensor/adm1272.c
> index 7310c769be2..2942ac75f90 100644
> --- a/hw/sensor/adm1272.c
> +++ b/hw/sensor/adm1272.c
> @@ -518,6 +518,7 @@ static void adm1272_class_init(ObjectClass *klass, void
> *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> PMBusDeviceClass *k = PMBUS_DEVICE_CLASS(klass);
>
> +set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
> dc->desc = "Analog Devices ADM1272 Hot Swap controller";
> dc->vmsd = _adm1272;
> k->write_data = adm1272_write_data;
> diff --git a/hw/sensor/dps310.c b/hw/sensor/dps310.c
> index d60a18ac41b..1e24a499b38 100644
> --- a/hw/sensor/dps310.c
> +++ b/hw/sensor/dps310.c
> @@ -208,6 +208,7 @@ static void dps310_class_init(ObjectClass *klass, void
> *data)
> k->send = dps310_tx;
> dc->reset = dps310_reset;
> dc->vmsd = _dps310;
> +set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
> }
>
> static const TypeInfo dps310_info = {
> diff --git a/hw/sensor/emc141x.c b/hw/sensor/emc141x.c
> index 7ce8f4e9794..4202d8f185a 100644
> --- a/hw/sensor/emc141x.c
> +++ b/hw/sensor/emc141x.c
> @@ -270,6 +270,7 @@ static void emc141x_class_init(ObjectClass *klass, void
> *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
>
> +set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
> dc->reset = emc141x_reset;
> k->event = emc141x_event;
> k->recv = emc141x_rx;
> diff --git a/hw/sensor/max34451.c b/hw/sensor/max34451.c
> index a91d8bd487c..8300bf4ff43 100644
> --- a/hw/sensor/max34451.c
> +++ b/hw/sensor/max34451.c
> @@ -751,6 +751,8 @@ static void max34451_class_init(ObjectClass *klass, void
> *data)
> ResettableClass *rc = RESETTABLE_CLASS(klass);
> DeviceClass *dc = DEVICE_CLASS(klass);
> PMBusDeviceClass *k = PMBUS_DEVICE_CLASS(klass);
> +
> +set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
> dc->desc = "Maxim MAX34451 16-Channel V/I monitor";
> dc->vmsd = _max34451;
> k->write_data = max34451_write_data;
> diff --git a/hw/sensor/tmp105.c b/hw/sensor/tmp105.c
> index 20564494899..43d79b9eeec 100644
> --- a/hw/sensor/tmp105.c
> +++ b/hw/sensor/tmp105.c
> @@ -305,6 +305,7 @@ static void tmp105_class_init(ObjectClass *klass, void
> *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
>
> +set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
> dc->realize = tmp105_realize;
> k->event = tmp105_event;
> k->recv = tmp105_rx;
> diff --git a/hw/sensor/tmp421.c b/hw/sensor/tmp421.c
> index
Re: [PATCH v6 4/5] block/nbd: drop connection_co
24.09.2021 20:44, Eric Blake wrote: On Thu, Sep 02, 2021 at 01:38:04PM +0300, Vladimir Sementsov-Ogievskiy wrote: OK, that's a big rewrite of the logic. And a time-consuming review on my part! Pre-patch we have an always running coroutine - connection_co. It does reply receiving and reconnecting. And it leads to a lot of difficult and unobvious code around drained sections and context switch. We also abuse bs->in_flight counter which is increased for connection_co and temporary decreased in points where we want to allow drained section to begin. One of these place is in another file: in nbd_read_eof() in nbd/client.c. We also cancel reconnect and requests waiting for reconnect on drained begin which is not correct. And this patch fixes that. Let's finally drop this always running coroutine and go another way: do both reconnect and receiving in request coroutines. The detailed list of changes below (in the sequence of diff hunks). Well, depending on the git order file in use ;) 1. receiving coroutines are woken directly from nbd_channel_error, when we change s->state 2. nbd_co_establish_connection_cancel(): we don't have drain_begin now, and in nbd_teardown_connection() all requests should already be finished (and reconnect is done from request). So nbd_co_establish_connection_cancel() is called from nbd_cancel_in_flight() (to cancel the request that is doing nbd_co_establish_connection()) and from reconnect_delay_timer_cb() (previously we didn't need it, as reconnect delay only should cancel active requests not the reconnection itself. But now reconnection Missing ) itself is done in the separate thread (we now call nbd_client_connection_enable_retry() in nbd_open()), and we need to cancel the requests that waits in nbd_co_establish_connection() Singular/plural disagreement. I think the intended meaning is 'requests that wait' and not 'request that waits'. now). 2. We do receive headers in request coroutine. But we also should Second point 2; I'll call it 2A below, because it looks related to point 8. Ohh. OK. dispatch replies for another pending requests. So, s/another/other/ nbd_connection_entry() is turned into nbd_receive_replies(), which does reply dispatching until it receive another request headers, and s/until/while/, s/another/other/ returns when it receive the requested header. receives 3. All old staff around drained sections and context switch is dropped. In details: - we don't need to move connection_co to new aio context, as we don't have connection_co anymore - we don't have a fake "request" of connection_co (extra increasing in_flight), so don't care with it in drain_begin/end - we don't stop reconnection during drained section anymore. This means that drain_begin may wait for a long time (up to reconnect_delay). But that's an improvement and more correct behavior see below[*] 4. In nbd_teardown_connection() we don't have to wait for connection_co, as it is dropped. And cleanup for s->ioc and nbd_yank is moved here from removed connection_co. 5. In nbd_co_do_establish_connection() we now should handle NBD_CLIENT_CONNECTING_NOWAIT: if new request comes when we are in NBD_CLIENT_CONNECTING_NOWAIT, it still should call nbd_co_establish_connection() (who knows, maybe connection already established by thread in background). But we shouldn't wait: if maybe the connection was already established by another thread in the background "another thread" sounds vague. I think better: by connection thread. nbd_co_establish_connection() can't return new channel immediately the request should fail (we are in NBD_CLIENT_CONNECTING_NOWAIT state). 6. nbd_reconnect_attempt() is simplified: it's now easier to wait for other requests in the caller, so here we just assert that fact. Also delay time is now initialized here: we can easily detect first attempt and start a timer. 7. nbd_co_reconnect_loop() is dropped, we don't need it. Reconnect retries are fully handle by thread (nbd/client-connection.c), delay timer we initialize in nbd_reconnect_attempt(), we don't have to bother with s->drained and friends. nbd_reconnect_attempt() now called from nbd_co_send_request(). A lot going on there, but it's making sense so far. 8. nbd_connection_entry is dropped: reconnect is now handled by nbd_co_send_request(), receiving reply is now handled by nbd_receive_replies(): all handled from request coroutines. 9. So, welcome new nbd_receive_replies() called from request coroutine, that receives reply header instead of nbd_connection_entry(). Like with sending requests, only one coroutine may receive in a moment. So we introduce receive_mutex, which is locked around nbd_receive_reply(). It also protects some related fields. Still, full audit of thread-safety in nbd
Re: [PATCH] hw/misc: Add a virtual pci device to dynamically attach memory to QEMU
On Mon, Sep 27, 2021 at 11:07:43AM +0200, David Hildenbrand (da...@redhat.com) wrote: > > CAUTION: This email originated from outside of the organization. Do not > click links or open attachments unless you recognize the sender and know the > content is safe. > > > On 27.09.21 10:27, Stefan Hajnoczi wrote: > > On Sun, Sep 26, 2021 at 10:16:14AM +0800, David Dai wrote: > > > Add a virtual pci to QEMU, the pci device is used to dynamically attach > > > memory > > > to VM, so driver in guest can apply host memory in fly without > > > virtualization > > > management software's help, such as libvirt/manager. The attached memory > > > is > > We do have virtio-mem to dynamically attach memory to a VM. It could be > extended by a mechanism for the VM to request more/less memory, that's > already a planned feature. But yeah, virito-mem memory is exposed as > ordinary system RAM, not only via a BAR to mostly be managed by user space > completely. > I wish virtio-mem can solve our problem, but it is a dynamic allocation mechanism for system RAM in virtualization. In heterogeneous computing environments, the attached memory usually comes from computing device, it should be managed separately. we doesn't hope Linux MM controls it. > > > isolated from System RAM, it can be used in heterogeneous memory > > > management for > > > virtualization. Multiple VMs dynamically share same computing device > > > memory > > > without memory overcommit. > > This sounds a lot like MemExpand/MemLego ... am I right that this is the > original design? I recall that VMs share a memory region and dynamically > agree upon which part of the memory region a VM uses. I further recall that > there were malloc() hooks that would dynamically allocate such memory in > user space from the shared memory region. > Thank you for telling me about Memexpand/MemLego, I have carefully read the paper. some ideas from it are same as this patch, such as software model and stack, but it may have a security risk that whole shared memory is visible to all VMs. --- application --- memory management driver --- pci driver --- virtual pci device --- > I can see some use cases for it, although the shared memory design isn't > what you typically want in most VM environments. > The original design for this patch is to share a computing device among multipile VMs. Each VM runs a computing application(for example, OpenCL application) Our computing device can support a few applications in parallel. In addition, it supports SVM(shared virtual memory) via IOMMU/ATS/PASID/PRI. Device exposes its memory to host vis PCIe bar or CXL.mem, host constructs memory pool to uniformly manage device memory, then attach device memory to VM via a virtual PCI device. but we don't know how much memory should be assigned when creating VM, so we hope memory is attached to VM on-demand. driver in guest triggers memory attaching, but not outside virtualization management software. so the original requirements are: 1> The managed memory comes from device, it should be isolated from system RAM 2> The memory can be dynamically attached to VM in fly 3> The attached memory supports SVM and DMA operation with IOMMU Thank you very much. Best Regards, David Dai > -- > Thanks, > > David / dhildenb > >
Re: [PATCH v7 12/40] accel/qtest: Implement AccelOpsClass::has_work()
On 25/09/2021 18:01, Philippe Mathieu-Daudé wrote:
On 9/25/21 17:32, Richard Henderson wrote:
On 9/25/21 11:27 AM, Philippe Mathieu-Daudé wrote:
+static bool qtest_cpu_has_work(CPUState *cpu)
+{
+ g_assert_not_reached();
+}
Sigh, this triggers:
Running test qtest-i386/cpu-plug-test
**
ERROR:../accel/qtest/qtest.c:52:qtest_cpu_has_work: code should not be reached
ERROR qtest-i386/cpu-plug-test - Bail out!
ERROR:../accel/qtest/qtest.c:52:qtest_cpu_has_work: code should not be reached
Broken pipe
Ha ha, yes. You beat me to the reply within minutes.
I suppose it is in my interest to 'return false' here and call it
a day...
I *think* that's the right thing, but I could see maybe "true" also makes sense. I'll
try and have a closer look.
So first I tested using "-machine pc,accel=qtest" -> no crash.
Looking closely at how check-qtest calls QEMU, it does:
"-machine pc -accel qtest". Isn't the sugar property supposed
to work that way?
Then the backtrace is:
Thread 5 "qemu-system-i38" hit Breakpoint 1, qtest_cpu_has_work (cpu=0x56a08400) at
accel/qtest/qtest.c:52
52 g_assert_not_reached();
(gdb) bt
#0 qtest_cpu_has_work (cpu=0x56a08400) at accel/qtest/qtest.c:52
#1 0x55c330ba in cpu_has_work (cpu=0x56a08400) at
softmmu/cpus.c:254
#2 0x55c32ac8 in cpu_thread_is_idle (cpu=0x56a08400) at
softmmu/cpus.c:91
#3 0x55c33584 in qemu_wait_io_event (cpu=0x56a08400) at
softmmu/cpus.c:417
#4 0x55d8a7f4 in dummy_cpu_thread_fn (arg=0x56a08400) at
accel/dummy-cpus.c:53
#5 0x55f469f6 in qemu_thread_start (args=0x574edae0) at
util/qemu-thread-posix.c:557
#6 0x74ff3299 in start_thread () at /lib64/libpthread.so.0
#7 0x74f1b353 in clone () at /lib64/libc.so.6
dummy_cpu_thread_fn() content didn't change since its introduction
in commit c7f0f3b1c82 ("qtest: add test framework"):
"The idea behind qtest is pretty simple. Instead of executing
a CPU via TCG or KVM, rely on an external process to send events
to the device model that the CPU would normally generate."
Based on that description, qtest should provide a command to notify
whether the CPU has work to do or not.
Meanwhile, no qtest command = no work = 'return false'.
In fact, with the migration test we have CPU running (it's the purpose of the test), qtest
allows to have several accelerators, "-accel qtest" adds the qtest control in QEMU and we
have then a KVM or TCG accel to be able to run some qtest commands with a CPU running.
Thanks,
Laurent
[PULL 1/5] docs/nvdimm: Update nvdimm option value in machine example
From: Pankaj Gupta Update nvdimm option value in example command from "-machine pc,nvdimm" to "-machine pc,nvdimm=on" as former complains with the below error: "qemu-system-x86_64: -machine pc,nvdimm: Expected '=' after parameter 'nvdimm'" Reviewed-by: Laurent Vivier Signed-off-by: Pankaj Gupta Message-Id: <20210923103015.135262-1-pankaj.gupta.li...@gmail.com> Signed-off-by: Laurent Vivier --- docs/nvdimm.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt index 0aae682be3eb..fd7773dc5abb 100644 --- a/docs/nvdimm.txt +++ b/docs/nvdimm.txt @@ -15,7 +15,7 @@ backend (i.e. memory-backend-file and memory-backend-ram). A simple way to create a vNVDIMM device at startup time is done via the following command line options: - -machine pc,nvdimm + -machine pc,nvdimm=on -m $RAM_SIZE,slots=$N,maxmem=$MAX_SIZE -object memory-backend-file,id=mem1,share=on,mem-path=$PATH,size=$NVDIMM_SIZE,readonly=off -device nvdimm,id=nvdimm1,memdev=mem1,unarmed=off -- 2.31.1
Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
On Mon, 27 Sept 2021 at 10:40, Philippe Mathieu-Daudé wrote:
>
> On Mon, Sep 27, 2021 at 11:15 AM Peter Maydell
> wrote:
> > On Sun, 26 Sept 2021 at 23:04, Philippe Mathieu-Daudé
> > wrote:
> > > Reported-by: Warner Losh
> > > Signed-off-by: Philippe Mathieu-Daudé
> > > ---
> > > bsd-user/meson.build | 4
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/bsd-user/meson.build b/bsd-user/meson.build
> > > index 03695493408..a7607e1c884 100644
> > > --- a/bsd-user/meson.build
> > > +++ b/bsd-user/meson.build
> > > @@ -1,3 +1,7 @@
> > > +if not config_host.has_key('CONFIG_BSD')
> > > + subdir_done()
> > > +endif
> > > +
> > > bsd_user_ss.add(files(
> > >'bsdload.c',
> > >'elfload.c',
> >
> >
> > So, what's the reason for this change?
>
> https://lore.kernel.org/qemu-devel/canczdfprc16ezjqcwjmyeapx6eym9nxsoqatbagr+czis4r...@mail.gmail.com/
>
> linux-user/meson.build is evaluated on bsd, and bsd-user/meson.build on Linux.
True, but "meson.build is evaluated but just does nothing or
adds files to a sourceset that isn't used" is pretty common
(hw/pci/meson.build is evaluated even if we're not building
a system with PCI support, for example). If there's a reason
why bsd-user and linux-user are special we should explain it,
I think.
thanks
-- PMM
[PULL 4/5] hmp: Drop a bogus sentence from set_password's documentation
From: Markus Armbruster
Signed-off-by: Markus Armbruster
Reviewed-by: Daniel P. Berrangé
Reviewed-by: Gerd Hoffmann
Message-Id: <20210909081219.308065-3-arm...@redhat.com>
Signed-off-by: Laurent Vivier
---
hmp-commands.hx | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8e45bce2cd9d..cf723c69acb7 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1522,12 +1522,11 @@ ERST
SRST
``set_password [ vnc | spice ] password [ action-if-connected ]``
- Change spice/vnc password. Use zero to make the password stay valid
- forever. *action-if-connected* specifies what should happen in
- case a connection is established: *fail* makes the password change
- fail. *disconnect* changes the password and disconnects the
- client. *keep* changes the password and keeps the connection up.
- *keep* is the default.
+ Change spice/vnc password. *action-if-connected* specifies what
+ should happen in case a connection is established: *fail* makes the
+ password change fail. *disconnect* changes the password and
+ disconnects the client. *keep* changes the password and keeps the
+ connection up. *keep* is the default.
ERST
{
--
2.31.1
Re: [PATCH v10 11/14] machine: Make smp_parse generic enough for all arches
On Sun, Sep 26, 2021 at 04:45:38PM +0800, Yanan Wang wrote:
> Currently the only difference between smp_parse and pc_smp_parse
> is the support of dies parameter and the related error reporting.
> With some arch compat variables like "bool dies_supported", we can
> make smp_parse generic enough for all arches and the PC specific
> one can be removed.
>
> Making smp_parse() generic enough can reduce code duplication and
> ease the code maintenance, and also allows extending the topology
> with more arch specific members (e.g., clusters) in the future.
>
> Suggested-by: Andrew Jones
> Signed-off-by: Yanan Wang
> Reviewed-by: Andrew Jones
> ---
> hw/core/machine.c | 110
> hw/i386/pc.c| 84 +
> include/hw/boards.h | 9
> 3 files changed, 100 insertions(+), 103 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index a21fcd7700..4b5c943f8e 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -15,6 +15,7 @@
> #include "qapi/qmp/qerror.h"
> #include "sysemu/replay.h"
> #include "qemu/units.h"
> +#include "qemu/cutils.h"
> #include "hw/boards.h"
> #include "hw/loader.h"
> #include "qapi/error.h"
> @@ -746,20 +747,87 @@ void machine_set_cpu_numa_node(MachineState *machine,
> }
> }
>
> +static char *cpu_topology_hierarchy(MachineState *ms)
> +{
> +MachineClass *mc = MACHINE_GET_CLASS(ms);
> +SMPCompatProps *smp_props = >smp_props;
> +char topo_msg[256] = "";
> +
> +/*
> + * Topology members should be ordered from the largest to the smallest.
> + * Concept of sockets/cores/threads is supported by default and will be
> + * reported in the hierarchy. Unsupported members will not be reported.
> + */
> +g_autofree char *sockets_msg = g_strdup_printf(
> +" * sockets (%u)", ms->smp.sockets);
> +pstrcat(topo_msg, sizeof(topo_msg), sockets_msg);
> +
> +if (smp_props->dies_supported) {
> +g_autofree char *dies_msg = g_strdup_printf(
> +" * dies (%u)", ms->smp.dies);
> +pstrcat(topo_msg, sizeof(topo_msg), dies_msg);
> +}
> +
> +g_autofree char *cores_msg = g_strdup_printf(
> +" * cores (%u)", ms->smp.cores);
> +pstrcat(topo_msg, sizeof(topo_msg), cores_msg);
> +
> +g_autofree char *threads_msg = g_strdup_printf(
> +" * threads (%u)", ms->smp.threads);
> +pstrcat(topo_msg, sizeof(topo_msg), threads_msg);
> +
> +return g_strdup_printf("%s", topo_msg + 3);
> +}
Mixing g_strdup_printf + pstrcat + fixed buffer is quite
unpleasant. This method is begging to use 'GString' APIs
for formatting.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 11/11] Deprecate stable non-JSON -device and -object
Am 27.09.2021 um 10:21 hat Daniel P. Berrangé geschrieben: > On Mon, Sep 27, 2021 at 10:15:43AM +0200, Paolo Bonzini wrote: > > On 24/09/21 11:04, Kevin Wolf wrote: > > > We want to switch both from QemuOpts to the keyval parser in the future, > > > which results in some incompatibilities, mainly around list handling. > > > Mark the non-JSON version of both as unstable syntax so that management > > > tools switch to JSON and we can later make the change without breaking > > > things. > > > > Maybe we need a different section for unstable syntaxes, rather than > > overloading deprecated.rst? > > This case feels like it hits two scenarios - we want to declare it > unstable, which is something we should document in qemu-options.hx. Actually, I think a section for unstable syntaxes or generally compatibility promises wouldn't hurt. When I checked, I couldn't find any documentation about the support status of HMP either. Basically, I imagine HMP and non-JSON -device/-object would be on the same level: We don't change things without a reason, but if we do want to change things, compatibility isn't an argument against making the change. > We want to also to warn of specific breakage when the impl changes > which is something suitable for deprecations. We don't do this for HMP either for individual changes. Basically this deprecation notice was meant to make people aware that we're lowering the support status from a long-term stable interface to HMP-like. Kevin
Re: [PATCH v5 03/26] hostmem: Add hostmem-epc as a backend for SGX EPC
On 9/24/21 13:24, Paolo Bonzini wrote: > From: Sean Christopherson > > EPC (Enclave Page Cahe) is a specialized type of memory used by Intel Typo "Enclave Page Cache". > SGX (Software Guard Extensions). The SDM desribes EPC as: > > The Enclave Page Cache (EPC) is the secure storage used to store > enclave pages when they are a part of an executing enclave. For an > EPC page, hardware performs additional access control checks to > restrict access to the page. After the current page access checks > and translations are performed, the hardware checks that the EPC > page is accessible to the program currently executing. Generally an > EPC page is only accessed by the owner of the executing enclave or > an instruction which is setting up an EPC page. > > Because of its unique requirements, Linux manages EPC separately from > normal memory. Similar to memfd, the device /dev/sgx_vepc can be > opened to obtain a file descriptor which can in turn be used to mmap() > EPC memory. > > Signed-off-by: Sean Christopherson > Signed-off-by: Yang Zhong > Message-Id: <20210719112136.57018-3-yang.zh...@intel.com> > Signed-off-by: Paolo Bonzini > --- > backends/hostmem-epc.c| 82 +++ > backends/meson.build | 1 + > include/hw/i386/hostmem-epc.h | 28 > 3 files changed, 111 insertions(+) > create mode 100644 backends/hostmem-epc.c > create mode 100644 include/hw/i386/hostmem-epc.h
Re: [PATCH v15] qapi: introduce 'query-x86-cpuid' QMP command.
On 22/09/2021 09.41, Vladimir Sementsov-Ogievskiy wrote: Ping. Hi! Any chance for this to land? Sorry if I missed the outcome of the discussion - but what about the idea to introduce this with a "x-" prefix first, since there was no 100% certainty that we really fully want to support this command in the current fashion? Thomas
[PATCH v1 1/1] hw/riscv: shakti_c: Mark as not user creatable
From: Alistair Francis
Mark the shakti_c machine as not user creatable as it uses serial_hd.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/639
Signed-off-by: Alistair Francis
---
hw/riscv/shakti_c.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c
index 2f084d3c8d..603992d442 100644
--- a/hw/riscv/shakti_c.c
+++ b/hw/riscv/shakti_c.c
@@ -150,6 +150,8 @@ static void shakti_c_soc_class_init(ObjectClass *klass,
void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
dc->realize = shakti_c_soc_state_realize;
+/* Reason: Uses serial_hds in realize function, thus can't be used twice */
+dc->user_creatable = false;
}
static void shakti_c_soc_instance_init(Object *obj)
--
2.31.1
Re: [PATCH] configure: Loosen GCC requirement from 7.5.0 to 7.4.0
On Sun, Sep 26, 2021 at 07:22:14AM +, nia wrote: > As discussed in issue 614, we're shipping GCC 7.4.0 as the > system compiler in NetBSD 9, the most recent stable branch, > and are still actively interested in QEMU on this platform. > > The differences between GCC 7.5.0 and 7.4.0 are trivial. > > Signed-off-by: Nia Alarie > --- > configure | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/configure b/configure > index 1043ccce4f..f918ad67a1 100755 > --- a/configure > +++ b/configure > @@ -2094,8 +2094,8 @@ cat > $TMPC << EOF > # endif > # endif > #elif defined(__GNUC__) && defined(__GNUC_MINOR__) > -# if __GNUC__ < 7 || (__GNUC__ == 7 && __GNUC_MINOR__ < 5) > -# error You need at least GCC v7.5.0 to compile QEMU > +# if __GNUC__ < 7 || (__GNUC__ == 7 && __GNUC_MINOR__ < 4) > +# error You need at least GCC v7.4.0 to compile QEMU > # endif > #else > # error You either need GCC or Clang to compiler QEMU You missed another version number change just after here Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 0/6] iotests: update environment and linting configuration
Am 24.09.2021 um 20:13 hat John Snow geschrieben: > On Thu, Sep 23, 2021 at 2:07 PM John Snow wrote: > > > GitLab: https://gitlab.com/jsnow/qemu/-/commits/python-package-iotest-pt1 > > CI: https://gitlab.com/jsnow/qemu/-/pipelines/376236687 > > > > This series partially supersedes: > > [PATCH v3 00/16] python/iotests: Run iotest linters during Python CI' > > > > Howdy, this is good stuff we want even if we aren't yet in agreement > > about the best way to run iotest 297 from CI. > > > > - Update linting config to tolerate pylint 2.11.1 > > - Eliminate sys.path hacking in individual test files > > - make mypy execution in test 297 faster > > > > The rest of the actual "run at CI time" stuff can get handled separately > > and later pending some discussion on the other series. > > > > V2: > > > > 001/6:[0011] [FC] 'iotests: add 'qemu' package location to PYTHONPATH in > > testenv' > > 002/6:[0025] [FC] 'iotests: add warning for rogue 'qemu' packages' > > > > - Squashed in a small optimization from Vladimir to 001, kept R-Bs. > > - Fixed the package detection logic to not panic if it can't find > > 'qemu' at all (kwolf) > > - Updated commit messages for the first two patches. > Patch 2 can just be dropped, and everything else is reviewed, so I think > this can be staged at your leisure. Thanks, applied to the block branch (without patch 2). Kevin
Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
On Mon, Sep 27, 2021 at 12:01:02AM +0200, Philippe Mathieu-Daudé wrote:
> Reported-by: Warner Losh
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> bsd-user/meson.build | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/bsd-user/meson.build b/bsd-user/meson.build
> index 03695493408..a7607e1c884 100644
> --- a/bsd-user/meson.build
> +++ b/bsd-user/meson.build
> @@ -1,3 +1,7 @@
> +if not config_host.has_key('CONFIG_BSD')
> + subdir_done()
> +endif
> +
> bsd_user_ss.add(files(
>'bsdload.c',
>'elfload.c',
If we look at the big picture across the root meson.build, and this
meson.build we have
bsd_user_ss = ss.source_set()
...
bsd_user_ss.add(files(
'bsdload.c',
'elfload.c',
'main.c',
'mmap.c',
'signal.c',
'strace.c',
'syscall.c',
'uaccess.c',
))
...
bsd_user_ss.add(files('gdbstub.c'))
specific_ss.add_all(when: 'CONFIG_BSD_USER', if_true: bsd_user_ss)
So without this change, we're already correctly dropping bsd_user_ss
in its entirity, when not on BSD.
With this change, we're dropping some, but not all, of bsd_user_ss
files - gdbstub.c remains.
So this change on its own doesn't make a whole lot of sense.
If we look at linux-user/meson.build though things are more complex.
There we have alot of sub-dirs, and meson.biuld in those dirs adds
generators for various files. So conceivably skipping linux-user
will mean we won't auto-generate files we don't need on non-Linux.
With that in mind, I think it makes conceptual sense to have this
bsd-user/meson.build change, for the purpose of design consistency,
even if it doesn't have any real world benefit for bsd-user today.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PULL 1/5] hw/pcie-root-port: Fix hotplug for PCI devices requiring IO
On Mon, Sep 27, 2021 at 10:33:42AM +0100, Daniel P. Berrangé wrote:
> On Tue, Aug 03, 2021 at 04:52:03PM -0400, Michael S. Tsirkin wrote:
> > From: Marcel Apfelbaum
> >
> > Q35 has now ACPI hotplug enabled by default for PCI(e) devices.
> > As opposed to native PCIe hotplug, guests like Fedora 34
> > will not assign IO range to pcie-root-ports not supporting
> > native hotplug, resulting into a regression.
> >
> > Reproduce by:
> > qemu-bin -M q35 -device pcie-root-port,id=p1 -monitor stdio
> > device_add e1000,bus=p1
> > In the Guest OS the respective pcie-root-port will have the IO range
> > disabled.
> >
> > Fix it by setting the "reserve-io" hint capability of the
> > pcie-root-ports so the firmware will allocate the IO range instead.
> >
> > Acked-by: Igor Mammedov
> > Signed-off-by: Marcel Apfelbaum
> > Message-Id: <20210802090057.1709775-1-mar...@redhat.com>
> > Reviewed-by: Michael S. Tsirkin
> > Signed-off-by: Michael S. Tsirkin
> > ---
> > hw/pci-bridge/gen_pcie_root_port.c | 5 +
> > 1 file changed, 5 insertions(+)
>
> This change, when combined with the switch to ACPI based hotplug by
> default, is responsible for a significant regression in QEMU 6.1.0
>
> It is no longer possible to have more than 15 pcie-root-port devices
> added to a q35 VM in 6.1.0. Before this I've had as many as 80+ devices
> present before I stopped trying to add more.
>
> https://gitlab.com/qemu-project/qemu/-/issues/641
>
> This regression is significant, because it has broken the out of the
> box default configuration that OpenStack uses for booting all VMs.
> They add 16 pcie-root-ports by defalt to allow empty slots for device
> hotplug under q35 [1].
Indeed, oops. Thanks for the report!
Going back and looking at seabios code, didn't we get confused?
Shouldn't we have reserved memory and not IO?
I see:
int resource_optional = pcie_cap && (type == PCI_REGION_TYPE_IO);
if (!sum && hotplug_support && !resource_optional)
sum = align; /* reserve min size for hot-plug */
generally maybe we should just add an ACPI-hotplug capability and
teach seabios about it?
Marcel?
> > diff --git a/hw/pci-bridge/gen_pcie_root_port.c
> > b/hw/pci-bridge/gen_pcie_root_port.c
> > index ec9907917e..20099a8ae3 100644
> > --- a/hw/pci-bridge/gen_pcie_root_port.c
> > +++ b/hw/pci-bridge/gen_pcie_root_port.c
> > @@ -28,6 +28,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(GenPCIERootPort,
> > GEN_PCIE_ROOT_PORT)
> > (GEN_PCIE_ROOT_PORT_AER_OFFSET + PCI_ERR_SIZEOF)
> >
> > #define GEN_PCIE_ROOT_PORT_MSIX_NR_VECTOR 1
> > +#define GEN_PCIE_ROOT_DEFAULT_IO_RANGE 4096
> >
> > struct GenPCIERootPort {
> > /*< private >*/
> > @@ -75,6 +76,7 @@ static bool gen_rp_test_migrate_msix(void *opaque, int
> > version_id)
> > static void gen_rp_realize(DeviceState *dev, Error **errp)
> > {
> > PCIDevice *d = PCI_DEVICE(dev);
> > +PCIESlot *s = PCIE_SLOT(d);
> > GenPCIERootPort *grp = GEN_PCIE_ROOT_PORT(d);
> > PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
> > Error *local_err = NULL;
> > @@ -85,6 +87,9 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
> > return;
> > }
> >
> > +if (grp->res_reserve.io == -1 && s->hotplug && !s->native_hotplug) {
> > +grp->res_reserve.io = GEN_PCIE_ROOT_DEFAULT_IO_RANGE;
> > +}
> > int rc = pci_bridge_qemu_reserve_cap_init(d, 0,
> >grp->res_reserve, errp);
> >
> > --
> > MST
> >
> >
>
> Regards,
> Daniel
>
> [1]
> https://github.com/openstack/tripleo-heat-templates/blob/7a6cd0640ec390a330f5699d8ed60f71b2a9f514/deployment/nova/nova-compute-container-puppet.yaml#L462-L472
> --
> |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v5 04/26] qom: Add memory-backend-epc ObjectOptions support
On Fri, Sep 24, 2021 at 08:56:40AM -0500, Eric Blake wrote:
> On Fri, Sep 24, 2021 at 01:24:47PM +0200, Paolo Bonzini wrote:
> > From: Yang Zhong
> >
> > Add the new 'memory-backend-epc' user creatable QOM object in
> > the ObjectOptions to support SGX since v6.1, or the sgx backend
> > object cannot bootup.
> >
> > Signed-off-by: Yang Zhong
> > Message-Id: <20210719112136.57018-4-yang.zh...@intel.com>
> > Signed-off-by: Paolo Bonzini
> > ---
> > qapi/qom.json | 19 +++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/qapi/qom.json b/qapi/qom.json
> > index a25616bc7a..0222bb4506 100644
> > --- a/qapi/qom.json
> > +++ b/qapi/qom.json
> > @@ -647,6 +647,23 @@
> > '*hugetlbsize': 'size',
> > '*seal': 'bool' } }
> >
> > +##
> > +# @MemoryBackendEpcProperties:
> > +#
> > +# Properties for memory-backend-epc objects.
> > +#
> > +# The @share boolean option is true by default with epc
> > +#
> > +# The @merge boolean option is false by default with epc
> > +#
> > +# The @dump boolean option is false by default with epc
> > +#
> > +# Since: 6.2
> > +##
> > +{ 'struct': 'MemoryBackendEpcProperties',
> > + 'base': 'MemoryBackendProperties',
> > + 'data': {} }
>
> Is the intent to add more members to data in later patches? Otherwise,...
No new members will be added. thanks! MemoryBackendProperties will replace
this.
Yang
>
> > +
> > ##
> > # @PrManagerHelperProperties:
> > #
> > @@ -797,6 +814,7 @@
> > { 'name': 'memory-backend-memfd',
> >'if': 'CONFIG_LINUX' },
> > 'memory-backend-ram',
> > +'memory-backend-epc',
> > 'pef-guest',
> > 'pr-manager-helper',
> > 'qtest',
> > @@ -855,6 +873,7 @@
> >'memory-backend-memfd': { 'type':
> > 'MemoryBackendMemfdProperties',
> >'if': 'CONFIG_LINUX' },
> >'memory-backend-ram': 'MemoryBackendProperties',
> > + 'memory-backend-epc': 'MemoryBackendEpcProperties',
>
> ...this could have just been MemoryBackendProperties.
Ditto, thanks!
Yang
>
> >'pr-manager-helper': 'PrManagerHelperProperties',
> >'qtest': 'QtestProperties',
> >'rng-builtin':'RngProperties',
> > --
> > 2.31.1
> >
> >
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
Re: [PATCH 1/2] hw/dma: sifive_pdma: Improve code readability for "!!foo & bar"
Hi Philippe,
On Mon, Sep 27, 2021 at 12:47 PM Philippe Mathieu-Daudé wrote:
>
> On 9/27/21 04:21, Bin Meng wrote:
> > GCC seems to be strict about processing pattern like "!!for & bar".
> > When 'bar' is not 0 or 1, it complains with -Werror=parentheses:
> >
> > suggest parentheses around operand of ‘!’ or change ‘&’ to ‘&&’ or ‘!’ to
> > ‘~’ [-Werror=parentheses]
> >
> > Add a () around "foo && bar", which also improves code readability.
> >
> > Signed-off-by: Bin Meng
> > ---
> >
> > hw/dma/sifive_pdma.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
> > index b4fd40573a..b8ec7621f3 100644
> > --- a/hw/dma/sifive_pdma.c
> > +++ b/hw/dma/sifive_pdma.c
> > @@ -243,7 +243,7 @@ static void sifive_pdma_write(void *opaque, hwaddr
> > offset,
> > offset &= 0xfff;
> > switch (offset) {
> > case DMA_CONTROL:
> > -claimed = !!s->chan[ch].control & CONTROL_CLAIM;
> > +claimed = !!(s->chan[ch].control & CONTROL_CLAIM);
>
> AFAIK in C logical NOT has precedence over bitwise AND, so IIUC
> compilers should read the current code as:
>
>claimed (!!s->chan[ch].control) & CONTROL_CLAIM;
>
> meaning this patch is doing more than "improve code readability",
> this is a logical change and likely a bug fix...
Yes, you are correct. Indeed this is a bug fix. I did not dig into the
operator precedence in detail. I will reword this in v2.
>
> BTW GCC suggestions are:
>
>claimed (!!s->chan[ch].control) & CONTROL_CLAIM;
>
>claimed (!!s->chan[ch].control) && CONTROL_CLAIM;
> >
> > if (!claimed && (value & CONTROL_CLAIM)) {
> > /* reset Next* registers */
> >
I was using GCC 9.3.0 on Ubuntu 20.04.
Regards,
Bin
Re: [PATCH 1/2] hw/dma: sifive_pdma: Improve code readability for "!!foo & bar"
Hi Markus,
On Mon, Sep 27, 2021 at 2:51 PM Markus Armbruster wrote:
>
> Bin Meng writes:
>
> > GCC seems to be strict about processing pattern like "!!for & bar".
> > When 'bar' is not 0 or 1, it complains with -Werror=parentheses:
> >
> > suggest parentheses around operand of ‘!’ or change ‘&’ to ‘&&’ or ‘!’ to
> > ‘~’ [-Werror=parentheses]
> >
> > Add a () around "foo && bar", which also improves code readability.
> >
> > Signed-off-by: Bin Meng
> > ---
> >
> > hw/dma/sifive_pdma.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
> > index b4fd40573a..b8ec7621f3 100644
> > --- a/hw/dma/sifive_pdma.c
> > +++ b/hw/dma/sifive_pdma.c
> > @@ -243,7 +243,7 @@ static void sifive_pdma_write(void *opaque, hwaddr
> > offset,
> > offset &= 0xfff;
> > switch (offset) {
> > case DMA_CONTROL:
> > -claimed = !!s->chan[ch].control & CONTROL_CLAIM;
> > +claimed = !!(s->chan[ch].control & CONTROL_CLAIM);
> >
> > if (!claimed && (value & CONTROL_CLAIM)) {
> > /* reset Next* registers */
>
> Old code
>
> first double-negate, mapping zero to zero, non-zero to one
> then mask, which does nothing, because CONTROL_CLAIM is 1
>
> New code:
>
> first mask, yielding 0 or 1
> then double-negate, which does nothing
>
> Looks like a bug fix to me. If I'm right, the commit message is wrong,
> and the double negate is redundant.
>
Thanks for the review. The double negate is not needed with
CONTROL_CLAIM which is 1, but is needed if the bit is in another
position.
Regards,
Bin
Re: [PATCH 11/11] Deprecate stable non-JSON -device and -object
On 24/09/21 11:04, Kevin Wolf wrote: We want to switch both from QemuOpts to the keyval parser in the future, which results in some incompatibilities, mainly around list handling. Mark the non-JSON version of both as unstable syntax so that management tools switch to JSON and we can later make the change without breaking things. Maybe we need a different section for unstable syntaxes, rather than overloading deprecated.rst? Paolo
Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
On Sun, 26 Sept 2021 at 23:04, Philippe Mathieu-Daudé wrote:
>
> Reported-by: Warner Losh
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> bsd-user/meson.build | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/bsd-user/meson.build b/bsd-user/meson.build
> index 03695493408..a7607e1c884 100644
> --- a/bsd-user/meson.build
> +++ b/bsd-user/meson.build
> @@ -1,3 +1,7 @@
> +if not config_host.has_key('CONFIG_BSD')
> + subdir_done()
> +endif
> +
> bsd_user_ss.add(files(
>'bsdload.c',
>'elfload.c',
So, what's the reason for this change? The commit messages and
the cover letter don't really explain it. Is this fixing a bug
(if so what?), a precaution to avoid possible future bugs,
fixing a performance issue with how long meson takes to run (if
so, how much effect does this have), or something else?
thanks
-- PMM
Re: [PULL 1/5] hw/pcie-root-port: Fix hotplug for PCI devices requiring IO
On Tue, Aug 03, 2021 at 04:52:03PM -0400, Michael S. Tsirkin wrote:
> From: Marcel Apfelbaum
>
> Q35 has now ACPI hotplug enabled by default for PCI(e) devices.
> As opposed to native PCIe hotplug, guests like Fedora 34
> will not assign IO range to pcie-root-ports not supporting
> native hotplug, resulting into a regression.
>
> Reproduce by:
> qemu-bin -M q35 -device pcie-root-port,id=p1 -monitor stdio
> device_add e1000,bus=p1
> In the Guest OS the respective pcie-root-port will have the IO range
> disabled.
>
> Fix it by setting the "reserve-io" hint capability of the
> pcie-root-ports so the firmware will allocate the IO range instead.
>
> Acked-by: Igor Mammedov
> Signed-off-by: Marcel Apfelbaum
> Message-Id: <20210802090057.1709775-1-mar...@redhat.com>
> Reviewed-by: Michael S. Tsirkin
> Signed-off-by: Michael S. Tsirkin
> ---
> hw/pci-bridge/gen_pcie_root_port.c | 5 +
> 1 file changed, 5 insertions(+)
This change, when combined with the switch to ACPI based hotplug by
default, is responsible for a significant regression in QEMU 6.1.0
It is no longer possible to have more than 15 pcie-root-port devices
added to a q35 VM in 6.1.0. Before this I've had as many as 80+ devices
present before I stopped trying to add more.
https://gitlab.com/qemu-project/qemu/-/issues/641
This regression is significant, because it has broken the out of the
box default configuration that OpenStack uses for booting all VMs.
They add 16 pcie-root-ports by defalt to allow empty slots for device
hotplug under q35 [1].
> diff --git a/hw/pci-bridge/gen_pcie_root_port.c
> b/hw/pci-bridge/gen_pcie_root_port.c
> index ec9907917e..20099a8ae3 100644
> --- a/hw/pci-bridge/gen_pcie_root_port.c
> +++ b/hw/pci-bridge/gen_pcie_root_port.c
> @@ -28,6 +28,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(GenPCIERootPort,
> GEN_PCIE_ROOT_PORT)
> (GEN_PCIE_ROOT_PORT_AER_OFFSET + PCI_ERR_SIZEOF)
>
> #define GEN_PCIE_ROOT_PORT_MSIX_NR_VECTOR 1
> +#define GEN_PCIE_ROOT_DEFAULT_IO_RANGE 4096
>
> struct GenPCIERootPort {
> /*< private >*/
> @@ -75,6 +76,7 @@ static bool gen_rp_test_migrate_msix(void *opaque, int
> version_id)
> static void gen_rp_realize(DeviceState *dev, Error **errp)
> {
> PCIDevice *d = PCI_DEVICE(dev);
> +PCIESlot *s = PCIE_SLOT(d);
> GenPCIERootPort *grp = GEN_PCIE_ROOT_PORT(d);
> PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
> Error *local_err = NULL;
> @@ -85,6 +87,9 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> +if (grp->res_reserve.io == -1 && s->hotplug && !s->native_hotplug) {
> +grp->res_reserve.io = GEN_PCIE_ROOT_DEFAULT_IO_RANGE;
> +}
> int rc = pci_bridge_qemu_reserve_cap_init(d, 0,
>grp->res_reserve, errp);
>
> --
> MST
>
>
Regards,
Daniel
[1]
https://github.com/openstack/tripleo-heat-templates/blob/7a6cd0640ec390a330f5699d8ed60f71b2a9f514/deployment/nova/nova-compute-container-puppet.yaml#L462-L472
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PULL 2/5] hw/loader: Restrict PC_ROM_* definitions to hw/i386/pc
From: Philippe Mathieu-Daudé The PC_ROM_* definitions are only used by the PC machine, and are irrelevant to the other architectures / machines. Reduce their scope by moving them to hw/i386/pc.c. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Laurent Vivier Message-Id: <20210917185949.2244956-1-phi...@redhat.com> Signed-off-by: Laurent Vivier --- hw/i386/pc.c| 6 ++ include/hw/loader.h | 6 -- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 7e523b913caa..557d49c9f8f1 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -843,6 +843,12 @@ void xen_load_linux(PCMachineState *pcms) x86ms->fw_cfg = fw_cfg; } +#define PC_ROM_MIN_VGA 0xc +#define PC_ROM_MIN_OPTION 0xc8000 +#define PC_ROM_MAX 0xe +#define PC_ROM_ALIGN 0x800 +#define PC_ROM_SIZE(PC_ROM_MAX - PC_ROM_MIN_VGA) + void pc_memory_init(PCMachineState *pcms, MemoryRegion *system_memory, MemoryRegion *rom_memory, diff --git a/include/hw/loader.h b/include/hw/loader.h index cbfc1848737c..81104cb02fed 100644 --- a/include/hw/loader.h +++ b/include/hw/loader.h @@ -336,12 +336,6 @@ void hmp_info_roms(Monitor *mon, const QDict *qdict); #define rom_add_blob_fixed_as(_f, _b, _l, _a, _as) \ rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, _as, true) -#define PC_ROM_MIN_VGA 0xc -#define PC_ROM_MIN_OPTION 0xc8000 -#define PC_ROM_MAX 0xe -#define PC_ROM_ALIGN 0x800 -#define PC_ROM_SIZE(PC_ROM_MAX - PC_ROM_MIN_VGA) - int rom_add_vga(const char *file); int rom_add_option(const char *file, int32_t bootindex); -- 2.31.1
Re: [PATCH 1/1] hw: aspeed_gpio: Fix GPIO array indexing
On 9/25/21 16:28, Peter Delevoryas wrote:
On Sep 25, 2021, at 4:03 AM, Philippe Mathieu-Daudé wrote:
Hi Peter,
On 9/24/21 08:19, p...@fb.com wrote:
From: Peter Delevoryas
The gpio array is declared as a dense array:
...
qemu_irq gpios[ASPEED_GPIO_NR_PINS];
(AST2500 has 228, AST2400 has 216, AST2600 has 208)
However, this array is used like a matrix of GPIO sets
(e.g. gpio[NR_SETS][NR_PINS_PER_SET] = gpio[8][32])
size_t offset = set * GPIOS_PER_SET + gpio;
qemu_set_irq(s->gpios[offset], !!(new & mask));
This can result in an out-of-bounds access to "s->gpios" because the
gpio sets do _not_ have the same length. Some of the groups (e.g.
GPIOAB) only have 4 pins. 228 != 8 * 32 == 256.
To fix this, I converted the gpio array from dense to sparse, to that
match both the hardware layout and this existing indexing code.
This is one logical change: 1 patch
Also, I noticed that some of the property specifications looked wrong:
the lower 8 bits in the input and output u32's correspond to the first
group label, and the upper 8 bits correspond to the last group label.
Another logical change: another patch :)
So please split this patch in 2. Maybe easier to fix GPIOSetProperties
first, then convert from dense to sparse array.
Good point, I’ll split it up then!
Yes. We can surely merge the GPIOSetProperties patch quickly.
I hope Rashmica has some time to check the second part.
Thanks,
C.
Regards,
Phil.
I looked at the datasheet and several of these declarations seemed
incorrect to me (I was basing it off of the I/O column). If somebody
can double-check this, I'd really appreciate it!
Some were definitely wrong though, like "Y" and "Z" in the 2600.
@@ -796,7 +776,7 @@ static const GPIOSetProperties ast2500_set_props[] = {
[3] = {0x, 0x, {"M", "N", "O", "P"} },
[4] = {0x, 0x, {"Q", "R", "S", "T"} },
[5] = {0x, 0x, {"U", "V", "W", "X"} },
-[6] = {0xff0f, 0x0f0f, {"Y", "Z", "AA", "AB"} },
+[6] = {0x0fff, 0x0fff, {"Y", "Z", "AA", "AB"} },
[7] = {0x00ff, 0x00ff, {"AC"} },
};
@@ -805,9 +785,9 @@ static GPIOSetProperties ast2600_3_3v_set_props[] = {
[1] = {0x, 0x, {"E", "F", "G", "H"} },
[2] = {0x, 0x, {"I", "J", "K", "L"} },
[3] = {0x, 0x, {"M", "N", "O", "P"} },
-[4] = {0x, 0x, {"Q", "R", "S", "T"} },
-[5] = {0x, 0x, {"U", "V", "W", "X"} },
-[6] = {0x, 0x0fff, {"Y", "Z", "", ""} },
+[4] = {0x, 0x00ff, {"Q", "R", "S", "T"} },
+[5] = {0x, 0xff00, {"U", "V", "W", "X"} },
+[6] = {0x, 0x, {"Y", "Z", "", ""} },
};
Signed-off-by: Peter Delevoryas
---
hw/gpio/aspeed_gpio.c | 80 +++
include/hw/gpio/aspeed_gpio.h | 5 +--
2 files changed, 35 insertions(+), 50 deletions(-)
Re: [PATCH 11/11] Deprecate stable non-JSON -device and -object
On Mon, Sep 27, 2021 at 10:15:43AM +0200, Paolo Bonzini wrote: > On 24/09/21 11:04, Kevin Wolf wrote: > > We want to switch both from QemuOpts to the keyval parser in the future, > > which results in some incompatibilities, mainly around list handling. > > Mark the non-JSON version of both as unstable syntax so that management > > tools switch to JSON and we can later make the change without breaking > > things. > > Maybe we need a different section for unstable syntaxes, rather than > overloading deprecated.rst? This case feels like it hits two scenarios - we want to declare it unstable, which is something we should document in qemu-options.hx. We want to also to warn of specific breakage when the impl changes which is something suitable for deprecations. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
On Sun, 26 Sept 2021 at 19:55, Simon Glass wrote: > In the case of U-Boot at least, it uses the devicetree for > configuration (it is a boot loader, so there is no user space to > provide configuration). So the current setup is not sufficient to boot > it correctly in all cases. On the other hand, the 'virt' feature is > very useful for testing U-Boot, so it would be great to be able to > support this. So what is missing in the QEMU-provided DTB that it needs? thanks -- PMM
Re: gitlab-ci: amd64-opensuse-leap-container job failing
On Mon, 2021-09-27 at 09:35 +0100, Daniel P. Berrangé wrote: > On Sun, Sep 26, 2021 at 07:23:56PM +0200, Philippe Mathieu-Daudé wrote: > > Hi, > > > > FYI the OpenSUSE job is failing since few days, i.e.: > > https://gitlab.com/qemu-project/qemu/-/jobs/1622345026 > > > > Retrieving repository 'Main Repository' metadata > > [..error] > > Repository 'Main Repository' is invalid. > > > > [repo- > > oss|http://download.opensuse.org/distribution/leap/15.2/repo/oss/] > > Valid metadata not found at specified URL > > History: > > - Download (curl) error for > > ' > > http://download.opensuse.org/distribution/leap/15.2/repo/oss/repodata/repomd.xml' > > : > > Error code: Curl error 56 > > Error message: Recv failure: Connection reset by peer > > - Can't provide /repodata/repomd.xml > > Please check if the URIs defined for this repository are pointing > > to a > > valid repository. > > Warning: Skipping repository 'Main Repository' because of the above > > error. > > > > I tried to run 'zypper ref' with: > > It isn't confined to only SuSE. In libvirt we've had similar problems > with several other jobs, though are suse jobs are the worst affected. > > GitLab have finally acknowledged it is an general infra issue affecting > many things: > > https://status.gitlab.com/ > https://gitlab.com/gitlab-com/gl-infra/production/-/issues/5590 > > > -- >8 -- > > --- a/tests/docker/dockerfiles/opensuse-leap.docker > > +++ b/tests/docker/dockerfiles/opensuse-leap.docker > > @@ -109,5 +109,7 @@ ENV PACKAGES \ > > zlib-devel > > ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3.6 > > > > -RUN zypper update -y && zypper --non-interactive install -y > > $PACKAGES > > +RUN zypper refresh && \ > > + zypper update -y && \ > > + zypper --non-interactive install -y $PACKAGES > > RUN rpm -q $PACKAGES | sort > /packages.txt > > --- > > > > but no luck: https://gitlab.com/philmd/qemu/-/jobs/1623554962 > > > > Should we temporarily disable to job and its dependencies? > > Given it is believed to be a gitlab infra issue, rather than a problem > of ours, or something we're using, I think best to wait a little longer > to see if they get fix the infra. > agree, and I am also checking the status of it. for now the http://download.opensuse.org/distribution/leap/15.2 and the repo works. Will follow up it. Cheers, AL
[PULL 0/5] Trivial branch for 6.2 patches
The following changes since commit 11a11998460ed84d9a127c025f50f7234e5a483f: Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20210921' into staging (2021-09-24 13:21:18 -0400) are available in the Git repository at: git://github.com/vivier/qemu.git tags/trivial-branch-for-6.2-pull-request for you to fetch changes up to 45b09cb12f5440971b321fc255e3930f38366ace: multi-process: fix usage information (2021-09-27 10:57:21 +0200) Trivial patches pull request 20210927 Dongli Zhang (1): multi-process: fix usage information Markus Armbruster (2): hmp: Unbreak "change vnc" hmp: Drop a bogus sentence from set_password's documentation Pankaj Gupta (1): docs/nvdimm: Update nvdimm option value in machine example Philippe Mathieu-Daudé (1): hw/loader: Restrict PC_ROM_* definitions to hw/i386/pc docs/nvdimm.txt | 2 +- docs/system/multi-process.rst | 2 +- hmp-commands.hx | 11 +-- hw/i386/pc.c | 6 ++ include/hw/loader.h | 6 -- monitor/hmp-cmds.c| 2 +- 6 files changed, 14 insertions(+), 15 deletions(-) -- 2.31.1
[PULL 5/5] multi-process: fix usage information
From: Dongli Zhang >From source code, the 'devid' of x-remote-object should be one of devices in remote QEMU process. Signed-off-by: Dongli Zhang Reviewed-by: Jagannathan Raman Reviewed-by: Laurent Vivier Message-Id: <20210713004718.20381-1-dongli.zh...@oracle.com> Signed-off-by: Laurent Vivier --- docs/system/multi-process.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/system/multi-process.rst b/docs/system/multi-process.rst index 46bb0cafc27a..210531ee17df 100644 --- a/docs/system/multi-process.rst +++ b/docs/system/multi-process.rst @@ -45,7 +45,7 @@ Following is a description of command-line used to launch mpqemu. -device lsi53c895a,id=lsi0 \ -drive id=drive_image2,file=/build/ol7-nvme-test-1.qcow2 \ -device scsi-hd,id=drive2,drive=drive_image2,bus=lsi0.0,scsi-id=0 \ - -object x-remote-object,id=robj1,devid=lsi1,fd=4, + -object x-remote-object,id=robj1,devid=lsi0,fd=4, * QEMU: -- 2.31.1
Re: [PATCH 1/2] hw/dma: sifive_pdma: Improve code readability for "!!foo & bar"
Bin Meng writes:
> GCC seems to be strict about processing pattern like "!!for & bar".
> When 'bar' is not 0 or 1, it complains with -Werror=parentheses:
>
> suggest parentheses around operand of ‘!’ or change ‘&’ to ‘&&’ or ‘!’ to
> ‘~’ [-Werror=parentheses]
>
> Add a () around "foo && bar", which also improves code readability.
>
> Signed-off-by: Bin Meng
> ---
>
> hw/dma/sifive_pdma.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
> index b4fd40573a..b8ec7621f3 100644
> --- a/hw/dma/sifive_pdma.c
> +++ b/hw/dma/sifive_pdma.c
> @@ -243,7 +243,7 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
> offset &= 0xfff;
> switch (offset) {
> case DMA_CONTROL:
> -claimed = !!s->chan[ch].control & CONTROL_CLAIM;
> +claimed = !!(s->chan[ch].control & CONTROL_CLAIM);
>
> if (!claimed && (value & CONTROL_CLAIM)) {
> /* reset Next* registers */
Old code
first double-negate, mapping zero to zero, non-zero to one
then mask, which does nothing, because CONTROL_CLAIM is 1
New code:
first mask, yielding 0 or 1
then double-negate, which does nothing
Looks like a bug fix to me. If I'm right, the commit message is wrong,
and the double negate is redundant.
[PATCH v3] migration/rdma: Fix out of order wrid
destination:
../qemu/build/qemu-system-x86_64 -enable-kvm -netdev
tap,id=hn0,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown -device
e1000,netdev=hn0,mac=50:52:54:00:11:22 -boot c -drive
if=none,file=./Fedora-rdma-server-migration.qcow2,id=drive-virtio-disk0 -device
virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0 -m
2048 -smp 2 -device piix3-usb-uhci -device usb-tablet -monitor stdio -vga qxl
-spice streaming-video=filter,port=5902,disable-ticketing -incoming
rdma:192.168.22.23:
qemu-system-x86_64: -spice streaming-video=filter,port=5902,disable-ticketing:
warning: short-form boolean option 'disable-ticketing' deprecated
Please use disable-ticketing=on instead
QEMU 6.0.50 monitor - type 'help' for more information
(qemu) trace-event qemu_rdma_block_for_wrid_miss on
(qemu) dest_init RDMA Device opened: kernel name rxe_eth0 uverbs device name
uverbs2, infiniband_verbs class device path
/sys/class/infiniband_verbs/uverbs2, infiniband class device path
/sys/class/infiniband/rxe_eth0, transport: (2) Ethernet
qemu_rdma_block_for_wrid_miss A Wanted wrid CONTROL SEND (2000) but got CONTROL
RECV (4000)
source:
../qemu/build/qemu-system-x86_64 -enable-kvm -netdev
tap,id=hn0,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown -device
e1000,netdev=hn0,mac=50:52:54:00:11:22 -boot c -drive
if=none,file=./Fedora-rdma-server.qcow2,id=drive-virtio-disk0 -device
virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0 -m
2048 -smp 2 -device piix3-usb-uhci -device usb-tablet -monitor stdio -vga qxl
-spice streaming-video=filter,port=5901,disable-ticketing -S
qemu-system-x86_64: -spice streaming-video=filter,port=5901,disable-ticketing:
warning: short-form boolean option 'disable-ticketing' deprecated
Please use disable-ticketing=on instead
QEMU 6.0.50 monitor - type 'help' for more information
(qemu)
(qemu) trace-event qemu_rdma_block_for_wrid_miss on
(qemu) migrate -d rdma:192.168.22.23:
source_resolve_host RDMA Device opened: kernel name rxe_eth0 uverbs device name
uverbs2, infiniband_verbs class device path
/sys/class/infiniband_verbs/uverbs2, infiniband class device path
/sys/class/infiniband/rxe_eth0, transport: (2) Ethernet
(qemu) qemu_rdma_block_for_wrid_miss A Wanted wrid WRITE RDMA (1) but got
CONTROL RECV (4000)
NOTE: we use soft RoCE as the rdma device.
[root@iaas-rpma images]# rdma link show rxe_eth0/1
link rxe_eth0/1 state ACTIVE physical_state LINK_UP netdev eth0
This migration could not be completed when out of order(OOO) CQ event occurs.
The send queue and receive queue shared a same completion queue, and
qemu_rdma_block_for_wrid() will drop the CQs it's not interested in. But
the dropped CQs by qemu_rdma_block_for_wrid() could be later CQs it wants.
So in this case, qemu_rdma_block_for_wrid() will block forever.
OOO cases will occur in both source side and destination side. And a
forever blocking happens on only SEND and RECV are out of order. OOO between
'WRITE RDMA' and 'RECV' doesn't matter.
below the OOO sequence:
source destination
rdma_write_one() qemu_rdma_registration_handle()
1.S1: post_recv XD1: post_recv Y
2.wait for recv CQ event X
3. D2: post_send X ---+
4. wait for send CQ send event X (D2) |
5.recv CQ event X reaches (D2) |
6. +-S2: post_send Y |
7. | wait for send CQ event Y |
8. |recv CQ event Y (S2) (drop it) |
9. +-send CQ event Y reaches (S2) |
10. send CQ event X reaches (D2) -+
11. wait recv CQ event Y (dropped by (8))
Although a hardware IB works fine in my a hundred of runs, the IB specification
doesn't guaratee the CQ order in such case.
Here we introduce a independent send completion queue to distinguish
ibv_post_send completion queue from the original mixed completion queue.
It helps us to poll the specific CQE we are really interested in.
Signed-off-by: Li Zhijian
---
V3: rebase code, and combine 2/2 to 1/2
V2: Introduce send completion queue
---
migration/rdma.c | 132 +++
1 file changed, 98 insertions(+), 34 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index 5c2d113aa94..bb19a5afe73 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -358,9 +358,11 @@ typedef struct RDMAContext {
struct ibv_context *verbs;
struct rdma_event_channel *channel;
struct ibv_qp *qp; /* queue pair */
-struct ibv_comp_channel *comp_channel; /* completion channel */
+struct ibv_comp_channel *recv_comp_channel; /* recv
Re: [PATCH] hw/misc: Add a virtual pci device to dynamically attach memory to QEMU
On 27.09.21 10:27, Stefan Hajnoczi wrote: On Sun, Sep 26, 2021 at 10:16:14AM +0800, David Dai wrote: Add a virtual pci to QEMU, the pci device is used to dynamically attach memory to VM, so driver in guest can apply host memory in fly without virtualization management software's help, such as libvirt/manager. The attached memory is We do have virtio-mem to dynamically attach memory to a VM. It could be extended by a mechanism for the VM to request more/less memory, that's already a planned feature. But yeah, virito-mem memory is exposed as ordinary system RAM, not only via a BAR to mostly be managed by user space completely. isolated from System RAM, it can be used in heterogeneous memory management for virtualization. Multiple VMs dynamically share same computing device memory without memory overcommit. This sounds a lot like MemExpand/MemLego ... am I right that this is the original design? I recall that VMs share a memory region and dynamically agree upon which part of the memory region a VM uses. I further recall that there were malloc() hooks that would dynamically allocate such memory in user space from the shared memory region. I can see some use cases for it, although the shared memory design isn't what you typically want in most VM environments. -- Thanks, David / dhildenb
Re: [PATCH v4 29/35] acpi: arm/virt: convert build_iort() to endian agnostic build_append_FOO() API
On 9/24/21 2:27 PM, Igor Mammedov wrote:
> Drop usage of packed structures and explicit endian conversions
> when building IORT table use endian agnostic build_append_int_noprefix()
> API to build it.
>
> Signed-off-by: Igor Mammedov
> ---
> CC: Eric Auger
>
> v4:
> - (Eric Auger )
> * keep nb_nodes
> * encode 'Memory access properties' as separate entries according to
> Table 13
> * keep some comments
> v3:
> * practically rewritten, due to conflicts wiht bypass iommu feature
>
> CC: drjo...@redhat.com
> CC: peter.mayd...@linaro.org
> CC: shannon.zha...@gmail.com
> CC: qemu-...@nongnu.org
> CC: wangxinga...@huawei.com
> CC: Eric Auger
> ---
> include/hw/acpi/acpi-defs.h | 71
> hw/arm/virt-acpi-build.c| 164
> 2 files changed, 93 insertions(+), 142 deletions(-)
>
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 195f90caf6..6f2f08a9de 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -188,75 +188,4 @@ struct AcpiGenericTimerTable {
> } QEMU_PACKED;
> typedef struct AcpiGenericTimerTable AcpiGenericTimerTable;
>
> -/*
> - * IORT node types
> - */
> -
> -#define ACPI_IORT_NODE_HEADER_DEF /* Node format common fields */ \
> -uint8_t type; \
> -uint16_t length;\
> -uint8_t revision; \
> -uint32_t reserved; \
> -uint32_t mapping_count; \
> -uint32_t mapping_offset;
> -
> -/* Values for node Type above */
> -enum {
> -ACPI_IORT_NODE_ITS_GROUP = 0x00,
> -ACPI_IORT_NODE_NAMED_COMPONENT = 0x01,
> -ACPI_IORT_NODE_PCI_ROOT_COMPLEX = 0x02,
> -ACPI_IORT_NODE_SMMU = 0x03,
> -ACPI_IORT_NODE_SMMU_V3 = 0x04
> -};
> -
> -struct AcpiIortIdMapping {
> -uint32_t input_base;
> -uint32_t id_count;
> -uint32_t output_base;
> -uint32_t output_reference;
> -uint32_t flags;
> -} QEMU_PACKED;
> -typedef struct AcpiIortIdMapping AcpiIortIdMapping;
> -
> -struct AcpiIortMemoryAccess {
> -uint32_t cache_coherency;
> -uint8_t hints;
> -uint16_t reserved;
> -uint8_t memory_flags;
> -} QEMU_PACKED;
> -typedef struct AcpiIortMemoryAccess AcpiIortMemoryAccess;
> -
> -struct AcpiIortItsGroup {
> -ACPI_IORT_NODE_HEADER_DEF
> -uint32_t its_count;
> -uint32_t identifiers[];
> -} QEMU_PACKED;
> -typedef struct AcpiIortItsGroup AcpiIortItsGroup;
> -
> -#define ACPI_IORT_SMMU_V3_COHACC_OVERRIDE 1
> -
> -struct AcpiIortSmmu3 {
> -ACPI_IORT_NODE_HEADER_DEF
> -uint64_t base_address;
> -uint32_t flags;
> -uint32_t reserved2;
> -uint64_t vatos_address;
> -uint32_t model;
> -uint32_t event_gsiv;
> -uint32_t pri_gsiv;
> -uint32_t gerr_gsiv;
> -uint32_t sync_gsiv;
> -AcpiIortIdMapping id_mapping_array[];
> -} QEMU_PACKED;
> -typedef struct AcpiIortSmmu3 AcpiIortSmmu3;
> -
> -struct AcpiIortRC {
> -ACPI_IORT_NODE_HEADER_DEF
> -AcpiIortMemoryAccess memory_properties;
> -uint32_t ats_attribute;
> -uint32_t pci_segment_number;
> -AcpiIortIdMapping id_mapping_array[];
> -} QEMU_PACKED;
> -typedef struct AcpiIortRC AcpiIortRC;
> -
> #endif
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 42ea460313..8c382915a9 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -240,6 +240,28 @@ static void acpi_dsdt_add_tpm(Aml *scope,
> VirtMachineState *vms)
> }
> #endif
>
> +#define ID_MAPPING_ENTRY_SIZE 20
> +#define SMMU_V3_ENTRY_SIZE 60
> +#define ROOT_COMPLEX_ENTRY_SIZE 32
> +#define IORT_NODE_OFFSET 48
> +
> +static void build_iort_id_mapping(GArray *table_data, uint32_t input_base,
> + uint32_t id_count, uint32_t out_ref)
> +{
> +/* Identity RID mapping covering the whole input RID range */
> +build_append_int_noprefix(table_data, input_base, 4); /* Input base */
> +build_append_int_noprefix(table_data, id_count, 4); /* Number of IDs */
> +build_append_int_noprefix(table_data, input_base, 4); /* Output base */
> +build_append_int_noprefix(table_data, out_ref, 4); /* Output Reference */
> +build_append_int_noprefix(table_data, 0, 4); /* Flags */
> +}
> +
> +struct AcpiIortIdMapping {
> +uint32_t input_base;
> +uint32_t id_count;
> +};
> +typedef struct AcpiIortIdMapping AcpiIortIdMapping;
> +
> /* Build the iort ID mapping to SMMUv3 for a given PCI host bridge */
> static int
> iort_host_bridges(Object *obj, void *opaque)
> @@ -282,17 +304,16 @@ static void
> build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> {
> int i, nb_nodes, rc_mapping_count;
> -AcpiIortIdMapping *idmap;
> -AcpiIortItsGroup *its;
> -AcpiIortSmmu3 *smmu;
> -AcpiIortRC *rc;
> -const uint32_t iort_node_offset = 48;
> +const uint32_t iort_node_offset = IORT_NODE_OFFSET;
> size_t node_size, smmu_offset = 0;
> +AcpiIortIdMapping *idmap;
> GArray
virtio-gpu: Get FD for texture
Hi, I am trying to support a Vulkan application in the guest (GTKGlArea+VirGL+venus) which needs to import a GL texture from a GL context. Before doing that, I need to get a FD for that texture, therefore I tried with calling egl-helpers.h:egl_get_fd_for_texture() but I get an epoxy error: > No provider of eglCreateImageKHR found. Requires one of: > EGL_KHR_image > EGL_KHR_image_base This is a bit weird to me as I am sure I am running QEMU with iris and according to eglinfo both of these extensions are available. Do you think my approach makes sense or I am doing something wrong somewhere? Kind regards, Antonio Caggiano
Re: [PATCH] allwinner-h3: Switch to SMC as PSCI conduit
On Wed, 22 Sept 2021 at 20:41, Niek Linnenbank wrote: > > Hi Alexander, > > I've tested your patch on the acceptance tests and they all pass: > > ARMBIAN_ARTIFACTS_CACHED=yes AVOCADO_ALLOW_LARGE_STORAGE=yes avocado > --show=app,console run -t machine:orangepi-pc > tests/acceptance/boot_linux_console.py > ... > RESULTS: PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | > CANCEL 0 > JOB TIME : 116.08 s > > Also the latest linux kernel is working OK with all cores booting up fine. > > At first I couldn't really figure out why simply changing the conduit there > works, without also changing the Linux kernel to match. > But it turns out we just override the provided DTB for this in > fdt_add_psci_node() in hw/arm/boot.c and the Linux kernel then uses that to > decide between HVC and SMC. > > So looks fine to me: > Reviewed-by: Niek Linnenbank > Tested-by: Niek Linnenbank Applied to target-arm.next, thanks. PS: if you put spaces in front of 'Reviewed-by' type tags, the automated tooling doesn't recognize them. -- PMM
Re: Add LoongArch support to RISU?
On Sun, Sep 26, 2021 at 04:35:37PM +0200, Philippe Mathieu-Daudé wrote: > Hi, I meant to include the qemu-devel@ mailing list in Cc but forgot to... > Doing that now. > > On Sun, Sep 26, 2021 at 11:25 AM Song Gao wrote: > > > > Hi, Phil > > > > On 09/26/2021 04:25 PM, Philippe Mathieu-Daudé wrote: > > > Hi Xuerui, > > > > > > Looking at the script [1] used in your series adding LoongArch TCG > > > backend [2], I think all the bits are in place to also generate most > > > of the files required to run RISU [3] and use it to be able to test > > > Song Gao's LoongArch TCG frontend [4]. > > > > > > Do you know developers / companies who might be interested in working > > > on this? > > > > > We can do it. Our plan is to complete LoongArch linux-user emulation, > > system emulation , TCG backend and others support. > > We plan to submit the system emulation code in mid and late October. Xue > > rui had finished TCG backend. So we may doing this work after system > > emulation. > > > > We welcome others to do the work！ > > This might be a good project to present to Outreachy internship: > https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg03970.html > > "QEMU will apply for the Outreachy December-March round. This > internship program offers paid, full-time, remote work internships for > contributing to open source. QEMU can act as an umbrella organization > for KVM kernel projects." > > Stefan, are we past the deadline for submission? The Outreachy project idea deadline has been extended to September 29th. QEMU contributors can submit project ideas they would like to mentor here: https://www.outreachy.org/communities/cfp/qemu/ Please take a look at this email for more details on designing Outreachy projects: https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg03970.html Thanks, Stefan signature.asc Description: PGP signature
Re: [PATCH 11/11] Deprecate stable non-JSON -device and -object
On Fri, 24 Sept 2021 at 10:14, Kevin Wolf wrote: > > We want to switch both from QemuOpts to the keyval parser in the future, > which results in some incompatibilities, mainly around list handling. > Mark the non-JSON version of both as unstable syntax so that management > tools switch to JSON and we can later make the change without breaking > things. > > Signed-off-by: Kevin Wolf > +Stable non-JSON ``-device`` and ``-object`` syntax (since 6.2) > +'' > + > +If you rely on a stable interface for ``-device`` and ``-object`` that > doesn't > +change incompatibly between QEMU versions (e.g. because you are using the > QEMU > +command line as a machine interface in scripts rather than interactively), > use > +JSON syntax for these options instead. > + > +There is no intention to remove support for non-JSON syntax entirely, but > +future versions may change the way to spell some options. As it stands, this is basically saying "pretty much anybody using the command line, your stuff may break in future, instead use some other interface you've never heard of, which doesn't appear to be documented in the manual and which none of the documentation's examples use". Is there some more limited deprecation we can do rather than "the entire commandline for almost all users" ? thanks -- PMM
Re: [PATCH 00/16] Acceptance Tests: use Avocado 91.0 features and other improvements
Hi, Cleber! What about record/replay tests from 25.06? On 24.09.2021 21:54, Cleber Rosa wrote: This is a collection of patches for the Acceptance Tests to leverage some of the features of Avocado 91.0. With the Avocado version bump by itself, there would be a change in the default "test runner" implementation that Avocado uses, from the one simply known as "runner" to the new one called "nrunner". Among the changes from one implementation to the other, is the fact that "nrunner" will run tests in parallel by default. This is *not yet* enabled by default on "make check-acceptance", but users can choose to use simply by setting the "AVOCADO_RUNNER" variable, that is: make AVOCADO_RUNNER=nrunner check-acceptance If you are curious about the architectural differences of the nrunner, please refer to: https://avocado-framework.readthedocs.io/en/91.0/guides/contributor/chapters/runners.html One other noteworthy proposal is a convention to tag tests that either have known issues, or that touch on QEMU features that have known issues. By tagging those tests accordingly, they will be automatically excluded from the regular execution with "make check-acceptance". Finally, some updates to assets locations and some tests refactors and cleanups. Cleber Rosa (16): Acceptance Tests: bump Avocado requirement to 91.0 Acceptance Tests: improve check-acceptance description Acceptance Tests: add mechanism for listing tests Acceptance Tests: keep track and disable tests with known issues Acceptance Tests: add standard clean up at test tearDown() Acceptance Tests: use extract from package from avocado.utils Acceptance Tests: workaround expired mipsdistros.mips.com HTTPS cert acceptance/tests/vnc.py: use explicit syntax for enabling passwords tests/acceptance/boot_xen.py: merge base classes tests/acceptance/boot_xen.py: unify tags tests/acceptance/boot_xen.py: fetch kernel during test setUp() tests/acceptance/boot_xen.py: removed unused import tests/acceptance/boot_xen.py: use class attribute tests/acceptance/ppc_prep_40p.py: NetBSD 7.1.2 location update tests/acceptance/ppc_prep_40p.py: clean up unused import tests/acceptance/ppc_prep_40p.py: unify tags docs/devel/testing.rst| 40 ++ tests/Makefile.include| 15 +++- tests/acceptance/avocado_qemu/__init__.py | 1 + tests/acceptance/boot_linux_console.py| 93 +-- tests/acceptance/boot_xen.py | 54 - tests/acceptance/machine_rx_gdbsim.py | 3 + tests/acceptance/ppc_prep_40p.py | 17 ++--- tests/acceptance/replay_kernel.py | 18 ++--- tests/acceptance/tcg_plugins.py | 2 +- tests/acceptance/vnc.py | 2 +- tests/requirements.txt| 2 +- 11 files changed, 128 insertions(+), 119 deletions(-)
Re: [PATCH] hw: Add a 'Sensor devices' qdev category
On 9/27/21 00:15, Philippe Mathieu-Daudé wrote:
Sensors models are listed in the 'Misc devices' category.
Move them to their own category.
For the devices in the hw/sensor/ directory, the category
is obvious.
hw/arm/z2.c models the AER915 model which is described
on [*] as:
The 14-pin chip marked AER915 just below the expansion
port is a 80C51-type microcontroller, similar to Philips
P89LPC915. It has an 8-bit A/D which is used to determine
which of six buttons are pressed on the resistor-network
wired remote. It communicates with the main cpu via I2C.
It was introduced in commit 3bf11207c06 ("Add support for
Zipit Z2 machine") with this comment:
248 static uint8_t aer915_recv(I2CSlave *slave)
249 {
...
253 switch (s->buf[0]) {
254 /* Return hardcoded battery voltage,
255 * 0xf0 means ~4.1V
256 */
257 case 0x02:
258 retval = 0xf0;
259 break;
For QEMU the AER915 is a very simple sensor model.
[*] https://www.bealecorner.org/best/measure/z2/index.html
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Cédric Le Goater
---
include/hw/qdev-core.h | 1 +
hw/arm/z2.c| 1 +
hw/sensor/adm1272.c| 1 +
hw/sensor/dps310.c | 1 +
hw/sensor/emc141x.c| 1 +
hw/sensor/max34451.c | 2 ++
hw/sensor/tmp105.c | 1 +
hw/sensor/tmp421.c | 1 +
softmmu/qdev-monitor.c | 1 +
9 files changed, 10 insertions(+)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 34c8a7506a1..f6241212247 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -26,6 +26,7 @@ typedef enum DeviceCategory {
DEVICE_CATEGORY_SOUND,
DEVICE_CATEGORY_MISC,
DEVICE_CATEGORY_CPU,
+DEVICE_CATEGORY_SENSOR,
DEVICE_CATEGORY_MAX
} DeviceCategory;
diff --git a/hw/arm/z2.c b/hw/arm/z2.c
index 9c1e876207b..62db9741106 100644
--- a/hw/arm/z2.c
+++ b/hw/arm/z2.c
@@ -288,6 +288,7 @@ static void aer915_class_init(ObjectClass *klass, void
*data)
k->recv = aer915_recv;
k->send = aer915_send;
dc->vmsd = _aer915_state;
+set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
}
static const TypeInfo aer915_info = {
diff --git a/hw/sensor/adm1272.c b/hw/sensor/adm1272.c
index 7310c769be2..2942ac75f90 100644
--- a/hw/sensor/adm1272.c
+++ b/hw/sensor/adm1272.c
@@ -518,6 +518,7 @@ static void adm1272_class_init(ObjectClass *klass, void
*data)
DeviceClass *dc = DEVICE_CLASS(klass);
PMBusDeviceClass *k = PMBUS_DEVICE_CLASS(klass);
+set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
dc->desc = "Analog Devices ADM1272 Hot Swap controller";
dc->vmsd = _adm1272;
k->write_data = adm1272_write_data;
diff --git a/hw/sensor/dps310.c b/hw/sensor/dps310.c
index d60a18ac41b..1e24a499b38 100644
--- a/hw/sensor/dps310.c
+++ b/hw/sensor/dps310.c
@@ -208,6 +208,7 @@ static void dps310_class_init(ObjectClass *klass, void
*data)
k->send = dps310_tx;
dc->reset = dps310_reset;
dc->vmsd = _dps310;
+set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
}
static const TypeInfo dps310_info = {
diff --git a/hw/sensor/emc141x.c b/hw/sensor/emc141x.c
index 7ce8f4e9794..4202d8f185a 100644
--- a/hw/sensor/emc141x.c
+++ b/hw/sensor/emc141x.c
@@ -270,6 +270,7 @@ static void emc141x_class_init(ObjectClass *klass, void
*data)
DeviceClass *dc = DEVICE_CLASS(klass);
I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
+set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
dc->reset = emc141x_reset;
k->event = emc141x_event;
k->recv = emc141x_rx;
diff --git a/hw/sensor/max34451.c b/hw/sensor/max34451.c
index a91d8bd487c..8300bf4ff43 100644
--- a/hw/sensor/max34451.c
+++ b/hw/sensor/max34451.c
@@ -751,6 +751,8 @@ static void max34451_class_init(ObjectClass *klass, void
*data)
ResettableClass *rc = RESETTABLE_CLASS(klass);
DeviceClass *dc = DEVICE_CLASS(klass);
PMBusDeviceClass *k = PMBUS_DEVICE_CLASS(klass);
+
+set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
dc->desc = "Maxim MAX34451 16-Channel V/I monitor";
dc->vmsd = _max34451;
k->write_data = max34451_write_data;
diff --git a/hw/sensor/tmp105.c b/hw/sensor/tmp105.c
index 20564494899..43d79b9eeec 100644
--- a/hw/sensor/tmp105.c
+++ b/hw/sensor/tmp105.c
@@ -305,6 +305,7 @@ static void tmp105_class_init(ObjectClass *klass, void
*data)
DeviceClass *dc = DEVICE_CLASS(klass);
I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
+set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
dc->realize = tmp105_realize;
k->event = tmp105_event;
k->recv = tmp105_rx;
diff --git a/hw/sensor/tmp421.c b/hw/sensor/tmp421.c
index a3db57dcb5a..c328978af9c 100644
--- a/hw/sensor/tmp421.c
+++ b/hw/sensor/tmp421.c
@@ -343,6 +343,7 @@ static void tmp421_class_init(ObjectClass *klass, void
*data)
I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
TMP421Class *sc = TMP421_CLASS(klass);
+
Re: [PATCH v3 11/27] linux-user/x86_64: Raise SIGSEGV if SA_RESTORER not set
On Fri, 24 Sept 2021 at 17:59, Richard Henderson wrote: > > This has been a fixme for some time. The effect of > returning -EFAULT from the kernel code is to raise SIGSEGV. > > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2] nbd/server: Add --selinux-label option
On Wed, Aug 25, 2021 at 02:35:04PM -0500, Eric Blake wrote: > On Fri, Jul 23, 2021 at 05:38:06PM +0100, Daniel P. Berrangé wrote: > > On Fri, Jul 23, 2021 at 06:18:55PM +0200, Kevin Wolf wrote: > > > Am 23.07.2021 um 12:33 hat Richard W.M. Jones geschrieben: > > > > Under SELinux, Unix domain sockets have two labels. One is on the > > > > disk and can be set with commands such as chcon(1). There is a > > > > different label stored in memory (called the process label). This can > > > > only be set by the process creating the socket. When using SELinux + > > > > SVirt and wanting qemu to be able to connect to a qemu-nbd instance, > > > > you must set both labels correctly first. > > > > > > > > For qemu-nbd the options to set the second label are awkward. You can > > > > create the socket in a wrapper program and then exec into qemu-nbd. > > > > Or you could try something with LD_PRELOAD. > > > > > > > > This commit adds the ability to set the label straightforwardly on the > > > > command line, via the new --selinux-label flag. (The name of the flag > > > > is the same as the equivalent nbdkit option.) > > > > > > > > A worked example showing how to use the new option can be found in > > > > this bug: https://bugzilla.redhat.com/show_bug.cgi?id=1984938 > > > > > > > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1984938 > > > > Signed-off-by: Richard W.M. Jones > > > > > > I suppose this would also be relevant for the built-in NBD server, > > > especially in the context of qemu-storage-daemon? > > > > It depends on the usage scenario really. nbdkit / qemu-nbd are > > not commonly run under any SELinux policy, so then end up being > > unconfined_t. A QEMU NBD client can't connect to an unconfined_t > > socket, so we need to override it with this arg. > > > > In the case of qemu system emulator, under libvirt, it will > > already have a svirt_t type, so in that case there is no need > > to override the type for the socket. > > > > For qsd there's not really any strong practice established > > but i expect most current usage is unconfined_t too and > > would benefit from setting label. > > > > > If so, is this something specific to NBD sockets, or would it actually > > > make sense to have it as a generic option in UnixSocketAddress? > > > > It is applicable to inet sockets too in fact. > > So now that 6.2 is open, should I queue the patch as is, or wait for a > v3 that makes the option more generic to all socket usage? My gut feeling is that it makes sense to have a more generic option, with the selinux label specified in the "SocketAddress" QAPI type, and then have util/qemu-sockets.h be setting the context in socket_listen(). I don't think this invalidates the patch that Richard proprosed here, as we'll still need the command line argument he's added. All that will differ is that the setsockcreatecon_raw will get moved down. So from that POV, I don't think we need the general solution to be a blocker, it can be additive. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PULL v2 25/25] tests/qapi-schema: Make test-qapi.py -u work when files are absent
test-qapi.py -u updates the expected files. Since it fails when they
are absent, users have to create them manually before they can use
test-qapi.py to fill in the contents, say for a new test. Silly.
Improve -u to create them.
Signed-off-by: Markus Armbruster
Message-Id: <20210922125619.670673-3-arm...@redhat.com>
Reviewed-by: John Snow
---
tests/qapi-schema/test-qapi.py | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index 2e384f5efd..c717a7a90b 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -132,6 +132,17 @@ def test_frontend(fname):
print('section=%s\n%s' % (section.name, section.text))
+def open_test_result(dir_name, file_name, update):
+mode = 'r+' if update else 'r'
+try:
+fp = open(os.path.join(dir_name, file_name), mode)
+except FileNotFoundError:
+if not update:
+raise
+fp = open(os.path.join(dir_name, file_name), 'w+')
+return fp
+
+
def test_and_diff(test_name, dir_name, update):
sys.stdout = StringIO()
try:
@@ -148,10 +159,9 @@ def test_and_diff(test_name, dir_name, update):
sys.stdout.close()
sys.stdout = sys.__stdout__
-mode = 'r+' if update else 'r'
try:
-outfp = open(os.path.join(dir_name, test_name + '.out'), mode)
-errfp = open(os.path.join(dir_name, test_name + '.err'), mode)
+outfp = open_test_result(dir_name, test_name + '.out', update)
+errfp = open_test_result(dir_name, test_name + '.err', update)
expected_out = outfp.readlines()
expected_err = errfp.readlines()
except OSError as err:
--
2.31.1
[PULL v2 24/25] tests/qapi-schema: Use Python OSError instead of outmoded IOError
https://docs.python.org/3.6/library/exceptions.html has
Changed in version 3.3: EnvironmentError, IOError, WindowsError,
socket.error, select.error and mmap.error have been merged into
OSError, and the constructor may return a subclass.
and
The following exceptions are kept for compatibility with previous
versions; starting from Python 3.3, they are aliases of OSError.
exception EnvironmentError
exception IOError
exception WindowsError
Only available on Windows.
Switch to the preferred name.
Signed-off-by: Markus Armbruster
Message-Id: <20210922125619.670673-2-arm...@redhat.com>
Reviewed-by: John Snow
Reviewed-by: Philippe Mathieu-Daudé
[Details added to commit message]
---
tests/qapi-schema/test-qapi.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index 73cffae2b6..2e384f5efd 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -154,7 +154,7 @@ def test_and_diff(test_name, dir_name, update):
errfp = open(os.path.join(dir_name, test_name + '.err'), mode)
expected_out = outfp.readlines()
expected_err = errfp.readlines()
-except IOError as err:
+except OSError as err:
print("%s: can't open '%s': %s"
% (sys.argv[0], err.filename, err.strerror),
file=sys.stderr)
@@ -180,7 +180,7 @@ def test_and_diff(test_name, dir_name, update):
errfp.truncate(0)
errfp.seek(0)
errfp.writelines(actual_err)
-except IOError as err:
+except OSError as err:
print("%s: can't write '%s': %s"
% (sys.argv[0], err.filename, err.strerror),
file=sys.stderr)
--
2.31.1
[PULL v2 07/25] qapi: Convert simple union ChardevBackend to flat one
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
To prepare for their removal, convert simple union ChardevBackend to
an equivalent flat one. Adds some boilerplate to the schema, which is
a bit ugly, but a lot easier to maintain than the simple union
feature.
Cc: "Marc-André Lureau"
Cc: Paolo Bonzini
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210917143134.412106-8-arm...@redhat.com>
[Missing conditionals added]
---
qapi/char.json | 190 +++--
1 file changed, 168 insertions(+), 22 deletions(-)
diff --git a/qapi/char.json b/qapi/char.json
index 9b18ee3305..f5133a5eeb 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -407,39 +407,185 @@
'base': 'ChardevCommon',
'if': 'CONFIG_SPICE_PROTOCOL' }
+##
+# @ChardevBackendKind:
+#
+# @pipe: Since 1.5
+# @udp: Since 1.5
+# @mux: Since 1.5
+# @msmouse: Since 1.5
+# @wctablet: Since 2.9
+# @braille: Since 1.5
+# @testdev: Since 2.2
+# @stdio: Since 1.5
+# @console: Since 1.5
+# @spicevmc: Since 1.5
+# @spiceport: Since 1.5
+# @qemu-vdagent: Since 6.1
+# @vc: v1.5
+# @ringbuf: Since 1.6
+# @memory: Since 1.5
+#
+# Since: 1.4
+##
+{ 'enum': 'ChardevBackendKind',
+ 'data': [ 'file',
+'serial',
+'parallel',
+'pipe',
+'socket',
+'udp',
+'pty',
+'null',
+'mux',
+'msmouse',
+'wctablet',
+'braille',
+'testdev',
+'stdio',
+'console',
+{ 'name': 'spicevmc', 'if': 'CONFIG_SPICE' },
+{ 'name': 'spiceport', 'if': 'CONFIG_SPICE' },
+{ 'name': 'qemu-vdagent', 'if': 'CONFIG_SPICE_PROTOCOL' },
+'vc',
+'ringbuf',
+# next one is just for compatibility
+'memory' ] }
+
+##
+# @ChardevFileWrapper:
+#
+# Since: 1.4
+##
+{ 'struct': 'ChardevFileWrapper',
+ 'data': { 'data': 'ChardevFile' } }
+
+##
+# @ChardevHostdevWrapper:
+#
+# Since: 1.4
+##
+{ 'struct': 'ChardevHostdevWrapper',
+ 'data': { 'data': 'ChardevHostdev' } }
+
+##
+# @ChardevSocketWrapper:
+#
+# Since: 1.4
+##
+{ 'struct': 'ChardevSocketWrapper',
+ 'data': { 'data': 'ChardevSocket' } }
+
+##
+# @ChardevUdpWrapper:
+#
+# Since: 1.5
+##
+{ 'struct': 'ChardevUdpWrapper',
+ 'data': { 'data': 'ChardevUdp' } }
+
+##
+# @ChardevCommonWrapper:
+#
+# Since: 2.6
+##
+{ 'struct': 'ChardevCommonWrapper',
+ 'data': { 'data': 'ChardevCommon' } }
+
+##
+# @ChardevMuxWrapper:
+#
+# Since: 1.5
+##
+{ 'struct': 'ChardevMuxWrapper',
+ 'data': { 'data': 'ChardevMux' } }
+
+##
+# @ChardevStdioWrapper:
+#
+# Since: 1.5
+##
+{ 'struct': 'ChardevStdioWrapper',
+ 'data': { 'data': 'ChardevStdio' } }
+
+##
+# @ChardevSpiceChannelWrapper:
+#
+# Since: 1.5
+##
+{ 'struct': 'ChardevSpiceChannelWrapper',
+ 'data': { 'data': 'ChardevSpiceChannel' },
+ 'if': 'CONFIG_SPICE' }
+
+##
+# @ChardevSpicePortWrapper:
+#
+# Since: 1.5
+##
+{ 'struct': 'ChardevSpicePortWrapper',
+ 'data': { 'data': 'ChardevSpicePort' },
+ 'if': 'CONFIG_SPICE' }
+
+##
+# @ChardevQemuVDAgentWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ChardevQemuVDAgentWrapper',
+ 'data': { 'data': 'ChardevQemuVDAgent' },
+ 'if': 'CONFIG_SPICE_PROTOCOL' }
+
+##
+# @ChardevVCWrapper:
+#
+# Since: 1.5
+##
+{ 'struct': 'ChardevVCWrapper',
+ 'data': { 'data': 'ChardevVC' } }
+
+##
+# @ChardevRingbufWrapper:
+#
+# Since: 1.5
+##
+{ 'struct': 'ChardevRingbufWrapper',
+ 'data': { 'data': 'ChardevRingbuf' } }
+
##
# @ChardevBackend:
#
# Configuration info for the new chardev backend.
#
-# Since: 1.4 (testdev since 2.2, wctablet since 2.9, vdagent since 6.1)
+# Since: 1.4
##
{ 'union': 'ChardevBackend',
- 'data': { 'file': 'ChardevFile',
-'serial': 'ChardevHostdev',
-'parallel': 'ChardevHostdev',
-'pipe': 'ChardevHostdev',
-'socket': 'ChardevSocket',
-'udp': 'ChardevUdp',
-'pty': 'ChardevCommon',
-'null': 'ChardevCommon',
-'mux': 'ChardevMux',
-'msmouse': 'ChardevCommon',
-'wctablet': 'ChardevCommon',
-'braille': 'ChardevCommon',
-'testdev': 'ChardevCommon',
-'stdio': 'ChardevStdio',
-'console': 'ChardevCommon',
-'spicevmc': { 'type': 'ChardevSpiceChannel',
+ 'base': { 'type': 'ChardevBackendKind' },
+ 'discriminator': 'type',
+ 'data': { 'file': 'ChardevFileWrapper',
+'serial': 'ChardevHostdevWrapper',
+'parallel': 'ChardevHostdevWrapper',
+'pipe': 'ChardevHostdevWrapper',
+'socket': 'ChardevSocketWrapper',
+'udp': 'ChardevUdpWrapper',
+'pty': 'ChardevCommonWrapper',
+'null': 'ChardevCommonWrapper',
+
[PULL v2 20/25] tests/qapi-schema: Purge simple unions from tests
Drop tests that are specifically about simple unions:
* SugaredUnion in doc-good: flat unions are covered by @Object.
* union-branch-case and union-clash-branches: branch naming for flat
unions is enforced for the tag enum instead, which is covered by
enum-member-case and enum-clash-member.
* union-empty: empty flat unions are covered by flat-union-empty.
Rewrite the remainder to use flat unions: args-union, bad-base,
flat-union-base-union, union-branch-invalid-dict, union-unknown.
Except drop union-optional-branch. because converting this one is not
worth the trouble; we don't explicitly check names beginning with '*'
in other places, either.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210917143134.412106-21-arm...@redhat.com>
---
tests/qapi-schema/args-union.err | 2 +-
tests/qapi-schema/args-union.json | 8 ++-
tests/qapi-schema/bad-base.err| 2 +-
tests/qapi-schema/bad-base.json | 8 ++-
tests/qapi-schema/doc-good.json | 9
tests/qapi-schema/doc-good.out| 22 ---
tests/qapi-schema/doc-good.txt| 20 -
tests/qapi-schema/flat-union-base-union.err | 2 +-
tests/qapi-schema/flat-union-base-union.json | 3 +++
tests/qapi-schema/meson.build | 4
tests/qapi-schema/union-branch-case.err | 2 --
tests/qapi-schema/union-branch-case.json | 2 --
tests/qapi-schema/union-branch-case.out | 0
.../qapi-schema/union-branch-invalid-dict.err | 2 +-
.../union-branch-invalid-dict.json| 4
tests/qapi-schema/union-clash-branches.err| 2 --
tests/qapi-schema/union-clash-branches.json | 7 --
tests/qapi-schema/union-clash-branches.out| 0
tests/qapi-schema/union-empty.err | 2 --
tests/qapi-schema/union-empty.json| 2 --
tests/qapi-schema/union-empty.out | 0
tests/qapi-schema/union-optional-branch.err | 2 --
tests/qapi-schema/union-optional-branch.json | 2 --
tests/qapi-schema/union-optional-branch.out | 0
tests/qapi-schema/union-unknown.err | 2 +-
tests/qapi-schema/union-unknown.json | 5 -
26 files changed, 30 insertions(+), 84 deletions(-)
delete mode 100644 tests/qapi-schema/union-branch-case.err
delete mode 100644 tests/qapi-schema/union-branch-case.json
delete mode 100644 tests/qapi-schema/union-branch-case.out
delete mode 100644 tests/qapi-schema/union-clash-branches.err
delete mode 100644 tests/qapi-schema/union-clash-branches.json
delete mode 100644 tests/qapi-schema/union-clash-branches.out
delete mode 100644 tests/qapi-schema/union-empty.err
delete mode 100644 tests/qapi-schema/union-empty.json
delete mode 100644 tests/qapi-schema/union-empty.out
delete mode 100644 tests/qapi-schema/union-optional-branch.err
delete mode 100644 tests/qapi-schema/union-optional-branch.json
delete mode 100644 tests/qapi-schema/union-optional-branch.out
diff --git a/tests/qapi-schema/args-union.err b/tests/qapi-schema/args-union.err
index 4bf4955027..4b80a99f74 100644
--- a/tests/qapi-schema/args-union.err
+++ b/tests/qapi-schema/args-union.err
@@ -1,2 +1,2 @@
args-union.json: In command 'oops':
-args-union.json:3: command's 'data' can take union type 'Uni' only with
'boxed': true
+args-union.json:9: command's 'data' can take union type 'Uni' only with
'boxed': true
diff --git a/tests/qapi-schema/args-union.json
b/tests/qapi-schema/args-union.json
index 2fcaeaae16..aabb159063 100644
--- a/tests/qapi-schema/args-union.json
+++ b/tests/qapi-schema/args-union.json
@@ -1,3 +1,9 @@
# use of union arguments requires 'boxed':true
-{ 'union': 'Uni', 'data': { 'case1': 'int', 'case2': 'str' } }
+{ 'enum': 'Enum', 'data': [ 'case1', 'case2' ] }
+{ 'struct': 'Case1', 'data': { 'data': 'int' } }
+{ 'struct': 'Case2', 'data': { 'data': 'str' } }
+{ 'union': 'Uni',
+ 'base': { 'type': 'Enum' },
+ 'discriminator': 'type',
+ 'data': { 'case1': 'Case1', 'case2': 'Case2' } }
{ 'command': 'oops', 'data': 'Uni' }
diff --git a/tests/qapi-schema/bad-base.err b/tests/qapi-schema/bad-base.err
index 61a1efc2c0..1fad63e392 100644
--- a/tests/qapi-schema/bad-base.err
+++ b/tests/qapi-schema/bad-base.err
@@ -1,2 +1,2 @@
bad-base.json: In struct 'MyType':
-bad-base.json:3: 'base' requires a struct type, union type 'Union' isn't
+bad-base.json:9: 'base' requires a struct type, union type 'Union' isn't
diff --git a/tests/qapi-schema/bad-base.json b/tests/qapi-schema/bad-base.json
index a634331cdd..8c773ff544 100644
--- a/tests/qapi-schema/bad-base.json
+++ b/tests/qapi-schema/bad-base.json
@@ -1,3 +1,9 @@
# we reject a base that is not a struct
-{ 'union': 'Union', 'data': { 'a': 'int', 'b': 'str' } }
+{ 'enum': 'Enum', 'data': [ 'a', 'b' ] }
+{ 'struct': 'Int', 'data': { 'data': 'int' } }
+{ 'struct': 'Str', 'data': { 'data': 'str' } }
+{ 'union': 'Union',
+ 'base': { 'type': 'Enum' },
+
[PULL v2 23/25] test-clone-visitor: Correct an accidental rename
Commit b359f4b203 "tests: Rename UserDefNativeListUnion to
UserDefListUnion" renamed test_clone_native_list() to
test_clone_list_union(). The function has nothing to do with unions.
Rename it to test_clone_list().
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210917143134.412106-24-arm...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé
---
tests/unit/test-clone-visitor.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/unit/test-clone-visitor.c b/tests/unit/test-clone-visitor.c
index 4048018607..5d48e125b8 100644
--- a/tests/unit/test-clone-visitor.c
+++ b/tests/unit/test-clone-visitor.c
@@ -63,7 +63,7 @@ static void test_clone_alternate(void)
qapi_free_AltEnumBool(s_dst);
}
-static void test_clone_list_union(void)
+static void test_clone_list(void)
{
uint8List *src = NULL, *dst;
uint8List *tmp = NULL;
@@ -203,7 +203,7 @@ int main(int argc, char **argv)
g_test_add_func("/visitor/clone/struct", test_clone_struct);
g_test_add_func("/visitor/clone/alternate", test_clone_alternate);
-g_test_add_func("/visitor/clone/list_union", test_clone_list_union);
+g_test_add_func("/visitor/clone/list", test_clone_list);
g_test_add_func("/visitor/clone/empty", test_clone_empty);
g_test_add_func("/visitor/clone/complex1", test_clone_complex1);
g_test_add_func("/visitor/clone/complex2", test_clone_complex2);
--
2.31.1
Re: [PATCH v3 16/27] linux-user/nios2: Properly emulate EXCP_TRAP
On Fri, 24 Sept 2021 at 17:59, Richard Henderson
wrote:
>
> The real kernel has to load the instruction and extract
> the imm5 field; for qemu, modify the translator to do this.
>
> The use of R_AT for this in cpu_loop was a bug. Handle
> the other trap numbers as per the kernel's trap_table.
>
> Signed-off-by: Richard Henderson
> ---
> target/nios2/cpu.h | 5 +++--
> linux-user/nios2/cpu_loop.c | 35 ++-
> target/nios2/translate.c| 17 -
> 3 files changed, 37 insertions(+), 20 deletions(-)
>
> diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h
> index 2ab82fdc71..395e4d3281 100644
> --- a/target/nios2/cpu.h
> +++ b/target/nios2/cpu.h
> @@ -158,9 +158,10 @@ struct Nios2CPUClass {
> struct CPUNios2State {
> uint32_t regs[NUM_CORE_REGS];
>
> -#if !defined(CONFIG_USER_ONLY)
> +#ifdef CONFIG_USER_ONLY
> +int trap_code;
> +#else
> Nios2MMU mmu;
> -
> uint32_t irq_pending;
> #endif
> };
Loading the insn and fishing out the imm5 field is about 2
lines of code, isn't it ? It's how we handle similar cases
for other targets. I think I prefer that over putting
linux-user specific fields and handling into the target/nios2
code.
> diff --git a/linux-user/nios2/cpu_loop.c b/linux-user/nios2/cpu_loop.c
> index 34290fb3b5..246293a501 100644
> --- a/linux-user/nios2/cpu_loop.c
> +++ b/linux-user/nios2/cpu_loop.c
> @@ -39,9 +39,10 @@ void cpu_loop(CPUNios2State *env)
> case EXCP_INTERRUPT:
> /* just indicate that signals should be handled asap */
> break;
> +
> case EXCP_TRAP:
> -if (env->regs[R_AT] == 0) {
> -abi_long ret;
> +switch (env->trap_code) {
> +case 0:
> qemu_log_mask(CPU_LOG_INT, "\nSyscall\n");
>
> ret = do_syscall(env, env->regs[2],
> @@ -55,26 +56,26 @@ void cpu_loop(CPUNios2State *env)
>
> env->regs[2] = abs(ret);
> /* Return value is 0..4096 */
> -env->regs[7] = (ret > 0xf000ULL);
> -env->regs[CR_ESTATUS] = env->regs[CR_STATUS];
> -env->regs[CR_STATUS] &= ~0x3;
> -env->regs[R_EA] = env->regs[R_PC] + 4;
> +env->regs[7] = ret > 0xf000u;
> env->regs[R_PC] += 4;
> break;
> -} else {
> -qemu_log_mask(CPU_LOG_INT, "\nTrap\n");
>
> -env->regs[CR_ESTATUS] = env->regs[CR_STATUS];
> -env->regs[CR_STATUS] &= ~0x3;
> -env->regs[R_EA] = env->regs[R_PC] + 4;
> -env->regs[R_PC] = cpu->exception_addr;
> -
> -info.si_signo = TARGET_SIGTRAP;
> -info.si_errno = 0;
> -info.si_code = TARGET_TRAP_BRKPT;
> -queue_signal(env, info.si_signo, QEMU_SI_FAULT, );
> +case 1:
> +qemu_log_mask(CPU_LOG_INT, "\nTrap 1\n");
> +force_sig_fault(TARGET_SIGUSR1, 0, env->regs[R_PC]);
> +break;
> +case 2:
> +qemu_log_mask(CPU_LOG_INT, "\nTrap 2\n");
> +force_sig_fault(TARGET_SIGUSR2, 0, env->regs[R_PC]);
> +break;
> +default:
> +qemu_log_mask(CPU_LOG_INT, "\nTrap %d\n", env->trap_code);
> +force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLTRP,
> +env->regs[R_PC]);
> break;
> }
The kernel also defines:
* trap 31 ("breakpoint"), which should wind PC back by 4 and
send a SIGTRAP/TRAP_BRKPT
* trap 30 ("KGDB breakpoint"), which we should treat the same
as the "default" case since we should be acting like "kernel
with CONFIG_KGDB not defined"
Side note: the kernel code for the "CONFIG_KGDB not defined" case
of trap 30 seems buggy to me. It points the trap at 'instruction_trap',
but that is the "emulate multiply and divide insns" entry point, and
that emulation code assumes that it really is getting a mul or div,
not a trap, so I think it will do something bogus. This seems to
be an error introduced in kernel commit baa54ab93c2e1, which refactored
trap handling and changed the reserved-trap-number handling from
"instruction_trap" to "handle_trap_reserved" but forgot this one entry.
> +break;
> +
> case EXCP_DEBUG:
> info.si_signo = TARGET_SIGTRAP;
> info.si_errno = 0;
thanks
-- PMM
Re: [PATCH v3 21/27] linux-user/ppc: Implement setup_sigtramp
On Fri, 24 Sept 2021 at 17:59, Richard Henderson wrote: > > Create and record the two signal trampolines. > > Cc: qemu-...@nongnu.org > Signed-off-by: Richard Henderson > --- > linux-user/ppc/target_signal.h | 2 ++ > linux-user/ppc/signal.c| 34 ++ > 2 files changed, 20 insertions(+), 16 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v3 17/27] linux-user/nios2: Map a real kuser page
On Fri, 24 Sept 2021 at 17:59, Richard Henderson
wrote:
>
> The first word of page1 is data, so the whole thing
> can't be implemented with emulation of addresses.
>
> Hijack trap number 31 to implement cmpxchg.
31 is used -- it's "breakpoint". We need to pick something else...
>
> Set default_rt_sigreturn based on the kuser page.
>
> Signed-off-by: Richard Henderson
> ---
> linux-user/nios2/target_signal.h | 3 ++
> linux-user/elfload.c | 35 ++
> linux-user/nios2/cpu_loop.c | 51 +---
> linux-user/nios2/signal.c| 2 +-
> target/nios2/translate.c | 9 --
> 5 files changed, 66 insertions(+), 34 deletions(-)
>
> diff --git a/linux-user/nios2/target_signal.h
> b/linux-user/nios2/target_signal.h
> index aebf749f12..fe266c4c51 100644
> --- a/linux-user/nios2/target_signal.h
> +++ b/linux-user/nios2/target_signal.h
> @@ -19,4 +19,7 @@ typedef struct target_sigaltstack {
>
> #include "../generic/signal.h"
>
> +/* Nios2 uses a fixed address on the kuser page for sigreturn. */
> +#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 0
> +
> #endif /* NIOS2_TARGET_SIGNAL_H */
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 459a26ef1d..7b3a91b3ed 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -1100,6 +1100,41 @@ static void elf_core_copy_regs(target_elf_gregset_t
> *regs, const CPUMBState *env
>
> static void init_thread(struct target_pt_regs *regs, struct image_info
> *infop)
> {
> +static const uint8_t kuser_page[4 + 2 * 64] = {
> +/* __kuser_helper_version */
> +[0x00] = 0x02, 0x00, 0x00, 0x00,
> +
> +/* __kuser_cmpxchg */
> +[0x04] = 0xfa, 0x6f, 0x3b, 0x00, /* trap 31 */
> + 0x3a, 0x28, 0x00, 0xf8, /* ret */
> +
> +/* __kuser_sigtramp */
> +[0x44] = 0xc4, 0x22, 0x80, 0x00, /* movi r2, __NR_rt_sigreturn */
> + 0x3a, 0x68, 0x3b, 0x00, /* trap 0 */
> +};
> +
> +abi_ulong pg;
> +uint8_t *ph;
> +
> +pg = target_mmap(TARGET_PAGE_SIZE, TARGET_PAGE_SIZE,
> + PROT_READ | PROT_WRITE,
> + MAP_ANON | MAP_PRIVATE | MAP_FIXED, -1, 0);
> +
> +/*
> + * If the mmap doesn't give us exactly page 1, there's nothing
> + * we can do, and it's unlikely that the program will be able
> + * to continue. FIXME: Error out now?
> + */
> +assert(pg == TARGET_PAGE_SIZE);
Shouldn't we be doing this via the probe_guest_base machinery
the way we do for the Arm commpage ?
> +
> +ph = lock_user(VERIFY_WRITE, pg, sizeof(kuser_page), 0);
> +memcpy(ph, kuser_page, sizeof(kuser_page));
> +unlock_user(ph, pg, sizeof(kuser_page));
> +target_mprotect(TARGET_PAGE_SIZE, TARGET_PAGE_SIZE, PROT_READ |
> PROT_EXEC);
> +
> +/* Install __kuser_sigtramp */
> +default_rt_sigreturn = TARGET_PAGE_SIZE + 0x44;
> +
> regs->ea = infop->entry;
> regs->sp = infop->start_stack;
> regs->estatus = 0x3;
-- PMM
[PATCH v2 0/2] modules: Improve modinfo.c support
This patchset introduces the modinfo_need and changes
modinfo-generate.py/meson.build to generate/link one modinfo per target.
modinfo-generate.py will know, thanks to modinfo_need, which modules are
currently enabled for a given target before adding it in the array of
modules. It will give a hint about why some modules failed, so
developers can have a clue about it:
},{
/* hw-display-qxl.modinfo */
/* module QXL is missing. */
/* hw-display-virtio-gpu.modinfo */
.name = "hw-display-virtio-gpu",
.objs = ((const char*[]){ "virtio-gpu-base", "virtio-gpu-device",
"vhost-user-gpu", NULL }),
},{
The main reason of this change is to fix problems like:
$ ./qemu-system-s390x -nodefaults -display none -accel tcg -M none -device help
| head
Failed to open module: /.../hw-display-qxl.so: undefined symbol: vga_ioport_read
Failed to open module: /.../hw-display-virtio-vga.so: undefined symbol:
vmstate_vga_common
Failed to open module: /.../hw-display-virtio-vga.so: undefined symbol:
vmstate_vga_common
Failed to open module: /.../hw-display-virtio-vga-gl.so: undefined symbol:
have_vga
Failed to open module: /.../hw-usb-smartcard.so: undefined symbol:
ccid_card_ccid_attach
Failed to open module: /.../hw-usb-redirect.so: undefined symbol:
vmstate_usb_device
Failed to open module: /.../hw-usb-host.so: undefined symbol: vmstate_usb_device
With this patch, I run this small script successfuly:
#!/bin/bash
pushd ~/suse/virtualization/qemu/build
for qemu in qemu-system-*
do
[[ -f "$qemu" ]] || continue
res=$(./$qemu -nodefaults -display none -accel tcg -M none -device help
2>&1 | grep "Failed to" > /dev/null; echo $?)
[[ $res -eq 0 ]] && echo "Error: $qemu"
done
popd
Also run make check and check-acceptance without any failures.
Todo:
- accelerators can be filtered as well (this only covers the device
part), then the field QemuModinfo.arch can be removed.
v1 -> v2:
- Changed the approach to this problem after suggestions made by Paolo and
Gerd.
Thank you!
Jose R. Ziviani (2):
modules: introduces module_needs directive
modules: generates per-target modinfo
hw/display/qxl.c| 1 +
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 | 1 +
hw/display/virtio-gpu-pci-gl.c | 1 +
hw/display/virtio-gpu-pci.c | 1 +
hw/display/virtio-gpu.c | 1 +
hw/display/virtio-vga-gl.c | 1 +
hw/display/virtio-vga.c | 1 +
hw/s390x/virtio-ccw-gpu.c | 1 +
hw/usb/ccid-card-emulated.c | 1 +
hw/usb/ccid-card-passthru.c | 1 +
hw/usb/host-libusb.c| 1 +
hw/usb/redirect.c | 1 +
include/qemu/module.h | 10 +
meson.build | 25 ++---
scripts/modinfo-generate.py | 40 -
19 files changed, 67 insertions(+), 24 deletions(-)
--
2.33.0
Re: [PATCH 4/6] avocado_qemu: tweak ssh connect method
On 9/20/21 22:49, Willian Rampazzo wrote:
> The current implementation will crash if the connection fails as the
> `time` module is not imported. This fixes the import problem and tweaks
> the connection to wait progressively when the connection fails.
>
> Signed-off-by: Willian Rampazzo
> ---
> tests/acceptance/avocado_qemu/__init__.py | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py
> b/tests/acceptance/avocado_qemu/__init__.py
> index edb9ed7485..c3613f9262 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -13,6 +13,7 @@
> import shutil
> import sys
> import tempfile
> +import time
> import uuid
>
> import avocado
> @@ -305,8 +306,7 @@ def ssh_connect(self, username, credential,
> credential_is_key=True):
> self.ssh_session.connect()
> return
> except:
> -time.sleep(4)
10 * 4 = 40
> -pass
> +time.sleep(i)
sum([0..10[) = 45
The described tweak. Almost the same, OK.
> self.fail('ssh connection timeout')
>
> def ssh_command(self, command):
>
[PATCH 0/1] virtio-gpu: CONTEXT_INIT feature
This is a different attempt at upstreaming the work I have been doing to enable support for the Venus Virtio-GPU Vulkan driver. I believe the previous one [0] was a bit too much stuff in one place, therefore with this I would like to try a more fine-grained approach. I will just start by the CONTEXT_INIT feature as it was the first commit of the series aforementioned and the virtio-spec has been updated recently on that regard [1]. Hopefully this would also answer Gerd's comment on the previous patch [2]. [0] https://www.mail-archive.com/qemu-devel@nongnu.org/msg826897.html [1] https://github.com/oasis-tcs/virtio-spec/commit/aad2b6f3620ec0c9d16aaf046db8c282c24cce3e [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg827304.html Antonio Caggiano (1): virtio-gpu: CONTEXT_INIT feature hw/display/virtio-gpu-base.c| 2 ++ hw/display/virtio-gpu-virgl.c | 10 -- include/hw/virtio/virtio-gpu-bswap.h| 2 +- include/standard-headers/linux/virtio_gpu.h | 9 +++-- 4 files changed, 18 insertions(+), 5 deletions(-) -- 2.30.2
Re: gitlab-ci: amd64-opensuse-leap-container job failing
On Mon, Sep 27, 2021 at 04:35:04PM +0200, Philippe Mathieu-Daudé wrote: > On 9/27/21 15:47, Daniel P. Berrangé wrote: > > On Mon, Sep 27, 2021 at 09:35:22AM +0100, Daniel P. Berrangé wrote: > >> On Sun, Sep 26, 2021 at 07:23:56PM +0200, Philippe Mathieu-Daudé wrote: > >>> Hi, > >>> > >>> FYI the OpenSUSE job is failing since few days, i.e.: > >>> https://gitlab.com/qemu-project/qemu/-/jobs/1622345026 > >>> > >>> Retrieving repository 'Main Repository' metadata > >>> [..error] > >>> Repository 'Main Repository' is invalid. > >>> > >>> [repo-oss|http://download.opensuse.org/distribution/leap/15.2/repo/oss/] > >>> Valid metadata not found at specified URL > >>> History: > >>>- Download (curl) error for > >>> 'http://download.opensuse.org/distribution/leap/15.2/repo/oss/repodata/repomd.xml': > >>> Error code: Curl error 56 > >>> Error message: Recv failure: Connection reset by peer > >>>- Can't provide /repodata/repomd.xml > >>> Please check if the URIs defined for this repository are pointing to a > >>> valid repository. > >>> Warning: Skipping repository 'Main Repository' because of the above > >>> error. > >>> > >>> I tried to run 'zypper ref' with: > >> > >> It isn't confined to only SuSE. In libvirt we've had similar problems > >> with several other jobs, though are suse jobs are the worst affected. > >> > >> GitLab have finally acknowledged it is an general infra issue affecting > >> many things: > >> > >>https://status.gitlab.com/ > >>https://gitlab.com/gitlab-com/gl-infra/production/-/issues/5590 > > > > Setting GitLab CI env var works around the problem temporarily: > > > > FF_NETWORK_PER_BUILD=true > > Do you know if we need to recreate the pipeline? > (It didn't work on already failing one, I'm going to test > a freshly created one now). You shoudln't need to re-create the pipeline, just retry the job. If setting the variable in the web UI, make sure "Protect variable" is *not* set. Only the "master" branch is protected by default so other branches won't get protected variables set in their jobs. > > > You can set it for all repos under a group eg > > > > https://gitlab.com/groups/qemu-project/-/settings/ci_cd > > > > or per repo eg > > > > https://gitlab.com/berrange/libvirt/-/settings/ci_cd > > > > > > Regards, > > Daniel > > > Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] hw: Add a 'Sensor devices' qdev category
On 9/27/21 13:33, Corey Minyard wrote:
> On Mon, Sep 27, 2021 at 12:15:18AM +0200, Philippe Mathieu-Daudé wrote:
>> Sensors models are listed in the 'Misc devices' category.
>> Move them to their own category.
>>
>> For the devices in the hw/sensor/ directory, the category
>> is obvious.
>>
>> hw/arm/z2.c models the AER915 model which is described
>> on [*] as:
>>
>> The 14-pin chip marked AER915 just below the expansion
>> port is a 80C51-type microcontroller, similar to Philips
>> P89LPC915. It has an 8-bit A/D which is used to determine
>> which of six buttons are pressed on the resistor-network
>> wired remote. It communicates with the main cpu via I2C.
>>
>> It was introduced in commit 3bf11207c06 ("Add support for
>> Zipit Z2 machine") with this comment:
>>
>> 248 static uint8_t aer915_recv(I2CSlave *slave)
>> 249 {
>> ...
>> 253 switch (s->buf[0]) {
>> 254 /* Return hardcoded battery voltage,
>> 255 * 0xf0 means ~4.1V
>> 256 */
>> 257 case 0x02:
>> 258 retval = 0xf0;
>> 259 break;
>>
>> For QEMU the AER915 is a very simple sensor model.
>>
>> [*] https://www.bealecorner.org/best/measure/z2/index.html
>>
>> Signed-off-by: Philippe Mathieu-Daudé
>
> This makes sense to me. I'd like to hear from others on this.
Devices on a bus (in particular I2C) are usually user-creatable
by default. The AER915 is more a band aid for the z2 machine,
but is not really a device. IMO it would be better to hide it
as non-user-creatable qdev.
>> ---
>> include/hw/qdev-core.h | 1 +
>> hw/arm/z2.c| 1 +
>> hw/sensor/adm1272.c| 1 +
>> hw/sensor/dps310.c | 1 +
>> hw/sensor/emc141x.c| 1 +
>> hw/sensor/max34451.c | 2 ++
>> hw/sensor/tmp105.c | 1 +
>> hw/sensor/tmp421.c | 1 +
>> softmmu/qdev-monitor.c | 1 +
>> 9 files changed, 10 insertions(+)
>> diff --git a/hw/arm/z2.c b/hw/arm/z2.c
>> index 9c1e876207b..62db9741106 100644
>> --- a/hw/arm/z2.c
>> +++ b/hw/arm/z2.c
>> @@ -288,6 +288,7 @@ static void aer915_class_init(ObjectClass *klass, void
>> *data)
>> k->recv = aer915_recv;
>> k->send = aer915_send;
>> dc->vmsd = _aer915_state;
>> +set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
>> }
[PULL v2 14/25] test-clone-visitor: Wean off UserDefListUnion
test_clone_complex1() uses simple union UserDefListUnion to cover
unions. Use UserDefFlatUnion instead. Arrays are still covered by
test_clone_complex3().
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210917143134.412106-15-arm...@redhat.com>
---
tests/unit/test-clone-visitor.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/tests/unit/test-clone-visitor.c b/tests/unit/test-clone-visitor.c
index 4944b3d857..8357a90e60 100644
--- a/tests/unit/test-clone-visitor.c
+++ b/tests/unit/test-clone-visitor.c
@@ -99,18 +99,26 @@ static void test_clone_empty(void)
static void test_clone_complex1(void)
{
-UserDefListUnion *src, *dst;
+UserDefFlatUnion *src, *dst;
-src = g_new0(UserDefListUnion, 1);
-src->type = USER_DEF_LIST_UNION_KIND_STRING;
+src = g_new0(UserDefFlatUnion, 1);
+src->integer = 123;
+src->string = g_strdup("abc");
+src->enum1 = ENUM_ONE_VALUE1;
+src->u.value1.boolean = true;
-dst = QAPI_CLONE(UserDefListUnion, src);
+dst = QAPI_CLONE(UserDefFlatUnion, src);
g_assert(dst);
-g_assert_cmpint(dst->type, ==, src->type);
-g_assert(!dst->u.string.data);
-qapi_free_UserDefListUnion(src);
-qapi_free_UserDefListUnion(dst);
+g_assert_cmpint(dst->integer, ==, 123);
+g_assert_cmpstr(dst->string, ==, "abc");
+g_assert_cmpint(dst->enum1, ==, ENUM_ONE_VALUE1);
+g_assert(dst->u.value1.boolean);
+g_assert(!dst->u.value1.has_a_b);
+g_assert_cmpint(dst->u.value1.a_b, ==, 0);
+
+qapi_free_UserDefFlatUnion(src);
+qapi_free_UserDefFlatUnion(dst);
}
static void test_clone_complex2(void)
--
2.31.1
[PULL v2 15/25] tests/qapi-schema: Wean off UserDefListUnion
Command boxed-union uses simple union UserDefListUnion to cover
unions. Use UserDefFlatUnion instead.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210917143134.412106-16-arm...@redhat.com>
---
tests/unit/test-qmp-cmds.c | 2 +-
tests/qapi-schema/qapi-schema-test.json | 2 +-
tests/qapi-schema/qapi-schema-test.out | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/unit/test-qmp-cmds.c b/tests/unit/test-qmp-cmds.c
index 83efa39720..83c9ef5b7c 100644
--- a/tests/unit/test-qmp-cmds.c
+++ b/tests/unit/test-qmp-cmds.c
@@ -119,7 +119,7 @@ void qmp_boxed_struct(UserDefZero *arg, Error **errp)
{
}
-void qmp_boxed_union(UserDefListUnion *arg, Error **errp)
+void qmp_boxed_union(UserDefFlatUnion *arg, Error **errp)
{
}
diff --git a/tests/qapi-schema/qapi-schema-test.json
b/tests/qapi-schema/qapi-schema-test.json
index b2d795cb19..a4b4405f94 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -175,7 +175,7 @@
'returns': 'int' }
{ 'command': 'guest-sync', 'data': { 'arg': 'any' }, 'returns': 'any' }
{ 'command': 'boxed-struct', 'boxed': true, 'data': 'UserDefZero' }
-{ 'command': 'boxed-union', 'data': 'UserDefListUnion', 'boxed': true }
+{ 'command': 'boxed-union', 'data': 'UserDefFlatUnion', 'boxed': true }
{ 'command': 'boxed-empty', 'boxed': true, 'data': 'Empty1' }
# Smoke test on out-of-band and allow-preconfig-test
diff --git a/tests/qapi-schema/qapi-schema-test.out
b/tests/qapi-schema/qapi-schema-test.out
index 7a488c1d06..f120f10616 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -232,7 +232,7 @@ command guest-sync q_obj_guest-sync-arg -> any
gen=True success_response=True boxed=False oob=False preconfig=False
command boxed-struct UserDefZero -> None
gen=True success_response=True boxed=True oob=False preconfig=False
-command boxed-union UserDefListUnion -> None
+command boxed-union UserDefFlatUnion -> None
gen=True success_response=True boxed=True oob=False preconfig=False
command boxed-empty Empty1 -> None
gen=True success_response=True boxed=True oob=False preconfig=False
--
2.31.1
[PULL v2 09/25] qapi: Convert simple union ImageInfoSpecific to flat one
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
To prepare for their removal, convert simple union ImageInfoSpecific
to an equivalent flat one. Adds some boilerplate to the schema, which
is a bit ugly, but a lot easier to maintain than the simple union
feature.
Implicit enum ImageInfoSpecificKind becomes explicit. It duplicates
part of enum BlockdevDriver. We could reuse BlockdevDriver instead.
Cc: Kevin Wolf
Cc: Hanna Reitz
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Acked-by: Hanna Reitz
Message-Id: <20210917143134.412106-10-arm...@redhat.com>
---
qapi/block-core.json | 59 ++--
1 file changed, 52 insertions(+), 7 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index c8ce1d9d5d..623a4f4a3f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -139,6 +139,52 @@
'*encryption-format': 'RbdImageEncryptionFormat'
} }
+##
+# @ImageInfoSpecificKind:
+#
+# @luks: Since 2.7
+# @rbd: Since 6.1
+#
+# Since: 1.7
+##
+{ 'enum': 'ImageInfoSpecificKind',
+ 'data': [ 'qcow2', 'vmdk', 'luks', 'rbd' ] }
+
+##
+# @ImageInfoSpecificQCow2Wrapper:
+#
+# Since: 1.7
+##
+{ 'struct': 'ImageInfoSpecificQCow2Wrapper',
+ 'data': { 'data': 'ImageInfoSpecificQCow2' } }
+
+##
+# @ImageInfoSpecificVmdkWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ImageInfoSpecificVmdkWrapper',
+ 'data': { 'data': 'ImageInfoSpecificVmdk' } }
+
+##
+# @ImageInfoSpecificLUKSWrapper:
+#
+# Since: 2.7
+##
+{ 'struct': 'ImageInfoSpecificLUKSWrapper',
+ 'data': { 'data': 'QCryptoBlockInfoLUKS' } }
+# If we need to add block driver specific parameters for
+# LUKS in future, then we'll subclass QCryptoBlockInfoLUKS
+# to define a ImageInfoSpecificLUKS
+
+##
+# @ImageInfoSpecificRbdWrapper:
+#
+# Since: 6.1
+##
+{ 'struct': 'ImageInfoSpecificRbdWrapper',
+ 'data': { 'data': 'ImageInfoSpecificRbd' } }
+
##
# @ImageInfoSpecific:
#
@@ -147,14 +193,13 @@
# Since: 1.7
##
{ 'union': 'ImageInfoSpecific',
+ 'base': { 'type': 'ImageInfoSpecificKind' },
+ 'discriminator': 'type',
'data': {
- 'qcow2': 'ImageInfoSpecificQCow2',
- 'vmdk': 'ImageInfoSpecificVmdk',
- # If we need to add block driver specific parameters for
- # LUKS in future, then we'll subclass QCryptoBlockInfoLUKS
- # to define a ImageInfoSpecificLUKS
- 'luks': 'QCryptoBlockInfoLUKS',
- 'rbd': 'ImageInfoSpecificRbd'
+ 'qcow2': 'ImageInfoSpecificQCow2Wrapper',
+ 'vmdk': 'ImageInfoSpecificVmdkWrapper',
+ 'luks': 'ImageInfoSpecificLUKSWrapper',
+ 'rbd': 'ImageInfoSpecificRbdWrapper'
} }
##
--
2.31.1
[PULL v2 22/25] tests/qapi-schema: Rename flat-union-* test cases to union-*
Signed-off-by: Markus Armbruster
Message-Id: <20210917143134.412106-23-arm...@redhat.com>
Reviewed-by: Eric Blake union-array-branch.json} | 0
...rray-branch.out => union-array-branch.out} | 0
tests/qapi-schema/union-bad-base.err | 2 ++
...nion-bad-base.json => union-bad-base.json} | 0
...-union-bad-base.out => union-bad-base.out} | 0
tests/qapi-schema/union-bad-discriminator.err | 2 ++
...ator.json => union-bad-discriminator.json} | 0
...inator.out => union-bad-discriminator.out} | 0
tests/qapi-schema/union-base-any.err | 2 ++
...nion-base-any.json => union-base-any.json} | 0
...-union-base-any.out => union-base-any.out} | 0
tests/qapi-schema/union-base-union.err| 2 ++
...-base-union.json => union-base-union.json} | 0
...on-base-union.out => union-base-union.out} | 0
tests/qapi-schema/union-clash-member.err | 2 ++
...sh-member.json => union-clash-member.json} | 0
...lash-member.out => union-clash-member.out} | 0
.../union-discriminator-bad-name.err | 2 ++
...json => union-discriminator-bad-name.json} | 0
...e.out => union-discriminator-bad-name.out} | 0
tests/qapi-schema/union-empty.err | 2 ++
...flat-union-empty.json => union-empty.json} | 0
.../{flat-union-empty.out => union-empty.out} | 0
.../qapi-schema/union-inline-invalid-dict.err | 2 ++
...ct.json => union-inline-invalid-dict.json} | 0
...dict.out => union-inline-invalid-dict.out} | 0
tests/qapi-schema/union-int-branch.err| 2 ++
...-int-branch.json => union-int-branch.json} | 0
...on-int-branch.out => union-int-branch.out} | 0
.../qapi-schema/union-invalid-branch-key.err | 2 ++
...key.json => union-invalid-branch-key.json} | 0
...h-key.out => union-invalid-branch-key.out} | 0
.../union-invalid-discriminator.err | 2 ++
json => union-invalid-discriminator.json} | 0
...or.out => union-invalid-discriminator.out} | 0
.../union-invalid-if-discriminator.err| 2 ++
...on => union-invalid-if-discriminator.json} | 0
...out => union-invalid-if-discriminator.out} | 0
tests/qapi-schema/union-no-base.err | 2 ++
...-union-no-base.json => union-no-base.json} | 0
...at-union-no-base.out => union-no-base.out} | 0
.../union-optional-discriminator.err | 2 ++
...json => union-optional-discriminator.json} | 0
...r.out => union-optional-discriminator.out} | 0
.../union-string-discriminator.err| 2 ++
...r.json => union-string-discriminator.json} | 0
...tor.out => union-string-discriminator.out} | 0
65 files changed, 48 insertions(+), 48 deletions(-)
delete mode 100644 tests/qapi-schema/flat-union-array-branch.err
delete mode 100644 tests/qapi-schema/flat-union-bad-base.err
delete mode 100644 tests/qapi-schema/flat-union-bad-discriminator.err
delete mode 100644 tests/qapi-schema/flat-union-base-any.err
delete mode 100644 tests/qapi-schema/flat-union-base-union.err
delete mode 100644 tests/qapi-schema/flat-union-clash-member.err
delete mode 100644 tests/qapi-schema/flat-union-discriminator-bad-name.err
delete mode 100644 tests/qapi-schema/flat-union-empty.err
delete mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.err
delete mode 100644 tests/qapi-schema/flat-union-int-branch.err
delete mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.err
delete mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.err
delete mode 100644 tests/qapi-schema/flat-union-invalid-if-discriminator.err
delete mode 100644 tests/qapi-schema/flat-union-no-base.err
delete mode 100644 tests/qapi-schema/flat-union-optional-discriminator.err
delete mode 100644 tests/qapi-schema/flat-union-string-discriminator.err
create mode 100644 tests/qapi-schema/union-array-branch.err
rename tests/qapi-schema/{flat-union-array-branch.json =>
union-array-branch.json} (100%)
rename tests/qapi-schema/{flat-union-array-branch.out =>
union-array-branch.out} (100%)
create mode 100644 tests/qapi-schema/union-bad-base.err
rename tests/qapi-schema/{flat-union-bad-base.json => union-bad-base.json}
(100%)
rename tests/qapi-schema/{flat-union-bad-base.out => union-bad-base.out} (100%)
create mode 100644 tests/qapi-schema/union-bad-discriminator.err
rename tests/qapi-schema/{flat-union-bad-discriminator.json =>
union-bad-discriminator.json} (100%)
rename tests/qapi-schema/{flat-union-bad-discriminator.out =>
union-bad-discriminator.out} (100%)
create mode 100644 tests/qapi-schema/union-base-any.err
rename tests/qapi-schema/{flat-union-base-any.json => union-base-any.json}
(100%)
rename tests/qapi-schema/{flat-union-base-any.out => union-base-any.out} (100%)
create mode 100644 tests/qapi-schema/union-base-union.err
rename tests/qapi-schema/{flat-union-base-union.json => union-base-union.json}
(100%)
rename tests/qapi-schema/{flat-union-base-union.out => union-base-union.out}
(100%)
create mode 100644 tests/qapi-schema/union-clash-member.err
rename
[PULL v2 16/25] tests/qapi-schema: Simple union UserDefListUnion is now unused, drop
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210917143134.412106-17-arm...@redhat.com>
---
tests/qapi-schema/qapi-schema-test.json | 17 ---
tests/qapi-schema/qapi-schema-test.out | 64 -
2 files changed, 81 deletions(-)
diff --git a/tests/qapi-schema/qapi-schema-test.json
b/tests/qapi-schema/qapi-schema-test.json
index a4b4405f94..eae43f41c4 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -123,23 +123,6 @@
# for testing use of 'str' within alternates
{ 'alternate': 'AltStrObj', 'data': { 's': 'str', 'o': 'TestStruct' } }
-# for testing lists
-{ 'union': 'UserDefListUnion',
- 'data': { 'integer': ['int'],
-'s8': ['int8'],
-'s16': ['int16'],
-'s32': ['int32'],
-'s64': ['int64'],
-'u8': ['uint8'],
-'u16': ['uint16'],
-'u32': ['uint32'],
-'u64': ['uint64'],
-'number': ['number'],
-'boolean': ['bool'],
-'string': ['str'],
-'sizes': ['size'],
-'any': ['any'],
-'user': ['Status'] } } # intentional forward ref. to sub-module
{ 'struct': 'ArrayStruct',
'data': { 'integer': ['int'],
's8': ['int8'],
diff --git a/tests/qapi-schema/qapi-schema-test.out
b/tests/qapi-schema/qapi-schema-test.out
index f120f10616..e43073d795 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -125,70 +125,6 @@ alternate AltStrObj
tag type
case s: str
case o: TestStruct
-object q_obj_intList-wrapper
-member data: intList optional=False
-object q_obj_int8List-wrapper
-member data: int8List optional=False
-object q_obj_int16List-wrapper
-member data: int16List optional=False
-object q_obj_int32List-wrapper
-member data: int32List optional=False
-object q_obj_int64List-wrapper
-member data: int64List optional=False
-object q_obj_uint8List-wrapper
-member data: uint8List optional=False
-object q_obj_uint16List-wrapper
-member data: uint16List optional=False
-object q_obj_uint32List-wrapper
-member data: uint32List optional=False
-object q_obj_uint64List-wrapper
-member data: uint64List optional=False
-object q_obj_numberList-wrapper
-member data: numberList optional=False
-object q_obj_boolList-wrapper
-member data: boolList optional=False
-object q_obj_strList-wrapper
-member data: strList optional=False
-object q_obj_sizeList-wrapper
-member data: sizeList optional=False
-object q_obj_anyList-wrapper
-member data: anyList optional=False
-object q_obj_StatusList-wrapper
-member data: StatusList optional=False
-enum UserDefListUnionKind
-member integer
-member s8
-member s16
-member s32
-member s64
-member u8
-member u16
-member u32
-member u64
-member number
-member boolean
-member string
-member sizes
-member any
-member user
-object UserDefListUnion
-member type: UserDefListUnionKind optional=False
-tag type
-case integer: q_obj_intList-wrapper
-case s8: q_obj_int8List-wrapper
-case s16: q_obj_int16List-wrapper
-case s32: q_obj_int32List-wrapper
-case s64: q_obj_int64List-wrapper
-case u8: q_obj_uint8List-wrapper
-case u16: q_obj_uint16List-wrapper
-case u32: q_obj_uint32List-wrapper
-case u64: q_obj_uint64List-wrapper
-case number: q_obj_numberList-wrapper
-case boolean: q_obj_boolList-wrapper
-case string: q_obj_strList-wrapper
-case sizes: q_obj_sizeList-wrapper
-case any: q_obj_anyList-wrapper
-case user: q_obj_StatusList-wrapper
object ArrayStruct
member integer: intList optional=False
member s8: int8List optional=False
--
2.31.1
Re: [PATCH] tcg/riscv: Fix potential bug in clobbered call register set
On 9/27/21 1:36 AM, Philippe Mathieu-Daudé wrote: There are not 64 registers, so this is incorrect. Currently there are 32 registers, but I was looking at this draft: https://five-embeddev.com/riscv-v-spec/draft/v-spec.html#_vector_registers "The vector extension adds 32 architectural vector registers, v0-v31 to the base scalar RISC-V ISA." If this were to be implemented (and available on the host), wouldn't we have 64 registers? Sure. But there are *lots* of changes required before that happens, and certainly you shouldn't be assuming what the ABI is now. Eventually this line would be easier to review as: tcg_target_call_clobber_regs = MAKE_64BIT_MASK(0, TCG_TARGET_NB_REGS); Would it? Or would it be eaier to review with tcg_target_call_clobber_regs = 0; followed by a set of each register that is call clobbered. Why are you assuming that it's safer to list unknown registers as call-clobbered? IF ANYTHING, it might be safer to assume that all new registers are caller saved. But as a general principal, I also don't like register masks containing set bits outside the range of the mask. r~
[PATCH v3 1/3] tests/acpi: Add void table for virt/DBG2 bios-tables-test
Add placeholders for DBG2 reference table for virt tests and ignore till reference blob is added. Signed-off-by: Eric Auger Acked-by: Igor Mammedov --- v2 -> v3: - commit msg rewording according to Igor's suggestion and added Igor's A-b. --- tests/qtest/bios-tables-test-allowed-diff.h | 1 + tests/data/acpi/virt/DBG2 | 0 2 files changed, 1 insertion(+) create mode 100644 tests/data/acpi/virt/DBG2 diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..1910d154c2 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,2 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/virt/DBG2", diff --git a/tests/data/acpi/virt/DBG2 b/tests/data/acpi/virt/DBG2 new file mode 100644 index 00..e69de29bb2 -- 2.26.3
[PATCH v3 2/3] hw/arm/virt_acpi_build: Generate DBG2 table
ARM SBBR specification mandates DBG2 table (Debug Port Table 2)
since v1.0 (ARM DEN0044F 8.3.1.7 DBG2).
The DBG2 table allows to describe one or more debug ports.
Generate an DBG2 table featuring a single debug port, the PL011.
The DBG2 specification can be found at
"Microsoft Debug Port Table 2 (DBG2)"
https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/acpi-debug-port-table?redirectedfrom=MSDN
Signed-off-by: Eric Auger
---
v2 -> v3:
Took into account all comments from Igor on v2:
mostly style adjustment, revision references
v1 -> v2:
- rebased on Igor's refactoring
---
hw/arm/virt-acpi-build.c | 62 +++-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 6cec97352b..257d0fee17 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -616,6 +616,63 @@ build_gtdt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
acpi_table_end(linker, );
}
+/* Debug Port Table 2 (DBG2) */
+static void
+build_dbg2(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
+{
+AcpiTable table = { .sig = "DBG2", .rev = 3, .oem_id = vms->oem_id,
+.oem_table_id = vms->oem_table_id };
+int dbg2devicelength;
+const char name[] = "COM0";
+const int namespace_length = sizeof(name);
+
+acpi_table_begin(, table_data);
+
+dbg2devicelength = 22 /* BaseAddressRegister[] offset */ +
+ 12 /* BaseAddressRegister[] */ +
+ 4 /* AddressSize[] */ +
+ namespace_length /* NamespaceString[] */;
+
+/* OffsetDbgDeviceInfo */
+build_append_int_noprefix(table_data, 44, 4);
+/* NumberDbgDeviceInfo */
+build_append_int_noprefix(table_data, 1, 4);
+
+/* Table 2. Debug Device Information structure format */
+build_append_int_noprefix(table_data, 0, 1); /* Revision */
+build_append_int_noprefix(table_data, dbg2devicelength, 2); /* Length */
+/* NumberofGenericAddressRegisters */
+build_append_int_noprefix(table_data, 1, 1);
+/* NameSpaceStringLength */
+build_append_int_noprefix(table_data, namespace_length, 2);
+build_append_int_noprefix(table_data, 38, 2); /* NameSpaceStringOffset */
+build_append_int_noprefix(table_data, 0, 2); /* OemDataLength */
+/* OemDataOffset (0 means no OEM data) */
+build_append_int_noprefix(table_data, 0, 2);
+
+/* Port Type */
+build_append_int_noprefix(table_data, 0x8000 /* Serial */, 2);
+/* Port Subtype */
+build_append_int_noprefix(table_data, 0x3 /* ARM PL011 UART */, 2);
+build_append_int_noprefix(table_data, 0, 2); /* Reserved */
+/* BaseAddressRegisterOffset */
+build_append_int_noprefix(table_data, 22, 2);
+/* AddressSizeOffset */
+build_append_int_noprefix(table_data, 34, 2);
+
+/* BaseAddressRegister[] */
+build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 8, 0, 1,
+ vms->memmap[VIRT_UART].base);
+
+/* AddressSize[] */
+build_append_int_noprefix(table_data, 0x1000 /* Register Space */, 4);
+
+/* NamespaceString[] */
+g_array_append_vals(table_data, name, namespace_length);
+
+acpi_table_end(linker, );
+};
+
/*
* ACPI spec, Revision 5.1 Errata A
* 5.2.12 Multiple APIC Description Table (MADT)
@@ -875,7 +932,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables
*tables)
dsdt = tables_blob->len;
build_dsdt(tables_blob, tables->linker, vms);
-/* FADT MADT GTDT MCFG SPCR pointed to by RSDT */
+/* FADT MADT GTDT MCFG SPCR DBG2 pointed to by RSDT */
acpi_add_table(table_offsets, tables_blob);
build_fadt_rev5(tables_blob, tables->linker, vms, dsdt);
@@ -898,6 +955,9 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables
*tables)
acpi_add_table(table_offsets, tables_blob);
build_spcr(tables_blob, tables->linker, vms);
+acpi_add_table(table_offsets, tables_blob);
+build_dbg2(tables_blob, tables->linker, vms);
+
if (vms->ras) {
build_ghes_error_table(tables->hardware_errors, tables->linker);
acpi_add_table(table_offsets, tables_blob);
--
2.26.3
Re: gitlab-ci: amd64-opensuse-leap-container job failing
On Mon, Sep 27, 2021 at 09:35:22AM +0100, Daniel P. Berrangé wrote: > On Sun, Sep 26, 2021 at 07:23:56PM +0200, Philippe Mathieu-Daudé wrote: > > Hi, > > > > FYI the OpenSUSE job is failing since few days, i.e.: > > https://gitlab.com/qemu-project/qemu/-/jobs/1622345026 > > > > Retrieving repository 'Main Repository' metadata > > [..error] > > Repository 'Main Repository' is invalid. > > > > [repo-oss|http://download.opensuse.org/distribution/leap/15.2/repo/oss/] > > Valid metadata not found at specified URL > > History: > >- Download (curl) error for > > 'http://download.opensuse.org/distribution/leap/15.2/repo/oss/repodata/repomd.xml': > > Error code: Curl error 56 > > Error message: Recv failure: Connection reset by peer > >- Can't provide /repodata/repomd.xml > > Please check if the URIs defined for this repository are pointing to a > > valid repository. > > Warning: Skipping repository 'Main Repository' because of the above error. > > > > I tried to run 'zypper ref' with: > > It isn't confined to only SuSE. In libvirt we've had similar problems > with several other jobs, though are suse jobs are the worst affected. > > GitLab have finally acknowledged it is an general infra issue affecting > many things: > >https://status.gitlab.com/ >https://gitlab.com/gitlab-com/gl-infra/production/-/issues/5590 Setting GitLab CI env var works around the problem temporarily: FF_NETWORK_PER_BUILD=true You can set it for all repos under a group eg https://gitlab.com/groups/qemu-project/-/settings/ci_cd or per repo eg https://gitlab.com/berrange/libvirt/-/settings/ci_cd Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH v2 0/2] modules: Improve modinfo.c support
This patchset introduces the modinfo_need and changes
modinfo-generate.py/meson.build to generate/link one modinfo per target.
modinfo-generate.py will know, thanks to modinfo_need, which modules are
currently enabled for a given target before adding it in the array of
modules. It will give a hint about why some modules failed, so
developers can have a clue about it:
},{
/* hw-display-qxl.modinfo */
/* module QXL is missing. */
/* hw-display-virtio-gpu.modinfo */
.name = "hw-display-virtio-gpu",
.objs = ((const char*[]){ "virtio-gpu-base", "virtio-gpu-device",
"vhost-user-gpu", NULL }),
},{
The main reason of this change is to fix problems like:
$ ./qemu-system-s390x -nodefaults -display none -accel tcg -M none -device help
| head
Failed to open module: /.../hw-display-qxl.so: undefined symbol: vga_ioport_read
Failed to open module: /.../hw-display-virtio-vga.so: undefined symbol:
vmstate_vga_common
Failed to open module: /.../hw-display-virtio-vga.so: undefined symbol:
vmstate_vga_common
Failed to open module: /.../hw-display-virtio-vga-gl.so: undefined symbol:
have_vga
Failed to open module: /.../hw-usb-smartcard.so: undefined symbol:
ccid_card_ccid_attach
Failed to open module: /.../hw-usb-redirect.so: undefined symbol:
vmstate_usb_device
Failed to open module: /.../hw-usb-host.so: undefined symbol: vmstate_usb_device
With this patch, I run this small script successfuly:
#!/bin/bash
pushd ~/suse/virtualization/qemu/build
for qemu in qemu-system-*
do
[[ -f "$qemu" ]] || continue
res=$(./$qemu -nodefaults -display none -accel tcg -M none -device help
2>&1 | grep "Failed to" > /dev/null; echo $?)
[[ $res -eq 0 ]] && echo "Error: $qemu"
done
popd
Also run make check and check-acceptance without any failures.
Todo:
- accelerators can be filtered as well (this only covers the device
part), then the field QemuModinfo.arch can be removed.
v1 -> v2:
- Changed the approach to this problem after suggestions made by Paolo and
Gerd.
Thank you!
Jose R. Ziviani (2):
modules: introduces module_needs directive
modules: generates per-target modinfo
hw/display/qxl.c| 1 +
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 | 1 +
hw/display/virtio-gpu-pci-gl.c | 1 +
hw/display/virtio-gpu-pci.c | 1 +
hw/display/virtio-gpu.c | 1 +
hw/display/virtio-vga-gl.c | 1 +
hw/display/virtio-vga.c | 1 +
hw/s390x/virtio-ccw-gpu.c | 1 +
hw/usb/ccid-card-emulated.c | 1 +
hw/usb/ccid-card-passthru.c | 1 +
hw/usb/host-libusb.c| 1 +
hw/usb/redirect.c | 1 +
include/qemu/module.h | 10 +
meson.build | 25 ++---
scripts/modinfo-generate.py | 40 -
19 files changed, 67 insertions(+), 24 deletions(-)
--
2.33.0
Re: Need to merge - QEMU patch for booting eMMC image for AST2600 machine
Hello,
On 9/26/21 09:59, Philippe Mathieu-Daudé wrote:
Hi,
On 9/25/21 19:07, Shitalkumar Gandhi via wrote:
Hi,
I am attaching a patch that will fix eMMC image booting on QEMU for
AST2600 machine, without this patch it will be stuck after SPL saying,
"booting from RAM".
Please review and merge, thanks.
Let me know in case of any concern.
Thanks for your patch.
Please look at how to submit patches here:
https://wiki.qemu.org/Contribute/SubmitAPatch#Submitting_your_Patches
This patch has been added to boot eMMC image for AST2600 machine on
QEMU.
Run quemu as follows:
./qemu-system-arm -m 1G -M ast2600-evb -nographic -drive
file=mmc-evb-ast2600.img,format=raw,if=sd,index=2
What is index=2?
Is this mmc-evb-ast2600.img image publicly available?
Tested: Booted AST2600 eMMC image on QEMU.
Suggested-by:
Reviewed-by: Hao Wu
Reviewed-by: Cédric Le Goater
Message-Id: <20210416162426.3217033-1-vent...@google.com>
Signed-off-by: Cédric Le Goater
I don't remember having reviewed or signed this patch.
---
hw/arm/aspeed.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index ba5f1dc5af..6a890adb83 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -148,7 +148,7 @@ struct AspeedMachineState {
SCU_AST2400_HW_STRAP_BOOT_MODE(AST2400_SPI_BOOT))
/* AST2600 evb hardware value */
-#define AST2600_EVB_HW_STRAP1 0x00C0
+#define AST2600_EVB_HW_STRAP1 (0x00C0 |
AST26500_HW_STRAP_BOOT_SRC_EMMC)
IIUC you are not implementing any eMMC code, simply faking the
controller can support eMMC, but the card is used in SD mode?
I think this is related to this issue :
https://github.com/openbmc/openbmc/issues/3818
I'm surprised your guest is happy and boot that. If so, then
what is the point of announcing eMMC is supported if not used?
It should work on the aspeed branches I maintain which include the
emmc support but this is not for upstream.
Some comments,
I don't think the AST2600 evb boots by default on emmc. I agree
it's nice to have for tests and there are other ways to modify
slightly the default behavior.
We could add a machine property to define the 'hw-strap1' register
but it's a bit difficult to remember the value.
A custom '-boot' value setting the 'hw-strap1' for the AST2600
machine seems a better alternative. We could merge such a change
even if emmc is not ready.
Thanks,
C.
Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
On Mon, 27 Sept 2021 at 16:18, Simon Glass wrote: > On Mon, 27 Sept 2021 at 02:48, Peter Maydell wrote: > > So what is missing in the QEMU-provided DTB that it needs? > > Quite a lot. Here are some examples: > > U-Boot has limited pre-relocation memory so tries to avoid > binding/probing devices that are not used before relocation: > > https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#pre-relocation-support It's up to u-boot to decide what it wants to touch and what it does not. QEMU tells u-boot what all the available devices are; I don't think we should have extra stuff saying "and if you are u-boot, do something odd". > There is a configuration node (which is likely to change form in > future releases, but will still be there) > > https://github.com/u-boot/u-boot/blob/master/doc/device-tree-bindings/config.txt I think u-boot should be storing this kind of thing somewhere else (e.g. as part of the binary blob that is u-boot itself, or stored in flash or RAM as a separate blob). > Then there are various features which put things in U-Boot's control > dtb, such as verified boot, which adds public keys during signing: > > https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L135 > > More generally, the U-Boot tree has hundreds of files which add > properties for each board, since we try to keep the U-Boot-specific > things out of the Linux tree: > > $ find . -name *u-boot.dtsi |wc -l > 398 If any of this is actual information about the hardware then you should sort out getting the bindings documented officially (which I think is still in the Linux tree), and then QEMU can provide them. > Quite a bit of this is to do with SPL and so far it seems that QEMU > mostly runs U-Boot proper only, although I see that SPL is starting to > creep in too in the U-Boot CI. > > So at present QEMU is not able to support U-Boot fully. My take is that this is u-boot doing weird custom things with the DTB that aren't "describe the hardware". You should be able to boot u-boot by putting those custom DTB extra things in a separate blob and having u-boot combine that with the actual DTB when it starts. -- PMM
Re: [PATCH] hw: Add a 'Sensor devices' qdev category
On Mon, Sep 27, 2021 at 3:03 AM Cédric Le Goater wrote:
> On 9/27/21 00:15, Philippe Mathieu-Daudé wrote:
> > Sensors models are listed in the 'Misc devices' category.
> > Move them to their own category.
> >
> > For the devices in the hw/sensor/ directory, the category
> > is obvious.
> >
> > hw/arm/z2.c models the AER915 model which is described
> > on [*] as:
> >
> >The 14-pin chip marked AER915 just below the expansion
> >port is a 80C51-type microcontroller, similar to Philips
> >P89LPC915. It has an 8-bit A/D which is used to determine
> >which of six buttons are pressed on the resistor-network
> >wired remote. It communicates with the main cpu via I2C.
> >
> > It was introduced in commit 3bf11207c06 ("Add support for
> > Zipit Z2 machine") with this comment:
> >
> >248 static uint8_t aer915_recv(I2CSlave *slave)
> >249 {
> >...
> >253 switch (s->buf[0]) {
> >254 /* Return hardcoded battery voltage,
> >255 * 0xf0 means ~4.1V
> >256 */
> >257 case 0x02:
> >258 retval = 0xf0;
> >259 break;
> >
> > For QEMU the AER915 is a very simple sensor model.
> >
> > [*] https://www.bealecorner.org/best/measure/z2/index.html
> >
> > Signed-off-by: Philippe Mathieu-Daudé
>
> Reviewed-by: Cédric Le Goater
>
Reviewed-by: Hao Wu
>
>
> > ---
> > include/hw/qdev-core.h | 1 +
> > hw/arm/z2.c| 1 +
> > hw/sensor/adm1272.c| 1 +
> > hw/sensor/dps310.c | 1 +
> > hw/sensor/emc141x.c| 1 +
> > hw/sensor/max34451.c | 2 ++
> > hw/sensor/tmp105.c | 1 +
> > hw/sensor/tmp421.c | 1 +
> > softmmu/qdev-monitor.c | 1 +
> > 9 files changed, 10 insertions(+)
> >
> > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> > index 34c8a7506a1..f6241212247 100644
> > --- a/include/hw/qdev-core.h
> > +++ b/include/hw/qdev-core.h
> > @@ -26,6 +26,7 @@ typedef enum DeviceCategory {
> > DEVICE_CATEGORY_SOUND,
> > DEVICE_CATEGORY_MISC,
> > DEVICE_CATEGORY_CPU,
> > +DEVICE_CATEGORY_SENSOR,
> > DEVICE_CATEGORY_MAX
> > } DeviceCategory;
> >
> > diff --git a/hw/arm/z2.c b/hw/arm/z2.c
> > index 9c1e876207b..62db9741106 100644
> > --- a/hw/arm/z2.c
> > +++ b/hw/arm/z2.c
> > @@ -288,6 +288,7 @@ static void aer915_class_init(ObjectClass *klass,
> void *data)
> > k->recv = aer915_recv;
> > k->send = aer915_send;
> > dc->vmsd = _aer915_state;
> > +set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
> > }
> >
> > static const TypeInfo aer915_info = {
> > diff --git a/hw/sensor/adm1272.c b/hw/sensor/adm1272.c
> > index 7310c769be2..2942ac75f90 100644
> > --- a/hw/sensor/adm1272.c
> > +++ b/hw/sensor/adm1272.c
> > @@ -518,6 +518,7 @@ static void adm1272_class_init(ObjectClass *klass,
> void *data)
> > DeviceClass *dc = DEVICE_CLASS(klass);
> > PMBusDeviceClass *k = PMBUS_DEVICE_CLASS(klass);
> >
> > +set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
> > dc->desc = "Analog Devices ADM1272 Hot Swap controller";
> > dc->vmsd = _adm1272;
> > k->write_data = adm1272_write_data;
> > diff --git a/hw/sensor/dps310.c b/hw/sensor/dps310.c
> > index d60a18ac41b..1e24a499b38 100644
> > --- a/hw/sensor/dps310.c
> > +++ b/hw/sensor/dps310.c
> > @@ -208,6 +208,7 @@ static void dps310_class_init(ObjectClass *klass,
> void *data)
> > k->send = dps310_tx;
> > dc->reset = dps310_reset;
> > dc->vmsd = _dps310;
> > +set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
> > }
> >
> > static const TypeInfo dps310_info = {
> > diff --git a/hw/sensor/emc141x.c b/hw/sensor/emc141x.c
> > index 7ce8f4e9794..4202d8f185a 100644
> > --- a/hw/sensor/emc141x.c
> > +++ b/hw/sensor/emc141x.c
> > @@ -270,6 +270,7 @@ static void emc141x_class_init(ObjectClass *klass,
> void *data)
> > DeviceClass *dc = DEVICE_CLASS(klass);
> > I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
> >
> > +set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
> > dc->reset = emc141x_reset;
> > k->event = emc141x_event;
> > k->recv = emc141x_rx;
> > diff --git a/hw/sensor/max34451.c b/hw/sensor/max34451.c
> > index a91d8bd487c..8300bf4ff43 100644
> > --- a/hw/sensor/max34451.c
> > +++ b/hw/sensor/max34451.c
> > @@ -751,6 +751,8 @@ static void max34451_class_init(ObjectClass *klass,
> void *data)
> > ResettableClass *rc = RESETTABLE_CLASS(klass);
> > DeviceClass *dc = DEVICE_CLASS(klass);
> > PMBusDeviceClass *k = PMBUS_DEVICE_CLASS(klass);
> > +
> > +set_bit(DEVICE_CATEGORY_SENSOR, dc->categories);
> > dc->desc = "Maxim MAX34451 16-Channel V/I monitor";
> > dc->vmsd = _max34451;
> > k->write_data = max34451_write_data;
> > diff --git a/hw/sensor/tmp105.c b/hw/sensor/tmp105.c
> > index 20564494899..43d79b9eeec 100644
> > --- a/hw/sensor/tmp105.c
> > +++ b/hw/sensor/tmp105.c
> > @@ -305,6 +305,7 @@ static void tmp105_class_init(ObjectClass *klass,
> void *data)
> > DeviceClass *dc =
[PULL v2 04/25] qapi: Convert simple union InputEvent to flat one
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
To prepare for their removal, convert simple union InputEvent to an
equivalent flat one. Adds some boilerplate to the schema, which is a
bit ugly, but a lot easier to maintain than the simple union feature.
Cc: Gerd Hoffmann
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210917143134.412106-5-arm...@redhat.com>
---
qapi/ui.json | 42 ++
1 file changed, 38 insertions(+), 4 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 9e04f9d65d..d7567ac866 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -960,6 +960,38 @@
'data' : { 'axis': 'InputAxis',
'value' : 'int' } }
+##
+# @InputEventKind:
+#
+# Since: 2.0
+##
+{ 'enum': 'InputEventKind',
+ 'data': [ 'key', 'btn', 'rel', 'abs' ] }
+
+##
+# @InputKeyEventWrapper:
+#
+# Since: 2.0
+##
+{ 'struct': 'InputKeyEventWrapper',
+ 'data': { 'data': 'InputKeyEvent' } }
+
+##
+# @InputBtnEventWrapper:
+#
+# Since: 2.0
+##
+{ 'struct': 'InputBtnEventWrapper',
+ 'data': { 'data': 'InputBtnEvent' } }
+
+##
+# @InputMoveEventWrapper:
+#
+# Since: 2.0
+##
+{ 'struct': 'InputMoveEventWrapper',
+ 'data': { 'data': 'InputMoveEvent' } }
+
##
# @InputEvent:
#
@@ -975,10 +1007,12 @@
# Since: 2.0
##
{ 'union' : 'InputEvent',
- 'data' : { 'key' : 'InputKeyEvent',
- 'btn' : 'InputBtnEvent',
- 'rel' : 'InputMoveEvent',
- 'abs' : 'InputMoveEvent' } }
+ 'base': { 'type': 'InputEventKind' },
+ 'discriminator': 'type',
+ 'data' : { 'key' : 'InputKeyEventWrapper',
+ 'btn' : 'InputBtnEventWrapper',
+ 'rel' : 'InputMoveEventWrapper',
+ 'abs' : 'InputMoveEventWrapper' } }
##
# @input-send-event:
--
2.31.1
[PULL v2 02/25] qapi: Stop enforcing "type name should not end in 'Kind'
I'm about to convert simple unions to flat unions, then drop simple
union support. The conversion involves making the implict enum types
explicit. To reduce churn, I'd like to name them exactly like the
implicit types they replace. However, these names are reserved for
the generator's use. They won't be once simple unions are gone. Stop
enforcing this naming rule now rather than then.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210917143134.412106-3-arm...@redhat.com>
---
scripts/qapi/expr.py | 6 +++---
tests/qapi-schema/meson.build | 1 -
tests/qapi-schema/reserved-type-kind.err | 2 --
tests/qapi-schema/reserved-type-kind.json | 2 --
tests/qapi-schema/reserved-type-kind.out | 0
5 files changed, 3 insertions(+), 8 deletions(-)
delete mode 100644 tests/qapi-schema/reserved-type-kind.err
delete mode 100644 tests/qapi-schema/reserved-type-kind.json
delete mode 100644 tests/qapi-schema/reserved-type-kind.out
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 90bde501b0..91959ee79a 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -171,7 +171,7 @@ def check_defn_name_str(name: str, info: QAPISourceInfo,
meta: str) -> None:
- 'event' names adhere to `check_name_upper()`.
- 'command' names adhere to `check_name_lower()`.
- Else, meta is a type, and must pass `check_name_camel()`.
-These names must not end with ``Kind`` nor ``List``.
+These names must not end with ``List``.
:param name: Name to check.
:param info: QAPI schema source file information.
@@ -187,9 +187,9 @@ def check_defn_name_str(name: str, info: QAPISourceInfo,
meta: str) -> None:
permit_underscore=name in info.pragma.command_name_exceptions)
else:
check_name_camel(name, info, meta)
-if name.endswith('Kind') or name.endswith('List'):
+if name.endswith('List'):
raise QAPISemError(
-info, "%s name should not end in '%s'" % (meta, name[-4:]))
+info, "%s name should not end in 'List'" % meta)
def check_keys(value: _JSONObject,
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index 6b2a4ce41a..0798e94042 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -168,7 +168,6 @@ schemas = [
'reserved-member-q.json',
'reserved-member-u.json',
'reserved-member-underscore.json',
- 'reserved-type-kind.json',
'reserved-type-list.json',
'returns-alternate.json',
'returns-array-bad.json',
diff --git a/tests/qapi-schema/reserved-type-kind.err
b/tests/qapi-schema/reserved-type-kind.err
deleted file mode 100644
index d8fb769f9d..00
--- a/tests/qapi-schema/reserved-type-kind.err
+++ /dev/null
@@ -1,2 +0,0 @@
-reserved-type-kind.json: In enum 'UnionKind':
-reserved-type-kind.json:2: enum name should not end in 'Kind'
diff --git a/tests/qapi-schema/reserved-type-kind.json
b/tests/qapi-schema/reserved-type-kind.json
deleted file mode 100644
index 9ecaba12bc..00
--- a/tests/qapi-schema/reserved-type-kind.json
+++ /dev/null
@@ -1,2 +0,0 @@
-# we reject types that would conflict with implicit union enum
-{ 'enum': 'UnionKind', 'data': [ 'oops' ] }
diff --git a/tests/qapi-schema/reserved-type-kind.out
b/tests/qapi-schema/reserved-type-kind.out
deleted file mode 100644
index e69de29bb2..00
--
2.31.1
[PULL v2 11/25] tests/qapi-schema: Prepare for simple union UserDefListUnion removal
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
To prepare for their removal, simple union UserDefListUnion has to go.
It is used to cover arrays. The next few commits will eliminate its
uses, and then it gets deleted. As a first step, provide struct
ArrayStruct for the tests to be rewritten.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20210917143134.412106-12-arm...@redhat.com>
---
tests/qapi-schema/qapi-schema-test.json | 16
tests/qapi-schema/qapi-schema-test.out | 16
2 files changed, 32 insertions(+)
diff --git a/tests/qapi-schema/qapi-schema-test.json
b/tests/qapi-schema/qapi-schema-test.json
index 3c43e14e22..b2d795cb19 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -140,6 +140,22 @@
'sizes': ['size'],
'any': ['any'],
'user': ['Status'] } } # intentional forward ref. to sub-module
+{ 'struct': 'ArrayStruct',
+ 'data': { 'integer': ['int'],
+'s8': ['int8'],
+'s16': ['int16'],
+'s32': ['int32'],
+'s64': ['int64'],
+'u8': ['uint8'],
+'u16': ['uint16'],
+'u32': ['uint32'],
+'u64': ['uint64'],
+'number': ['number'],
+'boolean': ['bool'],
+'string': ['str'],
+'*sz': ['size'],
+'*any': ['any'],
+'*user': ['Status'] } } # intentional forward ref. to sub-module
# for testing sub-modules
{ 'include': 'include/sub-module.json' }
diff --git a/tests/qapi-schema/qapi-schema-test.out
b/tests/qapi-schema/qapi-schema-test.out
index d557fe2d89..7a488c1d06 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -189,6 +189,22 @@ object UserDefListUnion
case sizes: q_obj_sizeList-wrapper
case any: q_obj_anyList-wrapper
case user: q_obj_StatusList-wrapper
+object ArrayStruct
+member integer: intList optional=False
+member s8: int8List optional=False
+member s16: int16List optional=False
+member s32: int32List optional=False
+member s64: int64List optional=False
+member u8: uint8List optional=False
+member u16: uint16List optional=False
+member u32: uint32List optional=False
+member u64: uint64List optional=False
+member number: numberList optional=False
+member boolean: boolList optional=False
+member string: strList optional=False
+member sz: sizeList optional=True
+member any: anyList optional=True
+member user: StatusList optional=True
include include/sub-module.json
command user-def-cmd None -> None
gen=True success_response=True boxed=False oob=False preconfig=False
--
2.31.1
