[Qemu-devel] [Bug 1836430] Re: Can't install on Windows 10
[Expired for QEMU because there has been no activity for 60 days.] ** Changed in: qemu Status: Incomplete => Expired -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1836430 Title: Can't install on Windows 10 Status in QEMU: Expired Bug description: Latest release (20190712) 64-bit doesn't install: The setup seems to work fine at first and actually extract all the files needed for qemu in the correct location, but after it has done that, it proceeds to delete every file and leaves no trace of qemu except the installation folder. The setup then finishes and notifies the user that it has been installed succesfully. I downloaded the previous release and it installs correctly. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1836430/+subscriptions
Re: [Qemu-devel] [PATCH v2 00/24] Audio: Mixeng-free 5.1/7.1 audio support
On 2019-09-12 12:20, Gerd Hoffmann wrote: On Sun, Sep 08, 2019 at 11:22:00PM +0200, Kővágó, Zoltán wrote: Hi, This is the v2 of my patch series that makes mixeng optional and enables more than two audio channels. Changes from v1: * renamed "mixeng" option to "mixing-engine" * dropped patch "audio: remove hw->samples, buffer_size_in/out pcm_ops" What is the testing status of this? I've ran some very simple audio playback tests, here are my results: * alsa - works * pa - works * oss - works, but it looks like I have to specify both in.dev and out.dev if the default /dev/dsp does not exist, otherwise I just get a generic "Could not init `oss' audio driver". We need a better error message here IMHO. * sdl - works * spice - yes, it's broken now, I'll fix it in the next update. I've also found a second bug (SIGSEGV if the guest starts playing while no client is connected). What about windows+macos? No idea, I don't have a windows or mac computer. I could try it in a windows VM though, if I figure out how to compile qemu on windows. Regards, Zoltan
[Qemu-devel] [PATCH] configure: Add xkbcommon configure options
This dependency is currently "automagic", which is bad for distributions. Signed-off-by: James Le Cuirot --- configure | 5 + 1 file changed, 5 insertions(+) diff --git a/configure b/configure index 30aad233d1..30544f52e6 100755 --- a/configure +++ b/configure @@ -1521,6 +1521,10 @@ for opt do ;; --disable-libpmem) libpmem=no ;; + --enable-xkbcommon) xkbcommon=yes + ;; + --disable-xkbcommon) xkbcommon=no + ;; *) echo "ERROR: unknown option $opt" echo "Try '$0 --help' for more information" @@ -1804,6 +1808,7 @@ disabled with --disable-FEATURE, default is enabled if available: capstonecapstone disassembler support debug-mutex mutex debugging support libpmem libpmem support + xkbcommon xkbcommon support NOTE: The object files are built at the place where configure is launched EOF -- 2.23.0
[Qemu-devel] [Bug 1843133] Re: Possibly incorrect branch in qemu-system-hppa
This test case works for me. $ ./hppa-linux-user/qemu-hppa ~/a.out $ echo $? 0 >From -d in_asm,cpu logs: IN: main 0x000112d0: addb,*<,n r24,r23,0x112e4 IA_F 000112d3 IA_B 000112d7 PSW bf00 CB -- GR00 GR01 GR02 0001162b GR03 ff7fe9c0 GR04 00011b94 GR05 00011c6c GR06 GR07 GR08 GR09 GR10 GR11 GR12 GR13 GR14 GR15 GR16 GR17 GR18 GR19 ff7fe888 GR20 GR21 GR22 000112bc GR23 7fff GR24 0001 GR25 ff7fe674 GR26 0001 GR27 0009a0e0 GR28 0009f080 GR29 0001 GR30 ff7fea00 GR31 0001162b About to execute the addb; r23 and r24 as expected. IN: main 0x000112e4: ldi 0,ret0 IA_F 000112e7 IA_B 000112eb PSW bf00 CB -- GR00 GR01 GR02 0001162b GR03 ff7fe9c0 GR04 00011b94 GR05 00011c6c GR06 GR07 GR08 GR09 GR10 GR11 GR12 GR13 GR14 GR15 GR16 GR17 GR18 GR19 ff7fe888 GR20 GR21 GR22 000112bc GR23 8000 GR24 0001 GR25 ff7fe674 GR26 0001 GR27 0009a0e0 GR28 0009f080 GR29 0001 GR30 ff7fea00 GR31 0001162b The branch has been taken, correctly. We can see the expected result in r23. I've also tested this in system mode, though getting logs from that is significantly more difficult. I am testing git master, not v3.1.1. Can you please try the development version? -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1843133 Title: Possibly incorrect branch in qemu-system-hppa Status in QEMU: Incomplete Bug description: I plan to release a new GNU Lightning soon. I no longer have access to any physical HPPA, but code that was tested some years ago did work on HPPA/HP-UX, and now it appears qemu-system-hppa incorrectly branches in code generated by GNU Lightning. Currently only 32 bit hppa jit generation supported. In the lightning check/test tool, the code would be: .code prolog movi %r0 0x7fff movi %r1 1 boaddr L0 %r0 %r1 calli @abort L0: ret epilog The code/debug information looks like this: movi r4 0x7fff 0xf8ef5018 ldil L%7800,r4 0xf8ef501c ldo 7ff(r4),r4 movi r5 0x1 0xf8ef5020 ldi 1,r5 boaddr L1 r4 r5 0xf8ef5024 addb,sv,n r5,r4,0xf8ef5044 :a.tst:291 0xf8ef5028 nop calli 0xf8eeb68a [...] L1: Apparently it is not understanding 0x7fff + 1 is a signed overflow. Tested in Fedora with qemu-system-hppa-3.1.1-2.fc30.x86_64 and using the debian-10 image. To make it a bit easier to test (partially transformed the not so optimized code generated by lightning to gcc -S output): # cat a.s .LEVEL 1.1 .text .align 4 .globl main .type main, @function main: .PROC .CALLINFO FRAME=64,NO_CALLS,SAVE_SP,ENTRY_GR=3 .ENTRY copy %r3,%r1 copy %r30,%r3 stwm %r1,64(%r30) zdepi -1,31,31,%r23 ldi 1,%r24 addb,sv,n %r24,%r23,.L0 nop ldi 1,%r28 b,n .L1 nop .L0: ldi 0,%r28 .L1: ldo 64(%r3),%r30 ldwm -64(%r30),%r3 bv,n %r0(%r2) .EXIT .PROCEND .size main, .-main # gcc a.s # ./a.out; echo $? 1 It should have returned 0. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1843133/+subscriptions
Re: [Qemu-devel] [PATCH v1 15/28] riscv: plic: Always set sip.SEIP bit for HS
On Fri, 23 Aug 2019 16:38:29 PDT (-0700), Alistair Francis wrote:
When the PLIC generates an interrupt ensure we always set it for the SIP
CSR that corresponds to the HS (V=0) register.
Signed-off-by: Alistair Francis
---
hw/riscv/sifive_plic.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c
index 98e4304b66..8309e96f64 100644
--- a/hw/riscv/sifive_plic.c
+++ b/hw/riscv/sifive_plic.c
@@ -150,7 +150,17 @@ static void sifive_plic_update(SiFivePLICState *plic)
riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_MEIP,
BOOL_TO_MASK(level));
break;
case PLICMode_S:
-riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_SEIP,
BOOL_TO_MASK(level));
+if (riscv_cpu_virt_enabled(env)) {
+if (level) {
+atomic_or(>mip_novirt, MIP_SEIP);
+g_assert(riscv_cpu_virt_enabled(env));
+} else {
+atomic_and(>mip_novirt, ~MIP_SEIP);
+g_assert(riscv_cpu_virt_enabled(env));
+}
+} else {
+riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_SEIP,
BOOL_TO_MASK(level));
+}
break;
default:
break;
This is going to go when we change the interrupt delivery mechanism.
Re: [Qemu-devel] [PATCH v1 14/28] target/riscv: Generate illegal instruction on WFI when V=1
On Fri, 23 Aug 2019 16:38:26 PDT (-0700), Alistair Francis wrote:
Signed-off-by: Alistair Francis
---
target/riscv/op_helper.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index d150551bc9..beb34e705b 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -130,9 +130,10 @@ void helper_wfi(CPURISCVState *env)
{
CPUState *cs = env_cpu(env);
-if (env->priv == PRV_S &&
+if ((env->priv == PRV_S &&
env->priv_ver >= PRIV_VERSION_1_10_0 &&
-get_field(*env->mstatus, MSTATUS_TW)) {
+get_field(*env->mstatus, MSTATUS_TW)) ||
+riscv_cpu_virt_enabled(env)) {
riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
} else {
cs->halted = 1;
Reviewed-by: Palmer Dabbelt
Re: [Qemu-devel] [PATCH v1 12/28] target/riscv: Add support for virtual interrupt setting
On Fri, 23 Aug 2019 16:38:21 PDT (-0700), Alistair Francis wrote:
Signed-off-by: Alistair Francis
---
target/riscv/cpu_helper.c | 21 ++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 41d4368128..afb3e8579e 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -38,12 +38,27 @@ static int riscv_cpu_local_irq_pending(CPURISCVState *env)
{
target_ulong mstatus_mie = get_field(*env->mstatus, MSTATUS_MIE);
target_ulong mstatus_sie = get_field(*env->mstatus, MSTATUS_SIE);
-target_ulong pending = atomic_read(env->mip) & *env->mie;
-target_ulong mie = env->priv < PRV_M || (env->priv == PRV_M &&
mstatus_mie);
-target_ulong sie = env->priv < PRV_S || (env->priv == PRV_S &&
mstatus_sie);
+target_ulong vsstatus_sie = get_field(env->mstatus_novirt, MSTATUS_SIE);
+
+target_ulong pending = atomic_read(>mip) & *env->mie;
+target_ulong hspending = atomic_read(>mip_novirt) & env->mie_novirt;
+
+target_ulong mie = env->priv < PRV_M || (env->priv == PRV_M &&
mstatus_mie);
+target_ulong sie = env->priv < PRV_S || (env->priv == PRV_S &&
mstatus_sie);
+target_ulong vsie = env->priv < PRV_S || (env->priv == PRV_S &&
vsstatus_sie);
+
target_ulong irqs = (pending & ~env->mideleg & -mie) |
(pending & env->mideleg & -sie);
+if (riscv_cpu_virt_enabled(env)) {
+target_ulong pending_hs_irq = hspending & -vsie;
+
+if (pending_hs_irq) {
+riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
+return ctz64(pending_hs_irq);
+}
+}
+
if (irqs) {
return ctz64(irqs); /* since non-zero */
} else {
Reviewed-by: Palmer Dabbelt
Re: [Qemu-devel] [PATCH v1 13/28] target/ricsv: Flush the TLB on virtulisation mode changes
On Fri, 23 Aug 2019 16:38:23 PDT (-0700), Alistair Francis wrote:
To ensure our TLB isn't out-of-date we flush it on all virt mode
changes. Unlike priv mode this isn't saved in the mmu_idx as all
guests share V=1. The easiest option is just to flush on all changes.
Signed-off-by: Alistair Francis
---
target/riscv/cpu_helper.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index afb3e8579e..8e8b156fc0 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -203,6 +203,11 @@ void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool
enable)
return;
}
+/* Flush the TLB on all virt mode changes. */
+if (((env->virt & VIRT_MODE_MASK) >> VIRT_MODE_SHIFT) != enable) {
+tlb_flush(env_cpu(env));
+}
+
env->virt &= ~VIRT_MODE_MASK;
env->virt |= enable << VIRT_MODE_SHIFT;
}
Reviewed-by: Palmer Dabbelt
[Qemu-devel] [Bug 1843133] Re: Possibly incorrect branch in qemu-system-hppa
** Changed in: qemu Status: New => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1843133 Title: Possibly incorrect branch in qemu-system-hppa Status in QEMU: Incomplete Bug description: I plan to release a new GNU Lightning soon. I no longer have access to any physical HPPA, but code that was tested some years ago did work on HPPA/HP-UX, and now it appears qemu-system-hppa incorrectly branches in code generated by GNU Lightning. Currently only 32 bit hppa jit generation supported. In the lightning check/test tool, the code would be: .code prolog movi %r0 0x7fff movi %r1 1 boaddr L0 %r0 %r1 calli @abort L0: ret epilog The code/debug information looks like this: movi r4 0x7fff 0xf8ef5018 ldil L%7800,r4 0xf8ef501c ldo 7ff(r4),r4 movi r5 0x1 0xf8ef5020 ldi 1,r5 boaddr L1 r4 r5 0xf8ef5024 addb,sv,n r5,r4,0xf8ef5044 :a.tst:291 0xf8ef5028 nop calli 0xf8eeb68a [...] L1: Apparently it is not understanding 0x7fff + 1 is a signed overflow. Tested in Fedora with qemu-system-hppa-3.1.1-2.fc30.x86_64 and using the debian-10 image. To make it a bit easier to test (partially transformed the not so optimized code generated by lightning to gcc -S output): # cat a.s .LEVEL 1.1 .text .align 4 .globl main .type main, @function main: .PROC .CALLINFO FRAME=64,NO_CALLS,SAVE_SP,ENTRY_GR=3 .ENTRY copy %r3,%r1 copy %r30,%r3 stwm %r1,64(%r30) zdepi -1,31,31,%r23 ldi 1,%r24 addb,sv,n %r24,%r23,.L0 nop ldi 1,%r28 b,n .L1 nop .L0: ldi 0,%r28 .L1: ldo 64(%r3),%r30 ldwm -64(%r30),%r3 bv,n %r0(%r2) .EXIT .PROCEND .size main, .-main # gcc a.s # ./a.out; echo $? 1 It should have returned 0. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1843133/+subscriptions
Re: [Qemu-devel] [PATCH] 9p: Print error hints if option parsing fails
Greg Kurz writes: > Option parsing fonctions are called with _fatal, which functions > causes error_setg() to call exit() and the hints are never > printed. > > Use an intermediate error object so that exit() happens in > error_propagate() after error_append_hint() could be called. Hmm. Code that creates error objects should not need to know how they are handled. Your patch shows that error_append_hint() requires error_propagate() to work regardless of how the error is handled. We should amend error_append_hint()'s contract in error.h to spell this out, and search the tree for more misuse of error_append_hint().
Re: [Qemu-devel] [PATCH v2 0/2] refine memory_device_get_free_addr
On Fri, Sep 13, 2019 at 11:47:46PM +, Wei Yang wrote: > On Tue, Jul 30, 2019 at 08:37:38AM +0800, Wei Yang wrote: > >When we iterate the memory-device list to get the available range, it is not > >necessary to iterate the whole list. > > > >1) no more overlap for hinted range if tmp exceed it > > > >v2: > > * remove #2 as suggested by Igor and David > > * add some comment to inform address assignment stay the same as before > > this change > > > >Wei Yang (2): > > memory-device: not necessary to use goto for the last check > > memory-device: break the loop if tmp exceed the hinted range > > > > hw/mem/memory-device.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > Would someone take this patch set? yes looks good to me too. Eduardo? > >-- > >2.17.1 > > > > -- > Wei Yang > Help you, Help me
Re: [Qemu-devel] [PATCH v8 18/32] riscv: sifive_u: Set the minimum number of cpus to 2
On Fri, 13 Sep 2019 08:25:21 PDT (-0700), bmeng...@gmail.com wrote:
Hi Palmer,
On Fri, Sep 13, 2019 at 10:33 PM Palmer Dabbelt wrote:
On Fri, 06 Sep 2019 09:20:05 PDT (-0700), bmeng...@gmail.com wrote:
> It is not useful if we only have one management CPU.
>
> Signed-off-by: Bin Meng
> Reviewed-by: Alistair Francis
>
> ---
>
> Changes in v8: None
> Changes in v7: None
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3:
> - use management cpu count + 1 for the min_cpus
>
> Changes in v2:
> - update the file header to indicate at least 2 harts are created
>
> hw/riscv/sifive_u.c | 4 +++-
> include/hw/riscv/sifive_u.h | 2 ++
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 2947e06..2023b71 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -10,7 +10,8 @@
> * 1) CLINT (Core Level Interruptor)
> * 2) PLIC (Platform Level Interrupt Controller)
> *
> - * This board currently uses a hardcoded devicetree that indicates one hart.
> + * This board currently generates devicetree dynamically that indicates at
least
> + * two harts.
> *
> * This program is free software; you can redistribute it and/or modify it
> * under the terms and conditions of the GNU General Public License,
> @@ -433,6 +434,7 @@ static void riscv_sifive_u_machine_init(MachineClass *mc)
> * management CPU.
> */
> mc->max_cpus = 4;
> +mc->min_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + 1;
> }
>
> DEFINE_MACHINE("sifive_u", riscv_sifive_u_machine_init)
> diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
> index f25bad8..6d22741 100644
> --- a/include/hw/riscv/sifive_u.h
> +++ b/include/hw/riscv/sifive_u.h
> @@ -69,6 +69,8 @@ enum {
> SIFIVE_U_GEM_CLOCK_FREQ = 12500
> };
>
> +#define SIFIVE_U_MANAGEMENT_CPU_COUNT 1
> +
> #define SIFIVE_U_PLIC_HART_CONFIG "MS"
> #define SIFIVE_U_PLIC_NUM_SOURCES 54
> #define SIFIVE_U_PLIC_NUM_PRIORITIES 7
This fails "make check", so I'm going to squash in this
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index ca9f7fea41..adecbf1dd9 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -528,6 +528,7 @@ static void riscv_sifive_u_machine_init(MachineClass *mc)
mc->init = riscv_sifive_u_init;
mc->max_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + SIFIVE_U_COMPUTE_CPU_COUNT;
mc->min_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + 1;
+mc->default_cpus = mc->max_cpus;
Thank you for fixing the 'make check'. Shouldn't it be:
mc->default_cpus = mc->min_cpus;
We have 5 harts on the board that this matches, so I figured that'd be the
better default.
?
}
DEFINE_MACHINE("sifive_u", riscv_sifive_u_machine_init)
Regards,
Bin
Re: [Qemu-devel] [PATCH] error-report: Add info_report_once helper
On Sat, Sep 14, 2019 at 08:40:40PM +0200, Markus Armbruster wrote: > You neglected to cc: the maintainer (me). I spotted this anyway, more > or less by chance. Next time, please use scripts/get_maintainer.pl to > find people you might want to cc:. Ah, sorry for that. Will do next time, thanks! > Cyrill Gorcunov writes: > > > We already have error_report_once and warn_report_once, > > thus lets add info_report_once to complement. Actually > > I use this helper a lot so might be usefull for others. > > > > Signed-off-by: Cyrill Gorcunov > > Patch looks reasonable enough. However, the new info_report_once() and > info_report_once_cond() aren't used. Your commit message suggests you > have users in unpublished code. Is that correct? Exactly. I work on yet unpublished but open source code (https://src.openvz.org/projects/UP/repos/qemu/browse) to be precise. But work is still in progress and hasn't been pushed out. You know I posted the patch simply because it might be useful for someone and it looks a bit incomplete that we have "warn/error once" but no info_once. Up to you to decide if we need it in vanilla tree.
Re: [Qemu-devel] [PATCH] error-report: Add info_report_once helper
You neglected to cc: the maintainer (me). I spotted this anyway, more or less by chance. Next time, please use scripts/get_maintainer.pl to find people you might want to cc:. Cyrill Gorcunov writes: > We already have error_report_once and warn_report_once, > thus lets add info_report_once to complement. Actually > I use this helper a lot so might be usefull for others. > > Signed-off-by: Cyrill Gorcunov Patch looks reasonable enough. However, the new info_report_once() and info_report_once_cond() aren't used. Your commit message suggests you have users in unpublished code. Is that correct?
Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
On 9/14/19 1:51 PM, Alex Bennée wrote: > I think I'm going to stick with the gross TARGET for now. It mostly > seems a way of avoiding building per-arch plugins. I assume most out of > tree plugins will be targeted at a specific machine anyway. Ok. > Anyway I think that means I'll drop this series unless 1-3 are worth > keeping as simple clean-ups leaving out 4? Patch 1 was supposed to be in another patch set, right? Patch 2, split, is still a good cleanup, I think. r~
Re: [Qemu-devel] [PATCH v1 3/4] elf: move elf_ops.h into include/elf/ and rename
On 9/11/19 4:20 AM, Alex Bennée wrote: > Alex Bennée writes: > >> Lets keep all the Elf manipulation bits together. Also rename the file >> to better reflect how it is used and add a little header to the file. >> >> Signed-off-by: Alex Bennée >> --- >> hw/core/loader.c| 4 ++-- > It is arguable this could be a private header in hw/core as it is only > included in one place. > Let's leave it here. r~
Re: [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types
On 9/10/19 3:34 PM, Alex Bennée wrote: > Most of the users of elf.h just want the standard Elf definitions. The > couple that want more than that want an expansion based on ELF_CLASS > which can be used for size agnostic code. The later is moved into > elf/elf-types.inc.h to make it clearer what it is for. While doing > that I also removed the whitespace damage. > > Signed-off-by: Alex Bennée > --- This patch is hard to follow because it moves and splits at the same time. With this patch split into two, Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [PATCH] tests/tcg: add float_madds test to multiarch
On 9/14/19 1:59 PM, Alex Bennée wrote: > > Richard Henderson writes: > >> On 9/13/19 9:49 AM, Alex Bennée wrote: >>> +/* must be built with -O2 to generate fused op */ >>> +r = a * b + c; >> >> I would prefer to use fmaf() or __builtin_fmaf() here. >> >> Although you'll need to link with -lm just in case the >> target doesn't support an instruction for fmaf and so >> the builtin expands to the standard library call. > > I can do that - we have other tests that link to libm. > > I was expecting to see more breakage but the ppc64 tests all passed (or > at least against the power8 David ran it on). What am I missing to hit > the cases you know are broken? I would *expect* the test to pass when run natively on power8 hardware. Did it not fail when run via qemu? If not, then we didn't really choose the argument sets that are affected by double rounding. I would expect the inputs that Paul used in the original report to be candidates. Otherwise, we should grab some from the glibc fma test case(s). > I've also experimented with reducing the number of iterations because if > we want to have golden references we probably don't want to dump several > hundred kilobytes of "golden" references into the source tree. > >> I also like Paul's suggestion to use hex float constants. > > Hmm I guess - look a bit weird but I guess that's lack of familiarity. > Is is still normalised? I guess the frac shows up (without the implicit > bit). The implicit bit is there: 0x1.xxx. The representation is always normalized; you write denormal numbers by using an exponent that would require denormalization. r~
Re: [Qemu-devel] [PATCH] tests/tcg: add float_madds test to multiarch
Richard Henderson writes: > On 9/13/19 9:49 AM, Alex Bennée wrote: >> +/* must be built with -O2 to generate fused op */ >> +r = a * b + c; > > I would prefer to use fmaf() or __builtin_fmaf() here. > > Although you'll need to link with -lm just in case the > target doesn't support an instruction for fmaf and so > the builtin expands to the standard library call. I can do that - we have other tests that link to libm. I was expecting to see more breakage but the ppc64 tests all passed (or at least against the power8 David ran it on). What am I missing to hit the cases you know are broken? I've also experimented with reducing the number of iterations because if we want to have golden references we probably don't want to dump several hundred kilobytes of "golden" references into the source tree. > I also like Paul's suggestion to use hex float constants. Hmm I guess - look a bit weird but I guess that's lack of familiarity. Is is still normalised? I guess the frac shows up (without the implicit bit). > > > r~ -- Alex Bennée
Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
Richard Henderson writes: > On 9/11/19 5:26 AM, Alex Bennée wrote: >> >> Aleksandar Markovic writes: >> >>> 10.09.2019. 21.34, "Alex Bennée" је написао/ла: This is preparatory for plugins which will want to report the architecture to plugins. Move the ELF_ARCH definition out of the loader and into its own header. Signed-off-by: Alex Bennée -- >>> >>> Hi, Alex. >>> >>> I advice some caution here >>> . For example, EM_NANOMIPS, and some other EM_xxx constants are not >>> included in the new header. >> >> EM_ARCH was never set to EM_NANOMIPS. In fact I can see e_machine is >> checked against it so I guess it is only ever used to checking the >> loading elf directly. >> >> In fact EM_ARCH is only really the default fallback case for checking >> the elf type if there is not a more "forgiving" check done by arch >> specific code (elf_check_arch). The only other cace is the fallback for >> EM_MACHINE unless PPC does something with it. >> >>> I am not sure what exactly you want to achieve >>> with this refactoring. But you may miss your goal, since some EM_xxx will >>> be missing from your new header. Is this the right way to achieve what you >>> want, and could you unintentionally introduce bugs? Can you outline in more >>> details your intentions around the new header? Is this refactoring the only >>> way? >> >> As mentioned in the other reply maybe exposing a value from configure >> into config-target.[mak|h] would be a better approach? > > I think using EM_FOO to tell a plugin about the architecture is just going to > be confusing. As seen here wrt EM_NANOMIPS, but arguably elsewhere with > EM_SPARC vs EM_SPARC64. > > You need to decide what it is that you want the plugin to know. It is just be > gross architecture? In which case QEMU_ARCH_FOO is probably enough. Is it > the > instruction set currently executing? In which case neither EM_FOO nor > QEMU_ARCH_FOO is sufficient -- e.g. arm thumb -- and you'll have to invent > something new, because no two architectures handle this in the same way. The > closest you might be able to get without invention from whole cloth is the > capstone cap_arch+cap_mode values from cc->disas_set_info(). But that only > handles the most popular of targets. I think I'm going to stick with the gross TARGET for now. It mostly seems a way of avoiding building per-arch plugins. I assume most out of tree plugins will be targeted at a specific machine anyway. Anyway I think that means I'll drop this series unless 1-3 are worth keeping as simple clean-ups leaving out 4? > > In any case, a single header that every target must touch is the wrong > approach. If you want to do some cleanup, move these defines into e.g. > linux-user/$TARGET/target_elf.h. (And I wouldn't bother touching bsd-user in > its current condition.) > > > r~ -- Alex Bennée
Re: [Qemu-devel] [PATCH] ppc: Add support for 'mffsce' instruction
On 9/12/19 8:54 AM, Paul A. Clarke wrote:
> From: "Paul A. Clarke"
>
> ISA 3.0B added a set of Floating-Point Status and Control Register (FPSCR)
> instructions: mffsce, mffscdrn, mffscdrni, mffscrn, mffscrni, mffsl.
> This patch adds support for 'mffsce' instruction.
>
> 'mffsce' is identical to 'mffs', except that it also clears the exception
> enable bits in the FPSCR.
>
> On CPUs without support for 'mffsce' (below ISA 3.0), the
> instruction will execute identically to 'mffs'.
>
> Signed-off-by: Paul A. Clarke
> ---
> target/ppc/translate/fp-impl.inc.c | 30 ++
> target/ppc/translate/fp-ops.inc.c | 2 ++
> 2 files changed, 32 insertions(+)
>
> diff --git a/target/ppc/translate/fp-impl.inc.c
> b/target/ppc/translate/fp-impl.inc.c
> index 59a4faf..34edc45 100644
> --- a/target/ppc/translate/fp-impl.inc.c
> +++ b/target/ppc/translate/fp-impl.inc.c
> @@ -639,6 +639,36 @@ static void gen_mffsl(DisasContext *ctx)
> tcg_temp_free_i64(t0);
> }
>
> +/* mffsce */
> +static void gen_mffsce(DisasContext *ctx)
> +{
> +TCGv_i64 t0;
> +TCGv_i32 mask;
> +
> +if (unlikely(!(ctx->insns_flags2 & PPC2_ISA300))) {
> +return gen_mffs(ctx);
> +}
> +
> +if (unlikely(!ctx->fpu_enabled)) {
> +gen_exception(ctx, POWERPC_EXCP_FPU);
> +return;
> +}
> +
> +t0 = tcg_temp_new_i64();
> +
> +gen_reset_fpstatus();
Note for future cleanup: we should not need to sprinkle these all over. This
should be the steady-state condition after softfp exceptions have been
processed into powerpc exceptions, after every single fp instruction.
That said, you're mirroring gen_mffs here, and the cleanup should happen
globally.
Reviewed-by: Richard Henderson
r~
Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
On 9/11/19 5:26 AM, Alex Bennée wrote: > > Aleksandar Markovic writes: > >> 10.09.2019. 21.34, "Alex Bennée" је написао/ла: >>> >>> This is preparatory for plugins which will want to report the >>> architecture to plugins. Move the ELF_ARCH definition out of the >>> loader and into its own header. >>> >>> Signed-off-by: Alex Bennée >>> -- >> >> Hi, Alex. >> >> I advice some caution here >> . For example, EM_NANOMIPS, and some other EM_xxx constants are not >> included in the new header. > > EM_ARCH was never set to EM_NANOMIPS. In fact I can see e_machine is > checked against it so I guess it is only ever used to checking the > loading elf directly. > > In fact EM_ARCH is only really the default fallback case for checking > the elf type if there is not a more "forgiving" check done by arch > specific code (elf_check_arch). The only other cace is the fallback for > EM_MACHINE unless PPC does something with it. > >> I am not sure what exactly you want to achieve >> with this refactoring. But you may miss your goal, since some EM_xxx will >> be missing from your new header. Is this the right way to achieve what you >> want, and could you unintentionally introduce bugs? Can you outline in more >> details your intentions around the new header? Is this refactoring the only >> way? > > As mentioned in the other reply maybe exposing a value from configure > into config-target.[mak|h] would be a better approach? I think using EM_FOO to tell a plugin about the architecture is just going to be confusing. As seen here wrt EM_NANOMIPS, but arguably elsewhere with EM_SPARC vs EM_SPARC64. You need to decide what it is that you want the plugin to know. It is just be gross architecture? In which case QEMU_ARCH_FOO is probably enough. Is it the instruction set currently executing? In which case neither EM_FOO nor QEMU_ARCH_FOO is sufficient -- e.g. arm thumb -- and you'll have to invent something new, because no two architectures handle this in the same way. The closest you might be able to get without invention from whole cloth is the capstone cap_arch+cap_mode values from cc->disas_set_info(). But that only handles the most popular of targets. In any case, a single header that every target must touch is the wrong approach. If you want to do some cleanup, move these defines into e.g. linux-user/$TARGET/target_elf.h. (And I wouldn't bother touching bsd-user in its current condition.) r~
[Qemu-devel] [PATCH 07/19] qapi: Use quotes more consistently in frontend error messages
Consistently enclose error messages in double quotes. Use single
quotes within, except for one case of "'".
Signed-off-by: Markus Armbruster
---
scripts/qapi/common.py| 37 ++-
tests/qapi-schema/bad-type-int.err| 2 +-
tests/qapi-schema/doc-missing-colon.err | 2 +-
tests/qapi-schema/duplicate-key.err | 2 +-
tests/qapi-schema/enum-int-member.err | 2 +-
tests/qapi-schema/escape-outside-string.err | 1 +
tests/qapi-schema/funny-char.err | 2 +-
tests/qapi-schema/funny-word.err | 2 +-
tests/qapi-schema/include-before-err.err | 2 +-
tests/qapi-schema/include-nested-err.err | 2 +-
tests/qapi-schema/leading-comma-list.err | 2 +-
tests/qapi-schema/leading-comma-object.err| 2 +-
tests/qapi-schema/missing-colon.err | 2 +-
tests/qapi-schema/missing-comma-list.err | 2 +-
tests/qapi-schema/missing-comma-object.err| 2 +-
tests/qapi-schema/non-objects.err | 2 +-
tests/qapi-schema/quoted-structural-chars.err | 2 +-
tests/qapi-schema/trailing-comma-list.err | 2 +-
tests/qapi-schema/unclosed-list.err | 2 +-
tests/qapi-schema/unclosed-object.err | 2 +-
20 files changed, 38 insertions(+), 36 deletions(-)
create mode 100644 tests/qapi-schema/escape-outside-string.err
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index f27860540b..142ab276ff 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -214,7 +214,7 @@ class QAPIDoc(object):
# recognized, and get silently treated as ordinary text
if not self.symbol and not self.body.text and line.startswith('@'):
if not line.endswith(':'):
-raise QAPIParseError(self._parser, "Line should end with :")
+raise QAPIParseError(self._parser, "Line should end with ':'")
self.symbol = line[1:-1]
# FIXME invalid names other than the empty string aren't flagged
if not self.symbol:
@@ -470,7 +470,7 @@ class QAPISchemaParser(object):
else:
fobj = open(incl_fname, 'r')
except IOError as e:
-raise QAPISemError(info, '%s: %s' % (e.strerror, incl_fname))
+raise QAPISemError(info, "%s: %s" % (e.strerror, incl_fname))
return QAPISchemaParser(fobj, previously_included, info)
def _pragma(self, name, value, info):
@@ -522,7 +522,7 @@ class QAPISchemaParser(object):
ch = self.src[self.cursor]
self.cursor += 1
if ch == '\n':
-raise QAPIParseError(self, 'Missing terminating "\'"')
+raise QAPIParseError(self, "Missing terminating \"'\"")
if esc:
# Note: we recognize only \\ because we have
# no use for funny characters in strings
@@ -559,7 +559,7 @@ class QAPISchemaParser(object):
self.line += 1
self.line_pos = self.cursor
elif not self.tok.isspace():
-raise QAPIParseError(self, 'Stray "%s"' % self.tok)
+raise QAPIParseError(self, "Stray '%s'" % self.tok)
def get_members(self):
expr = OrderedDict()
@@ -567,24 +567,24 @@ class QAPISchemaParser(object):
self.accept()
return expr
if self.tok != "'":
-raise QAPIParseError(self, 'Expected string or "}"')
+raise QAPIParseError(self, "Expected string or '}'")
while True:
key = self.val
self.accept()
if self.tok != ':':
-raise QAPIParseError(self, 'Expected ":"')
+raise QAPIParseError(self, "Expected ':'")
self.accept()
if key in expr:
-raise QAPIParseError(self, 'Duplicate key "%s"' % key)
+raise QAPIParseError(self, "Duplicate key '%s'" % key)
expr[key] = self.get_expr(True)
if self.tok == '}':
self.accept()
return expr
if self.tok != ',':
-raise QAPIParseError(self, 'Expected "," or "}"')
+raise QAPIParseError(self, "Expected ',' or '}'")
self.accept()
if self.tok != "'":
-raise QAPIParseError(self, 'Expected string')
+raise QAPIParseError(self, "Expected string")
def get_values(self):
expr = []
@@ -592,20 +592,20 @@ class QAPISchemaParser(object):
self.accept()
return expr
if self.tok not in "{['tfn":
-raise QAPIParseError(self, 'Expected "{", "[", "]", string, '
- 'boolean or "null"')
+raise QAPIParseError(
+self, "Expected '{', '[', ']', string, boolean or 'null'")
while True:
[Qemu-devel] [PATCH 16/19] qapi: Delete useless check_exprs() code for simple union kind
Commit bceae7697f "qapi script: support enum type as discriminator in
union" made check_exprs() add the implicit enum types of simple unions
to global @enum_types. I'm not sure it was needed even then. It's
certainly not needed now. Delete it.
discriminator_find_enum_define() and add_name() parameter @implicit
are now dead. Bury them.
Signed-off-by: Markus Armbruster
---
scripts/qapi/common.py | 39 ++-
1 file changed, 2 insertions(+), 37 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 3c3154a039..7e79c42b6a 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -672,26 +672,6 @@ def find_alternate_member_qtype(qapi_type):
return None
-# Return the discriminator enum define if discriminator is specified as an
-# enum type, otherwise return None.
-def discriminator_find_enum_define(expr):
-base = expr.get('base')
-discriminator = expr.get('discriminator')
-
-if not (discriminator and base):
-return None
-
-base_members = find_base_members(base)
-if not base_members:
-return None
-
-discriminator_value = base_members.get(discriminator)
-if not discriminator_value:
-return None
-
-return enum_types.get(discriminator_value['type'])
-
-
# Names must be letters, numbers, -, and _. They must start with letter,
# except for downstream extensions which must start with __RFQDN_.
# Dots are only valid in the downstream extension prefix.
@@ -722,7 +702,7 @@ def check_name(info, source, name, allow_optional=False,
raise QAPISemError(info, "%s uses invalid name '%s'" % (source, name))
-def add_name(name, info, meta, implicit=False):
+def add_name(name, info, meta):
global all_names
check_name(info, "'%s'" % meta, name)
# FIXME should reject names that differ only in '_' vs. '.'
@@ -730,7 +710,7 @@ def add_name(name, info, meta, implicit=False):
if name in all_names:
raise QAPISemError(info, "%s '%s' is already defined"
% (all_names[name], name))
-if not implicit and (name.endswith('Kind') or name.endswith('List')):
+if name.endswith('Kind') or name.endswith('List'):
raise QAPISemError(info, "%s '%s' should not end in '%s'"
% (meta, name, name[-4:]))
all_names[name] = meta
@@ -1138,21 +1118,6 @@ def check_exprs(exprs):
raise QAPISemError(info, "Definition of '%s' follows documentation"
" for '%s'" % (name, doc.symbol))
-# Try again for hidden UnionKind enum
-for expr_elem in exprs:
-expr = expr_elem['expr']
-
-if 'include' in expr:
-continue
-if 'union' in expr and not discriminator_find_enum_define(expr):
-name = '%sKind' % expr['union']
-elif 'alternate' in expr:
-name = '%sKind' % expr['alternate']
-else:
-continue
-enum_types[name] = {'enum': name}
-add_name(name, info, 'enum', implicit=True)
-
# Validate that exprs make sense
for expr_elem in exprs:
expr = expr_elem['expr']
--
2.21.0
[Qemu-devel] [PATCH 09/19] qapi: Remove null from schema language
We represent the parse tree as OrderedDict. We fetch optional dict
members with .get(). So far, so good.
We represent null literals as None. .get() returns None both for
"absent" and for "present, value is the null literal". Uh-oh.
Test features-if-invalid exposes this bug: "'if': null" is
misinterpreted as absent "if".
We added null to the schema language to "allow [...] an explicit
default value" (commit e53188ada5 "qapi: Allow true, false and null in
schema json", v2.4.0). Hasn't happened; null is still unused except
as generic invalid value in tests/.
To fix, we'd have to replace .get() by something more careful, or
represent null differently. Feasible, but we got more and bigger fish
to fry right now. Remove the null literal from the schema language.
Replace null in tests by another invalid value.
Test features-if-invalid now behaves as it should.
Signed-off-by: Markus Armbruster
---
docs/devel/qapi-code-gen.txt | 4 ++--
scripts/qapi/common.py | 4
tests/qapi-schema/features-if-invalid.err | 1 +
tests/qapi-schema/features-if-invalid.exit | 2 +-
tests/qapi-schema/features-if-invalid.json | 3 +--
tests/qapi-schema/features-if-invalid.out | 14 --
.../pragma-name-case-whitelist-crap.json | 2 +-
7 files changed, 6 insertions(+), 24 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 94f3054898..b4df31813d 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -52,7 +52,7 @@ Differences:
* Strings are restricted to printable ASCII, and escape sequences to
just '\\'.
-* Numbers are not supported.
+* Numbers and null are not supported.
A second layer of syntax defines the sequences of JSON texts that are
a correctly structured QAPI schema. We provide a grammar for this
@@ -67,7 +67,7 @@ syntax in an EBNF-like notation:
expression A separated by ,
* Grouping: expression ( A ) matches expression A
* JSON's structural characters are terminals: { } [ ] : ,
-* JSON's literal names are terminals: false true null
+* JSON's literal names are terminals: false true
* String literals enclosed in 'single quotes' are terminal, and match
this JSON string, with a leading '*' stripped off
* When JSON object member's name starts with '*', the member is
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index b3383b17ef..ef7c7be4fd 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -548,10 +548,6 @@ class QAPISchemaParser(object):
self.val = False
self.cursor += 4
return
-elif self.src.startswith('null', self.pos):
-self.val = None
-self.cursor += 3
-return
elif self.tok == '\n':
if self.cursor == len(self.src):
self.tok = None
diff --git a/tests/qapi-schema/features-if-invalid.err
b/tests/qapi-schema/features-if-invalid.err
index e69de29bb2..295800b4fc 100644
--- a/tests/qapi-schema/features-if-invalid.err
+++ b/tests/qapi-schema/features-if-invalid.err
@@ -0,0 +1 @@
+tests/qapi-schema/features-if-invalid.json:2: 'if' condition must be a string
or a list of strings
diff --git a/tests/qapi-schema/features-if-invalid.exit
b/tests/qapi-schema/features-if-invalid.exit
index 573541ac97..d00491fd7e 100644
--- a/tests/qapi-schema/features-if-invalid.exit
+++ b/tests/qapi-schema/features-if-invalid.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/features-if-invalid.json
b/tests/qapi-schema/features-if-invalid.json
index e6a524196d..89c2a6c234 100644
--- a/tests/qapi-schema/features-if-invalid.json
+++ b/tests/qapi-schema/features-if-invalid.json
@@ -1,5 +1,4 @@
# Cover feature with invalid 'if'
-# FIXME not rejected, misinterpreded as unconditional
{ 'struct': 'Stru',
'data': {},
- 'features': [{'name': 'f', 'if': null }] }
+ 'features': [{'name': 'f', 'if': false }] }
diff --git a/tests/qapi-schema/features-if-invalid.out
b/tests/qapi-schema/features-if-invalid.out
index 9c2637baa3..e69de29bb2 100644
--- a/tests/qapi-schema/features-if-invalid.out
+++ b/tests/qapi-schema/features-if-invalid.out
@@ -1,14 +0,0 @@
-module None
-object q_empty
-enum QType
-prefix QTYPE
-member none
-member qnull
-member qnum
-member qstring
-member qdict
-member qlist
-member qbool
-module features-if-invalid.json
-object Stru
-feature f
diff --git a/tests/qapi-schema/pragma-name-case-whitelist-crap.json
b/tests/qapi-schema/pragma-name-case-whitelist-crap.json
index 58382bf4e4..734e1c617b 100644
--- a/tests/qapi-schema/pragma-name-case-whitelist-crap.json
+++ b/tests/qapi-schema/pragma-name-case-whitelist-crap.json
@@ -1,3 +1,3 @@
# 'name-case-whitelist' must be list of strings
-{ 'pragma': { 'name-case-whitelist': null } }
+{ 'pragma': { 'name-case-whitelist': false } }
--
2.21.0
[Qemu-devel] [PATCH 02/19] tests/qapi-schema: Delete two redundant tests
Tests duplicate-key and double-data test the same thing. The former
predates the latter, and it has a better name. Delete the latter, and
tweak the former's comment.
Tests include-format-err and include-extra-junk test the same thing.
The former predates the latter, but the latter has a better name and a
comment. Delete the former.
Signed-off-by: Markus Armbruster
---
tests/Makefile.include| 2 --
tests/qapi-schema/double-data.err | 1 -
tests/qapi-schema/double-data.exit| 1 -
tests/qapi-schema/double-data.json| 2 --
tests/qapi-schema/double-data.out | 0
tests/qapi-schema/duplicate-key.json | 2 +-
tests/qapi-schema/include-format-err.err | 1 -
tests/qapi-schema/include-format-err.exit | 1 -
tests/qapi-schema/include-format-err.json | 2 --
tests/qapi-schema/include-format-err.out | 0
10 files changed, 1 insertion(+), 11 deletions(-)
delete mode 100644 tests/qapi-schema/double-data.err
delete mode 100644 tests/qapi-schema/double-data.exit
delete mode 100644 tests/qapi-schema/double-data.json
delete mode 100644 tests/qapi-schema/double-data.out
delete mode 100644 tests/qapi-schema/include-format-err.err
delete mode 100644 tests/qapi-schema/include-format-err.exit
delete mode 100644 tests/qapi-schema/include-format-err.json
delete mode 100644 tests/qapi-schema/include-format-err.out
diff --git a/tests/Makefile.include b/tests/Makefile.include
index bb071d2ba9..a1deaa5456 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -360,7 +360,6 @@ qapi-schema += doc-missing-expr.json
qapi-schema += doc-missing-space.json
qapi-schema += doc-missing.json
qapi-schema += doc-no-symbol.json
-qapi-schema += double-data.json
qapi-schema += double-type.json
qapi-schema += duplicate-key.json
qapi-schema += empty.json
@@ -405,7 +404,6 @@ qapi-schema += ident-with-escape.json
qapi-schema += include-before-err.json
qapi-schema += include-cycle.json
qapi-schema += include-extra-junk.json
-qapi-schema += include-format-err.json
qapi-schema += include-nested-err.json
qapi-schema += include-no-file.json
qapi-schema += include-non-file.json
diff --git a/tests/qapi-schema/double-data.err
b/tests/qapi-schema/double-data.err
deleted file mode 100644
index cc765c4ff2..00
--- a/tests/qapi-schema/double-data.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/double-data.json:2:41: Duplicate key "data"
diff --git a/tests/qapi-schema/double-data.exit
b/tests/qapi-schema/double-data.exit
deleted file mode 100644
index d00491fd7e..00
--- a/tests/qapi-schema/double-data.exit
+++ /dev/null
@@ -1 +0,0 @@
-1
diff --git a/tests/qapi-schema/double-data.json
b/tests/qapi-schema/double-data.json
deleted file mode 100644
index e76b519538..00
--- a/tests/qapi-schema/double-data.json
+++ /dev/null
@@ -1,2 +0,0 @@
-# we reject an expression with duplicate top-level keys
-{ 'struct': 'bar', 'data': { }, 'data': { 'string': 'str'} }
diff --git a/tests/qapi-schema/double-data.out
b/tests/qapi-schema/double-data.out
deleted file mode 100644
index e69de29bb2..00
diff --git a/tests/qapi-schema/duplicate-key.json
b/tests/qapi-schema/duplicate-key.json
index 14ac0e8a40..06b55840c9 100644
--- a/tests/qapi-schema/duplicate-key.json
+++ b/tests/qapi-schema/duplicate-key.json
@@ -1,3 +1,3 @@
-# QAPI cannot include the same key more than once in any {}
+# Cannot include the same key more than once in any {}
{ 'key': 'value',
'key': 'value' }
diff --git a/tests/qapi-schema/include-format-err.err
b/tests/qapi-schema/include-format-err.err
deleted file mode 100644
index 721ff4eccc..00
--- a/tests/qapi-schema/include-format-err.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/include-format-err.json:1: Invalid 'include' directive
diff --git a/tests/qapi-schema/include-format-err.exit
b/tests/qapi-schema/include-format-err.exit
deleted file mode 100644
index d00491fd7e..00
--- a/tests/qapi-schema/include-format-err.exit
+++ /dev/null
@@ -1 +0,0 @@
-1
diff --git a/tests/qapi-schema/include-format-err.json
b/tests/qapi-schema/include-format-err.json
deleted file mode 100644
index 44980f026f..00
--- a/tests/qapi-schema/include-format-err.json
+++ /dev/null
@@ -1,2 +0,0 @@
-{ 'include': 'include-simple-sub.json',
- 'foo': 'bar' }
diff --git a/tests/qapi-schema/include-format-err.out
b/tests/qapi-schema/include-format-err.out
deleted file mode 100644
index e69de29bb2..00
--
2.21.0
[Qemu-devel] [PATCH 19/19] qapi: Assert .visit() and .check_clash() run only after .check()
Easy since the previous commit provides .checked. Signed-off-by: Markus Armbruster --- scripts/qapi/common.py | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index c199a2b58c..b00caacca3 100644 --- a/scripts/qapi/common.py +++ b/scripts/qapi/common.py @@ -1190,7 +1190,7 @@ class QAPISchemaEntity(object): return not self.info def visit(self, visitor): -pass +assert self._checked class QAPISchemaVisitor(object): @@ -1245,6 +1245,7 @@ class QAPISchemaInclude(QAPISchemaEntity): self.fname = fname def visit(self, visitor): +QAPISchemaEntity.visit(self, visitor) visitor.visit_include(self.fname, self.info) @@ -1309,6 +1310,7 @@ class QAPISchemaBuiltinType(QAPISchemaType): return self.json_type() def visit(self, visitor): +QAPISchemaType.visit(self, visitor) visitor.visit_builtin_type(self.name, self.info, self.json_type()) @@ -1344,6 +1346,7 @@ class QAPISchemaEnumType(QAPISchemaType): return 'string' def visit(self, visitor): +QAPISchemaType.visit(self, visitor) visitor.visit_enum_type(self.name, self.info, self.ifcond, self.members, self.prefix) @@ -1386,6 +1389,7 @@ class QAPISchemaArrayType(QAPISchemaType): return 'array of ' + elt_doc_type def visit(self, visitor): +QAPISchemaType.visit(self, visitor) visitor.visit_array_type(self.name, self.info, self.ifcond, self.element_type) @@ -1461,6 +1465,7 @@ class QAPISchemaObjectType(QAPISchemaType): # and update seen to track the members seen so far. Report any errors # on behalf of info, which is not necessarily self.info def check_clash(self, info, seen): +assert self._checked assert not self.variants # not implemented for m in self.members: m.check_clash(info, seen) @@ -1498,6 +1503,7 @@ class QAPISchemaObjectType(QAPISchemaType): return 'object' def visit(self, visitor): +QAPISchemaType.visit(self, visitor) visitor.visit_object_type(self.name, self.info, self.ifcond, self.base, self.local_members, self.variants, self.features) @@ -1665,6 +1671,7 @@ class QAPISchemaAlternateType(QAPISchemaType): return 'value' def visit(self, visitor): +QAPISchemaType.visit(self, visitor) visitor.visit_alternate_type(self.name, self.info, self.ifcond, self.variants) @@ -1698,6 +1705,7 @@ class QAPISchemaCommand(QAPISchemaEntity): assert isinstance(self.ret_type, QAPISchemaType) def visit(self, visitor): +QAPISchemaEntity.visit(self, visitor) visitor.visit_command(self.name, self.info, self.ifcond, self.arg_type, self.ret_type, self.gen, self.success_response, @@ -1723,6 +1731,7 @@ class QAPISchemaEvent(QAPISchemaEntity): raise QAPISemError(self.info, "Use of 'boxed' requires 'data'") def visit(self, visitor): +QAPISchemaEntity.visit(self, visitor) visitor.visit_event(self.name, self.info, self.ifcond, self.arg_type, self.boxed) -- 2.21.0
[Qemu-devel] [PATCH 08/19] qapi: Improve reporting of lexical errors
Show text up to next structural character, whitespace, or quote
character instead of just the first character.
Forgotten quotes now get reported like "Stray 'command'" instead of
"Stray 'c'".
Signed-off-by: Markus Armbruster
---
scripts/qapi/common.py | 6 +-
tests/qapi-schema/bad-type-int.err | 2 +-
tests/qapi-schema/funny-word.err | 2 +-
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 142ab276ff..b3383b17ef 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -559,7 +559,11 @@ class QAPISchemaParser(object):
self.line += 1
self.line_pos = self.cursor
elif not self.tok.isspace():
-raise QAPIParseError(self, "Stray '%s'" % self.tok)
+# Show up to next structural, whitespace or quote
+# character
+match = re.match('[^[\\]{}:,\\s\'"]+',
+ self.src[self.cursor-1:])
+raise QAPIParseError(self, "Stray '%s'" % match.group(0))
def get_members(self):
expr = OrderedDict()
diff --git a/tests/qapi-schema/bad-type-int.err
b/tests/qapi-schema/bad-type-int.err
index 2021fda5d1..9b2c12c1eb 100644
--- a/tests/qapi-schema/bad-type-int.err
+++ b/tests/qapi-schema/bad-type-int.err
@@ -1 +1 @@
-tests/qapi-schema/bad-type-int.json:3:13: Stray '1'
+tests/qapi-schema/bad-type-int.json:3:13: Stray '123'
diff --git a/tests/qapi-schema/funny-word.err b/tests/qapi-schema/funny-word.err
index 18aedb4a99..af92fe2551 100644
--- a/tests/qapi-schema/funny-word.err
+++ b/tests/qapi-schema/funny-word.err
@@ -1 +1 @@
-tests/qapi-schema/funny-word.json:1:3: Stray 'c'
+tests/qapi-schema/funny-word.json:1:3: Stray 'command'
--
2.21.0
[Qemu-devel] [PATCH 13/19] qapi: Normalize 'if' in check_exprs(), like other sugar
We normalize shorthand to longhand forms in check_expr(): enumeration
values with normalize_enum(), feature values with
normalize_features(), struct members, union branches and alternate
branches with normalize_members(). If conditions are an exception: we
normalize them in QAPISchemaEntity.check() and
QAPISchemaMember.__init(), with listify_cond(). The idea goes back to
commit 2cbc94376e "qapi: pass 'if' condition into QAPISchemaEntity
objects", v3.0.0.
Normalize in check_expr() instead, with new helper normalize_if().
Signed-off-by: Markus Armbruster
---
scripts/qapi/common.py | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index cacee9b8bb..4d1f62e808 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -804,6 +804,7 @@ def check_type(info, source, value,
check_known_keys(info, "member '%s' of %s" % (key, source),
arg, ['type'], ['if'])
check_if(arg, info)
+normalize_if(arg)
check_type(info, "Member '%s' of %s" % (key, source),
arg['type'], allow_array=True,
allow_metas=['built-in', 'union', 'alternate', 'struct',
@@ -904,6 +905,7 @@ def check_union(expr, info):
check_known_keys(info, "member '%s' of union '%s'" % (key, name),
value, ['type'], ['if'])
check_if(value, info)
+normalize_if(value)
# Each value must name a known type
check_type(info, "Member '%s' of union '%s'" % (key, name),
value['type'],
@@ -933,6 +935,7 @@ def check_alternate(expr, info):
"member '%s' of alternate '%s'" % (key, name),
value, ['type'], ['if'])
check_if(value, info)
+normalize_if(value)
typ = value['type']
# Ensure alternates have no type conflicts.
@@ -978,6 +981,7 @@ def check_enum(expr, info):
check_known_keys(info, "member of enum '%s'" % name, member,
['name'], ['if'])
check_if(member, info)
+normalize_if(member)
check_name(info, "Member of enum '%s'" % name, member['name'],
enum_member=True)
@@ -1003,6 +1007,7 @@ def check_struct(expr, info):
['name'], ['if'])
check_if(f, info)
+normalize_if(f)
check_name(info, "Feature of struct %s" % name, f['name'])
@@ -1067,6 +1072,12 @@ def normalize_features(features):
for f in features]
+def normalize_if(expr):
+ifcond = expr.get('if')
+if isinstance(ifcond, str):
+expr['if'] = [ifcond]
+
+
def check_exprs(exprs):
global all_names
@@ -1123,6 +1134,7 @@ def check_exprs(exprs):
else:
raise QAPISemError(expr_elem['info'],
"Expression is missing metatype")
+normalize_if(expr)
name = expr[meta]
add_name(name, info, meta)
if doc and doc.symbol != name:
@@ -1177,14 +1189,6 @@ def check_exprs(exprs):
# Schema compiler frontend
#
-def listify_cond(ifcond):
-if not ifcond:
-return []
-if not isinstance(ifcond, list):
-return [ifcond]
-return ifcond
-
-
class QAPISchemaEntity(object):
def __init__(self, name, info, doc, ifcond=None):
assert name is None or isinstance(name, str)
@@ -1197,7 +1201,7 @@ class QAPISchemaEntity(object):
# such place).
self.info = info
self.doc = doc
-self._ifcond = ifcond # self.ifcond is set only after .check()
+self._ifcond = ifcond or []
def c_name(self):
return c_name(self.name)
@@ -1209,7 +1213,7 @@ class QAPISchemaEntity(object):
typ.check(schema)
self.ifcond = typ.ifcond
else:
-self.ifcond = listify_cond(self._ifcond)
+self.ifcond = self._ifcond
if self.info:
self.module = os.path.relpath(self.info['file'],
os.path.dirname(schema.fname))
@@ -1515,7 +1519,7 @@ class QAPISchemaMember(object):
def __init__(self, name, ifcond=None):
assert isinstance(name, str)
self.name = name
-self.ifcond = listify_cond(ifcond)
+self.ifcond = ifcond or []
self.owner = None
def set_owner(self, name):
--
2.21.0
[Qemu-devel] [PATCH 11/19] qapi: Reject blank 'if' conditions in addition to empty ones
"'if': 'COND'" generates "#if COND". We reject empty COND because it
won't compile. Blank COND won't compile any better, so reject that,
too.
Signed-off-by: Markus Armbruster
---
scripts/qapi/common.py | 5 +++--
tests/qapi-schema/bad-if-list.err | 2 +-
tests/qapi-schema/bad-if-list.json | 2 +-
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index a58e904978..2b46164854 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -742,8 +742,9 @@ def check_if(expr, info):
if not isinstance(ifcond, str):
raise QAPISemError(
info, "'if' condition must be a string or a list of strings")
-if ifcond == '':
-raise QAPISemError(info, "'if' condition '' makes no sense")
+if ifcond.strip() == '':
+raise QAPISemError(info, "'if' condition '%s' makes no sense"
+ % ifcond)
ifcond = expr.get('if')
if ifcond is None:
diff --git a/tests/qapi-schema/bad-if-list.err
b/tests/qapi-schema/bad-if-list.err
index 0af6316f78..53af099083 100644
--- a/tests/qapi-schema/bad-if-list.err
+++ b/tests/qapi-schema/bad-if-list.err
@@ -1 +1 @@
-tests/qapi-schema/bad-if-list.json:2: 'if' condition '' makes no sense
+tests/qapi-schema/bad-if-list.json:2: 'if' condition ' ' makes no sense
diff --git a/tests/qapi-schema/bad-if-list.json
b/tests/qapi-schema/bad-if-list.json
index 49ced9b9ca..ea3d95bb6b 100644
--- a/tests/qapi-schema/bad-if-list.json
+++ b/tests/qapi-schema/bad-if-list.json
@@ -1,3 +1,3 @@
# check invalid 'if' content
{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
- 'if': ['foo', ''] }
+ 'if': ['foo', ' '] }
--
2.21.0
[Qemu-devel] [PATCH 03/19] tests/qapi-schema: Demonstrate misleading optional tag error
Test flat-union-optional-discriminator declares its union tag as
'*switch': 'Enum', and points to it with 'discriminator': '*switch'.
This gets rejected as "discriminator of flat union 'MyUnion' uses
invalid name '*switch'". Correct; member 'discriminator' doesn't
accept a '*' prefix.
However, this merely tests name validity checking, which we already
cover elsewhere. More interesting is testing the valid name 'switch'.
This reports "discriminator 'switch' is not a member of base struct
'Base'", which is misleading.
Copy the existing 'discriminator': '*switch' test to
flat-union-discriminator-bad-name, and rewrite its comment. Change
flat-union-optional-discriminator to test 'discriminator': 'switch',
and mark it FIXME.
Signed-off-by: Markus Armbruster
---
tests/Makefile.include| 1 +
.../qapi-schema/flat-union-discriminator-bad-name.err | 1 +
.../flat-union-discriminator-bad-name.exit| 1 +
.../flat-union-discriminator-bad-name.json| 11 +++
.../qapi-schema/flat-union-discriminator-bad-name.out | 0
.../qapi-schema/flat-union-optional-discriminator.err | 2 +-
.../flat-union-optional-discriminator.json| 3 ++-
7 files changed, 17 insertions(+), 2 deletions(-)
create mode 100644 tests/qapi-schema/flat-union-discriminator-bad-name.err
create mode 100644 tests/qapi-schema/flat-union-discriminator-bad-name.exit
create mode 100644 tests/qapi-schema/flat-union-discriminator-bad-name.json
create mode 100644 tests/qapi-schema/flat-union-discriminator-bad-name.out
diff --git a/tests/Makefile.include b/tests/Makefile.include
index a1deaa5456..a11bde743e 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -389,6 +389,7 @@ qapi-schema += flat-union-bad-discriminator.json
qapi-schema += flat-union-base-any.json
qapi-schema += flat-union-base-union.json
qapi-schema += flat-union-clash-member.json
+qapi-schema += flat-union-discriminator-bad-name.json
qapi-schema += flat-union-empty.json
qapi-schema += flat-union-inline.json
qapi-schema += flat-union-inline-invalid-dict.json
diff --git a/tests/qapi-schema/flat-union-discriminator-bad-name.err
b/tests/qapi-schema/flat-union-discriminator-bad-name.err
new file mode 100644
index 00..7238d126ca
--- /dev/null
+++ b/tests/qapi-schema/flat-union-discriminator-bad-name.err
@@ -0,0 +1 @@
+tests/qapi-schema/flat-union-discriminator-bad-name.json:7: Discriminator of
flat union 'MyUnion' does not allow optional name '*switch'
diff --git a/tests/qapi-schema/flat-union-discriminator-bad-name.exit
b/tests/qapi-schema/flat-union-discriminator-bad-name.exit
new file mode 100644
index 00..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/flat-union-discriminator-bad-name.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/flat-union-discriminator-bad-name.json
b/tests/qapi-schema/flat-union-discriminator-bad-name.json
new file mode 100644
index 00..66376084fc
--- /dev/null
+++ b/tests/qapi-schema/flat-union-discriminator-bad-name.json
@@ -0,0 +1,11 @@
+# discriminator '*switch' isn't a member of base, 'switch' is
+# reports "does not allow optional name", which is good enough
+{ 'enum': 'Enum', 'data': [ 'one', 'two' ] }
+{ 'struct': 'Base',
+ 'data': { '*switch': 'Enum' } }
+{ 'struct': 'Branch', 'data': { 'name': 'str' } }
+{ 'union': 'MyUnion',
+ 'base': 'Base',
+ 'discriminator': '*switch',
+ 'data': { 'one': 'Branch',
+'two': 'Branch' } }
diff --git a/tests/qapi-schema/flat-union-discriminator-bad-name.out
b/tests/qapi-schema/flat-union-discriminator-bad-name.out
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/qapi-schema/flat-union-optional-discriminator.err
b/tests/qapi-schema/flat-union-optional-discriminator.err
index aaabedb3bd..8b4a4ba847 100644
--- a/tests/qapi-schema/flat-union-optional-discriminator.err
+++ b/tests/qapi-schema/flat-union-optional-discriminator.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-optional-discriminator.json:6: Discriminator of
flat union 'MyUnion' does not allow optional name '*switch'
+tests/qapi-schema/flat-union-optional-discriminator.json:7: Discriminator
'switch' is not a member of base struct 'Base'
diff --git a/tests/qapi-schema/flat-union-optional-discriminator.json
b/tests/qapi-schema/flat-union-optional-discriminator.json
index 08a8f7ef8b..143ab23a0d 100644
--- a/tests/qapi-schema/flat-union-optional-discriminator.json
+++ b/tests/qapi-schema/flat-union-optional-discriminator.json
@@ -1,10 +1,11 @@
# we require the discriminator to be non-optional
+# FIXME reports "discriminator 'switch' is not a member of base struct 'Base'"
{ 'enum': 'Enum', 'data': [ 'one', 'two' ] }
{ 'struct': 'Base',
'data': { '*switch': 'Enum' } }
{ 'struct': 'Branch', 'data': { 'name': 'str' } }
{ 'union': 'MyUnion',
'base': 'Base',
- 'discriminator': '*switch',
+ 'discriminator': 'switch',
'data': { 'one': 'Branch',
'two': 'Branch' } }
--
2.21.0
[Qemu-devel] [PATCH 18/19] qapi: Fix excessive QAPISchemaEntity.check() recursion
Entity checking goes back to commit ac88219a6c "qapi: New QAPISchema intermediate representation", v2.5.0. It's designed to work as follows: QAPISchema.check() calls .check() for all the schema's entities. An entity's .check() recurses into another entity's .check() only if the C struct generated for the former contains the C struct generated for the latter (pointers don't count). This is used to detect "object contains itself". There are two instances of this: * An object's C struct contains its base's C struct QAPISchemaObjectType.check() calls self.base.check() * An object's C struct contains its variants' C structs QAPISchemaObjectTypeVariants().check calls v.type.check(). Since commit b807a1e1e3 "qapi: Check for QAPI collisions involving variant members", v2.6.0. Thus, only object types can participate in recursion. QAPISchemaObjectType.check() is made for that: it checks @self when called the first time, recursing into base and variants, it reports an "contains itself" error when this recursion reaches an object being checked, and does nothing it reaches an object that has been checked already. The other .check() may safely assume they get called exactly once. Sadly, this design has since eroded: * QAPISchemaCommand.check() and QAPISchemaEvent.check() call .args_type.check(). Since commit c818408e44 "qapi: Implement boxed types for commands/events", v2.7.0. Harmless, since args_type can only be an object type. * QAPISchemaEntity.check() calls ._ifcond.check() when inheriting the condition from another type. Since commit 4fca21c1b0 qapi: leave the ifcond attribute undefined until check(), v3.0.0. This makes simple union wrapper types recurse into the wrapped type (nothing else uses this condition inheritance). The .check() of types used as simple union branch type get called multiple times. * QAPISchemaObjectType.check() calls its super type's .check() *before* the conditional handling multiple calls. Also since commit 4fca21c1b0. QAPISchemaObjectType.check()'s guard against multiple checking doesn't protect QAPISchemaEntity.check(). * QAPISchemaArrayType.check() calls .element_type.check(). Also since commit 4fca21c1b0. The .check() of types used as array element types get called multiple times. Commit 56a4689582 "qapi: Fix array first used in a different module" (v4.0.0) added more code relying on this .element_type.check(). The absence of explosions suggests the .check() involved happen to be effectively idempotent. Fix the unwanted recursion anyway: * QAPISchemaCommand.check() and QAPISchemaEvent.check() calling .args_type.check() is unnecessary. Delete the calls. * Fix QAPISchemaObjectType.check() to call its super type's .check() after the conditional handling multiple calls. * A QAPISchemaEntity's .ifcond becomes valid at .check(). This is due to arrays and simple unions. Most types get ifcond and info passed to their constructor. Array types don't: they get it from their element type, which becomes known only in .element_type.check(). The implicit wrapper object types for simple union branches don't: they get it from the wrapped type, which might be an array. Ditch the idea to set .ifcond in .check(). Instead, turn it into a property and compute it on demand. Safe because it's only used after the schema has been checked. Most types simply return the ifcond passed to their constructor. Array types forward to their .element_type instead, and the wrapper types forward to the wrapped type. * A QAPISchemaEntity's .module becomes valid at .check(). This is because computing it needs info and schema.fname, and because array types get it from their element type instead. Make it a property just like .ifcond. Additionally, have QAPISchemaEntity.check() assert it gets called at most once, so the design invariant will stick this time. Neglecting that was entirely my fault. Signed-off-by: Markus Armbruster --- scripts/qapi/common.py | 74 +- 1 file changed, 52 insertions(+), 22 deletions(-) diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index e2c87d1349..c199a2b58c 100644 --- a/scripts/qapi/common.py +++ b/scripts/qapi/common.py @@ -1155,7 +1155,7 @@ class QAPISchemaEntity(object): def __init__(self, name, info, doc, ifcond=None): assert name is None or isinstance(name, str) self.name = name -self.module = None +self._module = None # For explicitly defined entities, info points to the (explicit) # definition. For builtins (and their arrays), info is None. # For implicitly defined entities, info points to a place that @@ -1164,21 +1164,27 @@ class QAPISchemaEntity(object): self.info = info self.doc = doc self._ifcond = ifcond or [] +self._checked = False def c_name(self): return c_name(self.name) def
[Qemu-devel] [PATCH 15/19] qapi: Clean up around check_known_keys()
All callers pass a dict argument to @keys, except check_keys() passes
a dict's .keys(). Drop .keys() there, and rename parameter @keys to
@value.
Signed-off-by: Markus Armbruster
---
scripts/qapi/common.py | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 4d4e0be770..3c3154a039 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1011,18 +1011,18 @@ def check_struct(expr, info):
check_name(info, "Feature of struct %s" % name, f['name'])
-def check_known_keys(info, source, keys, required, optional):
+def check_known_keys(info, source, value, required, optional):
def pprint(elems):
return ', '.join("'" + e + "'" for e in sorted(elems))
-missing = set(required) - set(keys)
+missing = set(required) - set(value)
if missing:
raise QAPISemError(info, "Key%s %s %s missing from %s"
% ('s' if len(missing) > 1 else '', pprint(missing),
'are' if len(missing) > 1 else 'is', source))
allowed = set(required + optional)
-unknown = set(keys) - allowed
+unknown = set(value) - allowed
if unknown:
raise QAPISemError(info, "Unknown key%s %s in %s\nValid keys are %s."
% ('s' if len(unknown) > 1 else '', pprint(unknown),
@@ -1035,7 +1035,7 @@ def check_keys(expr, info, meta, required, optional=[]):
raise QAPISemError(info, "'%s' key must have a string value" % meta)
required = required + [meta]
source = "%s '%s'" % (meta, name)
-check_known_keys(info, source, expr.keys(), required, optional)
+check_known_keys(info, source, expr, required, optional)
for (key, value) in expr.items():
if key in ['gen', 'success-response'] and value is not False:
raise QAPISemError(info,
--
2.21.0
[Qemu-devel] [PATCH 04/19] tests/qapi-schema: Demonstrate broken discriminator errors
When the union definition's base is an object, some error messages
show it as an OrderedDict. Oops. Mark FIXME.
Signed-off-by: Markus Armbruster
---
tests/qapi-schema/flat-union-invalid-discriminator.err | 2 +-
tests/qapi-schema/flat-union-invalid-discriminator.json| 6 ++
tests/qapi-schema/flat-union-invalid-if-discriminator.err | 2 +-
tests/qapi-schema/flat-union-invalid-if-discriminator.json | 6 ++
4 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.err
b/tests/qapi-schema/flat-union-invalid-discriminator.err
index 5f4055614e..947a6b73aa 100644
--- a/tests/qapi-schema/flat-union-invalid-discriminator.err
+++ b/tests/qapi-schema/flat-union-invalid-discriminator.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-invalid-discriminator.json:13: Discriminator
'enum_wrong' is not a member of base struct 'TestBase'
+tests/qapi-schema/flat-union-invalid-discriminator.json:11: Discriminator
'enum_wrong' is not a member of base struct 'OrderedDict([('enum1', {'type':
'TestEnum'})])'
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.json
b/tests/qapi-schema/flat-union-invalid-discriminator.json
index 48b94c3a4d..de86cf0760 100644
--- a/tests/qapi-schema/flat-union-invalid-discriminator.json
+++ b/tests/qapi-schema/flat-union-invalid-discriminator.json
@@ -1,9 +1,7 @@
+# FIXME error message shows base as OrderedDict
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
-{ 'struct': 'TestBase',
- 'data': { 'enum1': 'TestEnum' } }
-
{ 'struct': 'TestTypeA',
'data': { 'string': 'str' } }
@@ -11,7 +9,7 @@
'data': { 'integer': 'int' } }
{ 'union': 'TestUnion',
- 'base': 'TestBase',
+ 'base': { 'enum1': 'TestEnum' },
'discriminator': 'enum_wrong',
'data': { 'value1': 'TestTypeA',
'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/flat-union-invalid-if-discriminator.err
b/tests/qapi-schema/flat-union-invalid-if-discriminator.err
index 0c94c9860d..ec04c4840c 100644
--- a/tests/qapi-schema/flat-union-invalid-if-discriminator.err
+++ b/tests/qapi-schema/flat-union-invalid-if-discriminator.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-invalid-if-discriminator.json:13: The
discriminator TestBase.enum1 for union TestUnion must not be conditional
+tests/qapi-schema/flat-union-invalid-if-discriminator.json:11: The
discriminator OrderedDict([('enum1', OrderedDict([('type', 'TestEnum'), ('if',
'FOO')]))]).enum1 for union TestUnion must not be conditional
diff --git a/tests/qapi-schema/flat-union-invalid-if-discriminator.json
b/tests/qapi-schema/flat-union-invalid-if-discriminator.json
index 618ec36396..bbaa9a3f82 100644
--- a/tests/qapi-schema/flat-union-invalid-if-discriminator.json
+++ b/tests/qapi-schema/flat-union-invalid-if-discriminator.json
@@ -1,9 +1,7 @@
+# FIXME error message shows base as OrderedDict
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
-{ 'struct': 'TestBase',
- 'data': { 'enum1': { 'type': 'TestEnum', 'if': 'FOO' } } }
-
{ 'struct': 'TestTypeA',
'data': { 'string': 'str' } }
@@ -11,7 +9,7 @@
'data': { 'integer': 'int' } }
{ 'union': 'TestUnion',
- 'base': 'TestBase',
+ 'base': { 'enum1': { 'type': 'TestEnum', 'if': 'FOO' } },
'discriminator': 'enum1',
'data': { 'value1': 'TestTypeA',
'value2': 'TestTypeB' } }
--
2.21.0
[Qemu-devel] [PATCH 17/19] qapi: Fix to .check() empty structs just once
QAPISchemaObjectType.check() does nothing for types that have been checked already. Except the "has been checked" predicate is broken for empty types: self.members is [] then, which isn't true. The bug is harmless, but fix it anyway: use self.member is not None instead. Signed-off-by: Markus Armbruster --- scripts/qapi/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index 7e79c42b6a..e2c87d1349 100644 --- a/scripts/qapi/common.py +++ b/scripts/qapi/common.py @@ -1406,7 +1406,7 @@ class QAPISchemaObjectType(QAPISchemaType): if self.members is False: # check for cycles raise QAPISemError(self.info, "Object %s contains itself" % self.name) -if self.members: +if self.members is not None:# already checked return self.members = False# mark as being checked seen = OrderedDict() -- 2.21.0
[Qemu-devel] [PATCH 10/19] qapi: Fix broken discriminator error messages
check_union() checks the discriminator exists in base and makes sense.
Two error messages mention the base. These are broken for anonymous
bases, as demonstrated by tests flat-union-invalid-discriminator and
flat-union-invalid-if-discriminator.err. The third one doesn't
bother.
First broken when commit ac4338f8eb "qapi: Allow anonymous base for
flat union" (v2.6.0) neglected to adjust the "not a member of base"
error message. Commit ccadd6bcba "qapi: Add 'if' to implicit struct
members" (v4.0.0) then cloned the flawed error message.
Dumb them down not to mention the base.
Signed-off-by: Markus Armbruster
---
scripts/qapi/common.py | 9 -
tests/qapi-schema/flat-union-invalid-discriminator.err | 2 +-
tests/qapi-schema/flat-union-invalid-discriminator.json | 1 -
.../qapi-schema/flat-union-invalid-if-discriminator.err | 2 +-
.../qapi-schema/flat-union-invalid-if-discriminator.json | 1 -
tests/qapi-schema/flat-union-optional-discriminator.err | 2 +-
tests/qapi-schema/union-base-empty.err | 2 +-
7 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index ef7c7be4fd..a58e904978 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -877,14 +877,13 @@ def check_union(expr, info):
discriminator_value = base_members.get(discriminator)
if not discriminator_value:
raise QAPISemError(info,
- "Discriminator '%s' is not a member of base "
- "struct '%s'"
- % (discriminator, base))
+ "Discriminator '%s' is not a member of 'base'"
+ % discriminator)
if discriminator_value.get('if'):
raise QAPISemError(
info,
-"The discriminator %s.%s for union %s must not be conditional"
-% (base, discriminator, name))
+"The discriminator '%s' for union %s must not be conditional"
+% (discriminator, name))
enum_define = enum_types.get(discriminator_value['type'])
# Do not allow string discriminator
if not enum_define:
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.err
b/tests/qapi-schema/flat-union-invalid-discriminator.err
index 947a6b73aa..495d5a520e 100644
--- a/tests/qapi-schema/flat-union-invalid-discriminator.err
+++ b/tests/qapi-schema/flat-union-invalid-discriminator.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-invalid-discriminator.json:11: Discriminator
'enum_wrong' is not a member of base struct 'OrderedDict([('enum1', {'type':
'TestEnum'})])'
+tests/qapi-schema/flat-union-invalid-discriminator.json:10: Discriminator
'enum_wrong' is not a member of 'base'
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.json
b/tests/qapi-schema/flat-union-invalid-discriminator.json
index de86cf0760..c4fce97362 100644
--- a/tests/qapi-schema/flat-union-invalid-discriminator.json
+++ b/tests/qapi-schema/flat-union-invalid-discriminator.json
@@ -1,4 +1,3 @@
-# FIXME error message shows base as OrderedDict
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
diff --git a/tests/qapi-schema/flat-union-invalid-if-discriminator.err
b/tests/qapi-schema/flat-union-invalid-if-discriminator.err
index ec04c4840c..cc5c3fb80b 100644
--- a/tests/qapi-schema/flat-union-invalid-if-discriminator.err
+++ b/tests/qapi-schema/flat-union-invalid-if-discriminator.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-invalid-if-discriminator.json:11: The
discriminator OrderedDict([('enum1', OrderedDict([('type', 'TestEnum'), ('if',
'FOO')]))]).enum1 for union TestUnion must not be conditional
+tests/qapi-schema/flat-union-invalid-if-discriminator.json:10: The
discriminator 'enum1' for union TestUnion must not be conditional
diff --git a/tests/qapi-schema/flat-union-invalid-if-discriminator.json
b/tests/qapi-schema/flat-union-invalid-if-discriminator.json
index bbaa9a3f82..e49992b798 100644
--- a/tests/qapi-schema/flat-union-invalid-if-discriminator.json
+++ b/tests/qapi-schema/flat-union-invalid-if-discriminator.json
@@ -1,4 +1,3 @@
-# FIXME error message shows base as OrderedDict
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
diff --git a/tests/qapi-schema/flat-union-optional-discriminator.err
b/tests/qapi-schema/flat-union-optional-discriminator.err
index 8b4a4ba847..45f5407c34 100644
--- a/tests/qapi-schema/flat-union-optional-discriminator.err
+++ b/tests/qapi-schema/flat-union-optional-discriminator.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-optional-discriminator.json:7: Discriminator
'switch' is not a member of base struct 'Base'
+tests/qapi-schema/flat-union-optional-discriminator.json:7: Discriminator
'switch' is not a member of 'base'
diff --git a/tests/qapi-schema/union-base-empty.err
b/tests/qapi-schema/union-base-empty.err
index
[Qemu-devel] [PATCH 01/19] tests/qapi-schema: Cover unknown pragma
Signed-off-by: Markus Armbruster
---
tests/Makefile.include| 1 +
tests/qapi-schema/pragma-unknown.err | 1 +
tests/qapi-schema/pragma-unknown.exit | 1 +
tests/qapi-schema/pragma-unknown.json | 1 +
tests/qapi-schema/pragma-unknown.out | 0
5 files changed, 4 insertions(+)
create mode 100644 tests/qapi-schema/pragma-unknown.err
create mode 100644 tests/qapi-schema/pragma-unknown.exit
create mode 100644 tests/qapi-schema/pragma-unknown.json
create mode 100644 tests/qapi-schema/pragma-unknown.out
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 52caeb705e..bb071d2ba9 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -428,6 +428,7 @@ qapi-schema += pragma-doc-required-crap.json
qapi-schema += pragma-extra-junk.json
qapi-schema += pragma-name-case-whitelist-crap.json
qapi-schema += pragma-non-dict.json
+qapi-schema += pragma-unknown.json
qapi-schema += pragma-returns-whitelist-crap.json
qapi-schema += qapi-schema-test.json
qapi-schema += quoted-structural-chars.json
diff --git a/tests/qapi-schema/pragma-unknown.err
b/tests/qapi-schema/pragma-unknown.err
new file mode 100644
index 00..6ef2058316
--- /dev/null
+++ b/tests/qapi-schema/pragma-unknown.err
@@ -0,0 +1 @@
+tests/qapi-schema/pragma-unknown.json:1: Unknown pragma 'no-such-pragma'
diff --git a/tests/qapi-schema/pragma-unknown.exit
b/tests/qapi-schema/pragma-unknown.exit
new file mode 100644
index 00..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/pragma-unknown.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/pragma-unknown.json
b/tests/qapi-schema/pragma-unknown.json
new file mode 100644
index 00..c5153f
--- /dev/null
+++ b/tests/qapi-schema/pragma-unknown.json
@@ -0,0 +1 @@
+{ 'pragma': { 'no-such-pragma': false } }
diff --git a/tests/qapi-schema/pragma-unknown.out
b/tests/qapi-schema/pragma-unknown.out
new file mode 100644
index 00..e69de29bb2
--
2.21.0
[Qemu-devel] [PATCH 12/19] qapi: Fix missing 'if' checks in struct, union, alternate 'data'
Commit 87adbbffd4..3e270dcacc "qapi: Add 'if' to (implicit
struct|union|alternate) members" (v4.0.0) neglected test coverage, and
promptly failed to check the conditions. Review fail.
Recent commit "tests/qapi-schema: Demonstrate insufficient 'if'
checking" added test coverage, demonstrating the bug. Fix it by add
the missing check_if().
Signed-off-by: Markus Armbruster
---
scripts/qapi/common.py| 3 +++
.../alternate-branch-if-invalid.err | 1 +
.../alternate-branch-if-invalid.exit | 2 +-
.../alternate-branch-if-invalid.json | 1 -
.../alternate-branch-if-invalid.out | 16 -
.../qapi-schema/struct-member-if-invalid.err | 1 +
.../qapi-schema/struct-member-if-invalid.exit | 2 +-
.../qapi-schema/struct-member-if-invalid.json | 1 -
.../qapi-schema/struct-member-if-invalid.out | 15
tests/qapi-schema/union-branch-if-invalid.err | 1 +
.../qapi-schema/union-branch-if-invalid.exit | 2 +-
.../qapi-schema/union-branch-if-invalid.json | 1 -
tests/qapi-schema/union-branch-if-invalid.out | 23 ---
13 files changed, 9 insertions(+), 60 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 2b46164854..cacee9b8bb 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -803,6 +803,7 @@ def check_type(info, source, value,
# an optional argument.
check_known_keys(info, "member '%s' of %s" % (key, source),
arg, ['type'], ['if'])
+check_if(arg, info)
check_type(info, "Member '%s' of %s" % (key, source),
arg['type'], allow_array=True,
allow_metas=['built-in', 'union', 'alternate', 'struct',
@@ -902,6 +903,7 @@ def check_union(expr, info):
check_known_keys(info, "member '%s' of union '%s'" % (key, name),
value, ['type'], ['if'])
+check_if(value, info)
# Each value must name a known type
check_type(info, "Member '%s' of union '%s'" % (key, name),
value['type'],
@@ -930,6 +932,7 @@ def check_alternate(expr, info):
check_known_keys(info,
"member '%s' of alternate '%s'" % (key, name),
value, ['type'], ['if'])
+check_if(value, info)
typ = value['type']
# Ensure alternates have no type conflicts.
diff --git a/tests/qapi-schema/alternate-branch-if-invalid.err
b/tests/qapi-schema/alternate-branch-if-invalid.err
index e69de29bb2..f1d6c10e00 100644
--- a/tests/qapi-schema/alternate-branch-if-invalid.err
+++ b/tests/qapi-schema/alternate-branch-if-invalid.err
@@ -0,0 +1 @@
+tests/qapi-schema/alternate-branch-if-invalid.json:2: 'if' condition ' ' makes
no sense
diff --git a/tests/qapi-schema/alternate-branch-if-invalid.exit
b/tests/qapi-schema/alternate-branch-if-invalid.exit
index 573541ac97..d00491fd7e 100644
--- a/tests/qapi-schema/alternate-branch-if-invalid.exit
+++ b/tests/qapi-schema/alternate-branch-if-invalid.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/alternate-branch-if-invalid.json
b/tests/qapi-schema/alternate-branch-if-invalid.json
index 6497f53475..fea6d9080c 100644
--- a/tests/qapi-schema/alternate-branch-if-invalid.json
+++ b/tests/qapi-schema/alternate-branch-if-invalid.json
@@ -1,4 +1,3 @@
# Cover alternative with invalid 'if'
-# FIXME not rejected, would generate '#if \n'
{ 'alternate': 'Alt',
'data': { 'branch': { 'type': 'int', 'if': ' ' } } }
diff --git a/tests/qapi-schema/alternate-branch-if-invalid.out
b/tests/qapi-schema/alternate-branch-if-invalid.out
index 89305d7f21..e69de29bb2 100644
--- a/tests/qapi-schema/alternate-branch-if-invalid.out
+++ b/tests/qapi-schema/alternate-branch-if-invalid.out
@@ -1,16 +0,0 @@
-module None
-object q_empty
-enum QType
-prefix QTYPE
-member none
-member qnull
-member qnum
-member qstring
-member qdict
-member qlist
-member qbool
-module alternate-branch-if-invalid.json
-alternate Alt
-tag type
-case branch: int
-if [' ']
diff --git a/tests/qapi-schema/struct-member-if-invalid.err
b/tests/qapi-schema/struct-member-if-invalid.err
index e69de29bb2..bfd65db97b 100644
--- a/tests/qapi-schema/struct-member-if-invalid.err
+++ b/tests/qapi-schema/struct-member-if-invalid.err
@@ -0,0 +1 @@
+tests/qapi-schema/struct-member-if-invalid.json:2: 'if' condition must be a
string or a list of strings
diff --git a/tests/qapi-schema/struct-member-if-invalid.exit
b/tests/qapi-schema/struct-member-if-invalid.exit
index 573541ac97..d00491fd7e 100644
--- a/tests/qapi-schema/struct-member-if-invalid.exit
+++ b/tests/qapi-schema/struct-member-if-invalid.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/struct-member-if-invalid.json
b/tests/qapi-schema/struct-member-if-invalid.json
index 73987e04fc..35078bd660 100644
--- a/tests/qapi-schema/struct-member-if-invalid.json
+++
[Qemu-devel] [PATCH 06/19] tests/qapi-schema: Demonstrate suboptimal lexical errors
The error message for forgotten quotes around a name shows just the
name's first character, which isn't as nice as it could be. Same for
attempting to use a number.
Signed-off-by: Markus Armbruster
---
tests/Makefile.include | 1 +
tests/qapi-schema/bad-type-int.json | 2 +-
tests/qapi-schema/funny-word.err| 1 +
tests/qapi-schema/funny-word.exit | 1 +
tests/qapi-schema/funny-word.json | 1 +
tests/qapi-schema/funny-word.out| 0
6 files changed, 5 insertions(+), 1 deletion(-)
create mode 100644 tests/qapi-schema/funny-word.err
create mode 100644 tests/qapi-schema/funny-word.exit
create mode 100644 tests/qapi-schema/funny-word.json
create mode 100644 tests/qapi-schema/funny-word.out
diff --git a/tests/Makefile.include b/tests/Makefile.include
index c108a83076..f5a4266992 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -403,6 +403,7 @@ qapi-schema += flat-union-no-base.json
qapi-schema += flat-union-optional-discriminator.json
qapi-schema += flat-union-string-discriminator.json
qapi-schema += funny-char.json
+qapi-schema += funny-word.json
qapi-schema += ident-with-escape.json
qapi-schema += include-before-err.json
qapi-schema += include-cycle.json
diff --git a/tests/qapi-schema/bad-type-int.json
b/tests/qapi-schema/bad-type-int.json
index 56fc6f8126..f3ad803cb6 100644
--- a/tests/qapi-schema/bad-type-int.json
+++ b/tests/qapi-schema/bad-type-int.json
@@ -1,3 +1,3 @@
# we reject an expression with a metatype that is not a string
# FIXME: once the parser understands integer inputs, improve the error message
-{ 'struct': 1, 'data': { } }
+{ 'struct': 123, 'data': { } }
diff --git a/tests/qapi-schema/funny-word.err b/tests/qapi-schema/funny-word.err
new file mode 100644
index 00..0a440574bd
--- /dev/null
+++ b/tests/qapi-schema/funny-word.err
@@ -0,0 +1 @@
+tests/qapi-schema/funny-word.json:1:3: Stray "c"
diff --git a/tests/qapi-schema/funny-word.exit
b/tests/qapi-schema/funny-word.exit
new file mode 100644
index 00..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/funny-word.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/funny-word.json
b/tests/qapi-schema/funny-word.json
new file mode 100644
index 00..1153b9f12f
--- /dev/null
+++ b/tests/qapi-schema/funny-word.json
@@ -0,0 +1 @@
+{ command: 'foo' }
diff --git a/tests/qapi-schema/funny-word.out b/tests/qapi-schema/funny-word.out
new file mode 100644
index 00..e69de29bb2
--
2.21.0
[Qemu-devel] [PATCH 05/19] tests/qapi-schema: Demonstrate insufficient 'if' checking
Cover invalid 'if' in struct members, features, union and alternate
branches. Four out of four are broken. Mark FIXME.
Signed-off-by: Markus Armbruster
---
tests/Makefile.include| 4
.../alternate-branch-if-invalid.err | 0
.../alternate-branch-if-invalid.exit | 1 +
.../alternate-branch-if-invalid.json | 4
.../alternate-branch-if-invalid.out | 16 +
tests/qapi-schema/features-if-invalid.err | 0
tests/qapi-schema/features-if-invalid.exit| 1 +
tests/qapi-schema/features-if-invalid.json| 5
tests/qapi-schema/features-if-invalid.out | 14 +++
.../qapi-schema/struct-member-if-invalid.err | 0
.../qapi-schema/struct-member-if-invalid.exit | 1 +
.../qapi-schema/struct-member-if-invalid.json | 4
.../qapi-schema/struct-member-if-invalid.out | 15
tests/qapi-schema/union-branch-if-invalid.err | 0
.../qapi-schema/union-branch-if-invalid.exit | 1 +
.../qapi-schema/union-branch-if-invalid.json | 7 ++
tests/qapi-schema/union-branch-if-invalid.out | 23 +++
17 files changed, 96 insertions(+)
create mode 100644 tests/qapi-schema/alternate-branch-if-invalid.err
create mode 100644 tests/qapi-schema/alternate-branch-if-invalid.exit
create mode 100644 tests/qapi-schema/alternate-branch-if-invalid.json
create mode 100644 tests/qapi-schema/alternate-branch-if-invalid.out
create mode 100644 tests/qapi-schema/features-if-invalid.err
create mode 100644 tests/qapi-schema/features-if-invalid.exit
create mode 100644 tests/qapi-schema/features-if-invalid.json
create mode 100644 tests/qapi-schema/features-if-invalid.out
create mode 100644 tests/qapi-schema/struct-member-if-invalid.err
create mode 100644 tests/qapi-schema/struct-member-if-invalid.exit
create mode 100644 tests/qapi-schema/struct-member-if-invalid.json
create mode 100644 tests/qapi-schema/struct-member-if-invalid.out
create mode 100644 tests/qapi-schema/union-branch-if-invalid.err
create mode 100644 tests/qapi-schema/union-branch-if-invalid.exit
create mode 100644 tests/qapi-schema/union-branch-if-invalid.json
create mode 100644 tests/qapi-schema/union-branch-if-invalid.out
diff --git a/tests/Makefile.include b/tests/Makefile.include
index a11bde743e..c108a83076 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -295,6 +295,7 @@ check-qtest-generic-y += tests/test-hmp$(EXESUF)
qapi-schema += alternate-any.json
qapi-schema += alternate-array.json
qapi-schema += alternate-base.json
+qapi-schema += alternate-branch-if-invalid.json
qapi-schema += alternate-clash.json
qapi-schema += alternate-conflict-dict.json
qapi-schema += alternate-conflict-enum-bool.json
@@ -379,6 +380,7 @@ qapi-schema += event-member-invalid-dict.json
qapi-schema += event-nest-struct.json
qapi-schema += features-bad-type.json
qapi-schema += features-duplicate-name.json
+qapi-schema += features-if-invalid.json
qapi-schema += features-missing-name.json
qapi-schema += features-name-bad-type.json
qapi-schema += features-no-list.json
@@ -453,6 +455,7 @@ qapi-schema += string-code-point-127.json
qapi-schema += struct-base-clash-deep.json
qapi-schema += struct-base-clash.json
qapi-schema += struct-data-invalid.json
+qapi-schema += struct-member-if-invalid.json
qapi-schema += struct-member-invalid-dict.json
qapi-schema += struct-member-invalid.json
qapi-schema += trailing-comma-list.json
@@ -464,6 +467,7 @@ qapi-schema += unclosed-string.json
qapi-schema += union-base-empty.json
qapi-schema += union-base-no-discriminator.json
qapi-schema += union-branch-case.json
+qapi-schema += union-branch-if-invalid.json
qapi-schema += union-branch-invalid-dict.json
qapi-schema += union-clash-branches.json
qapi-schema += union-empty.json
diff --git a/tests/qapi-schema/alternate-branch-if-invalid.err
b/tests/qapi-schema/alternate-branch-if-invalid.err
new file mode 100644
index 00..e69de29bb2
diff --git a/tests/qapi-schema/alternate-branch-if-invalid.exit
b/tests/qapi-schema/alternate-branch-if-invalid.exit
new file mode 100644
index 00..573541ac97
--- /dev/null
+++ b/tests/qapi-schema/alternate-branch-if-invalid.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/alternate-branch-if-invalid.json
b/tests/qapi-schema/alternate-branch-if-invalid.json
new file mode 100644
index 00..6497f53475
--- /dev/null
+++ b/tests/qapi-schema/alternate-branch-if-invalid.json
@@ -0,0 +1,4 @@
+# Cover alternative with invalid 'if'
+# FIXME not rejected, would generate '#if \n'
+{ 'alternate': 'Alt',
+ 'data': { 'branch': { 'type': 'int', 'if': ' ' } } }
diff --git a/tests/qapi-schema/alternate-branch-if-invalid.out
b/tests/qapi-schema/alternate-branch-if-invalid.out
new file mode 100644
index 00..89305d7f21
--- /dev/null
+++ b/tests/qapi-schema/alternate-branch-if-invalid.out
@@ -0,0 +1,16 @@
+module None
+object q_empty
+enum QType
+prefix QTYPE
+member none
+
[Qemu-devel] [PATCH 14/19] qapi: Simplify check_keys()
check_keys() parameter expr_elem expects a dictionary with keys 'expr'
and 'info'. Passing the two values separately is simpler, so do that.
Signed-off-by: Markus Armbruster
---
scripts/qapi/common.py | 19 ---
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 4d1f62e808..4d4e0be770 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1029,9 +1029,7 @@ def check_known_keys(info, source, keys, required,
optional):
source, pprint(allowed)))
-def check_keys(expr_elem, meta, required, optional=[]):
-expr = expr_elem['expr']
-info = expr_elem['info']
+def check_keys(expr, info, meta, required, optional=[]):
name = expr[meta]
if not isinstance(name, str):
raise QAPISemError(info, "'%s' key must have a string value" % meta)
@@ -1100,40 +1098,39 @@ def check_exprs(exprs):
if 'enum' in expr:
meta = 'enum'
-check_keys(expr_elem, 'enum', ['data'], ['if', 'prefix'])
+check_keys(expr, info, 'enum', ['data'], ['if', 'prefix'])
normalize_enum(expr)
enum_types[expr[meta]] = expr
elif 'union' in expr:
meta = 'union'
-check_keys(expr_elem, 'union', ['data'],
+check_keys(expr, info, 'union', ['data'],
['base', 'discriminator', 'if'])
normalize_members(expr.get('base'))
normalize_members(expr['data'])
union_types[expr[meta]] = expr
elif 'alternate' in expr:
meta = 'alternate'
-check_keys(expr_elem, 'alternate', ['data'], ['if'])
+check_keys(expr, info, 'alternate', ['data'], ['if'])
normalize_members(expr['data'])
elif 'struct' in expr:
meta = 'struct'
-check_keys(expr_elem, 'struct', ['data'],
+check_keys(expr, info, 'struct', ['data'],
['base', 'if', 'features'])
normalize_members(expr['data'])
normalize_features(expr.get('features'))
struct_types[expr[meta]] = expr
elif 'command' in expr:
meta = 'command'
-check_keys(expr_elem, 'command', [],
+check_keys(expr, info, 'command', [],
['data', 'returns', 'gen', 'success-response',
'boxed', 'allow-oob', 'allow-preconfig', 'if'])
normalize_members(expr.get('data'))
elif 'event' in expr:
meta = 'event'
-check_keys(expr_elem, 'event', [], ['data', 'boxed', 'if'])
+check_keys(expr, info, 'event', [], ['data', 'boxed', 'if'])
normalize_members(expr.get('data'))
else:
-raise QAPISemError(expr_elem['info'],
- "Expression is missing metatype")
+raise QAPISemError(info, "Expression is missing metatype")
normalize_if(expr)
name = expr[meta]
add_name(name, info, meta)
--
2.21.0
[Qemu-devel] [PATCH 00/19] qapi: Frontend fixes and cleanups
Here's the next batch of qapi patches, based on my "[PATCH v3 00/16] qapi: Schema language cleanups & doc improvements". There's more in the pipeline. Based-on: <20190913201349.24332-1-arm...@redhat.com> Markus Armbruster (19): tests/qapi-schema: Cover unknown pragma tests/qapi-schema: Delete two redundant tests tests/qapi-schema: Demonstrate misleading optional tag error tests/qapi-schema: Demonstrate broken discriminator errors tests/qapi-schema: Demonstrate insufficient 'if' checking tests/qapi-schema: Demonstrate suboptimal lexical errors qapi: Use quotes more consistently in frontend error messages qapi: Improve reporting of lexical errors qapi: Remove null from schema language qapi: Fix broken discriminator error messages qapi: Reject blank 'if' conditions in addition to empty ones qapi: Fix missing 'if' checks in struct, union, alternate 'data' qapi: Normalize 'if' in check_exprs(), like other sugar qapi: Simplify check_keys() qapi: Clean up around check_known_keys() qapi: Delete useless check_exprs() code for simple union kind qapi: Fix to .check() empty structs just once qapi: Fix excessive QAPISchemaEntity.check() recursion qapi: Assert .visit() and .check_clash() run only after .check() docs/devel/qapi-code-gen.txt | 4 +- scripts/qapi/common.py| 233 +- tests/Makefile.include| 9 +- .../alternate-branch-if-invalid.err | 1 + exit => alternate-branch-if-invalid.exit} | 0 .../alternate-branch-if-invalid.json | 3 + ...ta.out => alternate-branch-if-invalid.out} | 0 tests/qapi-schema/bad-if-list.err | 2 +- tests/qapi-schema/bad-if-list.json| 2 +- tests/qapi-schema/bad-type-int.err| 2 +- tests/qapi-schema/bad-type-int.json | 2 +- tests/qapi-schema/doc-missing-colon.err | 2 +- tests/qapi-schema/double-data.err | 1 - tests/qapi-schema/double-data.json| 2 - tests/qapi-schema/duplicate-key.err | 2 +- tests/qapi-schema/duplicate-key.json | 2 +- tests/qapi-schema/enum-int-member.err | 2 +- tests/qapi-schema/escape-outside-string.err | 1 + tests/qapi-schema/features-if-invalid.err | 1 + ...rmat-err.exit => features-if-invalid.exit} | 0 tests/qapi-schema/features-if-invalid.json| 4 + ...format-err.out => features-if-invalid.out} | 0 .../flat-union-discriminator-bad-name.err | 1 + .../flat-union-discriminator-bad-name.exit| 1 + .../flat-union-discriminator-bad-name.json| 11 + .../flat-union-discriminator-bad-name.out | 0 .../flat-union-invalid-discriminator.err | 2 +- .../flat-union-invalid-discriminator.json | 5 +- .../flat-union-invalid-if-discriminator.err | 2 +- .../flat-union-invalid-if-discriminator.json | 5 +- .../flat-union-optional-discriminator.err | 2 +- .../flat-union-optional-discriminator.json| 3 +- tests/qapi-schema/funny-char.err | 2 +- tests/qapi-schema/funny-word.err | 1 + tests/qapi-schema/funny-word.exit | 1 + tests/qapi-schema/funny-word.json | 1 + tests/qapi-schema/funny-word.out | 0 tests/qapi-schema/include-before-err.err | 2 +- tests/qapi-schema/include-format-err.err | 1 - tests/qapi-schema/include-format-err.json | 2 - tests/qapi-schema/include-nested-err.err | 2 +- tests/qapi-schema/leading-comma-list.err | 2 +- tests/qapi-schema/leading-comma-object.err| 2 +- tests/qapi-schema/missing-colon.err | 2 +- tests/qapi-schema/missing-comma-list.err | 2 +- tests/qapi-schema/missing-comma-object.err| 2 +- tests/qapi-schema/non-objects.err | 2 +- .../pragma-name-case-whitelist-crap.json | 2 +- tests/qapi-schema/pragma-unknown.err | 1 + tests/qapi-schema/pragma-unknown.exit | 1 + tests/qapi-schema/pragma-unknown.json | 1 + tests/qapi-schema/pragma-unknown.out | 0 tests/qapi-schema/quoted-structural-chars.err | 2 +- .../qapi-schema/struct-member-if-invalid.err | 1 + .../qapi-schema/struct-member-if-invalid.exit | 1 + .../qapi-schema/struct-member-if-invalid.json | 3 + .../qapi-schema/struct-member-if-invalid.out | 0 tests/qapi-schema/trailing-comma-list.err | 2 +- tests/qapi-schema/unclosed-list.err | 2 +- tests/qapi-schema/unclosed-object.err | 2 +- tests/qapi-schema/union-base-empty.err| 2 +- tests/qapi-schema/union-branch-if-invalid.err | 1 + .../qapi-schema/union-branch-if-invalid.exit | 1 + .../qapi-schema/union-branch-if-invalid.json | 6 + tests/qapi-schema/union-branch-if-invalid.out | 0 65 files changed, 202 insertions(+), 157 deletions(-) create mode 100644 tests/qapi-schema/alternate-branch-if-invalid.err rename
Re: [Qemu-devel] [PATCH] tests/tcg: add float_madds test to multiarch
On 9/13/19 9:49 AM, Alex Bennée wrote: > +/* must be built with -O2 to generate fused op */ > +r = a * b + c; I would prefer to use fmaf() or __builtin_fmaf() here. Although you'll need to link with -lm just in case the target doesn't support an instruction for fmaf and so the builtin expands to the standard library call. I also like Paul's suggestion to use hex float constants. r~
Re: [Qemu-devel] [PATCH v2] ppc: Add support for 'mffscrn', 'mffscrni' instructions
On 9/14/19 10:58 AM, Richard Henderson wrote:
> On 9/12/19 8:54 AM, Paul A. Clarke wrote:
>> +static void gen_helper_mffscrn(DisasContext *ctx, TCGv_i64 t1)
>> +{
>> +TCGv_i64 t0 = tcg_temp_new_i64();
>> +TCGv_i32 mask = tcg_const_i32(0x0001);
>> +
>> +gen_reset_fpstatus();
>> +tcg_gen_extu_tl_i64(t0, cpu_fpscr);
>> +tcg_gen_andi_i64(t0, t0, FP_MODE | FP_ENABLES);
>
> Missing the decimal rounding mode (DRN) field at 29:31.
>
> Otherwise,
Dang it, in the wrong order.
Reviewed-by: Richard Henderson
r~
Re: [Qemu-devel] [PATCH v2] ppc: Add support for 'mffscrn', 'mffscrni' instructions
On 9/12/19 8:54 AM, Paul A. Clarke wrote:
> +static void gen_helper_mffscrn(DisasContext *ctx, TCGv_i64 t1)
> +{
> +TCGv_i64 t0 = tcg_temp_new_i64();
> +TCGv_i32 mask = tcg_const_i32(0x0001);
> +
> +gen_reset_fpstatus();
> +tcg_gen_extu_tl_i64(t0, cpu_fpscr);
> +tcg_gen_andi_i64(t0, t0, FP_MODE | FP_ENABLES);
Missing the decimal rounding mode (DRN) field at 29:31.
Otherwise,
Re: [Qemu-devel] [RFC v3 3/3] ARM: KVM: Check KVM_CAP_ARM_IRQ_LINE_LAYOUT_2 for smp_cpus > 256
On 9/13/19 5:56 AM, Eric Auger wrote: > Host kernel within [4.18, 5.3] report an erroneous KVM_MAX_VCPUS=512 > for ARM. The actual capability to instantiate more than 256 vcpus > was fixed in 5.4 with the upgrade of the KVM_IRQ_LINE ABI to support > vcpu id encoded on 12 bits instead of 8 and a redistributor consuming > a single KVM IO device instead of 2. > > So let's check this capability when attempting to use more than 256 > vcpus within any ARM kvm accelerated machine. > > Signed-off-by: Eric Auger Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [RFC v3 2/3] intc/arm_gic: Support IRQ injection for more than 256 vpus
On 9/13/19 5:56 AM, Eric Auger wrote: > Host kernels that expose the KVM_CAP_ARM_IRQ_LINE_LAYOUT_2 capability > allow injection of interrupts along with vcpu ids larger than 255. > Let's encode the vpcu id on 12 bits according to the upgraded KVM_IRQ_LINE > ABI when needed. > > Given that we have two callsites that need to assemble > the value for kvm_set_irq(), a new helper routine, kvm_arm_set_irq > is introduced. > > Without that patch qemu exits with "kvm_set_irq: Invalid argument" > message. > > Signed-off-by: Eric Auger > Reported-by: Zenghui Yu Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [PATCH v2 03/17] RISC-V: support vector extension csr
On Wed, 11 Sep 2019 15:43:29 PDT (-0700), richard.hender...@linaro.org wrote:
On 9/11/19 2:25 AM, liuzhiwei wrote:
@@ -873,7 +925,12 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_FFLAGS] = { fs, read_fflags, write_fflags },
[CSR_FRM] = { fs, read_frm, write_frm },
[CSR_FCSR] ={ fs, read_fcsr,write_fcsr},
-
+/* Vector CSRs */
+[CSR_VSTART] = { any, read_vstart, write_vstart },
+[CSR_VXSAT] = { any, read_vxsat, write_vxsat },
+[CSR_VXRM] ={ any, read_vxrm, write_vxrm},
+[CSR_VL] = { any, read_vl},
+[CSR_VTYPE] = { any, read_vtype },
Is there really no MSTATUS bit to disable the vector unit,
as there is for the FPU? That seems like a defect in the
specification if true...
The privileged part of the V extension hasn't been written yet, which is part
of the reason this is a draft that we know will change. We're letting it into
QEMU so people can more easily prototype software, but won't be letting it into
Linux or GCC to avoid users depending on behavior that will change in the
future.
Re: [Qemu-devel] [PATCH v2 00/17] RISC-V: support vector extension
On Wed, 11 Sep 2019 00:00:56 PDT (-0700), aleksandar.m.m...@gmail.com wrote:
11.09.2019. 08.35, "liuzhiwei" је написао/ла:
Features:
* support specification riscv-v-spec-0.7.1(
https://content.riscv.org/wp-content/uploads/2019/06/17.40-Vector_RISCV-20190611-Vectors.pdf
).
Hi, Zhivei.
The linked document is a presentation, outlining general concepts of the
instruction set in question, which is certainly useful and nice to have,
but, for review process, we need *specifications* (especially given that
they are in draft phase, and therefore "moving target"). Please provide
such link.
Here's the V spec repository
https://github.com/riscv/riscv-v-spec
and the exact 0.7.1 specification PDF
https://github.com/riscv/riscv-v-spec/releases/download/0.7.1/riscv-v-spec-0.7.1.pdf
In RISC-V land this constitutes an official draft -- there's a whole process
for getting a specification ratified, but that isn't done for these draft
specifications. The RISC-V QEMU maintainers agree that we'll take
implementations of drafts as long as there's a concrete definition we can point
at, like this one.
I also noticed lack of commit messages, and was really disappointed by
that. It looks to me you did not honor in entirety our guidlines for
submitting patches.
Yours,
Aleksandar
* support basic vector extension.
* support Zvlsseg.
* support Zvamo.
* not support Zvediv as it is changing.
* fixed VLEN 128bit.
* fixed SLEN 128bit.
* ELEN support 8bit, 16bit, 32bit, 64bit.
Todo:
* support VLEN configure from qemu command line.
* move check code from execution-time to translation-time
Changelog:
V2
* use float16_compare{_quiet}
* only use GETPC() in outer most helper
* add ctx.ext_v Property
LIU Zhiwei (17):
RISC-V: add vfp field in CPURISCVState
RISC-V: turn on vector extension from command line by cfg.ext_v
Property
RISC-V: support vector extension csr
RISC-V: add vector extension configure instruction
RISC-V: add vector extension load and store instructions
RISC-V: add vector extension fault-only-first implementation
RISC-V: add vector extension atomic instructions
RISC-V: add vector extension integer instructions part1,
add/sub/adc/sbc
RISC-V: add vector extension integer instructions part2, bit/shift
RISC-V: add vector extension integer instructions part3, cmp/min/max
RISC-V: add vector extension integer instructions part4, mul/div/merge
RISC-V: add vector extension fixed point instructions
RISC-V: add vector extension float instruction part1, add/sub/mul/div
RISC-V: add vector extension float instructions part2,
sqrt/cmp/cvt/others
RISC-V: add vector extension reduction instructions
RISC-V: add vector extension mask instructions
RISC-V: add vector extension premutation instructions
linux-user/riscv/cpu_loop.c | 7 +
target/riscv/Makefile.objs | 2 +-
target/riscv/cpu.c | 6 +-
target/riscv/cpu.h |30 +
target/riscv/cpu_bits.h |15 +
target/riscv/cpu_helper.c | 7 +
target/riscv/csr.c |65 +-
target/riscv/helper.h | 358 +
target/riscv/insn32.decode | 373 +
target/riscv/insn_trans/trans_rvv.inc.c | 490 +
target/riscv/translate.c| 1 +
target/riscv/vector_helper.c| 25701
++
12 files changed, 27049 insertions(+), 6 deletions(-)
create mode 100644 target/riscv/insn_trans/trans_rvv.inc.c
create mode 100644 target/riscv/vector_helper.c
--
2.7.4
