Re: [Qemu-devel] Qemu-devel Digest, Vol 180, Issue 211

[Aishwarya Kadlag]

I want to contribute to QEMU as a developer. I have worked on ARM Cortex
board and implemented its peripherals. I have gone through QEMU and
micro_bit board documentation. But I am not able to understand what I am
exactly supposed to do to complete any bite sized tasks. Please help me
with this.

On Sat, Mar 10, 2018 at 4:13 AM,  wrote:

> Send Qemu-devel mailing list submissions to
> qemu-devel@nongnu.org
>
> To subscribe or unsubscribe via the World Wide Web, visit
> https://lists.nongnu.org/mailman/listinfo/qemu-devel
> or, via email, send a message with subject or body 'help' to
> qemu-devel-requ...@nongnu.org
>
> You can reach the person managing the list at
> qemu-devel-ow...@nongnu.org
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of Qemu-devel digest..."
>
>
> Today's Topics:
>
>1. [PATCH v2] fsl-imx6: Swap Ethernet interrupt defines
>   (Guenter Roeck)
>2. Re: Apparently fpu/softfloat.c:1374 is reachable (Emilio G. Cota)
>3. Re: [PATCH 3/5] nbd/server: fix: check client->closing before
>   reply sending (Eric Blake)
>4. Re: [PATCH 4/6] luks: Turn invalid assertion into check
>   (Kevin Wolf)
>5. Re: [PATCH 3/7] qcow: Support .bdrv_co_create (Eric Blake)
>6. [Bug 1726733] Re: ?qemu-img info replication:? causes
>   segfault (Fabiano Rosas)
>7. Re: [PATCH 4/7] qed: Support .bdrv_co_create (Eric Blake)
>8. Re: [PATCH] qemu-doc: Rework the network options chapter to
>   make "-net" less prominent (Thomas Huth)
>9. Re: [PATCH 4/5] nbd/server: refactor nbd_trip: cmd_read and
>   generic reply (Eric Blake)
>   10. Re: [PATCH 5/5] nbd/server: refactor nbd_trip: split out
>   nbd_handle_request (Eric Blake)
>   11. Re: Apparently fpu/softfloat.c:1374 is reachable (Palmer Dabbelt)
>   12. Re: [PATCH] qemu-doc: Rework the network options chapter to
>   make "-net" less prominent (Eric Blake)
>   13. Re: [PATCH v2 1/3] scripts/update-linux-headers: add
>   ethtool.h and update to 4.16.0-rc4 (Michael S. Tsirkin)
>
>
> --
>
> Message: 1
> Date: Fri,  9 Mar 2018 13:47:03 -0800
> From: Guenter Roeck 
> To: Peter Maydell 
> Cc: Peter Chubb , Jason Wang
> ,  qemu-...@nongnu.org, qemu-devel@nongnu.org
> ,
> Andrey Smirnov , Chris Healy
> ,Bill Paul , Guenter
> Roeck
> 
> Subject: [Qemu-devel] [PATCH v2] fsl-imx6: Swap Ethernet interrupt
> defines
> Message-ID: <1520632024-31535-1-git-send-email-li...@roeck-us.net>
>
> The sabrelite machine model used by qemu-system-arm is based on the
> Freescale/NXP i.MX6Q processor. This SoC has an on-board ethernet
> controller which is supported in QEMU using the imx_fec.c module
> (actually called imx.enet for this model.)
>
> The include/hw/arm/fsm-imx6.h file defines the interrupt vectors for the
> imx.enet device like this:
>
>  #define FSL_IMX6_ENET_MAC_1588_IRQ 118
>  #define FSL_IMX6_ENET_MAC_IRQ 119
>
> According to https://www.nxp.com/docs/en/reference-manual/IMX6DQRM.pdf,
> page 225, in Table 3-1. ARM Cortex A9 domain interrupt summary,
> interrupts are as follows.
>
> 150 ENET MAC 0 IRQ
> 151 ENET MAC 0 1588 Timer interrupt
>
> where
>
> 150 - 32 == 118
> 151 - 32 == 119
>
> In other words, the vector definitions in the fsl-imx6.h file are reversed.
>
> Fixing the interrupts alone causes problems with older Linux kernels:
> The Ethernet interface will fail to probe with Linux v4.9 and earlier.
> Linux v4.1 and earlier will crash due to a bug in Ethernet driver probe
> error handling. This is a Linux kernel problem, not a qemu problem:
> the Linux kernel only worked by accident since it requested both
> interrupts.
>
> For backward compatibility, generate the Ethernet interrupt on both
> interrupt
> lines. This was shown to work from all Linux kernel releases starting with
> v3.16.
>
> Link: https://bugs.launchpad.net/qemu/+bug/1753309
> Signed-off-by: Guenter Roeck 
> ---
> v2: Generate Ethernet interrupts on both interrupt lines
>
>  hw/net/imx_fec.c  | 7 ++-
>  include/hw/arm/fsl-imx6.h | 4 ++--
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
> index 9506f9b..d3ae7db 100644
> --- a/hw/net/imx_fec.c
> +++ b/hw/net/imx_fec.c
> @@ -417,7 +417,12 @@ static void imx_enet_write_bd(IMXENETBufDesc *bd,
> dma_addr_t addr)
>
>  static void imx_eth_update(IMXFECState *s)
>  {
> -if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] & ENET_INT_TS_TIMER) {
> +/*
> + * Generate ENET_INT_MAC interrrupts on both interrupt lines for
> + * backward compatibility with Linux kernel versions 4.9 and older.
> + */
> +if 

Re: [Qemu-devel] [PATCH v8 19/23] SiFive RISC-V UART Device

[Michael Clark]

On Sat, Mar 10, 2018 at 1:39 AM, Philippe Mathieu-Daudé 
wrote:

> On 03/02/2018 02:51 PM, Michael Clark wrote:
> > QEMU model of the UART on the SiFive E300 and U500 series SOCs.
> > BBL supports the SiFive UART for early console access via the SBI
> > (Supervisor Binary Interface) and the linux kernel SBI console.
> >
> > The SiFive UART implements the pre qom legacy interface consistent
> > with the 16550a UART in 'hw/char/serial.c'.
> >
> > Acked-by: Richard Henderson 
> > Signed-off-by: Stefan O'Rear 
> > Signed-off-by: Palmer Dabbelt 
> > Signed-off-by: Michael Clark 
> > ---
> >  hw/riscv/sifive_uart.c | 176 ++
> +++
> >  include/hw/riscv/sifive_uart.h |  71 +
> >  2 files changed, 247 insertions(+)
> >  create mode 100644 hw/riscv/sifive_uart.c
> >  create mode 100644 include/hw/riscv/sifive_uart.h
> >
> > diff --git a/hw/riscv/sifive_uart.c b/hw/riscv/sifive_uart.c
> > new file mode 100644
> > index 000..b0c3798
> > --- /dev/null
> > +++ b/hw/riscv/sifive_uart.c
> > @@ -0,0 +1,176 @@
> > +/*
> > + * QEMU model of the UART on the SiFive E300 and U500 series SOCs.
> > + *
> > + * Copyright (c) 2016 Stefan O'Rear
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but
> WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> along with
> > + * this program.  If not, see .
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qapi/error.h"
> > +#include "hw/sysbus.h"
> > +#include "chardev/char.h"
> > +#include "chardev/char-fe.h"
> > +#include "target/riscv/cpu.h"
> > +#include "hw/riscv/sifive_uart.h"
> > +
> > +/*
> > + * Not yet implemented:
> > + *
> > + * Transmit FIFO using "qemu/fifo8.h"
> > + * SIFIVE_UART_IE_TXWM interrupts
> > + * SIFIVE_UART_IE_RXWM interrupts must honor fifo watermark
> > + * Rx FIFO watermark interrupt trigger threshold
> > + * Tx FIFO watermark interrupt trigger threshold.
> > + */
> > +
> > +static void update_irq(SiFiveUARTState *s)
> > +{
> > +int cond = 0;
> > +if ((s->ie & SIFIVE_UART_IE_RXWM) && s->rx_fifo_len) {
> > +cond = 1;
> > +}
> > +if (cond) {
> > +qemu_irq_raise(s->irq);
> > +} else {
> > +qemu_irq_lower(s->irq);
> > +}
>
> or qemu_set_irq(s->irq, cond);
>
> > +}
> > +
> > +static uint64_t
> > +uart_read(void *opaque, hwaddr addr, unsigned int size)
> > +{
> > +SiFiveUARTState *s = opaque;
> > +unsigned char r;
> > +switch (addr) {
> > +case SIFIVE_UART_RXFIFO:
> > +if (s->rx_fifo_len) {
> > +r = s->rx_fifo[0];
> > +memmove(s->rx_fifo, s->rx_fifo + 1, s->rx_fifo_len - 1);
> > +s->rx_fifo_len--;
> > +qemu_chr_fe_accept_input(>chr);
> > +update_irq(s);
> > +return r;
> > +}
> > +return 0x8000;
>
> Can you add a #define for this bit?
>
> > +
> > +case SIFIVE_UART_TXFIFO:
> > +return 0; /* Should check tx fifo */
> > +case SIFIVE_UART_IE:
> > +return s->ie;
> > +case SIFIVE_UART_IP:
> > +return s->rx_fifo_len ? SIFIVE_UART_IP_RXWM : 0;
> > +case SIFIVE_UART_TXCTRL:
> > +return s->txctrl;
> > +case SIFIVE_UART_RXCTRL:
> > +return s->rxctrl;
> > +case SIFIVE_UART_DIV:
> > +return s->div;
> > +}
> > +
> > +hw_error("%s: bad read: addr=0x%x\n",
> > +__func__, (int)addr);
>
> qemu_log(GUEST_ERROR)?
>
> > +return 0;
> > +}
> > +
> > +static void
> > +uart_write(void *opaque, hwaddr addr,
> > +   uint64_t val64, unsigned int size)
> > +{
> > +SiFiveUARTState *s = opaque;
> > +uint32_t value = val64;
> > +unsigned char ch = value;
> > +
> > +switch (addr) {
> > +case SIFIVE_UART_TXFIFO:
> > +qemu_chr_fe_write(>chr, , 1);
> > +return;
> > +case SIFIVE_UART_IE:
> > +s->ie = val64;
> > +update_irq(s);
> > +return;
> > +case SIFIVE_UART_TXCTRL:
> > +s->txctrl = val64;
> > +return;
> > +case SIFIVE_UART_RXCTRL:
> > +s->rxctrl = val64;
> > +return;
> > +case SIFIVE_UART_DIV:
> > +s->div = val64;
> > +return;
> > +}
> > +hw_error("%s: bad write: addr=0x%x v=0x%x\n",
> > +__func__, (int)addr, (int)value);
>
> Ditto.
>
> > +}
> > +
> > +static const MemoryRegionOps uart_ops = {
> > +.read = uart_read,

Re: [Qemu-devel] [PATCH v8 17/23] SiFive RISC-V Test Finisher

[Michael Clark]

On Sat, Mar 10, 2018 at 12:57 AM, Philippe Mathieu-Daudé 
wrote:

> On 03/02/2018 02:51 PM, Michael Clark wrote:
> > Test finisher memory mapped device used to exit simulation.
> >
> > Acked-by: Richard Henderson 
> > Signed-off-by: Palmer Dabbelt 
> > Signed-off-by: Michael Clark 
> > ---
> >  hw/riscv/sifive_test.c | 93 ++
> 
> >  include/hw/riscv/sifive_test.h | 42 +++
> >  2 files changed, 135 insertions(+)
> >  create mode 100644 hw/riscv/sifive_test.c
> >  create mode 100644 include/hw/riscv/sifive_test.h
> >
> > diff --git a/hw/riscv/sifive_test.c b/hw/riscv/sifive_test.c
> > new file mode 100644
> > index 000..8abd2cd
> > --- /dev/null
> > +++ b/hw/riscv/sifive_test.c
> > @@ -0,0 +1,93 @@
> > +/*
> > + * QEMU SiFive Test Finisher
> > + *
> > + * Copyright (c) 2018 SiFive, Inc.
> > + *
> > + * Test finisher memory mapped device used to exit simulation
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but
> WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> along with
> > + * this program.  If not, see .
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/sysbus.h"
> > +#include "target/riscv/cpu.h"
> > +#include "hw/riscv/sifive_test.h"
> > +
> > +static uint64_t sifive_test_read(void *opaque, hwaddr addr, unsigned
> int size)
> > +{
> > +return 0;
> > +}
> > +
> > +static void sifive_test_write(void *opaque, hwaddr addr,
> > +   uint64_t val64, unsigned int size)
> > +{
> > +if (addr == 0) {
> > +int status = val64 & 0x;
> > +int code = (val64 >> 16) & 0x;
> > +switch (status) {
> > +case FINISHER_FAIL:
> > +exit(code);
> > +case FINISHER_PASS:
> > +exit(0);
> > +default:
> > +break;
> > +}
> > +}
> > +hw_error("%s: write: addr=0x%x val=0x%016" PRIx64 "\n",
> > +__func__, (int)addr, val64);
> > +}
> > +
> > +static const MemoryRegionOps sifive_test_ops = {
> > +.read = sifive_test_read,
> > +.write = sifive_test_write,
> > +.endianness = DEVICE_NATIVE_ENDIAN,
> > +.valid = {
> > +.min_access_size = 4,
> > +.max_access_size = 4
> > +}
> > +};
> > +
> > +static void sifive_test_init(Object *obj)
> > +{
> > +SiFiveTestState *s = SIFIVE_TEST(obj);
> > +
> > +memory_region_init_io(>mmio, obj, _test_ops, s,
> > +  TYPE_SIFIVE_TEST, 0x1000);
>
> 0x1000? 0x8 is enough :)
>
>
In this case we were following the aperture size of SiFive's test finisher.
See the device tree here. 0x4000 is the offset, 0x1000 is the length.

L20: teststatus@4000 {
compatible = "sifive,test0";
reg = <0x0 0x4000 0x0 0x1000>;
reg-names = "control";
};

I can change it to 8 and it will still work. There should probably be a
SIFIVE_TEST_APERTURE_SIZE constant (perhaps something a little shorter).

BTW We used the test finisher because the firmware we have already knows
how to use it to shutdown a device in the SBI (Supervisor Binary Interface)
sbi_shutdown() method.

Apparently there is a generic device-tree mechanism of passing a reference
to a GPIO node for shutdown and a GPIO node for reset, that is recognised
by Linux Kernel device-tree code. When we have GPIOs we may change the
method to use the standard Linux mechanism.

> +sysbus_init_mmio(SYS_BUS_DEVICE(obj), >mmio);
> > +}
> > +
> > +static const TypeInfo sifive_test_info = {
> > +.name  = TYPE_SIFIVE_TEST,
> > +.parent= TYPE_SYS_BUS_DEVICE,
> > +.instance_size = sizeof(SiFiveTestState),
> > +.instance_init = sifive_test_init,
> > +};
> > +
> > +static void sifive_test_register_types(void)
> > +{
> > +type_register_static(_test_info);
> > +}
> > +
> > +type_init(sifive_test_register_types)
> > +
> > +
> > +/*
> > + * Create Test device.
> > + */
> > +DeviceState *sifive_test_create(hwaddr addr)
> > +{
> > +DeviceState *dev = qdev_create(NULL, TYPE_SIFIVE_TEST);
> > +qdev_init_nofail(dev);
> > +sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
> > +return dev;
> > +}
> > diff --git a/include/hw/riscv/sifive_test.h b/include/hw/riscv/sifive_
> test.h
> > new file mode 100644
> > index 000..71d4c9f
> > --- /dev/null
> > +++ b/include/hw/riscv/sifive_test.h
> > @@ -0,0 +1,42 @@
> > +/*
> > + * QEMU Test Finisher interface
> > + *
> > + * Copyright (c) 2018 

Re: [Qemu-devel] [PATCH v2] target-i386: add KVM_HINTS_DEDICATED performance hint

[Wanpeng Li]

2018-03-09 22:16 GMT+08:00 Eduardo Habkost :
> On Fri, Feb 09, 2018 at 06:15:25AM -0800, Wanpeng Li wrote:
>> From: Wanpeng Li 
>>
>> Add KVM_HINTS_DEDICATED performance hint, guest checks this feature bit
>> to determine if they run on dedicated vCPUs, allowing optimizations such
>> as usage of qspinlocks.
>>
>> Cc: Paolo Bonzini 
>> Cc: Radim Krčmář 
>> Cc: Eduardo Habkost 
>> Signed-off-by: Wanpeng Li 
>> ---
>> v1 -> v2:
>>  * add a new feature word
>>
>>  target/i386/cpu.c | 14 ++
>>  target/i386/cpu.h |  3 +++
>>  target/i386/kvm.c |  4 
>>  3 files changed, 21 insertions(+)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index d70954b..e2974ad 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -358,6 +358,20 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] 
>> = {
>>  .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX,
>>  .tcg_features = TCG_KVM_FEATURES,
>>  },
>> +[FEAT_KVM_HINTS] = {
>> +.feat_names = {
>> +"hint-dedicated", NULL, NULL, NULL,
>
> I suggest naming it "kvm-hint-dedicated", to indicate it's
> KVM-specific.
>
> If there are no objections, I can rename it while applying.

Looks good to me, thanks Eduardo.

Regards,
Wanpeng Li

>
> With the rename:
>
> Reviewed-by: Eduardo Habkost 
>
>
>> +NULL, NULL, NULL, NULL,
>> +NULL, NULL, NULL, NULL,
>> +NULL, NULL, NULL, NULL,
>> +NULL, NULL, NULL, NULL,
>> +NULL, NULL, NULL, NULL,
>> +NULL, NULL, NULL, NULL,
>> +NULL, NULL, NULL, NULL,
>> +},
>> +.cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EDX,
>> +.tcg_features = TCG_KVM_FEATURES,
>> +},
>>  [FEAT_HYPERV_EAX] = {
>>  .feat_names = {
>>  NULL /* hv_msr_vp_runtime_access */, NULL /* 
>> hv_msr_time_refcount_access */,
>> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
>> index f91e37d..9f73692 100644
>> --- a/target/i386/cpu.h
>> +++ b/target/i386/cpu.h
>> @@ -475,6 +475,7 @@ typedef enum FeatureWord {
>>  FEAT_8000_0008_EBX, /* CPUID[8000_0008].EBX */
>>  FEAT_C000_0001_EDX, /* CPUID[C000_0001].EDX */
>>  FEAT_KVM,   /* CPUID[4000_0001].EAX (KVM_CPUID_FEATURES) */
>> +FEAT_KVM_HINTS, /* CPUID[4000_0001].EDX */
>>  FEAT_HYPERV_EAX,/* CPUID[4000_0003].EAX */
>>  FEAT_HYPERV_EBX,/* CPUID[4000_0003].EBX */
>>  FEAT_HYPERV_EDX,/* CPUID[4000_0003].EDX */
>> @@ -670,6 +671,8 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
>>  #define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply 
>> Accumulation Single Precision */
>>  #define CPUID_7_0_EDX_SPEC_CTRL (1U << 26) /* Speculation Control */
>>
>> +#define KVM_HINTS_DEDICATED (1U << 0)
>> +
>>  #define CPUID_8000_0008_EBX_IBPB(1U << 12) /* Indirect Branch 
>> Prediction Barrier */
>>
>>  #define CPUID_XSAVE_XSAVEOPT   (1U << 0)
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index ad4b159..44ee524 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -383,6 +383,9 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, 
>> uint32_t function,
>>  if (!kvm_irqchip_in_kernel()) {
>>  ret &= ~(1U << KVM_FEATURE_PV_UNHALT);
>>  }
>> +} else if (function == KVM_CPUID_FEATURES && reg == R_EDX) {
>> +ret |= KVM_HINTS_DEDICATED;
>> +found = 1;
>>  }
>>
>>  /* fallback for older kernels */
>> @@ -801,6 +804,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>  c = _data.entries[cpuid_i++];
>>  c->function = KVM_CPUID_FEATURES | kvm_base;
>>  c->eax = env->features[FEAT_KVM];
>> +c->edx = env->features[FEAT_KVM_HINTS];
>>  }
>>
>>  cpu_x86_cpuid(env, 0, 0, , , , );
>> --
>> 2.7.4
>>
>
> --
> Eduardo

Re: [Qemu-devel] [PATCH] fsl-imx6: Swap Ethernet interrupt defines

[Bill Paul]

Of all the gin joints in all the towns in all the world, Guenter Roeck had to 
walk into mine at 13:38 on Friday 09 March 2018 and say:

[...]
> > > > Do I have that right?
> > > 
> > > Pretty much.
> > 
> > There may be a 4th option.
> > 
> > Since older kernels work because they were looking at vector 151, you
> > could patch the imx_fec.c module so that when it signals an event for
> > vector 150, it also signals vector 151 too. This would keep newer
> > versions of QEMU "bug- compatible" with older versions while also fixing
> > support for newer Linux kernels and other guests (e.g. VxWorks) that
> > follow the hardware spec correctly.
> > 
> > I think this would require only a small modification to this function:
> > 
> > static void imx_eth_update(IMXFECState *s)
> > {
> > 
> > if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] & ENET_INT_TS_TIMER) {
> > 
> > qemu_set_irq(s->irq[1], 1);
> > 
> > } else {
> > 
> > qemu_set_irq(s->irq[1], 0);
> > 
> > }
> > 
> > if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] & ENET_INT_MAC) {
> > 
> > qemu_set_irq(s->irq[0], 1);
> > 
> > } else {
> > 
> > qemu_set_irq(s->irq[0], 0);
> > 
> > }
> > 
> > }
> > 
> > (For ENET_INT_MAC events, just signal both s->irq[0] and s->irq[1]).
> 
> Now this is an excellent idea.
> 
> > This of course means signaling spurious events on vector 151, but you're
> > doing that now anyway. :)
> 
> Actually, it doesn't. It looks like the first interrupt is handled,
> resetting the interrupt status, and the second interrupt is never even
> executed. I tested this with all kernel releases back to v3.16.

I just did a quick test with your patch and I can confirm that VxWorks 7 also 
works correctly.

-Bill

> I'll send out patch v2 with this change and let Peter decide which version
> to apply.
> 
> Thanks,
> Guenter
-- 
=
-Bill Paul(510) 749-2329 | Senior Member of Technical Staff,
 wp...@windriver.com | Master of Unix-Fu - Wind River Systems
=
   "I put a dollar in a change machine. Nothing changed." - George Carlin
=

Re: [Qemu-devel] [PATCH v2 1/3] scripts/update-linux-headers: add ethtool.h and update to 4.16.0-rc4

[Michael S. Tsirkin]

On Wed, Mar 07, 2018 at 10:25:39PM -0500, Jason Baron wrote:
> A subsequent patch to add support for setting linkspeed/duplex in
> virtio-net, requires a few definitions from ethtool.h, which ends up
> pulling in kernel.h and sysinfo.h as well.
> 
> Signed-off-by: Jason Baron 
> Cc: "Michael S. Tsirkin" 
> Cc: Jason Wang 
> Cc: virtio-...@lists.oasis-open.org

I will drop unrelated stuff and just update files we need.

> ---
>  include/standard-headers/linux/ethtool.h | 1821 
> ++
>  include/standard-headers/linux/input.h   |4 +-
>  include/standard-headers/linux/kernel.h  |   15 +
>  include/standard-headers/linux/sysinfo.h |   25 +
>  linux-headers/asm-x86/kvm_para.h |1 +
>  linux-headers/linux/kvm.h|2 +
>  scripts/update-linux-headers.sh  |   11 +-
>  7 files changed, 1876 insertions(+), 3 deletions(-)
>  create mode 100644 include/standard-headers/linux/ethtool.h
>  create mode 100644 include/standard-headers/linux/kernel.h
>  create mode 100644 include/standard-headers/linux/sysinfo.h
> 
> diff --git a/include/standard-headers/linux/ethtool.h 
> b/include/standard-headers/linux/ethtool.h
> new file mode 100644
> index 000..94aacb7
> --- /dev/null
> +++ b/include/standard-headers/linux/ethtool.h
> @@ -0,0 +1,1821 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * ethtool.h: Defines for Linux ethtool.
> + *
> + * Copyright (C) 1998 David S. Miller (da...@redhat.com)
> + * Copyright 2001 Jeff Garzik 
> + * Portions Copyright 2001 Sun Microsystems (thoc...@sun.com)
> + * Portions Copyright 2002 Intel (eli.kuperm...@intel.com,
> + *christopher.le...@intel.com,
> + *scott.feld...@intel.com)
> + * Portions Copyright (C) Sun Microsystems 2008
> + */
> +
> +#ifndef _LINUX_ETHTOOL_H
> +#define _LINUX_ETHTOOL_H
> +
> +#include "net/eth.h"
> +
> +#include "standard-headers/linux/kernel.h"
> +#include "standard-headers/linux/types.h"
> +#include "standard-headers/linux/if_ether.h"
> +
> +#include  /* for INT_MAX */
> +
> +/* All structures exposed to userland should be defined such that they
> + * have the same layout for 32-bit and 64-bit userland.
> + */
> +
> +/**
> + * struct ethtool_cmd - DEPRECATED, link control and status
> + * This structure is DEPRECATED, please use struct ethtool_link_settings.
> + * @cmd: Command number = %ETHTOOL_GSET or %ETHTOOL_SSET
> + * @supported: Bitmask of %SUPPORTED_* flags for the link modes,
> + *   physical connectors and other link features for which the
> + *   interface supports autonegotiation or auto-detection.
> + *   Read-only.
> + * @advertising: Bitmask of %ADVERTISED_* flags for the link modes,
> + *   physical connectors and other link features that are
> + *   advertised through autonegotiation or enabled for
> + *   auto-detection.
> + * @speed: Low bits of the speed, 1Mb units, 0 to INT_MAX or SPEED_UNKNOWN
> + * @duplex: Duplex mode; one of %DUPLEX_*
> + * @port: Physical connector type; one of %PORT_*
> + * @phy_address: MDIO address of PHY (transceiver); 0 or 255 if not
> + *   applicable.  For clause 45 PHYs this is the PRTAD.
> + * @transceiver: Historically used to distinguish different possible
> + *   PHY types, but not in a consistent way.  Deprecated.
> + * @autoneg: Enable/disable autonegotiation and auto-detection;
> + *   either %AUTONEG_DISABLE or %AUTONEG_ENABLE
> + * @mdio_support: Bitmask of %ETH_MDIO_SUPPORTS_* flags for the MDIO
> + *   protocols supported by the interface; 0 if unknown.
> + *   Read-only.
> + * @maxtxpkt: Historically used to report TX IRQ coalescing; now
> + *   obsoleted by  ethtool_coalesce.  Read-only; deprecated.
> + * @maxrxpkt: Historically used to report RX IRQ coalescing; now
> + *   obsoleted by  ethtool_coalesce.  Read-only; deprecated.
> + * @speed_hi: High bits of the speed, 1Mb units, 0 to INT_MAX or 
> SPEED_UNKNOWN
> + * @eth_tp_mdix: Ethernet twisted-pair MDI(-X) status; one of
> + *   %ETH_TP_MDI_*.  If the status is unknown or not applicable, the
> + *   value will be %ETH_TP_MDI_INVALID.  Read-only.
> + * @eth_tp_mdix_ctrl: Ethernet twisted pair MDI(-X) control; one of
> + *   %ETH_TP_MDI_*.  If MDI(-X) control is not implemented, reads
> + *   yield %ETH_TP_MDI_INVALID and writes may be ignored or rejected.
> + *   When written successfully, the link should be renegotiated if
> + *   necessary.
> + * @lp_advertising: Bitmask of %ADVERTISED_* flags for the link modes
> + *   and other link features that the link partner advertised
> + *   through autonegotiation; 0 if unknown or not applicable.
> + *   Read-only.
> + *
> + * The link speed in Mbps is split between @speed and @speed_hi.  Use
> + * the ethtool_cmd_speed() and ethtool_cmd_speed_set() functions to
> + * access it.
> + *
> + * If autonegotiation is disabled, the speed and @duplex represent the
> + * 

Re: [Qemu-devel] [PATCH] qemu-doc: Rework the network options chapter to make "-net" less prominent

[Eric Blake]

On 03/09/2018 04:07 PM, Thomas Huth wrote:


... but looks like I even got it wrong - it should be "--device e1000",
without "=". Will fix it.


Really?  As I understand it, both long-opt spellings work ('--long=opt'
as one arg, and '--long' 'opt' as two args).  So the only reason to drop
'=' would be consistency with other examples.


Well, that would be true if we used getopt_long_only().  But we don't 
(we stupidly rolled our own command line parser, meaning it has quirks 
that are different from everything else that uses the standardized parser).




$ qemu-system-x86_64 --device=e1000
qemu-system-x86_64: --device=e1000: invalid option

Does at least not work for me.


Maybe our QemuOpts parser should be taught to make it work.  But that's 
orthogonal, so you are right that for now we just always document things 
as the two-arg form, since the one-arg form with '=' isn't implemented.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable

[Palmer Dabbelt]

On Fri, 09 Mar 2018 13:49:57 PST (-0800), c...@braap.org wrote:

On Fri, Mar 09, 2018 at 11:34:56 +, Michael Clark wrote:
Isn't Cc'ing riscv-patches an obvious use case for using the --cc flag?
(BTW You can add as many --cc's as you want, and these apply to all patches
in a series.)


FWIW, that's what I do with my patches.  I generally only add CC lines to the
patches if the address won't be found by the normal means (ie,
scripts/get_maintainer.pl) -- for example, maybe someone helped out a lot with
the patch but hasn't provided a reviewed-by yet.  Everyone else should come out
of the script, which makes it easy to avoid forgetting :).

Of course, that's all for my Linux stuff.  While I haven't really submitted any
QEMU patches, the flow seems very similar to Linux.  The GNU flow is a bit
different, but the git-send-email flow is very similar (without
get_maintainers).

Re: [Qemu-devel] [PATCH 5/5] nbd/server: refactor nbd_trip: split out nbd_handle_request

[Eric Blake]

On 03/08/2018 12:46 PM, Vladimir Sementsov-Ogievskiy wrote:

Split out request handling logic.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  nbd/server.c | 129 +++
  1 file changed, 67 insertions(+), 62 deletions(-)




+
+switch (request->type) {
+case NBD_CMD_READ:
+return nbd_do_cmd_read(client, request, data, errp);
+
+case NBD_CMD_WRITE:
+flags = 0;
+if (request->flags & NBD_CMD_FLAG_FUA) {
+flags |= BDRV_REQ_FUA;
+}
+ret = blk_pwrite(exp->blk, request->from + exp->dev_offset,
+ data, request->len, flags);
+
+return nbd_send_generic_reply(client, request->handle, ret,
+  "writing to file failed", errp);
+case NBD_CMD_WRITE_ZEROES:


Inconsistent spacing between return and the next case label.

But switching whitespace is trivial, so

Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 4/5] nbd/server: refactor nbd_trip: cmd_read and generic reply

[Eric Blake]

On 03/08/2018 12:46 PM, Vladimir Sementsov-Ogievskiy wrote:

nbd_trip has difficult logic of sending reply: it tries to use one
code path for all replies. It is ok for simple replies, but is not
comfortable for structured replies. Also, two types of error (and
corresponding message in local_err) - fatal (leading to disconnect)
and not-fatal (just to be sent to the client) are difficult to follow.

To make things a bit clearer, the following is done:
  - split CMD_READ logic to separate function. It is the most difficult
command for now, and it is definitely cramped inside nbd_trip. Also,
it is difficult to follow CMD_READ logic, shared between
"case NBD_CMD_READ" and "if"s under "reply:" label.


Yay - I already admitted when adding sparse read replies that splitting 
the logic was weird.



  - create separate helper function nbd_send_generic_reply() and use it
both in new nbd_do_cmd_read and for other command in nbd_trip instead


s/command/commands/


of common code-path under "reply:" label in nbd_trip. The helper
supports error message, so logic with local_err in nbd_trip is


s/supports/supports an/


simplified.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  nbd/server.c | 164 ---
  1 file changed, 88 insertions(+), 76 deletions(-)


Gains a few lines, but I think the end result is more legible.



diff --git a/nbd/server.c b/nbd/server.c
index 97b45a21fa..a2f5f73d52 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1520,6 +1520,70 @@ static int nbd_co_receive_request(NBDRequestData *req, 
NBDRequest *request,
  return 0;
  }
  
+/* Send simple reply without a payload or structured error


s/payload or/payload, or a /


+ * @error_msg is ignored if @ret >= 0 */


Maybe mention the return value (0 if connection is still live, -errno on 
failure to communicate to client)



+static coroutine_fn int nbd_send_generic_reply(NBDClient *client,
+   uint64_t handle,
+   int ret,
+   const char *error_msg,
+   Error **errp)
+{
+if (client->structured_reply && ret < 0) {
+return nbd_co_send_structured_error(client, handle, -ret, error_msg,
+errp);
+} else {
+return nbd_co_send_simple_reply(client, handle, ret < 0 ? -ret : 0,
+NULL, 0, errp);
+}
+}
+
+/* Handle NBD_CMD_READ request.
+ * Return -errno if sending fails. Other errors are reported directly to the
+ * client as an error reply. */
+static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request,
+uint8_t *data, Error **errp)
+{



+
  /* Owns a reference to the NBDClient passed as opaque.  */
  static coroutine_fn void nbd_trip(void *opaque)
  {
@@ -1529,7 +1593,6 @@ static coroutine_fn void nbd_trip(void *opaque)
  NBDRequest request = { 0 };/* GCC thinks it can be used uninitialized 
*/
  int ret;
  int flags;
-int reply_data_len = 0;
  Error *local_err = NULL;
  char *msg = NULL;
  
@@ -1556,39 +1619,21 @@ static coroutine_fn void nbd_trip(void *opaque)

  }
  
  if (ret < 0) {

-goto reply;
+/* It's not a -EIO, so, accordingly to nbd_co_receive_request()


It wasn't -EIO, so according to nbd_co_receive_request()


+ * semantics, we should return the error to the client. */
+Error *export_err = local_err;
+
+local_err = NULL;
+ret = nbd_send_generic_reply(client, request.handle, -EINVAL,
+ error_get_pretty(export_err), _err);
+error_free(export_err);
+
+goto replied;
  }
  
  switch (request.type) {

  case NBD_CMD_READ:



-reply_data_len = request.len;
+ret = nbd_do_cmd_read(client, , req->data, _err);
  
  break;


We could also collapse the empty line before break.


  case NBD_CMD_WRITE:
@@ -1598,9 +1643,8 @@ static coroutine_fn void nbd_trip(void *opaque)
  }
  ret = blk_pwrite(exp->blk, request.from + exp->dev_offset,
   req->data, request.len, flags);
-if (ret < 0) {
-error_setg_errno(_err, -ret, "writing to file failed");
-}
+ret = nbd_send_generic_reply(client, request.handle, ret,
+ "writing to file failed", _err);


I like how this works out.

  
  break;

  default:
-error_setg(_err, "invalid request type (%" PRIu32 ") received",
-   request.type);
-ret = -EINVAL;
-}
-
-reply:
-if (local_err) {
-/* If we get here, local_err was not a fatal error, and should be sent
- * to the client. */
-assert(ret < 0);
-msg = 

Re: [Qemu-devel] [PATCH] qemu-doc: Rework the network options chapter to make "-net" less prominent

[Thomas Huth]

On 09.03.2018 20:00, Eric Blake wrote:
> On 03/09/2018 11:41 AM, Thomas Huth wrote:
>> On 09.03.2018 15:36, Eric Blake wrote:
>>> On 03/09/2018 12:13 AM, Thomas Huth wrote:
 "-net" is clearly a legacy option. Yet we still use it in almost all
 examples in the qemu documentation, and many other spots in the network
 chapter. We should make it less prominent that users are not lured into
 using it so often anymore. So instead of starting the network chapter
 with
 "-net nic" and documenting "-net " below "-netdev "
 everywhere, all the "-net" related documentation is now moved to the
 end
 of the chapter. The new "--nic" option is moved to the beginning of the
 chapter instead, with a new example that should demonstrate how "--nic"
 can be used to shortcut "--device" with "--netdev".
 And the examples in this chapter are changed to use the "--device" and
 "--netdev" options or "--nic" instead of "-net nic -net ".

> 
 +@example
 +qemu-system-i386 --netdev user,id=n1,ipv6=off
 --device=e1000,netdev=n1,mac=52:54:98:76:54:32
 +qemu-system-i386 --nic user,ipv6=off,model=e1000,mac=52:54:98:76:54:32
 +@end example
>>>
>>> Nice example.
>>
>> ... but looks like I even got it wrong - it should be "--device e1000",
>> without "=". Will fix it.
> 
> Really?  As I understand it, both long-opt spellings work ('--long=opt'
> as one arg, and '--long' 'opt' as two args).  So the only reason to drop
> '=' would be consistency with other examples.

$ qemu-system-x86_64 --device=e1000
qemu-system-x86_64: --device=e1000: invalid option

Does at least not work for me.

 Thomas

Re: [Qemu-devel] [PATCH 4/7] qed: Support .bdrv_co_create

[Eric Blake]

On 03/09/2018 03:46 PM, Kevin Wolf wrote:

This adds the .bdrv_co_create driver callback to qed, which
enables image creation over QMP.

Signed-off-by: Kevin Wolf 
---
  qapi/block-core.json |  25 ++-
  block/qed.c  | 204 ++-
  2 files changed, 162 insertions(+), 67 deletions(-)


Similar question as to qcow (who still creates qed images these days? 
and if no one seriously does it outside of our testsuite, would it be 
better to not allow QMP creation of qed images?).  On the other hand, 
qed is newer than qcow so it doesn't have quite the legacy of poor 
usage, so it may also mean that qed gets a longer deprecation cycle than 
qcow.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [Bug 1726733] Re: ‘qemu-img info replication:’ causes segfault

[Fabiano Rosas]

** Changed in: qemu
   Status: New => In Progress

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1726733

Title:
  ‘qemu-img info replication:’ causes segfault

Status in QEMU:
  In Progress

Bug description:
  Typing the literal command ‘qemu-img info replication:’ causes a
  segfault.  Note that ‘replication:’ is not a filename.

  $ ./qemu-img info replication:
  qemu-img: block.c:2609: bdrv_open_inherit: Assertion `!!(flags & 
BDRV_O_PROTOCOL) == !!drv->bdrv_file_open' failed.
  Aborted (core dumped)

  This was originally found by Han Han and reported in Fedora:
  https://bugzilla.redhat.com/show_bug.cgi?id=1505652

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1726733/+subscriptions

Re: [Qemu-devel] [PATCH 3/7] qcow: Support .bdrv_co_create

[Eric Blake]

On 03/09/2018 03:46 PM, Kevin Wolf wrote:

This adds the .bdrv_co_create driver callback to qcow, which
enables image creation over QMP.

Signed-off-by: Kevin Wolf 
---
  qapi/block-core.json |  21 +-
  block/qcow.c | 196 ++-
  2 files changed, 150 insertions(+), 67 deletions(-)


Pre-review question: do we REALLY want to support creation of new qcow 
images from QMP?  Or are we at the point where we want to declare qcow a 
read-only format where we only support it to the extent that you can 
convert an existing qcow file into a better supported format like qcow2?


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 4/6] luks: Turn invalid assertion into check

[Kevin Wolf]

Am 09.03.2018 um 21:19 hat Eric Blake geschrieben:
> On 03/09/2018 11:27 AM, Kevin Wolf wrote:
> > The .bdrv_getlength implementation of the crypto block driver asserted
> > that the payload offset isn't after EOF. This is an invalid assertion to
> > make as the image file could be corrupted. Instead, check it and return
> > -EIO if the file is too small for the payload offset.
> 
> Good catch.  Probably not a CVE (unless someone can argue some way that
> causing a crash on an attempt to load a maliciously corrupted file can be
> used as a denial of service across a privilege boundary), but definitely
> needs fixing.
> 
> > 
> > Zero length images are fine, so trigger -EIO only on offset > len, not
> > on offset >= len as the assertion did before.
> > 
> > Signed-off-by: Kevin Wolf 
> > ---
> >   block/crypto.c | 5 -
> >   1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/block/crypto.c b/block/crypto.c
> > index 2035f9ab13..4908d8627f 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -518,7 +518,10 @@ static int64_t block_crypto_getlength(BlockDriverState 
> > *bs)
> >   uint64_t offset = qcrypto_block_get_payload_offset(crypto->block);
> >   assert(offset < INT64_MAX);
> 
> Umm, if the file can be corrupted, what's to prevent someone from sticking
> in a negative size that fails this assertion?

In qcrypto_block_luks_open():

block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
block->payload_offset = luks->header.payload_offset *
block->sector_size

The sector size is 512LL, and luks->header.payload_offset is 32 bit.

But I just saw that block_crypto_truncate() has another wrong assertion.
Maybe I should fix that and write a test case for it. Not sure if I'll
add it to this series or as a follow-up during the freeze.

Kevin

[Qemu-devel] [PATCH 5/7] vdi: Support .bdrv_co_create

[Kevin Wolf]

This adds the .bdrv_co_create driver callback to vdi, which
enables image creation over QMP.

Signed-off-by: Kevin Wolf 
---
 qapi/block-core.json |  21 ++-
 block/vdi.c  | 169 ++-
 2 files changed, 148 insertions(+), 42 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1e2edbc063..2eba0eef7e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3680,6 +3680,25 @@
 'size': 'size' } }
 
 ##
+# @BlockdevCreateOptionsVdi:
+#
+# Driver specific image creation options for vdi.
+#
+# @file Node to create the image format on
+# @size Size of the virtual disk in bytes
+# @cluster-size Cluster size in bytes (default: 1 MB)
+# @static   Whether to create a static (preallocated) image
+#   (default: false)
+#
+# Since: 2.12
+##
+{ 'struct': 'BlockdevCreateOptionsVdi',
+  'data': { 'file': 'BlockdevRef',
+'size': 'size',
+'*cluster-size':'size',
+'*static':  'bool' } }
+
+##
 # @BlockdevCreateNotSupported:
 #
 # This is used for all drivers that don't support creating images.
@@ -3733,7 +3752,7 @@
   'sheepdog':   'BlockdevCreateOptionsSheepdog',
   'ssh':'BlockdevCreateOptionsSsh',
   'throttle':   'BlockdevCreateNotSupported',
-  'vdi':'BlockdevCreateNotSupported',
+  'vdi':'BlockdevCreateOptionsVdi',
   'vhdx':   'BlockdevCreateNotSupported',
   'vmdk':   'BlockdevCreateNotSupported',
   'vpc':'BlockdevCreateNotSupported',
diff --git a/block/vdi.c b/block/vdi.c
index 2b5ddd0666..c60ddc58c0 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -60,6 +60,9 @@
 #include "qemu/coroutine.h"
 #include "qemu/cutils.h"
 #include "qemu/uuid.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-block-core.h"
 
 /* Code configuration options. */
 
@@ -182,6 +185,8 @@ typedef struct {
 Error *migration_blocker;
 } BDRVVdiState;
 
+static QemuOptsList vdi_create_opts;
+
 static void vdi_header_to_cpu(VdiHeader *header)
 {
 le32_to_cpus(>signature);
@@ -716,67 +721,72 @@ nonallocating_write:
 return ret;
 }
 
-static int coroutine_fn vdi_co_create_opts(const char *filename, QemuOpts 
*opts,
-   Error **errp)
+static int coroutine_fn vdi_co_create(BlockdevCreateOptions *opts,
+  Error **errp)
 {
+BlockdevCreateOptionsVdi *vdi_opts;
+BlockBackend *blk = NULL;
+BlockDriverState *bs = NULL;
+
 int ret = 0;
-uint64_t bytes = 0;
 uint32_t blocks;
-size_t block_size = DEFAULT_CLUSTER_SIZE;
 uint32_t image_type = VDI_TYPE_DYNAMIC;
 VdiHeader header;
 size_t i;
 size_t bmap_size;
 int64_t offset = 0;
-Error *local_err = NULL;
-BlockBackend *blk = NULL;
 uint32_t *bmap = NULL;
 
 logout("\n");
 
-/* Read out options. */
-bytes = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
- BDRV_SECTOR_SIZE);
-#if defined(CONFIG_VDI_BLOCK_SIZE)
-/* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */
-block_size = qemu_opt_get_size_del(opts,
-   BLOCK_OPT_CLUSTER_SIZE,
-   DEFAULT_CLUSTER_SIZE);
-#endif
-#if defined(CONFIG_VDI_STATIC_IMAGE)
-if (qemu_opt_get_bool_del(opts, BLOCK_OPT_STATIC, false)) {
-image_type = VDI_TYPE_STATIC;
+assert(opts->driver == BLOCKDEV_DRIVER_VDI);
+vdi_opts = >u.vdi;
+
+/* Validate options and set default values */
+if (!vdi_opts->has_cluster_size) {
+vdi_opts->cluster_size = DEFAULT_CLUSTER_SIZE;
 }
-#endif
 
-if (bytes > VDI_DISK_SIZE_MAX) {
-ret = -ENOTSUP;
+if (vdi_opts->size > VDI_DISK_SIZE_MAX) {
 error_setg(errp, "Unsupported VDI image size (size is 0x%" PRIx64
   ", max supported is 0x%" PRIx64 ")",
-  bytes, VDI_DISK_SIZE_MAX);
-goto exit;
+  vdi_opts->size, VDI_DISK_SIZE_MAX);
+return -ENOTSUP;
 }
 
-ret = bdrv_create_file(filename, opts, _err);
-if (ret < 0) {
-error_propagate(errp, local_err);
-goto exit;
+#if !defined(CONFIG_VDI_BLOCK_SIZE)
+if (vdi_opts->has_cluster_size) {
+error_setg(errp, "Non-default cluster size not supported");
+return -ENOTSUP;
+}
+#endif
+#if !defined(CONFIG_VDI_STATIC_IMAGE)
+if (vdi_opts->has_static) {
+error_setg(errp, "Static images not supported");
+return -ENOTSUP;
+}
+#else
+if (vdi_opts->q_static) {
+image_type = VDI_TYPE_STATIC;
+}
+#endif
+
+/* Create BlockBackend to write to the image */
+bs = bdrv_open_blockdev_ref(vdi_opts->file, errp);
+if (bs == NULL) {
+  

Re: [Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable

[Emilio G. Cota]

On Fri, Mar 09, 2018 at 11:34:56 +, Michael Clark wrote:
> BTW how does one hide signed-off-by or cc email addresses with the
> git-send-email workflow?

You just don't.

> Seems like editing the patch after git format-patch is likely the only way
> around for contributors whose wishes I might not have honoured, except
> originally when I was using an advoc albeit buggy workflow, where the Cc’s
> et all we’re later added to the patch headers and not the commits. I’ve
> probably inadvertently violated someone’s wish to keep their email address
> out of the list archives, which is an understandable wish.

If that's their wish I'd say just use the --cc flag; they get to keep
their address hidden from public commit messages, at the expense of (possibly)
being Cc'ed on more emails than they need to. I think it's a
reasonable trade-off.

That said, if they really want to hide their address from others,
you should use --bcc. But then they wouldn't receive the replies.

> This is not an issue with GitHub PR’s as they keep identity information
> differently however slicing up the port for upstreaming has lost some of
> our contributor history. I have made tags before every squash and rebase so
> we can find all history in the riscv repo, as well as previously trying to
> keep personal emails out of the cover letters in the first series. With the
> change to the git-sendemail  workflow i’ve likely regressed here.
> 
> I also probably have to manually edit patches to add ‘Cc to the
> riscv-patches mailing list, as it doesn’t seem right to put that email in
> the commit messages.

Isn't Cc'ing riscv-patches an obvious use case for using the --cc flag?
(BTW You can add as many --cc's as you want, and these apply to all patches
in a series.)

E.

PS. As always, remember to use --dry-run =)

Re: [Qemu-devel] [PATCH 3/5] nbd/server: fix: check client->closing before reply sending

[Eric Blake]

On 03/08/2018 12:46 PM, Vladimir Sementsov-Ogievskiy wrote:

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

It's like an RFC. I'm not sure, but this place looks like a bug. Shouldn't
we chack client-closing even before nbd_client_receive_next_request() call?

  nbd/server.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index e0de431e10..97b45a21fa 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1547,10 +1547,6 @@ static coroutine_fn void nbd_trip(void *opaque)


More context:

ret = nbd_co_receive_request(req, , _err);
client->recv_coroutine = NULL;
nbd_client_receive_next_request(client);
if (ret == -EIO) {


  goto disconnect;
  }
  


Calling nbd_client_receive_next_request() checks whether recv_coroutine 
is NULL (it is, because we just set it that way) and whether we are up 
to our maximum in parallel request handling; so it likely queues another 
coroutine to call nbd_trip() again.  However, when the next nbd_trip() 
is invoked, the first thing it does (after a trace call) is check 
client->closing, at which point it is a nop.


Your argument is that if ret was -EIO, we goto disconnect (which sets 
client->closing if it was not already set), and thus the just-scheduled 
next nbd_trip() will be a nop.  If ret was anything else, we used to try 
to reply to the client no matter what, which generally works; although 
if client->closing is already set, the attempt to reply is instead 
likely to fail and result in a later attempt to goto disconnect - but at 
that point disconnect is a nop since client->closing is already set. 
Whereas your patch skips the reply to the client if client->closing (and 
can't reach the disconnect code, but that doesn't matter as the 
disconnect attempt did nothing).  Swapping the check for client->closing 
to be earlier than the reply to the client thus looks safe.


Your RFC question is whether we can just check client->closing before 
checking ret, and skip nbd_client_receive_next_request() in that case 
(in other words, why even bother to queue up a coroutine that will do 
nothing, if we already know the client is going away).  And my answer is 
yes, I think that makes more sense.  So that would be:


diff --git c/nbd/server.c w/nbd/server.c
index 5f292064af0..b230ecb4fb8 100644
--- c/nbd/server.c
+++ w/nbd/server.c
@@ -1543,14 +1543,6 @@ static coroutine_fn void nbd_trip(void *opaque)
 req = nbd_request_get(client);
 ret = nbd_co_receive_request(req, , _err);
 client->recv_coroutine = NULL;
-nbd_client_receive_next_request(client);
-if (ret == -EIO) {
-goto disconnect;
-}
-
-if (ret < 0) {
-goto reply;
-}

 if (client->closing) {
 /*
@@ -1560,6 +1552,15 @@ static coroutine_fn void nbd_trip(void *opaque)
 goto done;
 }

+nbd_client_receive_next_request(client);
+if (ret == -EIO) {
+goto disconnect;
+}
+
+if (ret < 0) {
+goto reply;
+}
+
 switch (request.type) {
 case NBD_CMD_READ:
 /* XXX: NBD Protocol only documents use of FUA with WRITE */


Unless this revised form fails testing or gets any other review 
comments, I will go ahead and amend your commit in this manner.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH 1/7] parallels: Support .bdrv_co_create

[Kevin Wolf]

This adds the .bdrv_co_create driver callback to parallels, which
enables image creation over QMP.

Signed-off-by: Kevin Wolf 
---
 qapi/block-core.json |  18 -
 block/parallels.c| 199 ++-
 2 files changed, 168 insertions(+), 49 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 07039bfe9c..d38058eeab 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3481,6 +3481,22 @@
 'size': 'size' } }
 
 ##
+# @BlockdevCreateOptionsParallels:
+#
+# Driver specific image creation options for parallels.
+#
+# @file Node to create the image format on
+# @size Size of the virtual disk in bytes
+# @cluster-size Cluster size in bytes (default: 1 MB)
+#
+# Since: 2.12
+##
+{ 'struct': 'BlockdevCreateOptionsParallels',
+  'data': { 'file': 'BlockdevRef',
+'size': 'size',
+'*cluster-size':'size' } }
+
+##
 # @BlockdevQcow2Version:
 #
 # @v2:  The original QCOW2 format as introduced in qemu 0.10 (version 2)
@@ -3664,7 +3680,7 @@
   'null-aio':   'BlockdevCreateNotSupported',
   'null-co':'BlockdevCreateNotSupported',
   'nvme':   'BlockdevCreateNotSupported',
-  'parallels':  'BlockdevCreateNotSupported',
+  'parallels':  'BlockdevCreateOptionsParallels',
   'qcow2':  'BlockdevCreateOptionsQcow2',
   'qcow':   'BlockdevCreateNotSupported',
   'qed':'BlockdevCreateNotSupported',
diff --git a/block/parallels.c b/block/parallels.c
index c13cb619e6..2da5e56a9d 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -34,6 +34,9 @@
 #include "sysemu/block-backend.h"
 #include "qemu/module.h"
 #include "qemu/option.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-block-core.h"
 #include "qemu/bswap.h"
 #include "qemu/bitmap.h"
 #include "migration/blocker.h"
@@ -79,6 +82,25 @@ static QemuOptsList parallels_runtime_opts = {
 },
 };
 
+static QemuOptsList parallels_create_opts = {
+.name = "parallels-create-opts",
+.head = QTAILQ_HEAD_INITIALIZER(parallels_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = "Virtual disk size",
+},
+{
+.name = BLOCK_OPT_CLUSTER_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = "Parallels image cluster size",
+.def_value_str = stringify(DEFAULT_CLUSTER_SIZE),
+},
+{ /* end of list */ }
+}
+};
+
 
 static int64_t bat2sect(BDRVParallelsState *s, uint32_t idx)
 {
@@ -480,46 +502,62 @@ out:
 }
 
 
-static int coroutine_fn parallels_co_create_opts(const char *filename,
- QemuOpts *opts,
- Error **errp)
+static int coroutine_fn parallels_co_create(BlockdevCreateOptions* opts,
+Error **errp)
 {
+BlockdevCreateOptionsParallels *parallels_opts;
+BlockDriverState *bs;
+BlockBackend *blk;
 int64_t total_size, cl_size;
-uint8_t tmp[BDRV_SECTOR_SIZE];
-Error *local_err = NULL;
-BlockBackend *file;
 uint32_t bat_entries, bat_sectors;
 ParallelsHeader header;
+uint8_t tmp[BDRV_SECTOR_SIZE];
 int ret;
 
-total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-  BDRV_SECTOR_SIZE);
-cl_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE,
-  DEFAULT_CLUSTER_SIZE), BDRV_SECTOR_SIZE);
+assert(opts->driver == BLOCKDEV_DRIVER_PARALLELS);
+parallels_opts = >u.parallels;
+
+/* Sanity checks */
+total_size = parallels_opts->size;
+
+if (parallels_opts->has_cluster_size) {
+cl_size = parallels_opts->cluster_size;
+} else {
+cl_size = DEFAULT_CLUSTER_SIZE;
+}
+
 if (total_size >= MAX_PARALLELS_IMAGE_FACTOR * cl_size) {
-error_propagate(errp, local_err);
+error_setg(errp, "Image size is too large for this cluster size");
 return -E2BIG;
 }
 
-ret = bdrv_create_file(filename, opts, _err);
-if (ret < 0) {
-error_propagate(errp, local_err);
-return ret;
+if (!QEMU_IS_ALIGNED(total_size, BDRV_SECTOR_SIZE)) {
+error_setg(errp, "Image size must be a multiple of 512 bytes");
+return -EINVAL;
 }
 
-file = blk_new_open(filename, NULL, NULL,
-BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL,
-_err);
-if (file == NULL) {
-error_propagate(errp, local_err);
+if (!QEMU_IS_ALIGNED(cl_size, BDRV_SECTOR_SIZE)) {
+error_setg(errp, "Cluster size must be a multiple of 512 bytes");
+return -EINVAL;
+}
+
+/* Create BlockBackend to write to the image */
+

[Qemu-devel] [PATCH v2] fsl-imx6: Swap Ethernet interrupt defines

[Guenter Roeck]

The sabrelite machine model used by qemu-system-arm is based on the
Freescale/NXP i.MX6Q processor. This SoC has an on-board ethernet
controller which is supported in QEMU using the imx_fec.c module
(actually called imx.enet for this model.)

The include/hw/arm/fsm-imx6.h file defines the interrupt vectors for the
imx.enet device like this:

 #define FSL_IMX6_ENET_MAC_1588_IRQ 118
 #define FSL_IMX6_ENET_MAC_IRQ 119

According to https://www.nxp.com/docs/en/reference-manual/IMX6DQRM.pdf,
page 225, in Table 3-1. ARM Cortex A9 domain interrupt summary,
interrupts are as follows.

150 ENET MAC 0 IRQ
151 ENET MAC 0 1588 Timer interrupt

where

150 - 32 == 118
151 - 32 == 119

In other words, the vector definitions in the fsl-imx6.h file are reversed.

Fixing the interrupts alone causes problems with older Linux kernels:
The Ethernet interface will fail to probe with Linux v4.9 and earlier.
Linux v4.1 and earlier will crash due to a bug in Ethernet driver probe
error handling. This is a Linux kernel problem, not a qemu problem:
the Linux kernel only worked by accident since it requested both interrupts.

For backward compatibility, generate the Ethernet interrupt on both interrupt
lines. This was shown to work from all Linux kernel releases starting with
v3.16.

Link: https://bugs.launchpad.net/qemu/+bug/1753309
Signed-off-by: Guenter Roeck 
---
v2: Generate Ethernet interrupts on both interrupt lines

 hw/net/imx_fec.c  | 7 ++-
 include/hw/arm/fsl-imx6.h | 4 ++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 9506f9b..d3ae7db 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -417,7 +417,12 @@ static void imx_enet_write_bd(IMXENETBufDesc *bd, 
dma_addr_t addr)
 
 static void imx_eth_update(IMXFECState *s)
 {
-if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] & ENET_INT_TS_TIMER) {
+/*
+ * Generate ENET_INT_MAC interrrupts on both interrupt lines for
+ * backward compatibility with Linux kernel versions 4.9 and older.
+ */
+if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] &
+(ENET_INT_MAC | ENET_INT_TS_TIMER)) {
 qemu_set_irq(s->irq[1], 1);
 } else {
 qemu_set_irq(s->irq[1], 0);
diff --git a/include/hw/arm/fsl-imx6.h b/include/hw/arm/fsl-imx6.h
index ec6c509..06f8aae 100644
--- a/include/hw/arm/fsl-imx6.h
+++ b/include/hw/arm/fsl-imx6.h
@@ -438,8 +438,8 @@ typedef struct FslIMX6State {
 #define FSL_IMX6_HDMI_MASTER_IRQ 115
 #define FSL_IMX6_HDMI_CEC_IRQ 116
 #define FSL_IMX6_MLB150_LOW_IRQ 117
-#define FSL_IMX6_ENET_MAC_1588_IRQ 118
-#define FSL_IMX6_ENET_MAC_IRQ 119
+#define FSL_IMX6_ENET_MAC_IRQ 118
+#define FSL_IMX6_ENET_MAC_1588_IRQ 119
 #define FSL_IMX6_PCIE1_IRQ 120
 #define FSL_IMX6_PCIE2_IRQ 121
 #define FSL_IMX6_PCIE3_IRQ 122
-- 
2.7.4

[Qemu-devel] [PATCH 4/7] qed: Support .bdrv_co_create

[Kevin Wolf]

This adds the .bdrv_co_create driver callback to qed, which
enables image creation over QMP.

Signed-off-by: Kevin Wolf 
---
 qapi/block-core.json |  25 ++-
 block/qed.c  | 204 ++-
 2 files changed, 162 insertions(+), 67 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index c81677c434..1e2edbc063 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3559,6 +3559,29 @@
 '*refcount-bits':   'int' } }
 
 ##
+# @BlockdevCreateOptionsQed:
+#
+# Driver specific image creation options for qed.
+#
+# @file Node to create the image format on
+# @size Size of the virtual disk in bytes
+# @backing-file File name of the backing file if a backing file
+#   should be used
+# @backing-fmt  Name of the block driver to use for the backing file
+# @cluster-size Cluster size in bytes (default: 65536)
+# @table-size   L1/L2 table size (in clusters)
+#
+# Since: 2.12
+##
+{ 'struct': 'BlockdevCreateOptionsQed',
+  'data': { 'file': 'BlockdevRef',
+'size': 'size',
+'*backing-file':'str',
+'*backing-fmt': 'BlockdevDriver',
+'*cluster-size':'size',
+'*table-size':  'int' } }
+
+##
 # @BlockdevCreateOptionsRbd:
 #
 # Driver specific image creation options for rbd/Ceph.
@@ -3702,7 +3725,7 @@
   'parallels':  'BlockdevCreateOptionsParallels',
   'qcow':   'BlockdevCreateOptionsQcow',
   'qcow2':  'BlockdevCreateOptionsQcow2',
-  'qed':'BlockdevCreateNotSupported',
+  'qed':'BlockdevCreateOptionsQed',
   'quorum': 'BlockdevCreateNotSupported',
   'raw':'BlockdevCreateNotSupported',
   'rbd':'BlockdevCreateOptionsRbd',
diff --git a/block/qed.c b/block/qed.c
index 5e6a6bfaa0..46a84beeed 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -20,6 +20,11 @@
 #include "trace.h"
 #include "qed.h"
 #include "sysemu/block-backend.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-block-core.h"
+
+static QemuOptsList qed_create_opts;
 
 static int bdrv_qed_probe(const uint8_t *buf, int buf_size,
   const char *filename)
@@ -594,57 +599,95 @@ static void bdrv_qed_close(BlockDriverState *bs)
 qemu_vfree(s->l1_table);
 }
 
-static int qed_create(const char *filename, uint32_t cluster_size,
-  uint64_t image_size, uint32_t table_size,
-  const char *backing_file, const char *backing_fmt,
-  QemuOpts *opts, Error **errp)
+static int coroutine_fn bdrv_qed_co_create(BlockdevCreateOptions *opts,
+   Error **errp)
 {
-QEDHeader header = {
-.magic = QED_MAGIC,
-.cluster_size = cluster_size,
-.table_size = table_size,
-.header_size = 1,
-.features = 0,
-.compat_features = 0,
-.l1_table_offset = cluster_size,
-.image_size = image_size,
-};
+BlockdevCreateOptionsQed *qed_opts;
+BlockBackend *blk = NULL;
+BlockDriverState *bs = NULL;
+
+QEDHeader header;
 QEDHeader le_header;
 uint8_t *l1_table = NULL;
-size_t l1_size = header.cluster_size * header.table_size;
-Error *local_err = NULL;
+size_t l1_size;
 int ret = 0;
-BlockBackend *blk;
 
-ret = bdrv_create_file(filename, opts, _err);
-if (ret < 0) {
-error_propagate(errp, local_err);
-return ret;
+assert(opts->driver == BLOCKDEV_DRIVER_QED);
+qed_opts = >u.qed;
+
+/* Validate options and set default values */
+if (!qed_opts->has_cluster_size) {
+qed_opts->cluster_size = QED_DEFAULT_CLUSTER_SIZE;
+}
+if (!qed_opts->has_table_size) {
+qed_opts->table_size = QED_DEFAULT_TABLE_SIZE;
 }
 
-blk = blk_new_open(filename, NULL, NULL,
-   BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL,
-   _err);
-if (blk == NULL) {
-error_propagate(errp, local_err);
+if (!qed_is_cluster_size_valid(qed_opts->cluster_size)) {
+error_setg(errp, "QED cluster size must be within range [%u, %u] "
+ "and power of 2",
+   QED_MIN_CLUSTER_SIZE, QED_MAX_CLUSTER_SIZE);
+return -EINVAL;
+}
+if (!qed_is_table_size_valid(qed_opts->table_size)) {
+error_setg(errp, "QED table size must be within range [%u, %u] "
+ "and power of 2",
+   QED_MIN_TABLE_SIZE, QED_MAX_TABLE_SIZE);
+return -EINVAL;
+}
+if (!qed_is_image_size_valid(qed_opts->size, qed_opts->cluster_size,
+ qed_opts->table_size))
+{
+error_setg(errp, "QED image size must be a non-zero multiple of "
+ "cluster 

[Qemu-devel] [PATCH 6/7] vhdx: Support .bdrv_co_create

[Kevin Wolf]

This adds the .bdrv_co_create driver callback to vhdx, which
enables image creation over QMP.

Signed-off-by: Kevin Wolf 
---
 qapi/block-core.json |  37 ++-
 block/vhdx.c | 174 ++-
 2 files changed, 167 insertions(+), 44 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 2eba0eef7e..3a65909c47 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3699,6 +3699,41 @@
 '*static':  'bool' } }
 
 ##
+# @BlockdevVhdxSubformat:
+#
+# @dynamic: Growing image file
+# @fixed:   Preallocated fixed-size imge file
+#
+# Since: 2.12
+##
+{ 'enum': 'BlockdevVhdxSubformat',
+  'data': [ 'dynamic', 'fixed' ] }
+
+##
+# @BlockdevCreateOptionsVhdx:
+#
+# Driver specific image creation options for vhdx.
+#
+# @file Node to create the image format on
+# @size Size of the virtual disk in bytes
+# @log-size Log size in bytes (default: 1 MB)
+# @block-size   Block size in bytes (default: 1 MB)
+# @subformatvhdx subformat (default: dynamic)
+# @block-state-zero Force use of payload blocks of type 'ZERO'. Non-standard,
+#   but default.  Do not set to 'off' when using 'qemu-img
+#   convert' with subformat=dynamic.
+#
+# Since: 2.12
+##
+{ 'struct': 'BlockdevCreateOptionsVhdx',
+  'data': { 'file': 'BlockdevRef',
+'size': 'size',
+'*log-size':'size',
+'*block-size':  'size',
+'*subformat':   'BlockdevVhdxSubformat',
+'*block-state-zero':'bool' } }
+
+##
 # @BlockdevCreateNotSupported:
 #
 # This is used for all drivers that don't support creating images.
@@ -3753,7 +3788,7 @@
   'ssh':'BlockdevCreateOptionsSsh',
   'throttle':   'BlockdevCreateNotSupported',
   'vdi':'BlockdevCreateOptionsVdi',
-  'vhdx':   'BlockdevCreateNotSupported',
+  'vhdx':   'BlockdevCreateOptionsVhdx',
   'vmdk':   'BlockdevCreateNotSupported',
   'vpc':'BlockdevCreateNotSupported',
   'vvfat':  'BlockdevCreateNotSupported',
diff --git a/block/vhdx.c b/block/vhdx.c
index d82350d07c..0ce972381f 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -26,6 +26,9 @@
 #include "block/vhdx.h"
 #include "migration/blocker.h"
 #include "qemu/uuid.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-block-core.h"
 
 /* Options for VHDX creation */
 
@@ -39,6 +42,8 @@ typedef enum VHDXImageType {
 VHDX_TYPE_DIFFERENCING,   /* Currently unsupported */
 } VHDXImageType;
 
+static QemuOptsList vhdx_create_opts;
+
 /* Several metadata and region table data entries are identified by
  * guids in  a MS-specific GUID format. */
 
@@ -1792,54 +1797,63 @@ exit:
  *. ~ --- ~  ~  ~ ---.
  *   1MB
  */
-static int coroutine_fn vhdx_co_create_opts(const char *filename, QemuOpts 
*opts,
-Error **errp)
+static int coroutine_fn vhdx_co_create(BlockdevCreateOptions *opts,
+   Error **errp)
 {
+BlockdevCreateOptionsVhdx *vhdx_opts;
+BlockBackend *blk = NULL;
+BlockDriverState *bs = NULL;
+
 int ret = 0;
-uint64_t image_size = (uint64_t) 2 * GiB;
-uint32_t log_size   = 1 * MiB;
-uint32_t block_size = 0;
+uint64_t image_size;
+uint32_t log_size;
+uint32_t block_size;
 uint64_t signature;
 uint64_t metadata_offset;
 bool use_zero_blocks = false;
 
 gunichar2 *creator = NULL;
 glong creator_items;
-BlockBackend *blk;
-char *type = NULL;
 VHDXImageType image_type;
-Error *local_err = NULL;
 
-image_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-  BDRV_SECTOR_SIZE);
-log_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_LOG_SIZE, 0);
-block_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_BLOCK_SIZE, 0);
-type = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
-use_zero_blocks = qemu_opt_get_bool_del(opts, VHDX_BLOCK_OPT_ZERO, true);
+assert(opts->driver == BLOCKDEV_DRIVER_VHDX);
+vhdx_opts = >u.vhdx;
+
+/* Validate options and set default values */
+
+image_size = vhdx_opts->size;
+block_size = vhdx_opts->block_size;
+
+if (!vhdx_opts->has_log_size) {
+log_size = DEFAULT_LOG_SIZE;
+} else {
+log_size = vhdx_opts->log_size;
+}
+
+if (!vhdx_opts->has_block_state_zero) {
+use_zero_blocks = true;
+} else {
+use_zero_blocks = vhdx_opts->block_state_zero;
+}
 
 if (image_size > VHDX_MAX_IMAGE_SIZE) {
 error_setg_errno(errp, EINVAL, "Image size too large; max of 64TB");
-ret = -EINVAL;
-goto exit;
+return -EINVAL;
 }
 
-if (type == NULL) {

[Qemu-devel] [PATCH 3/7] qcow: Support .bdrv_co_create

[Kevin Wolf]

This adds the .bdrv_co_create driver callback to qcow, which
enables image creation over QMP.

Signed-off-by: Kevin Wolf 
---
 qapi/block-core.json |  21 +-
 block/qcow.c | 196 ++-
 2 files changed, 150 insertions(+), 67 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index d38058eeab..c81677c434 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3497,6 +3497,25 @@
 '*cluster-size':'size' } }
 
 ##
+# @BlockdevCreateOptionsQcow:
+#
+# Driver specific image creation options for qcow.
+#
+# @file Node to create the image format on
+# @size Size of the virtual disk in bytes
+# @backing-file File name of the backing file if a backing file
+#   should be used
+# @encrypt  Encryption options if the image should be encrypted
+#
+# Since: 2.12
+##
+{ 'struct': 'BlockdevCreateOptionsQcow',
+  'data': { 'file': 'BlockdevRef',
+'size': 'size',
+'*backing-file':'str',
+'*encrypt': 'QCryptoBlockCreateOptions' } }
+
+##
 # @BlockdevQcow2Version:
 #
 # @v2:  The original QCOW2 format as introduced in qemu 0.10 (version 2)
@@ -3681,8 +3700,8 @@
   'null-co':'BlockdevCreateNotSupported',
   'nvme':   'BlockdevCreateNotSupported',
   'parallels':  'BlockdevCreateOptionsParallels',
+  'qcow':   'BlockdevCreateOptionsQcow',
   'qcow2':  'BlockdevCreateOptionsQcow2',
-  'qcow':   'BlockdevCreateNotSupported',
   'qed':'BlockdevCreateNotSupported',
   'quorum': 'BlockdevCreateNotSupported',
   'raw':'BlockdevCreateNotSupported',
diff --git a/block/qcow.c b/block/qcow.c
index 47a18d9a3a..2e3770ca63 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -33,6 +33,8 @@
 #include 
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qstring.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-block-core.h"
 #include "crypto/block.h"
 #include "migration/blocker.h"
 #include "block/crypto.h"
@@ -86,6 +88,8 @@ typedef struct BDRVQcowState {
 Error *migration_blocker;
 } BDRVQcowState;
 
+static QemuOptsList qcow_create_opts;
+
 static int decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset);
 
 static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename)
@@ -810,62 +814,50 @@ static void qcow_close(BlockDriverState *bs)
 error_free(s->migration_blocker);
 }
 
-static int coroutine_fn qcow_co_create_opts(const char *filename, QemuOpts 
*opts,
-Error **errp)
+static int coroutine_fn qcow_co_create(BlockdevCreateOptions *opts,
+   Error **errp)
 {
+BlockdevCreateOptionsQcow *qcow_opts;
 int header_size, backing_filename_len, l1_size, shift, i;
 QCowHeader header;
 uint8_t *tmp;
 int64_t total_size = 0;
-char *backing_file = NULL;
-Error *local_err = NULL;
 int ret;
+BlockDriverState *bs;
 BlockBackend *qcow_blk;
-char *encryptfmt = NULL;
-QDict *options;
-QDict *encryptopts = NULL;
-QCryptoBlockCreateOptions *crypto_opts = NULL;
 QCryptoBlock *crypto = NULL;
 
-/* Read out options */
-total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-  BDRV_SECTOR_SIZE);
+assert(opts->driver == BLOCKDEV_DRIVER_QCOW);
+qcow_opts = >u.qcow;
+
+/* Sanity checks */
+total_size = qcow_opts->size;
 if (total_size == 0) {
 error_setg(errp, "Image size is too small, cannot be zero length");
-ret = -EINVAL;
-goto cleanup;
+return -EINVAL;
 }
 
-backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
-encryptfmt = qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT_FORMAT);
-if (encryptfmt) {
-if (qemu_opt_get(opts, BLOCK_OPT_ENCRYPT)) {
-error_setg(errp, "Options " BLOCK_OPT_ENCRYPT " and "
-   BLOCK_OPT_ENCRYPT_FORMAT " are mutually exclusive");
-ret = -EINVAL;
-goto cleanup;
-}
-} else if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
-encryptfmt = g_strdup("aes");
+if (qcow_opts->has_encrypt &&
+qcow_opts->encrypt->format != Q_CRYPTO_BLOCK_FORMAT_QCOW)
+{
+error_setg(errp, "Unsupported encryption format");
+return -EINVAL;
 }
 
-ret = bdrv_create_file(filename, opts, _err);
-if (ret < 0) {
-error_propagate(errp, local_err);
-goto cleanup;
+/* Create BlockBackend to write to the image */
+bs = bdrv_open_blockdev_ref(qcow_opts->file, errp);
+if (bs == NULL) {
+return -EIO;
 }
 
-qcow_blk = blk_new_open(filename, NULL, NULL,
-BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL,
-  

[Qemu-devel] [PATCH 7/7] vpc: Support .bdrv_co_create

[Kevin Wolf]

This adds the .bdrv_co_create driver callback to vpc, which
enables image creation over QMP.

Signed-off-by: Kevin Wolf 
---
 qapi/block-core.json |  33 ++-
 block/vpc.c  | 152 ++-
 2 files changed, 147 insertions(+), 38 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 3a65909c47..ca645a0067 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3734,6 +3734,37 @@
 '*block-state-zero':'bool' } }
 
 ##
+# @BlockdevVpcSubformat:
+#
+# @dynamic: Growing image file
+# @fixed:   Preallocated fixed-size imge file
+#
+# Since: 2.12
+##
+{ 'enum': 'BlockdevVpcSubformat',
+  'data': [ 'dynamic', 'fixed' ] }
+
+##
+# @BlockdevCreateOptionsVpc:
+#
+# Driver specific image creation options for vpc (VHD).
+#
+# @file Node to create the image format on
+# @size Size of the virtual disk in bytes
+# @subformatvhdx subformat (default: dynamic)
+# @force-size   Force use of the exact byte size instead of rounding to the
+#   next size that can be represented in CHS geometry
+#   (default: false)
+#
+# Since: 2.12
+##
+{ 'struct': 'BlockdevCreateOptionsVpc',
+  'data': { 'file': 'BlockdevRef',
+'size': 'size',
+'*subformat':   'BlockdevVpcSubformat',
+'*force-size':  'bool' } }
+
+##
 # @BlockdevCreateNotSupported:
 #
 # This is used for all drivers that don't support creating images.
@@ -3790,7 +3821,7 @@
   'vdi':'BlockdevCreateOptionsVdi',
   'vhdx':   'BlockdevCreateOptionsVhdx',
   'vmdk':   'BlockdevCreateNotSupported',
-  'vpc':'BlockdevCreateNotSupported',
+  'vpc':'BlockdevCreateOptionsVpc',
   'vvfat':  'BlockdevCreateNotSupported',
   'vxhs':   'BlockdevCreateNotSupported'
   } }
diff --git a/block/vpc.c b/block/vpc.c
index b2e2b9ebd4..8824211713 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -32,6 +32,9 @@
 #include "migration/blocker.h"
 #include "qemu/bswap.h"
 #include "qemu/uuid.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-block-core.h"
 
 /**/
 
@@ -166,6 +169,8 @@ static QemuOptsList vpc_runtime_opts = {
 }
 };
 
+static QemuOptsList vpc_create_opts;
+
 static uint32_t vpc_checksum(uint8_t* buf, size_t size)
 {
 uint32_t res = 0;
@@ -897,12 +902,15 @@ static int create_fixed_disk(BlockBackend *blk, uint8_t 
*buf,
 return ret;
 }
 
-static int coroutine_fn vpc_co_create_opts(const char *filename, QemuOpts 
*opts,
-   Error **errp)
+static int coroutine_fn vpc_co_create(BlockdevCreateOptions *opts,
+  Error **errp)
 {
+BlockdevCreateOptionsVpc *vpc_opts;
+BlockBackend *blk = NULL;
+BlockDriverState *bs = NULL;
+
 uint8_t buf[1024];
 VHDFooter *footer = (VHDFooter *) buf;
-char *disk_type_param;
 int i;
 uint16_t cyls = 0;
 uint8_t heads = 0;
@@ -911,45 +919,38 @@ static int coroutine_fn vpc_co_create_opts(const char 
*filename, QemuOpts *opts,
 int64_t total_size;
 int disk_type;
 int ret = -EIO;
-bool force_size;
-Error *local_err = NULL;
-BlockBackend *blk = NULL;
 
-/* Read out options */
-total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-  BDRV_SECTOR_SIZE);
-disk_type_param = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
-if (disk_type_param) {
-if (!strcmp(disk_type_param, "dynamic")) {
-disk_type = VHD_DYNAMIC;
-} else if (!strcmp(disk_type_param, "fixed")) {
-disk_type = VHD_FIXED;
-} else {
-error_setg(errp, "Invalid disk type, %s", disk_type_param);
-ret = -EINVAL;
-goto out;
-}
-} else {
+assert(opts->driver == BLOCKDEV_DRIVER_VPC);
+vpc_opts = >u.vpc;
+
+/* Validate options and set default values */
+total_size = vpc_opts->size;
+
+if (!vpc_opts->has_subformat) {
+vpc_opts->subformat = BLOCKDEV_VPC_SUBFORMAT_DYNAMIC;
+}
+switch (vpc_opts->subformat) {
+case BLOCKDEV_VPC_SUBFORMAT_DYNAMIC:
 disk_type = VHD_DYNAMIC;
+break;
+case BLOCKDEV_VPC_SUBFORMAT_FIXED:
+disk_type = VHD_FIXED;
+break;
+default:
+g_assert_not_reached();
 }
 
-force_size = qemu_opt_get_bool_del(opts, VPC_OPT_FORCE_SIZE, false);
-
-ret = bdrv_create_file(filename, opts, _err);
-if (ret < 0) {
-error_propagate(errp, local_err);
-goto out;
+/* Create BlockBackend to write to the image */
+bs = bdrv_open_blockdev_ref(vpc_opts->file, errp);
+if (bs == NULL) {
+return -EIO;
 }
 
-blk = 

[Qemu-devel] [PATCH 2/7] qemu-iotests: Enable write tests for parallels

[Kevin Wolf]

Originally we added parallels as a read-only format to qemu-iotests
where we did just some tests with a binary image. Since then, write and
image creation support has been added to the driver, so we can now
enable it in _supported_fmt generic.

The driver doesn't support migration yet, though, so we need to add it
to the list of exceptions in 181.

Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/181   | 2 +-
 tests/qemu-iotests/check | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/181 b/tests/qemu-iotests/181
index 0c91e8f9de..5e767c6195 100755
--- a/tests/qemu-iotests/181
+++ b/tests/qemu-iotests/181
@@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt generic
 # Formats that do not support live migration
-_unsupported_fmt qcow vdi vhdx vmdk vpc vvfat
+_unsupported_fmt qcow vdi vhdx vmdk vpc vvfat parallels
 _supported_proto generic
 _supported_os Linux
 
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index e6b6ff7a04..469142cd58 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -284,7 +284,6 @@ testlist options
 
 -parallels)
 IMGFMT=parallels
-IMGFMT_GENERIC=false
 xpand=false
 ;;
 
-- 
2.13.6

[Qemu-devel] [PATCH 0/7] block: .bdrv_co_create for format drivers

[Kevin Wolf]

This series adds a .bdrv_co_create implementation to almost all format
drivers that support creating images where its still missing. The only
exception is VMDK because its support for extents will make the QAPI
design a bit more complicated.

The other format driver not covered in this series are qcow2 (already
merged) and luks (already posted in a separate series).

Kevin Wolf (7):
  parallels: Support .bdrv_co_create
  qemu-iotests: Enable write tests for parallels
  qcow: Support .bdrv_co_create
  qed: Support .bdrv_co_create
  vdi: Support .bdrv_co_create
  vhdx: Support .bdrv_co_create
  vpc: Support .bdrv_co_create

 qapi/block-core.json | 155 +--
 block/parallels.c| 199 ++---
 block/qcow.c | 196 ++---
 block/qed.c  | 204 ---
 block/vdi.c  | 169 +--
 block/vhdx.c | 174 ++--
 block/vpc.c  | 152 ++-
 tests/qemu-iotests/181   |   2 +-
 tests/qemu-iotests/check |   1 -
 9 files changed, 943 insertions(+), 309 deletions(-)

-- 
2.13.6

Re: [Qemu-devel] [PATCH] fsl-imx6: Swap Ethernet interrupt defines

[Guenter Roeck]

On Fri, Mar 09, 2018 at 10:53:43AM -0800, Bill Paul wrote:
> Of all the gin joints in all the towns in all the world, Guenter Roeck had to 
> walk into mine at 10:20 on Friday 09 March 2018 and say:
> 
> > On Fri, Mar 09, 2018 at 05:47:16PM +, Peter Maydell wrote:
> > > On 8 March 2018 at 18:28, Bill Paul  wrote:
> > > > Anyway, this means that the only reason older Linux kernels worked in
> > > > QEMU with the broken interrupt configuration is that they also
> > > > registered a handler on vector 151 (119). Even though QEMU could not
> > > > send events via GPIO6, it was mistakenly sending them via vector 151,
> > > > so it looked like things worked. On real hardware, the older kernels
> > > > would have gotten their interrupts via GPIO6 and also worked. The
> > > > older kernels would _not_ work if you fix QEMU because now they would
> > > > never get interrupts on either vector, unless you fudge things so that
> > > > the ENET module triggers both vector 150 and the vector for GPIO6 in
> > > > the GIC or patch them to back out the erratum 6678 workaround as later
> > > > kernels do.
> > > 
> > > Thanks for that really useful writeup. So if I understand correctly
> > > 
> > > we have several choices here:
> > >  (1) we could implement a model of the IOMUX block that is at least
> > >  sufficient to support guests that configure it to route the ENET
> > >  interrupt line to a GPIO pin. Then we could apply this patch that fixes
> > >  the ENET line definitions. Old kernels would continue to work (for the
> > >  same reason they worked on hardware), and new ones would work now too.
> > >  This is in some ways the preferred option, but it's possibly a lot of
> > >  code and we're nearly in freeze for 2.12.
> > >  
> > >  (2) we could leave everything as it is for 2.12. This would mean that
> > >  at least we don't regress setups that used to work on older QEMU
> > >  versions. Downside is that we wouldn't be able to run Linux v4.15+, or
> > >  other guest OSes that don't have the bug that older Linux kernels do.
> > >  (Presumably we'd only do this on the understanding that we were going
> > >  to go down route (1) for 2.13.)
> > >  
> > >  (3) we could apply this patch for 2.12. Linux v4.15+ now works, as
> > >  do other guest OSes that use the ENET interrupt. v4.1 and older Linux
> > >  guests that used to boot in QEMU stop doing so, and 4.2-4.9 boot but
> > >  lose the ethernet device support. Perhaps for 2.13 we might
> > >  take route (1) to make those older guests start working again.
> > > 
> > > Do I have that right?
> > 
> > Pretty much.
> 
> There may be a 4th option.
> 
> Since older kernels work because they were looking at vector 151, you could 
> patch the imx_fec.c module so that when it signals an event for vector 150, 
> it 
> also signals vector 151 too. This would keep newer versions of QEMU "bug-
> compatible" with older versions while also fixing support for newer Linux 
> kernels and other guests (e.g. VxWorks) that follow the hardware spec 
> correctly.
> 
> I think this would require only a small modification to this function:
> 
> static void imx_eth_update(IMXFECState *s)
> {
> if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] & ENET_INT_TS_TIMER) {
> qemu_set_irq(s->irq[1], 1);
> } else {
> qemu_set_irq(s->irq[1], 0);
> }
> 
> if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] & ENET_INT_MAC) {
> qemu_set_irq(s->irq[0], 1);
> } else {
> qemu_set_irq(s->irq[0], 0);
> }
> }
> 
> (For ENET_INT_MAC events, just signal both s->irq[0] and s->irq[1]).
> 
Now this is an excellent idea.

> This of course means signaling spurious events on vector 151, but you're 
> doing 
> that now anyway. :)
> 
Actually, it doesn't. It looks like the first interrupt is handled, resetting
the interrupt status, and the second interrupt is never even executed. I tested
this with all kernel releases back to v3.16.

I'll send out patch v2 with this change and let Peter decide which version to
apply.

Thanks,
Guenter

[Qemu-devel] [PATCH v2] RISC-V: Fix riscv_isa_string, use popcount to count bits

[Michael Clark]

Logic bug caused the string size calculation for the RISC-V
format ISA string to be small. This fix allows slack for rv128.

Cc: Palmer Dabbelt 
Cc: Peter Maydell 
Cc: Eric Blake 
Signed-off-by: Michael Clark 
---
 target/riscv/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 4851890..1456535 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -391,7 +391,7 @@ static const TypeInfo riscv_cpu_type_info = {
 char *riscv_isa_string(RISCVCPU *cpu)
 {
 int i;
-size_t maxlen = 5 + ctz32(cpu->env.misa);
+size_t maxlen = 8 + ctpop64(cpu->env.misa);
 char *isa_string = g_new0(char, maxlen);
 snprintf(isa_string, maxlen, "rv%d", TARGET_LONG_BITS);
 for (i = 0; i < sizeof(riscv_exts); i++) {
-- 
2.7.0

Re: [Qemu-devel] [PATCH v2 10/11] qemu-binfmt-conf.sh: add qemu-xtensa

[Max Filippov]

Hi Laurent,

On Fri, Mar 9, 2018 at 11:58 AM, Laurent Vivier  wrote:
> Le 28/02/2018 à 23:16, Max Filippov a écrit :
>> Register qemu-xtensa and qemu-xtensaeb for transparent linux userspace
>> emulation.
>>
>> Cc: Riku Voipio 
>> Cc: Laurent Vivier 
>> Signed-off-by: Max Filippov 
>> ---
>>  scripts/qemu-binfmt-conf.sh | 12 ++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>
>
> Applied to my 'linux-user-for-2.12' branch.

could you please also take a look at the patch that adds xtensa linux-user
support:
  http://lists.nongnu.org/archive/html/qemu-devel/2018-02/msg07348.html

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 0/8] tests: Run device-crash-test on "make check"

[Eduardo Habkost]

On Fri, Mar 09, 2018 at 12:48:42PM -0800, no-re...@patchew.org wrote:
[...]
> /tmp/qemu-test/src/scripts/device-crash-test -q -t machine=DEFAULT accel=tcg 
> --  x86_64-softmmu/qemu-system-x86_64  aarch64-softmmu/qemu-system-aarch64
> /usr/bin/env: python2.7: No such file or directory
> make: *** [check-device-crash-quick] Error 127
> make: *** Waiting for unfinished jobs
> make: *** wait: No child processes.  Stop.
> Traceback (most recent call last):
>   File "./tests/docker/docker.py", line 407, in 
> sys.exit(main())
>   File "./tests/docker/docker.py", line 404, in main
> return args.cmdobj.run(args, argv)
>   File "./tests/docker/docker.py", line 261, in run
> return Docker().run(argv, args.keep, quiet=args.quiet)
>   File "./tests/docker/docker.py", line 229, in run
> quiet=quiet)
>   File "./tests/docker/docker.py", line 147, in _do_check
> return subprocess.check_call(self._command + cmd, **kwargs)
>   File "/usr/lib64/python2.7/subprocess.py", line 186, in check_call
> raise CalledProcessError(retcode, cmd)
> subprocess.CalledProcessError: Command '['docker', 'run', '--label', 
> 'com.qemu.instance.uuid=02a6de1c23db11e8935d52540069c830', '-u', '0', 
> '--security-opt', 'seccomp=unconfined', '--rm', '--net=none', '-e', 
> 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=8', '-e', 
> 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
> '/root/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
> '/var/tmp/patchew-tester-tmp-i0mjl1iv/src/docker-src.2018-03-09-15.46.46.18385:/var/tmp/qemu:z,ro',
>  'qemu:centos6', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
> status 2
> make[1]: *** [tests/docker/Makefile.include:129: docker-run] Error 1
> make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-i0mjl1iv/src'
> make: *** [tests/docker/Makefile.include:163: docker-run-test-quick@centos6] 
> Error 2

Porting device-crash-test to Python 3 is on my plans, but I don't
think this should block us from running device-crash-test on
systems using Python 2.7.  I will look for a mechanism to skip
running device-crash-test if only Python 3 is available.

-- 
Eduardo

Re: [Qemu-devel] [PULL] RISC-V: Fix riscv_isa_string g_new0 size calculation

[Michael Clark]

Apologies for jumping the gun again with a PR before a review. It was most
likely because I thought it was a critical bug fix.

I'm incorporating Eric Blake's feedback.

On Sat, Mar 10, 2018 at 9:29 AM, Michael Clark  wrote:

> The following changes since commit e4ae62b802cec437f877f2cadc4ef0
> 59cc0eca76:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request'
> into staging (2018-03-09 17:28:16 +)
>
> are available in the git repository at:
>
>   https://github.com/michaeljclark/riscv-qemu.git fix-riscv-isa-string
>
> for you to fetch changes up to 32b3a5716062f805a42e1cae1df95f2f869a78c1:
>
>   RISC-V: Fix isa string logic bug, use popcount to count bits (2018-03-10
> 09:17:29 +1300)
>
> 
> Michael Clark (1):
>   RISC-V: Fix isa string logic bug, use popcount to count bits
>
>  target/riscv/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Re: [Qemu-devel] [PATCH 02/25] hw/dma/i8257: Rename DMA_init() to i8257_dma_init()

[Artyom Tarasenko]

On Fri, Mar 9, 2018 at 1:19 PM, Mark Cave-Ayland
 wrote:
> On 09/03/18 10:43, Philippe Mathieu-Daudé wrote:
>>
>> Hi Mark,
>>
>> On 03/09/2018 11:32 AM, Mark Cave-Ayland wrote:
>>>
>>> On 08/03/18 22:39, Philippe Mathieu-Daudé wrote:
>>>
 - Move the header from hw/isa/ to hw/dma/
 - Remove the old i386/pc dependency
 - use a bool type for the high_page_enable argument

 Signed-off-by: Philippe Mathieu-Daudé 
 ---
include/hw/{isa => dma}/i8257.h | 6 ++
include/hw/isa/isa.h| 2 --
hw/dma/i82374.c | 3 ++-
hw/dma/i8257.c  | 4 ++--
hw/i386/pc.c| 3 ++-
hw/mips/mips_fulong2e.c | 3 ++-
hw/mips/mips_jazz.c | 3 ++-
hw/mips/mips_malta.c| 3 ++-
hw/sparc/sun4m.c| 4 
hw/sparc64/sun4u.c  | 4 
MAINTAINERS | 2 +-
11 files changed, 19 insertions(+), 18 deletions(-)
rename include/hw/{isa => dma}/i8257.h (86%)

 diff --git a/include/hw/isa/i8257.h b/include/hw/dma/i8257.h
 similarity index 86%
 rename from include/hw/isa/i8257.h
 rename to include/hw/dma/i8257.h
 index 88a2766a3f..2cab50bb6c 100644
 --- a/include/hw/isa/i8257.h
 +++ b/include/hw/dma/i8257.h
 @@ -1,6 +1,10 @@
#ifndef HW_I8257_H
#define HW_I8257_H
+#include "hw/hw.h"
 +#include "hw/isa/isa.h"
 +#include "exec/ioport.h"
 +
#define TYPE_I8257 "i8257"
  typedef struct I8257Regs {
 @@ -40,4 +44,6 @@ typedef struct I8257State {
PortioList portio_pageh;
} I8257State;
+void i8257_dma_init(ISABus *bus, bool high_page_enable);
 +
#endif
 diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
 index 95593408ef..b9dbab24b4 100644
 --- a/include/hw/isa/isa.h
 +++ b/include/hw/isa/isa.h
 @@ -151,6 +151,4 @@ static inline ISABus
 *isa_bus_from_device(ISADevice *d)
return ISA_BUS(qdev_get_parent_bus(DEVICE(d)));
}
-/* i8257.c */
 -void DMA_init(ISABus *bus, int high_page_enable);
#endif
 diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c
 index 6c0f975df0..83c87d92e0 100644
 --- a/hw/dma/i82374.c
 +++ b/hw/dma/i82374.c
 @@ -24,6 +24,7 @@
  #include "qemu/osdep.h"
#include "hw/isa/isa.h"
 +#include "hw/dma/i8257.h"
  #define TYPE_I82374 "i82374"
#define I82374(obj) OBJECT_CHECK(I82374State, (obj), TYPE_I82374)
 @@ -123,7 +124,7 @@ static void i82374_realize(DeviceState *dev, Error
 **errp)
portio_list_add(>port_list,
 isa_address_space_io(>parent_obj),
s->iobase);
-DMA_init(isa_bus_from_device(ISA_DEVICE(dev)), 1);
 +i8257_dma_init(isa_bus_from_device(ISA_DEVICE(dev)), true);
memset(s->commands, 0, sizeof(s->commands));
}
diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
 index bd23e893bf..52675e97c9 100644
 --- a/hw/dma/i8257.c
 +++ b/hw/dma/i8257.c
 @@ -24,7 +24,7 @@
#include "qemu/osdep.h"
#include "hw/hw.h"
#include "hw/isa/isa.h"
 -#include "hw/isa/i8257.h"
 +#include "hw/dma/i8257.h"
#include "qemu/main-loop.h"
#include "trace.h"
@@ -622,7 +622,7 @@ static void i8257_register_types(void)
  type_init(i8257_register_types)
-void DMA_init(ISABus *bus, int high_page_enable)
 +void i8257_dma_init(ISABus *bus, bool high_page_enable)
{
ISADevice *isa1, *isa2;
DeviceState *d;
 diff --git a/hw/i386/pc.c b/hw/i386/pc.c
 index 81364932d3..ec75b09a8f 100644
 --- a/hw/i386/pc.c
 +++ b/hw/i386/pc.c
 @@ -41,6 +41,7 @@
#include "elf.h"
#include "multiboot.h"
#include "hw/timer/mc146818rtc.h"
 +#include "hw/dma/i8257.h"
#include "hw/timer/i8254.h"
#include "hw/audio/pcspk.h"
#include "hw/pci/msi.h"
 @@ -1609,7 +1610,7 @@ void pc_basic_device_init(ISABus *isa_bus,
 qemu_irq *gsi,
port92_init(port92, a20_line[1]);
g_free(a20_line);
-DMA_init(isa_bus, 0);
 +i8257_dma_init(isa_bus, 0);
  for(i = 0; i < MAX_FD; i++) {
fd[i] = drive_get(IF_FLOPPY, 0, i);
 diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
 index dc77b55755..0545fcd899 100644
 --- a/hw/mips/mips_fulong2e.c
 +++ b/hw/mips/mips_fulong2e.c
 @@ -22,6 +22,7 @@
#include "qapi/error.h"
#include "hw/hw.h"
#include "hw/i386/pc.h"
 +#include "hw/dma/i8257.h"
#include "hw/char/serial.h"
#include "hw/char/parallel.h"
#include "hw/block/fdc.h"
 @@ -360,7 +361,7 @@ static void mips_fulong2e_init(MachineState
 

Re: [Qemu-devel] [PATCH] RISC-V: Fix isa string logic bug, use popcount to count bits

[Michael Clark]

On Sat, Mar 10, 2018 at 9:33 AM, Eric Blake  wrote:

> [resend, this time with proper cc's]
>
> On 03/09/2018 02:20 PM, Michael Clark wrote:
>
>> Cc: Palmer Dabbelt 
>> Cc: Peter Maydell 
>> Signed-off-by: Michael Clark 
>> ---
>>   target/riscv/cpu.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 4851890..f0d6d1d 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -391,7 +391,7 @@ static const TypeInfo riscv_cpu_type_info = {
>>   char *riscv_isa_string(RISCVCPU *cpu)
>>   {
>>   int i;
>> -size_t maxlen = 5 + ctz32(cpu->env.misa);
>> +size_t maxlen = 5 + __builtin_popcountll(cpu->env.misa);
>>   char *isa_string = g_new0(char, maxlen);
>>   snprintf(isa_string, maxlen, "rv%d", TARGET_LONG_BITS);
>>   for (i = 0; i < sizeof(riscv_exts); i++) {
>>
>
> I'd rather you used ctpop64() from host-utils.h, so we have a centralized
> place to change things just once in case we have to tweak the use of
> __builtin_popcount when targetting a different compiler.


Okay. I'll revise the patch...

Re: [Qemu-devel] [PATCH 0/8] tests: Run device-crash-test on "make check"

[no-reply]

Hi,

This series failed docker-quick@centos6 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20180309202827.12085-1-ehabk...@redhat.com
Subject: [Qemu-devel] [PATCH 0/8] tests: Run device-crash-test on "make check"

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
43e25ed8f6 tests: Run device-crash-test on "make check"
41b49d1214 device-crash-test: Don't print warnings in quiet mode
b693bd7177 device-crash-test: Use WARN for known crashes
c593d69f22 device-crash-test: Remove runnable-machine check
5340dee618 device-crash-test: New known crashes
0481610e19 device-crash-test: Accept machine=DEFAULT to test the default machine
183fe35b74 device-crash-test: Add examples to script documentation
6b368c2850 device-crash-test: Refactor loglevel configuration code

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-i0mjl1iv/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   centos6
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-i0mjl1iv/src'
  GEN 
/var/tmp/patchew-tester-tmp-i0mjl1iv/src/docker-src.2018-03-09-15.46.46.18385/qemu.tar
Cloning into 
'/var/tmp/patchew-tester-tmp-i0mjl1iv/src/docker-src.2018-03-09-15.46.46.18385/qemu.tar.vroot'...
done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 
'/var/tmp/patchew-tester-tmp-i0mjl1iv/src/docker-src.2018-03-09-15.46.46.18385/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered 
for path 'ui/keycodemapdb'
Cloning into 
'/var/tmp/patchew-tester-tmp-i0mjl1iv/src/docker-src.2018-03-09-15.46.46.18385/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'6b3d716e2b6472eb7189d3220552280ef3d832ce'
  COPYRUNNER
RUN test-quick in qemu:centos6 
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
bison-2.4.1-5.el6.x86_64
bzip2-devel-1.0.5-7.el6_0.x86_64
ccache-3.1.6-2.el6.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el6.x86_64
flex-2.5.35-9.el6.x86_64
gcc-4.4.7-18.el6.x86_64
gettext-0.17-18.el6.x86_64
git-1.7.1-9.el6_9.x86_64
glib2-devel-2.28.8-9.el6.x86_64
libepoxy-devel-1.2-3.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
librdmacm-devel-1.0.21-0.el6.x86_64
lzo-devel-2.03-3.1.el6_5.1.x86_64
make-3.81-23.el6.x86_64
mesa-libEGL-devel-11.0.7-4.el6.x86_64
mesa-libgbm-devel-11.0.7-4.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
spice-glib-devel-0.26-8.el6.x86_64
spice-server-devel-0.12.4-16.el6.x86_64
tar-1.23-15.el6_8.x86_64
vte-devel-0.25.1-9.el6.x86_64
xen-devel-4.6.6-2.el6.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=bison bzip2-devel ccache csnappy-devel flex g++
 gcc gettext git glib2-devel libepoxy-devel libfdt-devel
 librdmacm-devel lzo-devel make mesa-libEGL-devel 
mesa-libgbm-devel pixman-devel SDL-devel spice-glib-devel 
spice-server-devel tar vte-devel xen-devel zlib-devel
HOSTNAME=19c8ab59833a
MAKEFLAGS= -j8
J=8
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
TARGET_LIST=
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
FEATURES= dtc
DEBUG=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/tmp/qemu-test/install
No C++ compiler available; disabling C++ specific optional code
Install prefix/tmp/qemu-test/install
BIOS directory/tmp/qemu-test/install/share/qemu
firmware path /tmp/qemu-test/install/share/qemu-firmware
binary directory  /tmp/qemu-test/install/bin
library directory /tmp/qemu-test/install/lib
module directory  /tmp/qemu-test/install/lib/qemu
libexec directory /tmp/qemu-test/install/libexec
include directory /tmp/qemu-test/install/include
config directory  /tmp/qemu-test/install/etc
local state directory   /tmp/qemu-test/install/var
Manual directory  /tmp/qemu-test/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /tmp/qemu-test/src
GIT binarygit
GIT submodules
C compilercc
Host C compiler   cc
C++ compiler  
Objective-C compiler cc
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g 
QEMU_CFLAGS   -I/usr/include/pixman-1   -I$(SRC_PATH)/dtc/libfdt -pthread 
-I/usr/include/glib-2.0 

Re: [Qemu-devel] [PATCH 07/10] linux-user: init_guest_space: Clean up control flow a bit

[Laurent Vivier]

Le 28/12/2017 à 19:08, Luke Shumaker a écrit :
> From: Luke Shumaker 
> 
> Instead of doing
> 
> if (check1) {
> if (check2) {
>success;
> }
> }
> 
> retry;
> 
> Do a clearer
> 
> if (!check1) {
>goto try_again;
> }
> 
> if (!check2) {
>goto try_again;
> }
> 
> success;
> 
> try_again:
> retry;
> 
> Besides being clearer, this makes it easier to insert more checks that
> need to trigger a retry on check failure, or rearrange them, or anything
> like that.
> 
> Because some indentation is changing, "ignore space change" may be useful
> for viewing this patch.
> 
> Signed-off-by: Luke Shumaker 
> ---
>  linux-user/elfload.c | 34 +++---
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index b560f5d6fe..5c0ad65611 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -1906,24 +1906,28 @@ unsigned long init_guest_space(unsigned long 
> host_start,
>  }
>  
>  /* Check to see if the address is valid.  */
> -if (!host_start || aligned_start == current_start) {
> +if (host_start && aligned_start != current_start) {
> +goto try_again;
> +}
> +
>  #if defined(TARGET_ARM) && !defined(TARGET_AARCH64)
> -/* On 32-bit ARM, we need to also be able to map the commpage.  
> */
> -int valid = init_guest_commpage(aligned_start - guest_start,
> -aligned_size + guest_start);
> -if (valid == 1) {
> -break;
> -} else if (valid == -1) {
> -munmap((void *)real_start, real_size);
> -return (unsigned long)-1;
> -}
> -/* valid == 0, so try again. */
> -#else
> -/* On other architectures, whatever we have here is fine.  */
> -break;
> -#endif
> +/* On 32-bit ARM, we need to also be able to map the commpage.  */
> +int valid = init_guest_commpage(aligned_start - guest_start,
> +aligned_size + guest_start);
> +if (valid == -1) {
> +munmap((void *)real_start, real_size);
> +return (unsigned long)-1;
> +} else if (valid == -1) {

I think it should be "if (valid == 0)" here.

> +goto try_again;
>  }
> +#endif
> +
> +/* If nothing has said `return -1` or `goto try_again` yet,
> + * then the address we have is good.
> + */
> +break;
>  
> +try_again:
>  /* That address didn't work.  Unmap and try a different one.
>   * The address the host picked because is typically right at
>   * the top of the host address space and leaves the guest with
> 

Thanks,
Laurent

[Qemu-devel] [Bug 1754295] Re: Incorrect en-us keymap in QEMU 2.11

[Evangelos Foutras via Qemu-devel]

*** This bug is a duplicate of bug 1738283 ***
https://bugs.launchpad.net/bugs/1738283

** This bug has been marked a duplicate of bug 1738283
   'Less than' (<), 'more than' (>), and 'pipe' (|) can't be typed via VNC

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1754295

Title:
  Incorrect en-us keymap in QEMU 2.11

Status in QEMU:
  New

Bug description:
  I'm using the latest Arch Linux installation ISO as a live system and
  start QEMU with the following command:

  $ qemu-system-x86_64 -enable-kvm -boot d -cdrom
  ~/isos/archlinux-2018.03.01-x86_64.iso -m 512 -vnc :0 -k en-us

  Then I use Vinagre to connect to the guest system, boot the default
  bootloader option, and type the character '<' at the command prompt.
  The guest prints the character '>' instead of the '<' I typed.

  I believe this is caused by the updated en-us keymap in QEMU 2.11. [1]

  If I start QEMU with `-k en-gb` (or without the -k switch at all), I
  can type '<' and get the same character to appear on the guest's
  command line. The issue happens with the updated en-us keymap. It is
  also fixed if I replace /usr/share/qemu/keymaps/en-us with the old
  keymap file (before commit a7815faf).

  This problem was originally reported against Packer because we were
  seeing '>' characters in place of '<' when using it with the QEMU
  builder. [2]

  [1] 
https://github.com/qemu/qemu/commit/a7815faffb2bd594b92aa3542d7b799cc89c5414
  [2] https://github.com/hashicorp/packer/issues/5769

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1754295/+subscriptions

Re: [Qemu-devel] [PATCH] RISC-V: Fix isa string logic bug, use popcount to count bits

[Eric Blake]

[resend, this time with proper cc's]

On 03/09/2018 02:20 PM, Michael Clark wrote:

Cc: Palmer Dabbelt 
Cc: Peter Maydell 
Signed-off-by: Michael Clark 
---
  target/riscv/cpu.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 4851890..f0d6d1d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -391,7 +391,7 @@ static const TypeInfo riscv_cpu_type_info = {
  char *riscv_isa_string(RISCVCPU *cpu)
  {
  int i;
-size_t maxlen = 5 + ctz32(cpu->env.misa);
+size_t maxlen = 5 + __builtin_popcountll(cpu->env.misa);
  char *isa_string = g_new0(char, maxlen);
  snprintf(isa_string, maxlen, "rv%d", TARGET_LONG_BITS);
  for (i = 0; i < sizeof(riscv_exts); i++) {


I'd rather you used ctpop64() from host-utils.h, so we have a 
centralized place to change things just once in case we have to tweak 
the use of __builtin_popcount when targetting a different compiler.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] RISC-V: Fix isa string logic bug, use popcount to count bits

[Eric Blake]

On 03/09/2018 02:20 PM, Michael Clark wrote:

Cc: Palmer Dabbelt 
Cc: Peter Maydell 
Signed-off-by: Michael Clark 
---
  target/riscv/cpu.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 4851890..f0d6d1d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -391,7 +391,7 @@ static const TypeInfo riscv_cpu_type_info = {
  char *riscv_isa_string(RISCVCPU *cpu)
  {
  int i;
-size_t maxlen = 5 + ctz32(cpu->env.misa);
+size_t maxlen = 5 + __builtin_popcountll(cpu->env.misa);


I'd rather you used ctpop64() from host-utils.h, so we have a 
centralized place to change things just once in case we have to tweak 
the use of __builtin_popcount when targetting a different compiler.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH 8/8] tests: Run device-crash-test on "make check"

[Eduardo Habkost]

Run a subset of tests using device-crash-test on "make check", to
help us catch device crashes earlier.

This also add a "check-device-crash-test-full" rule, that will
check all machine/device combinations.

Signed-off-by: Eduardo Habkost 
---
 tests/Makefile.include | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index ef9b88c369..0ba641b8d4 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -942,6 +942,17 @@ check-decodetree:
   ./check.sh "$(PYTHON)" "$(SRC_PATH)/scripts/decodetree.py", \
   TEST, decodetree.py)
 
+CRASH_TEST = $(SRC_PATH)/scripts/device-crash-test
+CRASH_TEST_OPTIONS = $(if $(V),-v,-q)
+CRASH_TEST_BINARIES = $(foreach TARGET,$(TARGETS), 
$(TARGET)-softmmu/qemu-system-$(TARGET))
+
+.PHONY: check-device-crash-quick
+check-device-crash-quick:
+   $(CRASH_TEST) $(CRASH_TEST_OPTIONS) -t machine=DEFAULT accel=tcg -- 
$(CRASH_TEST_BINARIES)
+
+check-device-crash-full:
+   $(CRASH_TEST) $(CRASH_TEST_OPTIONS) -F $(CRASH_TEST_BINARIES)
+
 # Consolidated targets
 
 .PHONY: check-qapi-schema check-qtest check-unit check check-clean
@@ -950,7 +961,7 @@ check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
 check-unit: $(patsubst %,check-%, $(check-unit-y))
 check-speed: $(patsubst %,check-%, $(check-speed-y))
 check-block: $(patsubst %,check-%, $(check-block-y))
-check: check-qapi-schema check-unit check-qtest check-decodetree
+check: check-qapi-schema check-unit check-qtest check-decodetree 
check-device-crash-quick
 check-clean:
$(MAKE) -C tests/tcg clean
rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
-- 
2.14.3

[Qemu-devel] [PULL] RISC-V: Fix riscv_isa_string g_new0 size calculation

[Michael Clark]

The following changes since commit e4ae62b802cec437f877f2cadc4ef059cc0eca76:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into 
staging (2018-03-09 17:28:16 +)

are available in the git repository at:

  https://github.com/michaeljclark/riscv-qemu.git fix-riscv-isa-string

for you to fetch changes up to 32b3a5716062f805a42e1cae1df95f2f869a78c1:

  RISC-V: Fix isa string logic bug, use popcount to count bits (2018-03-10 
09:17:29 +1300)


Michael Clark (1):
  RISC-V: Fix isa string logic bug, use popcount to count bits

 target/riscv/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Re: [Qemu-devel] [PATCH 05/10] linux-user: init_guest_space: Clarify page alignment logic

[Laurent Vivier]

Le 28/12/2017 à 19:08, Luke Shumaker a écrit :
> From: Luke Shumaker 
> 
> There are 3 parts to this change:
>  - Add a comment showing the relative sizes and positions of the blocks of
>memory
>  - introduce and use new aligned_{start,size} instead of adjusting
>real_{start_size}
>  - When we clean up (on failure), munmap(real_start, real_size) instead of
>munmap(aligned_start, aligned_size).  It *shouldn't* make any
>difference, but I will admit that this does mean we are making the
>syscall with different values, so this isn't quite a no-op patch.
> 
> Signed-off-by: Luke Shumaker 
> ---
>  linux-user/elfload.c | 43 +--
>  1 file changed, 33 insertions(+), 10 deletions(-)
> 

Applied to my 'linux-user-for-2.12' branch.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 06/10] linux-user: init_guest_commpage: Add a comment about size check

[Laurent Vivier]

Le 28/12/2017 à 19:08, Luke Shumaker a écrit :
> From: Luke Shumaker 
> 
> Signed-off-by: Luke Shumaker 
> ---
>  linux-user/elfload.c | 5 +
>  1 file changed, 5 insertions(+)

Applied to my 'linux-user-for-2.12' branch.

Thanks,
Laurent

[Qemu-devel] [PATCH 6/8] device-crash-test: Use WARN for known crashes

[Eduardo Habkost]

We're going to run device-crash-test on "make check", so it's
better to make the script less noisy on known errors.

Signed-off-by: Eduardo Habkost 
---
 scripts/device-crash-test | 44 ++--
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index f406890519..ed9a552fa1 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -228,28 +228,28 @@ ERROR_WHITELIST = [
 {'exitcode':1, 'loglevel':logging.INFO},
 
 # KNOWN CRASHES:
-# Known crashes will generate error messages, but won't be fatal.
-# Those entries must be removed once we fix the crashes.
-{'exitcode':-6, 'log':r"Device 'serial0' is in use", 
'loglevel':logging.ERROR},
-{'exitcode':-6, 'log':r"qemu_net_client_setup: Assertion `!peer->peer' 
failed", 'loglevel':logging.ERROR},
-{'exitcode':-6, 'log':r'RAMBlock "[\w.-]+" already registered', 
'loglevel':logging.ERROR},
-{'exitcode':-6, 'log':r"find_ram_offset: Assertion `size != 0' failed.", 
'loglevel':logging.ERROR},
-{'exitcode':-6, 'log':r"add_cpreg_to_hashtable: code should not be 
reached", 'loglevel':logging.ERROR},
-{'exitcode':-6, 'log':r"qemu_alloc_display: Assertion `surface->image != 
NULL' failed", 'loglevel':logging.ERROR},
-{'exitcode':-6, 'log':r"Unexpected error in 
error_set_from_qdev_prop_error", 'loglevel':logging.ERROR},
-{'exitcode':-6, 'log':r"Object .* is not an instance of type 
spapr-machine", 'loglevel':logging.ERROR},
-{'exitcode':-6, 'log':r"Object .* is not an instance of type 
generic-pc-machine", 'loglevel':logging.ERROR},
-{'exitcode':-6, 'log':r"Object .* is not an instance of type e500-ccsr", 
'loglevel':logging.ERROR},
-{'exitcode':-6, 'log':r"vmstate_register_with_alias_id: Assertion 
`!se->compat \|\| se->instance_id == 0' failed", 'loglevel':logging.ERROR},
-{'exitcode':-6, 'device':'isa-fdc', 'loglevel':logging.ERROR, 
'expected':True},
-{'exitcode':-11, 'device':'gus', 'loglevel':logging.ERROR, 
'expected':True},
-{'exitcode':-11, 'device':'isa-serial', 'loglevel':logging.ERROR, 
'expected':True},
-{'exitcode':-11, 'device':'sb16', 'loglevel':logging.ERROR, 
'expected':True},
-{'exitcode':-11, 'device':'cs4231a', 'loglevel':logging.ERROR, 
'expected':True},
-{'exitcode':-11, 'machine':'isapc', 'device':'.*-iommu', 
'loglevel':logging.ERROR, 'expected':True},
-{'exitcode':-11, 'device':'mioe3680_pci', 'loglevel':logging.ERROR, 
'expected':True},
-{'exitcode':-11, 'device':'pcm3680_pci', 'loglevel':logging.ERROR, 
'expected':True},
-{'exitcode':-11, 'device':'kvaser_pci', 'loglevel':logging.ERROR, 
'expected':True},
+# Known crashes will generate warn messages
+# FIXME: all those crashes MUST be fixed, and all these entries MUST be 
removed
+{'exitcode':-6, 'log':r"Device 'serial0' is in use", 
'loglevel':logging.WARN},
+{'exitcode':-6, 'log':r"qemu_net_client_setup: Assertion `!peer->peer' 
failed", 'loglevel':logging.WARN},
+{'exitcode':-6, 'log':r'RAMBlock "[\w.-]+" already registered', 
'loglevel':logging.WARN},
+{'exitcode':-6, 'log':r"find_ram_offset: Assertion `size != 0' failed.", 
'loglevel':logging.WARN},
+{'exitcode':-6, 'log':r"add_cpreg_to_hashtable: code should not be 
reached", 'loglevel':logging.WARN},
+{'exitcode':-6, 'log':r"qemu_alloc_display: Assertion `surface->image != 
NULL' failed", 'loglevel':logging.WARN},
+{'exitcode':-6, 'log':r"Unexpected error in 
error_set_from_qdev_prop_error", 'loglevel':logging.WARN},
+{'exitcode':-6, 'log':r"Object .* is not an instance of type 
spapr-machine", 'loglevel':logging.WARN},
+{'exitcode':-6, 'log':r"Object .* is not an instance of type 
generic-pc-machine", 'loglevel':logging.WARN},
+{'exitcode':-6, 'log':r"Object .* is not an instance of type e500-ccsr", 
'loglevel':logging.WARN},
+{'exitcode':-6, 'log':r"vmstate_register_with_alias_id: Assertion 
`!se->compat \|\| se->instance_id == 0' failed", 'loglevel':logging.WARN},
+{'exitcode':-6, 'device':'isa-fdc', 'loglevel':logging.WARN, 
'expected':True},
+{'exitcode':-11, 'device':'gus', 'loglevel':logging.WARN, 'expected':True},
+{'exitcode':-11, 'device':'isa-serial', 'loglevel':logging.WARN, 
'expected':True},
+{'exitcode':-11, 'device':'sb16', 'loglevel':logging.WARN, 
'expected':True},
+{'exitcode':-11, 'device':'cs4231a', 'loglevel':logging.WARN, 
'expected':True},
+{'exitcode':-11, 'machine':'isapc', 'device':'.*-iommu', 
'loglevel':logging.WARN, 'expected':True},
+{'exitcode':-11, 'device':'mioe3680_pci', 'loglevel':logging.WARN, 
'expected':True},
+{'exitcode':-11, 'device':'pcm3680_pci', 'loglevel':logging.WARN, 
'expected':True},
+{'exitcode':-11, 'device':'kvaser_pci', 'loglevel':logging.WARN, 
'expected':True},
 
 # everything else (including SIGABRT and SIGSEGV) will be a fatal error:
 {'exitcode':None, 

[Qemu-devel] [PATCH 3/8] device-crash-test: Accept machine=DEFAULT to test the default machine

[Eduardo Habkost]

This will be useful for running a smaller test set on
"make check", instead of testing every single machine-type/device
combination.

Signed-off-by: Eduardo Habkost 
---
 scripts/device-crash-test | 29 +++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 364c779cdb..632b128e44 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -41,6 +41,11 @@ Test a single QEMU binary:
 
 device-crash-test /path/to/qemu/binary
 
+Test all QEMU binaries found in the current directory, using only the default
+machine-type:
+
+device-crash-test -t machine=DEFAULT
+
 """
 
 import sys
@@ -357,7 +362,7 @@ class QemuBinaryInfo(object):
 # there's no way to query DeviceClass::user_creatable using QMP,
 # so use 'info qdm':
 self.no_user_devs = set([d['name'] for d in infoQDM(vm, ) if 
d['no-user']])
-self.machines = list(m['name'] for m in 
vm.command('query-machines'))
+self.machines = vm.command('query-machines')
 self.user_devs = self.alldevs.difference(self.no_user_devs)
 self.kvm_available = vm.command('query-kvm')['enabled']
 finally:
@@ -390,6 +395,19 @@ class QemuBinaryInfo(object):
 self._machine_info[machine] = mi
 return mi
 
+def defaultMachine(self):
+"""Default machine-type for the QEMU binary
+
+If no default machine-type is returned by query-machines, return the
+first machine-type in the list.
+"""
+machines = self.machines
+defmachine = [m['name'] for m in self.machines if m.get('is-default')]
+if defmachine:
+return defmachine[0]
+else:
+return self.machines[0]['name']
+
 
 BINARY_INFO = {}
 
@@ -455,7 +473,7 @@ def accelsToTest(args, testcase):
 
 
 def machinesToTest(args, testcase):
-return getBinaryInfo(args, testcase['binary']).machines
+return [m['name'] for m in getBinaryInfo(args, 
testcase['binary']).machines]
 
 
 def devicesToTest(args, testcase):
@@ -580,6 +598,13 @@ def main():
 return 1
 
 for t in casesToTest(args, tc):
+
+# expand some test case variables to their actual values before
+# using them:
+# "-t machine=DEFAULT" can be used to use the default machine-type
+if t['machine'] == 'DEFAULT':
+t['machine'] = getBinaryInfo(args, t['binary']).defaultMachine()
+
 logger.info("running test case: %s", formatTestCase(t))
 total += 1
 
-- 
2.14.3

[Qemu-devel] [PATCH 5/8] device-crash-test: Remove runnable-machine check

[Eduardo Habkost]

This optimization to skip tests if the machine is not runnable is
unreliable and makes the script ignore actual QEMU crashes, so
we're safer simply removing it.

Signed-off-by: Eduardo Habkost 
---
 scripts/device-crash-test | 31 +--
 1 file changed, 1 insertion(+), 30 deletions(-)

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index b9a7470efc..f406890519 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -372,33 +372,6 @@ class QemuBinaryInfo(object):
 finally:
 vm.shutdown()
 
-def machineInfo(self, machine):
-"""Query for information on a specific machine-type
-
-Results are cached internally, in case the same machine-
-type is queried multiple times.
-"""
-if machine in self._machine_info:
-return self._machine_info[machine]
-
-mi = {}
-args = ['-S', '-machine', '%s' % (machine)]
-dbg("querying machine info for binary=%s machine=%s", self.binary, 
machine)
-vm = QEMUMachine(binary=self.binary, args=args)
-try:
-vm.launch()
-mi['runnable'] = True
-except KeyboardInterrupt:
-raise
-except:
-dbg("exception trying to run binary=%s machine=%s", self.binary, 
machine, exc_info=sys.exc_info())
-dbg("log: %r", vm.get_log())
-mi['runnable'] = False
-
-vm.shutdown()
-self._machine_info[machine] = mi
-return mi
-
 def defaultMachine(self):
 """Default machine-type for the QEMU binary
 
@@ -613,9 +586,7 @@ def main():
 total += 1
 
 expected_match = findExpectedResult(t)
-if (args.quick and
-(expected_match or
- not getBinaryInfo(args, 
t['binary']).machineInfo(t['machine'])['runnable'])):
+if args.quick and expected_match:
 dbg("skipped: %s", formatTestCase(t))
 skipped += 1
 continue
-- 
2.14.3

[Qemu-devel] [PATCH 7/8] device-crash-test: Don't print warnings in quiet mode

[Eduardo Habkost]

This will be useful for hiding known crashes on "make check".

Signed-off-by: Eduardo Habkost 
---
 scripts/device-crash-test | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index ed9a552fa1..36194e4347 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -533,7 +533,7 @@ def main():
 dest='loglevel', const=logging.INFO,
 help='verbose output')
 parser.add_argument('-q', '--quiet',action='store_const',
-dest='loglevel', const=logging.WARN,
+dest='loglevel', const=logging.ERROR,
 help='non-verbose output')
 parser.add_argument('-r', '--random', type=int, metavar='COUNT',
 help='run a random sample of COUNT test cases',
-- 
2.14.3

[Qemu-devel] [PATCH 2/8] device-crash-test: Add examples to script documentation

[Eduardo Habkost]

Add simple examples for common use cases.

Signed-off-by: Eduardo Habkost 
---
 scripts/device-crash-test | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index c6a7875357..364c779cdb 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -22,6 +22,25 @@
 """
 Run QEMU with all combinations of -machine and -device types,
 check for crashes and unexpected errors.
+
+Example usage:
+
+Test all QEMU binaries found in the current directory, with all
+machine-type/device combinations, but skip the combinations that are expected 
to
+fail:
+
+device-crash-test
+
+Test all QEMU binaries found in the current directory, with all
+machine-type/device combinations, including the combinations that are expected
+to fail:
+
+device-crash-test -F
+
+Test a single QEMU binary:
+
+device-crash-test /path/to/qemu/binary
+
 """
 
 import sys
-- 
2.14.3

[Qemu-devel] [PATCH 4/8] device-crash-test: New known crashes

[Eduardo Habkost]

We are not running the script on "make check" yet, and additional
bugs were introduced recently in the tree.

Whitelist the new crashes while we investigate, to allow us to
run device-crash-test on "make check" as soon as possible to
prevent new bugs.

Cc: Pavel Pisa 
Cc: John Snow 
Signed-off-by: Eduardo Habkost 
---
 scripts/device-crash-test | 4 
 1 file changed, 4 insertions(+)

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 632b128e44..b9a7470efc 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -241,11 +241,15 @@ ERROR_WHITELIST = [
 {'exitcode':-6, 'log':r"Object .* is not an instance of type 
generic-pc-machine", 'loglevel':logging.ERROR},
 {'exitcode':-6, 'log':r"Object .* is not an instance of type e500-ccsr", 
'loglevel':logging.ERROR},
 {'exitcode':-6, 'log':r"vmstate_register_with_alias_id: Assertion 
`!se->compat \|\| se->instance_id == 0' failed", 'loglevel':logging.ERROR},
+{'exitcode':-6, 'device':'isa-fdc', 'loglevel':logging.ERROR, 
'expected':True},
 {'exitcode':-11, 'device':'gus', 'loglevel':logging.ERROR, 
'expected':True},
 {'exitcode':-11, 'device':'isa-serial', 'loglevel':logging.ERROR, 
'expected':True},
 {'exitcode':-11, 'device':'sb16', 'loglevel':logging.ERROR, 
'expected':True},
 {'exitcode':-11, 'device':'cs4231a', 'loglevel':logging.ERROR, 
'expected':True},
 {'exitcode':-11, 'machine':'isapc', 'device':'.*-iommu', 
'loglevel':logging.ERROR, 'expected':True},
+{'exitcode':-11, 'device':'mioe3680_pci', 'loglevel':logging.ERROR, 
'expected':True},
+{'exitcode':-11, 'device':'pcm3680_pci', 'loglevel':logging.ERROR, 
'expected':True},
+{'exitcode':-11, 'device':'kvaser_pci', 'loglevel':logging.ERROR, 
'expected':True},
 
 # everything else (including SIGABRT and SIGSEGV) will be a fatal error:
 {'exitcode':None, 'fatal':True, 'loglevel':logging.FATAL},
-- 
2.14.3

[Qemu-devel] [PATCH 0/8] tests: Run device-crash-test on "make check"

[Eduardo Habkost]

So, we're back to that time when we need to add new known crashes
to device-crash-test, because we haven't been running it in a
while and new bugs were introduced.

To help prevent this from happening again, this series adds new
rules to validate devices against the default machine-type on
"make check".  This should help us catch more obvious bugs, at
least.

Peter, is it possible to get this tested inside your system that
validates every pull request, just to see if it won't generate
any unexpected warning, error, or false positive?

Eduardo Habkost (8):
  device-crash-test: Refactor loglevel configuration code
  device-crash-test: Add examples to script documentation
  device-crash-test: Accept machine=DEFAULT to test the default machine
  device-crash-test: New known crashes
  device-crash-test: Remove runnable-machine check
  device-crash-test: Use WARN for known crashes
  device-crash-test: Don't print warnings in quiet mode
  tests: Run device-crash-test on "make check"

 scripts/device-crash-test | 135 ++
 tests/Makefile.include|  13 -
 2 files changed, 88 insertions(+), 60 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH 1/8] device-crash-test: Refactor loglevel configuration code

[Eduardo Habkost]

Use action='store_const', dest='loglevel' to represent the effect
of each option more clearly.

This will also make the last option in the command-line override
the previous ones (e.g.: "-d -q").

Signed-off-by: Eduardo Habkost 
---
 scripts/device-crash-test | 22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 7417177ebb..c6a7875357 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -484,7 +484,7 @@ def casesToTest(args, testcase):
 if args.random:
 cases = list(cases)
 cases = random.sample(cases, min(args.random, len(cases)))
-if args.debug:
+if logger.isEnabledFor(logging.DEBUG):
 cases = list(cases)
 dbg("%d test cases to test", len(cases))
 if args.shuffle:
@@ -511,11 +511,15 @@ def main():
 parser.add_argument('-t', metavar='KEY=VALUE', nargs='*',
 help="Limit test cases to KEY=VALUE",
 action='append', dest='testcases', default=[])
-parser.add_argument('-d', '--debug', action='store_true',
+parser.set_defaults(loglevel=logging.INFO)
+parser.add_argument('-d', '--debug',action='store_const',
+dest='loglevel', const=logging.DEBUG,
 help='debug output')
-parser.add_argument('-v', '--verbose', action='store_true', default=True,
+parser.add_argument('-v', '--verbose',action='store_const',
+dest='loglevel', const=logging.INFO,
 help='verbose output')
-parser.add_argument('-q', '--quiet', dest='verbose', action='store_false',
+parser.add_argument('-q', '--quiet',action='store_const',
+dest='loglevel', const=logging.WARN,
 help='non-verbose output')
 parser.add_argument('-r', '--random', type=int, metavar='COUNT',
 help='run a random sample of COUNT test cases',
@@ -536,13 +540,7 @@ def main():
 help='QEMU binary to run')
 args = parser.parse_args()
 
-if args.debug:
-lvl = logging.DEBUG
-elif args.verbose:
-lvl = logging.INFO
-else:
-lvl = logging.WARN
-logging.basicConfig(stream=sys.stdout, level=lvl, format='%(levelname)s: 
%(message)s')
+logging.basicConfig(stream=sys.stdout, level=args.loglevel, 
format='%(levelname)s: %(message)s')
 
 fatal_failures = []
 wl_stats = {}
@@ -599,7 +597,7 @@ def main():
 if skipped:
 logger.info("Skipped %d test cases", skipped)
 
-if args.debug:
+if logger.isEnabledFor(logging.DEBUG):
 stats = sorted([(len(wl_stats.get(i, [])), wl) for i, wl in 
enumerate(ERROR_WHITELIST)])
 for count, wl in stats:
 dbg("whitelist entry stats: %d: %r", count, wl)
-- 
2.14.3

Re: [Qemu-devel] [PATCH 04/10] linux-user: init_guest_space: Correctly handle guest_start in commpage initialization

[Laurent Vivier]

Le 28/12/2017 à 19:08, Luke Shumaker a écrit :
> From: Luke Shumaker 
> 
> init_guest_commpage  needs to check if the mapped space, which ends at
> real_start+real_size overlaps with where it needs to put the commpage,
> which is (assuming sane qemu_host_page_size) guest_base + 0x000, where
> guest_base is real_start - guest_start.
> 
> [guest_base][   0x  ][commpage]
> [guest_base][guest_start][real_size] [commpage]
> [   real_start  ][real_size] [commpage]
> ^
>  fail if this gap < 0
> 
> Since init_guest_commpage wants to do everything relative to guest_base
> (rather than real_start), it obviously needs to be comparing 0x
> against guest_start+real_size, not just real_size.
> 
> This bug has been present since 806d102141b99d4f1e55a97d68b7ea8c8ba3129f in
> 2012, but guest_start is usually 0, and prior to v2.11 real_size was
> usually much smaller than 0x, so it was uncommon for it to have
> made a difference.
> 
> Signed-off-by: Luke Shumaker 
> ---
>  linux-user/elfload.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to my 'linux-user-for-2.12' branch.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 6/6] qemu-iotests: Test luks QMP image creation

[Eric Blake]

On 03/09/2018 11:27 AM, Kevin Wolf wrote:

Signed-off-by: Kevin Wolf 
---
  tests/qemu-iotests/208   | 211 +++
  tests/qemu-iotests/208.out   | 136 
  tests/qemu-iotests/common.rc |   2 +-
  tests/qemu-iotests/group |   1 +
  4 files changed, 349 insertions(+), 1 deletion(-)
  create mode 100755 tests/qemu-iotests/208
  create mode 100644 tests/qemu-iotests/208.out


I've seen another patch using 208 - someone gets to renumber ;)


+# creator
+owner=kw...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1   # failure is the default!


Whatever happened to Jeff's efforts to reduce pointless boilerplate?



+function do_run_qemu()
+{
+echo Testing: "$@"
+$QEMU -nographic -qmp stdio -serial none "$@"


Is -nodefaults better than -serial none?

At any rate, my comments are trivial, whether or not you do something 
about them, so


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 02/10] linux-user: Rename validate_guest_space => init_guest_commpage

[Laurent Vivier]

Le 28/12/2017 à 19:08, Luke Shumaker a écrit :
> From: Luke Shumaker 
> 
> init_guest_commpage is a much more honest description of what the function
> does.  validate_guest_space not only suggests that the function has no
> side-effects, but also introduces confusion as to why it is only needed on
> 32-bit ARM targets.
> 
> Signed-off-by: Luke Shumaker 
> ---
>  linux-user/elfload.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 

Applied to my 'linux-user-for-2.12' branch.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 03/10] linux-user: init_guest_space: Clean up if we can't initialize the commpage

[Laurent Vivier]

Le 28/12/2017 à 19:08, Luke Shumaker a écrit :
> From: Luke Shumaker 
> 
> We'll just exit with an error anyway, so it doesn't really matter, but it
> is cleaned up in all of the other places were we error out.
> 
> Signed-off-by: Luke Shumaker 
> ---
>  linux-user/elfload.c | 1 +
>  1 file changed, 1 insertion(+)

Applied to my 'linux-user-for-2.12' branch.

Thanks,
Laurent

[Qemu-devel] [PATCH] RISC-V: Fix isa string logic bug, use popcount to count bits

[Michael Clark]

Cc: Palmer Dabbelt 
Cc: Peter Maydell 
Signed-off-by: Michael Clark 
---
 target/riscv/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 4851890..f0d6d1d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -391,7 +391,7 @@ static const TypeInfo riscv_cpu_type_info = {
 char *riscv_isa_string(RISCVCPU *cpu)
 {
 int i;
-size_t maxlen = 5 + ctz32(cpu->env.misa);
+size_t maxlen = 5 + __builtin_popcountll(cpu->env.misa);
 char *isa_string = g_new0(char, maxlen);
 snprintf(isa_string, maxlen, "rv%d", TARGET_LONG_BITS);
 for (i = 0; i < sizeof(riscv_exts); i++) {
-- 
2.7.0

Re: [Qemu-devel] [patches] Re: [PULL] RISC-V QEMU Port Submission v8.2

[Michael Clark]

On Sat, Mar 10, 2018 at 9:11 AM, Michael Clark  wrote:

>
>
> On Sat, Mar 10, 2018 at 5:49 AM, Peter Maydell 
> wrote:
>
>> On 9 March 2018 at 14:28, Peter Maydell  wrote:
>> > NB: there was a test failure on OpenBSD host:
>> >
>> > TEST: tests/qom-test... (pid=64016)
>> >   /riscv32/qom/spike_v1.9.1:
>>  **
>> > ERROR:/home/qemu/tests/qom-test.c:64:test_properties: assertion
>> > failed: (qdict_haskey(response, "return"))
>> > FAIL
>> >
>> > but this seems to have been intermittent -- it was only on that one
>> > host, and I reran the test suite there and it passed fine the second
>> > time. So it may be nothing to do with your code; we'll see if it
>> > comes up again.
>>
>> On a later test run I got this different one; openbsd again:
>>
>> TEST: tests/test-hmp... (pid=45236)
>>   /riscv32/hmp/spike_v1.9.1:   **
>> ERROR:/home/qemu/qom/object.c:488:object_new_with_type: assertion
>> failed: (type != NULL)
>> Broken pipe
>> FAIL
>>
>> My current best theory is that OpenBSD libc's memory allocator
>> happens to be more sensitive to a memory corruption bug in the risc
>> code, resulting in intermittent failures if the allocations happen
>> to come out the wrong way. You do have at least one invalid-write
>> off the end of a block according to valgrind:
>>
>> ==17441== Invalid write of size 1
>> ==17441==at 0x26517F: riscv_isa_string (cpu.c:399)
>> ==17441==by 0x25C14D: create_fdt (spike.c:125)
>> ==17441==by 0x25C14D: spike_v1_10_0_board_init (spike.c:199)
>> ==17441==by 0x2CCE0A: machine_run_board_init (machine.c:807)
>> ==17441==by 0x1BFF28: main (vl.c:4597)
>> ==17441==  Address 0x3055c425 is 0 bytes after a block of size 5 alloc'd
>> ==17441==at 0x4C2FB55: calloc (in
>> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==17441==by 0x70C8770: g_malloc0 (in
>> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4800.2)
>> ==17441==by 0x26511E: riscv_isa_string (cpu.c:395)
>> ==17441==by 0x25C14D: create_fdt (spike.c:125)
>> ==17441==by 0x25C14D: spike_v1_10_0_board_init (spike.c:199)
>> ==17441==by 0x2CCE0A: machine_run_board_init (machine.c:807)
>> ==17441==by 0x1BFF28: main (vl.c:4597)
>>
>> If you can prioritise a patch that fixes the bug in riscv_isa_string()
>> I'll apply that and hopefully these intermittent failures will go away.
>>
>
> I'm looking at this right now.
>

It's a glaringly obvious logic bug. The use of the wrong bit manipulation
instrinsic. I just sent a patch.

Re: [Qemu-devel] [PATCH 5/6] luks: Catch integer overflow for huge sizes

[Eric Blake]

On 03/09/2018 11:27 AM, Kevin Wolf wrote:

When you request an image size close to UINT64_MAX, the addition of the
crypto header may cause an integer overflow. Catch it instead of
silently truncating the image size.

Signed-off-by: Kevin Wolf 
---
  block/crypto.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/block/crypto.c b/block/crypto.c
index 4908d8627f..1b46519c53 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -102,6 +102,11 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
  {
  struct BlockCryptoCreateData *data = opaque;
  
+if (headerlen > UINT64_MAX - data->size) {


INT64_MAX, please.  We are further bounded by having to fit within off_t 
(signed) rather than uint64_t.



+error_setg(errp, "The requested file size is too large");
+return -EFBIG;
+}
+
  /* User provided size should reflect amount of space made
   * available to the guest, so we must take account of that
   * which will be used by the crypto header



--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 01/10] linux-user: Use #if to only call validate_guest_space for 32-bit ARM target

[Laurent Vivier]

Le 28/12/2017 à 19:08, Luke Shumaker a écrit :
> From: Luke Shumaker 
> 
> Instead of defining a bogus validate_guest_space that always returns 1 on
> targets other than 32-bit ARM, use #if blocks to only call it on 32-bit ARM
> targets.  This makes the "normal" flow control clearer.
> 
> Signed-off-by: Luke Shumaker 
> ---
>  linux-user/elfload.c | 21 +
>  1 file changed, 9 insertions(+), 12 deletions(-)

With the change request by Peter (condition "!= 1"),
applied to my 'linux-user-for-2.12' branch.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 4/6] luks: Turn invalid assertion into check

[Eric Blake]

On 03/09/2018 11:27 AM, Kevin Wolf wrote:

The .bdrv_getlength implementation of the crypto block driver asserted
that the payload offset isn't after EOF. This is an invalid assertion to
make as the image file could be corrupted. Instead, check it and return
-EIO if the file is too small for the payload offset.


Good catch.  Probably not a CVE (unless someone can argue some way that 
causing a crash on an attempt to load a maliciously corrupted file can 
be used as a denial of service across a privilege boundary), but 
definitely needs fixing.




Zero length images are fine, so trigger -EIO only on offset > len, not
on offset >= len as the assertion did before.

Signed-off-by: Kevin Wolf 
---
  block/crypto.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/crypto.c b/block/crypto.c
index 2035f9ab13..4908d8627f 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -518,7 +518,10 @@ static int64_t block_crypto_getlength(BlockDriverState *bs)
  
  uint64_t offset = qcrypto_block_get_payload_offset(crypto->block);

  assert(offset < INT64_MAX);


Umm, if the file can be corrupted, what's to prevent someone from 
sticking in a negative size that fails this assertion?



-assert(offset < len);
+
+if (offset > len) {
+   return -EIO;
+}
  
  len -= offset;
  



--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] fsl-imx6: Swap Ethernet interrupt defines

[Guenter Roeck]

On Fri, Mar 09, 2018 at 06:48:43PM +, Peter Maydell wrote:
> On 9 March 2018 at 18:20, Guenter Roeck  wrote:
> > On Fri, Mar 09, 2018 at 05:47:16PM +, Peter Maydell wrote:
> >> Thanks for that really useful writeup. So if I understand correctly
> >> we have several choices here:
> >>
> >>  (1) we could implement a model of the IOMUX block that is at least
> >>  sufficient to support guests that configure it to route the ENET interrupt
> >>  line to a GPIO pin. Then we could apply this patch that fixes the ENET
> >>  line definitions. Old kernels would continue to work (for the same
> >>  reason they worked on hardware), and new ones would work now too.
> >>  This is in some ways the preferred option, but it's possibly a lot
> >>  of code and we're nearly in freeze for 2.12.
> >>
> >>  (2) we could leave everything as it is for 2.12. This would mean that
> >>  at least we don't regress setups that used to work on older QEMU versions.
> >>  Downside is that we wouldn't be able to run Linux v4.15+, or other
> >>  guest OSes that don't have the bug that older Linux kernels do.
> >>  (Presumably we'd only do this on the understanding that we were going
> >>  to go down route (1) for 2.13.)
> >>
> >>  (3) we could apply this patch for 2.12. Linux v4.15+ now works, as
> >>  do other guest OSes that use the ENET interrupt. v4.1 and older Linux
> >>  guests that used to boot in QEMU stop doing so, and 4.2-4.9 boot but
> >>  lose the ethernet device support. Perhaps for 2.13 we might
> >>  take route (1) to make those older guests start working again.
> >>
> >> Do I have that right?
> >>
> > Pretty much.
> >
> >> None of these options seems especially palatable to me, so we're
> >> choosing the lesser evil, I think... (unless somebody wants to say
> >> that option (1) would be 20 lines of code and here's the patch :-))
> >> I guess in the absence of (1) that (3) is better than (2) ?
> >>
> >
> > I would prefer (2). This is what I decided to use in my "local"
> > version of qemu. Older versions of Linux can be fixed by applying one
> > (4.2..4.9) or two (4.1 and older) upstream patches; anyone interested
> > running those kernels in qemu with Ethernet working should apply those
> > patches (or, alternatively, provide a patch adding IOMUX support to
> > qemu).
> 
> Did you mean "prefer (3) [apply this patch]" ? The rest of the paragraph
> makes more sense if you did.
> 
Yes, sorry.

Guenter

Re: [Qemu-devel] [PATCH 3/6] luks: Support .bdrv_co_create

[Eric Blake]

On 03/09/2018 11:27 AM, Kevin Wolf wrote:

This adds the .bdrv_co_create driver callback to luks, which enables
image creation over QMP.

Signed-off-by: Kevin Wolf 
---
  qapi/block-core.json | 17 -
  block/crypto.c   | 34 ++
  2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 524d51567a..07039bfe9c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3452,6 +3452,21 @@
  '*preallocation':   'PreallocMode' } }
  
  ##

+# @BlockdevCreateOptionsLUKS:
+#
+# Driver specific image creation options for LUKS.
+#
+# @file Node to create the image format on
+# @size Size of the virtual disk in bytes
+#
+# Since: 2.12


Well, as long as we make it by Tuesday :)

Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v3 0/7] block: Handle null backing link

[Eric Blake]

On 02/24/2018 09:40 AM, Max Reitz wrote:

Currently, we try to rewrite every occurrence of "backing": null into
"backing": "" in qmp_blockdev_add().  However, that breaks using the
same "backing": null construction in json:{} file names (which do not go
through qmp_blockdev_add()).  Currently, these then just behave as if
the option has not been specified.

Since there is actually only one place where we evaluate the @backing
option to find out whether not to use a backing file, we can instead
just check for null there.  It doesn't matter that this changes the
runtime state of the option from "" to null, because nobody really does
anything with that runtime state anyway (except put it into qemu again,
but qemu doesn't care whether it's "" or null).

And in the future, it's much better if we get it to be null in that
runtime state sooner than later -- see patch 7.


Note that it was Markus (who's away having a good time, I hope) who
proposed qobject_to(), so I guess he won't object too much to seeing the
concept having landed in his tree once he returns.
(Although he hasn't reviewed the previous iteration of this series,
  which included it already.)


v3:
- Added patch 1 so we can use a common macro in patch 2 (instead of
   invoking _Static_assert() directly), but still keep the explanatory
   message



This series mostly touches QAPI, so I'm probably going to include it in 
my pending qapi pull request in time for soft freeze.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [patches] Re: [PULL] RISC-V QEMU Port Submission v8.2

[Michael Clark]

On Sat, Mar 10, 2018 at 5:49 AM, Peter Maydell 
wrote:

> On 9 March 2018 at 14:28, Peter Maydell  wrote:
> > NB: there was a test failure on OpenBSD host:
> >
> > TEST: tests/qom-test... (pid=64016)
> >   /riscv32/qom/spike_v1.9.1:   **
> > ERROR:/home/qemu/tests/qom-test.c:64:test_properties: assertion
> > failed: (qdict_haskey(response, "return"))
> > FAIL
> >
> > but this seems to have been intermittent -- it was only on that one
> > host, and I reran the test suite there and it passed fine the second
> > time. So it may be nothing to do with your code; we'll see if it
> > comes up again.
>
> On a later test run I got this different one; openbsd again:
>
> TEST: tests/test-hmp... (pid=45236)
>   /riscv32/hmp/spike_v1.9.1:   **
> ERROR:/home/qemu/qom/object.c:488:object_new_with_type: assertion
> failed: (type != NULL)
> Broken pipe
> FAIL
>
> My current best theory is that OpenBSD libc's memory allocator
> happens to be more sensitive to a memory corruption bug in the risc
> code, resulting in intermittent failures if the allocations happen
> to come out the wrong way. You do have at least one invalid-write
> off the end of a block according to valgrind:
>
> ==17441== Invalid write of size 1
> ==17441==at 0x26517F: riscv_isa_string (cpu.c:399)
> ==17441==by 0x25C14D: create_fdt (spike.c:125)
> ==17441==by 0x25C14D: spike_v1_10_0_board_init (spike.c:199)
> ==17441==by 0x2CCE0A: machine_run_board_init (machine.c:807)
> ==17441==by 0x1BFF28: main (vl.c:4597)
> ==17441==  Address 0x3055c425 is 0 bytes after a block of size 5 alloc'd
> ==17441==at 0x4C2FB55: calloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==17441==by 0x70C8770: g_malloc0 (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4800.2)
> ==17441==by 0x26511E: riscv_isa_string (cpu.c:395)
> ==17441==by 0x25C14D: create_fdt (spike.c:125)
> ==17441==by 0x25C14D: spike_v1_10_0_board_init (spike.c:199)
> ==17441==by 0x2CCE0A: machine_run_board_init (machine.c:807)
> ==17441==by 0x1BFF28: main (vl.c:4597)
>
> If you can prioritise a patch that fixes the bug in riscv_isa_string()
> I'll apply that and hopefully these intermittent failures will go away.
>

I'm looking at this right now.

Re: [Qemu-devel] [PATCH v8 03/23] qobject: introduce qobject_get_try_str()

[Eric Blake]

On 03/09/2018 02:59 AM, Peter Xu wrote:

A quick way to fetch string from qobject when it's a QString.

Reviewed-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Peter Xu 
---
  include/qapi/qmp/qstring.h |  1 +
  qobject/qstring.c  | 11 +++
  2 files changed, 12 insertions(+)




+++ b/qobject/qstring.c
@@ -137,6 +137,17 @@ const char *qstring_get_try_str(const QString *qstring)
  return qstring ? qstring_get_str(qstring) : NULL;
  }
  
+/**

+ * qobject_get_try_str(): Return a pointer to the corresponding string
+ *
+ * NOTE: the string will only be returned if the object is valid, and
+ * its type is QString, otherwise NULL is returned.
+ */
+const char *qobject_get_try_str(const QObject *qstring)
+{
+return qstring_get_try_str(qobject_to_qstring(qstring));


Conflicts with Max's refactoring to a qobject_to() macro.
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg06230.html

Whoever lands first gets to watch the other (or the maintainer) rebase ;)

Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 2/6] luks: Create block_crypto_co_create_generic()

[Eric Blake]

On 03/09/2018 11:27 AM, Kevin Wolf wrote:

Everything that refers to the protocol layer or QemuOpts is moved out of
block_crypto_create_generic(), so that the remaining function is
suitable to be called by a .bdrv_co_create implementation.

LUKS is the only driver that actually implements the old interface, and
we don't intend to use it in any new drivers, so put the moved out code
directly into a LUKS function rather than creating a generic
intermediate one.

Signed-off-by: Kevin Wolf 
---
  block/crypto.c | 95 +-
  1 file changed, 61 insertions(+), 34 deletions(-)



Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v2 08/11] linux-user: drop unused target_msync function

[Laurent Vivier]

Le 28/02/2018 à 23:16, Max Filippov a écrit :
> target_msync is not used, remove its declaration and implementation.
> 
> Cc: Riku Voipio 
> Cc: Laurent Vivier 
> Signed-off-by: Max Filippov 
> ---
>  linux-user/mmap.c | 17 -
>  linux-user/qemu.h |  1 -
>  2 files changed, 18 deletions(-)
>

Applied to my 'linux-user-for-2.12' branch.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 3/5] migration/block: rename MAX_INFLIGHT_IO to MAX_IO_BUFFERS

[Peter Lieven]

Am 09.03.2018 um 15:58 schrieb Dr. David Alan Gilbert:
> * Peter Lieven (p...@kamp.de) wrote:
>> this actually limits (as the original commit mesage suggests) the
>> number of I/O buffers that can be allocated and not the number
>> of parallel (inflight) I/O requests.
>>
>> Signed-off-by: Peter Lieven 
> I've queued 1-3 (which have Juan's R-b).

Thank you. It would be good if we find a proper solution for the rest as this 
is also important.

Peter

Re: [Qemu-devel] [PATCH v2 07/11] linux-user: fix target_mprotect/target_munmap error return values

[Laurent Vivier]

Le 28/02/2018 à 23:16, Max Filippov a écrit :
> target_mprotect/target_munmap return value goes through get_errno at the
> call site, thus the functions must either set errno to host error code
> and return -1 or return negative guest error code. Do the latter.
> 
> Cc: qemu-sta...@nongnu.org
> Cc: Riku Voipio 
> Cc: Laurent Vivier 
> Signed-off-by: Max Filippov 
> ---
>  linux-user/mmap.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 

Applied to my 'linux-user-for-2.12' branch.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2 10/11] qemu-binfmt-conf.sh: add qemu-xtensa

[Laurent Vivier]

Le 28/02/2018 à 23:16, Max Filippov a écrit :
> Register qemu-xtensa and qemu-xtensaeb for transparent linux userspace
> emulation.
> 
> Cc: Riku Voipio 
> Cc: Laurent Vivier 
> Signed-off-by: Max Filippov 
> ---
>  scripts/qemu-binfmt-conf.sh | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 

Applied to my 'linux-user-for-2.12' branch.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v4] linux-user: Support f_flags in statfs when available.

[Laurent Vivier]

Le 01/03/2018 à 12:15, Shea Levy a écrit :
> Signed-off-by: Shea Levy 
> ---
>  linux-user/syscall.c  |  5 +
>  linux-user/syscall_defs.h | 41 +++--
>  2 files changed, 24 insertions(+), 22 deletions(-)

Applied to my 'linux-user-for-2.12' branch.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2 06/11] linux-user: fix assertion in shmdt

[Laurent Vivier]

Le 28/02/2018 à 23:16, Max Filippov a écrit :
> shmdt fails to call mmap_lock/mmap_unlock around page_set_flags,
> resulting in the following assertion:
>   page_set_flags: Assertion `have_mmap_lock()' failed.
> 
> Wrap shmdt internals into mmap_lock/mmap_unlock.
> 
> Cc: qemu-sta...@nongnu.org
> Cc: Riku Voipio 
> Cc: Laurent Vivier 
> Signed-off-by: Max Filippov 
> ---
>  linux-user/syscall.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

Applied to my 'linux-user-for-2.12' branch.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2 2/5] nbd/server: fix sparse read

[Eric Blake]

On 03/08/2018 12:46 PM, Vladimir Sementsov-Ogievskiy wrote:

In case of io error in nbd_co_send_sparse_read we should not
"goto reply:", as it is fatal error and common behavior is
disconnect in this case. We should not try to send client an


s/send/send the/


error reply, representing channel-io error on previous try to
send a reply.


s/representing .../since we already hit a channel-io error on our 
previous attempt to send a reply/




Fix this by handle block-status error in nbd_co_send_sparse_read,


s/handle/handling/


so nbd_co_send_sparse_read fails only on io error. Then just skip
common "reply:" code path in nbd_trip.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

v2: code movement splitted to 01
 remove error_report
 fix indent
 fix commit message subject line
 add comment to nbd_co_send_sparse_read (be free to adjust it)


Not a problem, I know English is not your native tongue, so I don't mind 
touching it up, and I already admire your efforts at programming in a 
second language (a skill I lack).




  nbd/server.c | 17 ++---
  1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index c406b0656d..e0de431e10 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1365,6 +1365,10 @@ static int coroutine_fn 
nbd_co_send_structured_error(NBDClient *client,
  return nbd_co_send_iov(client, iov, 1 + !!iov[1].iov_len, errp);
  }
  
+/* Do sparse read and send structured reply to the client.


s/Do/Do a/


+ * Returns -errno if sending fails. bdrv_block_status_above() fail is not


s/fail/failure/


+ * considered as an error, but reported to the client instead (as a structured
+ * error). */


The comment tail */ usually gets its own line.

Here's what I used:

/* Do a sparse read and send the structured reply to the client.
 * Returns -errno if sending fails. bdrv_block_status_above() failure is
 * reported to the client, at which point this function succeeds.
 */


  static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
  uint64_t handle,
  uint64_t offset,
@@ -1385,8 +1389,13 @@ static int coroutine_fn 
nbd_co_send_sparse_read(NBDClient *client,
  bool final;
  
  if (status < 0) {

-error_setg_errno(errp, -status, "unable to check for holes");
-return status;
+char *msg = g_strdup_printf("unable to check for holes: %s",
+strerror(-status));


I had to double-check block/io.c, but it looks like 
bdrv_block-status_above() does indeed document a return of negative 
errno on failure.  (My suspicion is that the documentation might be 
wrong, because I did not audit error returns when doing my byte-based 
work; but a quick audit of a couple of random drivers shows that at 
least iscsi.c and qed.c appear to comply.)



+
+ret = nbd_co_send_structured_error(client, handle, -status, msg,
+   errp);
+g_free(msg);
+return ret;


I wonder if a separate patch should clean this up to let 
nbd_co_send_structured_error() do message formatting (making it varargs) 
- but doesn't affect this patch.


With grammar tweaks,
Reviewed-by: Eric Blake 

Re: [Qemu-devel] [PATCH] linux-user: allows to use "--systemd ALL" with qemu-binfmt-conf.sh

[Laurent Vivier]

Le 08/03/2018 à 11:48, Laurent Vivier a écrit :
> qemu-binfmt-conf.sh when it is used with systemd
> needs to know for which CPU the systemd-binfmt.service
> file must be created (i.e. "--systemd ppc").
> 
> But sometime, for instance for test purpose, we need to
> create an entry for all known architectures.
> This patch entroduce the "ALL" parameter for this purpose.
> 
> Signed-off-by: Laurent Vivier 
> ---
>  scripts/qemu-binfmt-conf.sh | 25 ++---
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 

Applied to my 'linux-user-for-2.12' branch.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v6 05/11] linux-user: fix mmap/munmap/mprotect/mremap/shmat

[Laurent Vivier]

Le 07/03/2018 à 22:50, Max Filippov a écrit :
> In linux-user QEMU that runs for a target with TARGET_ABI_BITS bigger
> than L1_MAP_ADDR_SPACE_BITS an assertion in page_set_flags fires when
> mmap, munmap, mprotect, mremap or shmat is called for an address outside
> the guest address space. mmap and mprotect should return ENOMEM in such
> case.
> 
> Change definition of GUEST_ADDR_MAX to always be the last valid guest
> address. Account for this change in open_self_maps.
> Add macro guest_addr_valid that verifies if the guest address is valid.
> Add function guest_range_valid that verifies if address range is within
> guest address space and does not wrap around. Use that macro in
> mmap/munmap/mprotect/mremap/shmat for error checking.
> 
> Cc: qemu-sta...@nongnu.org
> Cc: Riku Voipio 
> Cc: Laurent Vivier 
> Signed-off-by: Max Filippov 
> ---
> Changes v5->v6:
> - drop 'if (len)' clause from guest_range_valid and explicitly compare
>   len with GUEST_ADDR_MAX.
> 
> Changes v4->v5:
> - change definition of GUEST_ADDR_MAX to always be the last valid guest
>   address. Account for this change in guest_addr_valid and open_self_maps.
> - turn guest_range_valid into a function.
> 
> Changes v3->v4:
> - change GUEST_ADDR_MAX and h2g_valid definitions as suggested by Laurent
>   Vivier.
> 
> Changes v2->v3:
> - fix comparison in guest_valid: it must be 'less' to preserve the existing
>   functionality, not 'less or equal'.
> - fix guest_range_valid: it may not use guest_valid, because single range
>   that occupies all of the guest address space is valid.
> 
>  include/exec/cpu-all.h  |  6 +-
>  include/exec/cpu_ldst.h | 16 +++-
>  linux-user/mmap.c   | 20 +++-
>  linux-user/syscall.c|  5 -
>  4 files changed, 31 insertions(+), 16 deletions(-)
> 

Applied to my 'linux-user-for-2.12' branch.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 1/2] linux-user: Drop unicore32 code

[Laurent Vivier]

Le 08/03/2018 à 15:47, Peter Maydell a écrit :
> We dropped the unicore32-linux-user target in commit 5e2b40f7271cf9
> in 2016. Nobody has made any attempt to fix the issues that
> caused us to drop it, so remove the associated code.
> (The system emulation parts of unicore32 remain.)
> 
> Signed-off-by: Peter Maydell 
> ---
>  linux-user/qemu.h |   5 +-
>  linux-user/syscall_defs.h |   6 +-
>  linux-user/unicore32/syscall_nr.h | 371 
> --
>  linux-user/unicore32/target_cpu.h |  27 ---
>  linux-user/unicore32/target_elf.h |  14 --
>  linux-user/unicore32/target_signal.h  |  30 ---
>  linux-user/unicore32/target_structs.h |  58 --
>  linux-user/unicore32/target_syscall.h |  62 --
>  linux-user/unicore32/termbits.h   |   2 -
>  linux-user/elfload.c  |  72 ---
>  linux-user/main.c |  99 +
>  linux-user/signal.c   |   5 +-
>  12 files changed, 6 insertions(+), 745 deletions(-)
>  delete mode 100644 linux-user/unicore32/syscall_nr.h
>  delete mode 100644 linux-user/unicore32/target_cpu.h
>  delete mode 100644 linux-user/unicore32/target_elf.h
>  delete mode 100644 linux-user/unicore32/target_signal.h
>  delete mode 100644 linux-user/unicore32/target_structs.h
>  delete mode 100644 linux-user/unicore32/target_syscall.h
>  delete mode 100644 linux-user/unicore32/termbits.h
> 

Applied to my 'linux-user-for-2.12' branch.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 2/2] linux-user: Remove the unused "not implemented" signal handling stubs

[Laurent Vivier]

Le 08/03/2018 à 15:47, Peter Maydell a écrit :
> Now we've dropped unicore32, all of the architectures we support
> for linux-user implement the signal handling routines. The
> dummy "just print a message" versions are unimplemented, so we
> can drop them entirely.
> 
> Signed-off-by: Peter Maydell 
> ---
> IMHO signal handling support is too important to allow a
> hypothetical new architecture target to silently get away
> without implementing it. For initial development it's easy
> enough to stub out the per-architecture functions, and then
> we will have a clear view of which targets (if any) don't
> have the signal handling implemented yet, and the missing
> feature will show up in code review.
> ---
>  linux-user/signal.c | 27 +--
>  1 file changed, 1 insertion(+), 26 deletions(-)

Applied to my 'linux-user-for-2.12' branch.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v4 00/29] postcopy+vhost-user/shared ram

[Dr. David Alan Gilbert]

* Dr. David Alan Gilbert (git) (dgilb...@redhat.com) wrote:
> From: "Dr. David Alan Gilbert" 
> 
>   This set enables postcopy migration with shared memory to a vhost user 
> process.
> It's based off current head.
> 
>   Testing is mostly performed with dpdk, with corresponding modifications by
> Maxime.

Maxime's dpdk world is available here:
  https://gitlab.com/mcoquelin/dpdk-next-virtio.git
on the
   postcopy_proto_v4

branch.


Dave

>   v4 is mostly just fixes from comments received during review of v3,
> and the normal updating of all the virtio enum's to catch up with other
> things getting in before us.
> 
> Dave
> 
> 
> Dr. David Alan Gilbert (29):
>   migrate: Update ram_block_discard_range for shared
>   qemu_ram_block_host_offset
>   postcopy: use UFFDIO_ZEROPAGE only when available
>   postcopy: Add notifier chain
>   postcopy: Add vhost-user flag for postcopy and check it
>   vhost-user: Add 'VHOST_USER_POSTCOPY_ADVISE' message
>   libvhost-user: Support sending fds back to qemu
>   libvhost-user: Open userfaultfd
>   postcopy: Allow registering of fd handler
>   vhost+postcopy: Register shared ufd with postcopy
>   vhost+postcopy: Transmit 'listen' to client
>   postcopy+vhost-user: Split set_mem_table for postcopy
>   migration/ram: ramblock_recv_bitmap_test_byte_offset
>   libvhost-user+postcopy: Register new regions with the ufd
>   vhost+postcopy: Send address back to qemu
>   vhost+postcopy: Stash RAMBlock and offset
>   vhost+postcopy: Send requests to source for shared pages
>   vhost+postcopy: Resolve client address
>   postcopy: helper for waking shared
>   postcopy: postcopy_notify_shared_wake
>   vhost+postcopy: Add vhost waker
>   vhost+postcopy: Call wakeups
>   libvhost-user: mprotect & madvises for postcopy
>   vhost-user: Add VHOST_USER_POSTCOPY_END message
>   vhost+postcopy: Wire up POSTCOPY_END notify
>   vhost: Huge page align and merge
>   postcopy: Allow shared memory
>   libvhost-user: Claim support for postcopy
>   postcopy shared docs
> 
>  contrib/libvhost-user/libvhost-user.c | 303 -
>  contrib/libvhost-user/libvhost-user.h |  11 +
>  docs/devel/migration.rst  |  41 
>  docs/interop/vhost-user.txt   |  51 +
>  exec.c|  86 +--
>  hw/virtio/trace-events|  16 +-
>  hw/virtio/vhost-user.c| 411 
> +-
>  hw/virtio/vhost.c |  66 +-
>  include/exec/cpu-common.h |   4 +
>  migration/migration.c |   6 +
>  migration/migration.h |   4 +
>  migration/postcopy-ram.c  | 354 +++--
>  migration/postcopy-ram.h  |  69 ++
>  migration/ram.c   |   5 +
>  migration/ram.h   |   1 +
>  migration/savevm.c|  13 ++
>  migration/trace-events|   6 +
>  trace-events  |   3 +-
>  vl.c  |   2 +
>  19 files changed, 1354 insertions(+), 98 deletions(-)
> 
> -- 
> 2.14.3
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 1/6] luks: Separate image file creation from formatting

[Eric Blake]

On 03/09/2018 11:27 AM, Kevin Wolf wrote:

The crypto driver used to create the image file in a callback from the
crypto subsystem. If we want to implement .bdrv_co_create, this needs to
go away because that callback will get a reference to an already
existing block node.

Move the image file creation to block_crypto_create_generic().

Signed-off-by: Kevin Wolf 
---
  block/crypto.c | 37 +
  1 file changed, 17 insertions(+), 20 deletions(-)



Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 0/9] nbd block status base:allocation

[Vladimir Sementsov-Ogievskiy]

09.03.2018 22:08, Eric Blake wrote:

On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote:

Hi all.

Here is minimal realization of base:allocation context of NBD
block-status extension, which allows to get block status through
NBD.

Vladimir Sementsov-Ogievskiy (9):
   nbd/server: add nbd_opt_invalid helper
   nbd: change indenting in nbd.h
   nbd: BLOCK_STATUS for standard get_block_status function: server part
   block/nbd-client: save first fatal error in nbd_iter_error
   nbd/client: fix error messages in nbd_handle_reply_err
   nbd: BLOCK_STATUS for standard get_block_status function: client part
   iotests.py: tiny refactor: move system imports up
   iotests: add file_path helper
   iotests: new test 206 for NBD BLOCK_STATUS


I'd really like to send a PULL request for NBD on Monday, in order to 
make the 2.12 softfreeze deadline (this is a new feature, so if we 
miss Tuesday, we have to wait until 2.13 or whatever the next release 
is called).  Where do you stand on rebasing this, and what help can I 
offer? (I know you have factored out some of the patches in another 
thread that I'm in the middle of reviewing as well; you can submit the 
later patches even before the earlier ones land, and use a 'Based-on:' 
tag in the cover letter to make it obvious the dependencies between 
series).




I'm now at start of "Re: [PATCH 6/9] nbd: BLOCK_STATUS for standard 
get_block_status function: client part", and I think rebasing on 
byte-based is too much for me for that moment (10:20 pm =). I'll do my 
best on Monday morning as early as I can.


--
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 0/5] nbd server fixing and refactoring before BLOCK_STATUS

[Eric Blake]

On 03/09/2018 10:41 AM, Eric Blake wrote:

On 03/08/2018 12:46 PM, Vladimir Sementsov-Ogievskiy wrote:

01 and 02 are splitted and updated "[PATCH] nbd/server: fix space read",
others are new.

Vladimir Sementsov-Ogievskiy (5):
   nbd/server: move nbd_co_send_structured_error up
   nbd/server: fix sparse read
   nbd/server: fix: check client->closing before reply sending
   nbd/server: refactor nbd_trip: cmd_read and generic reply
   nbd/server: refactor nbd_trip: split out nbd_handle_request


I had a tough time applying this one:

Applying: nbd/server: move nbd_co_send_structured_error up
Applying: nbd/server: fix: check client->closing before reply sending
Applying: nbd/server: refactor nbd_trip: cmd_read and generic reply
error: sha1 information is lacking or useless (nbd/server.c).
error: could not build fake ancestor
Patch failed at 0004 nbd/server: refactor nbd_trip: cmd_read and generic 
reply


Aha - I see my problem - the patches were applied out of order because 
only patch 2 had a 'v2' in the subject line.  If I tell git to apply 
them one at a time, instead of trying to apply them on 'maildir/*' where 
the sorting botches the ordering, things work better.


I think I've resolved the conflicts correctly, but if I get through 
reviewing this series and posting it to my NBD queue, you may want to 
double-check things.


No need to repost this series, I'm doing a lot better now that I see my 
mistake.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [Bug 1738691] Re: Guest kernel crashes with kvm_pr on POWER8

[Murilo Opsfelder Araújo]

Hi, Timothy.

I tried to reproduce this issue on a POWER8 box and couldn't reproduce
it.

Whatever the issue was, it seems to be fixed on kernel v4.16-rc4 with
qemu 2.11.50.

I downloaded vmlinux/initrd.gz from Ubuntu 18.04 to boot guest. It
booted fine up to the installer initial screen.

Please find my environment information listed below.

I'm closing this bug but feel free to reopen it or file a new one.

Cheers
Murilo


Machine type/model: 8247-22L

[muriloo@baratheon ~]$ uname -a
Linux localhost.localdomain 4.16.0-rc4+ #1 SMP Thu Mar 8 22:54:31 UTC 2018 
ppc64le ppc64le ppc64le GNU/Linux

[muriloo@baratheon ~]$ lsmod | grep kvm
kvm_pr100276  0
kvm   217753  1 kvm_pr

[muriloo@baratheon ~]$ ~/qemu/build/ppc64-softmmu/qemu-system-ppc64 --version
QEMU emulator version 2.11.50 (v2.11.0-2108-g83d2e94)
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

[muriloo@baratheon ~]$ ~/qemu/build/ppc64-softmmu/qemu-system-ppc64
-kernel ~/ubuntu/18.04/vmlinux -initrd ~/ubuntu/18.04/initrd.gz -append
"console=hvc0 verbose" -nodefaults -nographic -serial mon:stdio -accel
kvm

vmlinux: 
http://ports.ubuntu.com/ubuntu-ports/dists/bionic/main/installer-ppc64el/current/images/netboot/ubuntu-installer/ppc64el/vmlinux
initrd.gz: 
http://ports.ubuntu.com/ubuntu-ports/dists/bionic/main/installer-ppc64el/current/images/netboot/ubuntu-installer/ppc64el/initrd.gz

** Changed in: qemu
   Status: New => Invalid

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1738691

Title:
  Guest kernel crashes with kvm_pr on POWER8

Status in QEMU:
  Invalid

Bug description:
  When attempting to use the kvm_pr module with QEMU 2.10 on a POWER8
  host, Debian and Ubuntu guests hang and show crashes.

  Host kernel is 4.14.  Issue is observed with host kernels 4.9 and 4.13
  as well; no other host kernels were tested.

  Is this the correct place to report a kvm_pr bug?

  Output from Ubuntu 17.10 guest:

  Quiescing Open Firmware ...
  Booting Linux via __start() @ 0x0200 ...
  [0.00] Page sizes from device-tree:
  [0.00] base_shift=12: shift=12, sllp=0x, avpnm=0x, 
tlbiel=1, penc=0
  [0.00] base_shift=16: shift=16, sllp=0x0110, avpnm=0x, 
tlbiel=1, penc=1
  [0.00] base_shift=24: shift=24, sllp=0x0100, avpnm=0x0001, 
tlbiel=0, penc=0
  [0.00] Using 1TB segments
  [0.00] Initializing hash mmu with SLB
  [0.00] Linux version 4.13.0-16-generic (buildd@bos01-ppc64el-029) 
(gcc version 7.2.0 (Ubuntu 7.2.0-8ubuntu2)) #19-Ubuntu SMP Wed Oct 11 18:37:02 
UTC 2017 (Ubuntu 4.13.0-16.19-generic 4.13.4)
  [0.00] Found initrd at 0xc3b0:0xc48cf68b
  [0.00] Using pSeries machine description
  [0.00] bootconsole [udbg0] enabled
  [0.00] Partition configured for 2 cpus.
  [0.00] CPU maps initialized for 1 thread per core
   -> smp_release_cpus()
  spinning_secondaries = 1
   <- smp_release_cpus()
  [0.00] -
  [0.00] ppc64_pft_size= 0x19
  [0.00] phys_mem_size = 0x1
  [0.00] dcache_bsize  = 0x80
  [0.00] icache_bsize  = 0x80
  [0.00] cpu_features  = 0x077c7a6c18500249
  [0.00]   possible= 0x5fff18500649
  [0.00]   always  = 0x18100040
  [0.00] cpu_user_features = 0xdc0065c2 0xae00
  [0.00] mmu_features  = 0x7c006001
  [0.00] firmware_features = 0x415a445f
  [0.00] htab_hash_mask= 0x3
  [0.00] -
  [0.00] numa:   NODE_DATA [mem 0xfffd7c80-0xfffe3fff]
  [0.00] PCI host bridge /pci@8002000  ranges:
  [0.00]   IO 0x2000..0x2000 -> 
0x
  [0.00]  MEM 0x20008000..0x2000 -> 
0x8000
  [0.00]  MEM 0x2100..0x21ff -> 
0x2100
  [0.00] PPC64 nvram contains 65536 bytes
  [0.00] Zone ranges:
  [0.00]   DMA  [mem 0x-0x]
  [0.00]   DMA32empty
  [0.00]   Normal   empty
  [0.00]   Device   empty
  [0.00] Movable zone start for each node
  [0.00] Early memory node ranges
  [0.00]   node   0: [mem 0x-0x]
  [0.00] Initmem setup node 0 [mem 
0x-0x]
  [0.00] percpu: Embedded 4 pages/cpu @c000ffe0 s162840 r0 
d99304 u524288
  [0.00] Built 1 zonelists in Node order, mobility grouping on.  Total 
pages: 65472
  [0.00] Policy zone: DMA
  [0.00] Kernel command line: BOOT_IMAGE=/install/vmlinux 
file=/cdrom/preseed/ubuntu-server.seed 

Re: [Qemu-devel] [PATCH v4 1/2] i386: Add Intel Processor Trace feature support

[Eduardo Habkost]

On Mon, Mar 05, 2018 at 12:48:35AM +0800, Luwei Kang wrote:
> From: Chao Peng 
> 
> Expose Intel Processor Trace feature to guest.
> 
> To make Intel PT live migration safe and get same CPUID information
> with same CPU model on diffrent host. CPUID[14] is constant in this
> patch. Intel PT use EPT is first supported in IceLake, the CPUID[14]
> get on this machine as default value. Intel PT would be disabled
> if any machine don't support this minial feature list.
> 
> Signed-off-by: Chao Peng 
> Signed-off-by: Luwei Kang 
> ---
> From V3:
>  - fix some typo;
>  - add some comments and safty check.
> 
> ---
>  target/i386/cpu.c | 78 
> +--
>  target/i386/cpu.h |  1 +
>  target/i386/kvm.c | 23 
>  3 files changed, 100 insertions(+), 2 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index b5e431e..24e1693 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -173,7 +173,32 @@
>  #define L2_ITLB_4K_ASSOC   4
>  #define L2_ITLB_4K_ENTRIES   512
>  
> -
> +/* CPUID Leaf 0x14 constants: */
> +#define INTEL_PT_MAX_SUBLEAF 0x1
> +/*
> + * bit[00]: IA32_RTIT_CTL.CR3 filter can be set to 1 and IA32_RTIT_CR3_MATCH
> + *  MSR can be accessed;
> + * bit[01]: Support Configurable PSB and Cycle-Accurate Mode;
> + * bit[02]: Support IP Filtering, TraceStop filtering, and preservation
> + *  of Intel PT MSRs across warm reset;
> + * bit[03]: Support MTC timing packet and suppression of COFI-based packets;
> + */
> +#define INTEL_PT_MINIMAL_EBX 0xf

Thanks!  I didn't expect a detailed description of each bit.  I
thought that just adding macros for each bit instead of
hardcoding 0xf would be enough.

But after reading the docs, I understand it could be difficult to
choose a macro name for something like "support of IP Filtering,
TraceStop filtering, and preservation of Intel PT MSRs across
warm reset", so this description looks like the best we can do.
:)


I only see a problem below:

> +/*
> + * bit[00]: Tracing can be enabled with IA32_RTIT_CTL.ToPA = 1 and
> + *  IA32_RTIT_OUTPUT_BASE and IA32_RTIT_OUTPUT_MASK_PTRS MSRs can be
> + *  accessed;
> + * bit[01]: ToPA tables can hold any number of output entries, up to the
> + *  maximum allowed by the MaskOrTableOffset field of
> + *  IA32_RTIT_OUTPUT_MASK_PTRS;
> + * bit[02]: Support Single-Range Output scheme;
> + */
> +#define INTEL_PT_MINIMAL_ECX 0x7
> +#define INTEL_PT_ADDR_RANGES_NUM 0x2 /* Number of configurable address 
> ranges */
> +#define INTEL_PT_ADDR_RANGES_NUM_MASK 0x3
> +#define INTEL_PT_MTC_BITMAP  (0x0249 << 16) /* Support ART(0,3,6,9) */
> +#define INTEL_PT_CYCLE_BITMAP0x1fff /* Support 0,2^(0~11) */
> +#define INTEL_PT_PSB_BITMAP  (0x003f << 16) /* Support 
> 2K,4K,8K,16K,32K,64K */
>  
>  static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
>   uint32_t vendor2, uint32_t vendor3)
[...]
> @@ -4083,6 +4129,34 @@ static int x86_cpu_filter_features(X86CPU *cpu)
>  }
>  }
>  
> +if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
> +kvm_enabled()) {
> +KVMState *s = CPU(cpu)->kvm_state;
> +uint32_t eax_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EAX);
> +uint32_t ebx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EBX);
> +uint32_t ecx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_ECX);
> +uint32_t eax_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EAX);
> +uint32_t ebx_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EBX);
> +
> +if (!eax_0 ||
> +   ((ebx_0 & INTEL_PT_MINIMAL_EBX) != INTEL_PT_MINIMAL_EBX) ||
> +   ((ecx_0 & INTEL_PT_MINIMAL_ECX) != INTEL_PT_MINIMAL_ECX) ||
> +   ((eax_1 & INTEL_PT_MTC_BITMAP) != INTEL_PT_MTC_BITMAP) ||
> +   ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) <
> +   INTEL_PT_ADDR_RANGES_NUM) ||
> +   ((ebx_1 & (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) !=
> +(INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP))) {

I still don't see a check to ensure the host has bit 31 on ecx_0
set to 0, as I mentioned when reviewing v3.

The rest of the patch looks good.

> +/*
> + * Processor Trace capabilities aren't configurable, so if the
> + * host can't emulate the capabilities we report on
> + * cpu_x86_cpuid(), intel-pt can't be enabled on the current 
> host.
> + */
> +env->features[FEAT_7_0_EBX] &= ~CPUID_7_0_EBX_INTEL_PT;
> +cpu->filtered_features[FEAT_7_0_EBX] |= CPUID_7_0_EBX_INTEL_PT;
> +rv = 1;
> +}
> +}
> +
>  return rv;
>  }
>  
[...]

-- 
Eduardo

Re: [Qemu-devel] [PATCH 0/9] nbd block status base:allocation

[Eric Blake]

On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote:

Hi all.

Here is minimal realization of base:allocation context of NBD
block-status extension, which allows to get block status through
NBD.

Vladimir Sementsov-Ogievskiy (9):
   nbd/server: add nbd_opt_invalid helper
   nbd: change indenting in nbd.h
   nbd: BLOCK_STATUS for standard get_block_status function: server part
   block/nbd-client: save first fatal error in nbd_iter_error
   nbd/client: fix error messages in nbd_handle_reply_err
   nbd: BLOCK_STATUS for standard get_block_status function: client part
   iotests.py: tiny refactor: move system imports up
   iotests: add file_path helper
   iotests: new test 206 for NBD BLOCK_STATUS


I'd really like to send a PULL request for NBD on Monday, in order to 
make the 2.12 softfreeze deadline (this is a new feature, so if we miss 
Tuesday, we have to wait until 2.13 or whatever the next release is 
called).  Where do you stand on rebasing this, and what help can I 
offer? (I know you have factored out some of the patches in another 
thread that I'm in the middle of reviewing as well; you can submit the 
later patches even before the earlier ones land, and use a 'Based-on:' 
tag in the cover letter to make it obvious the dependencies between series).


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] qemu-doc: Rework the network options chapter to make "-net" less prominent

[Eric Blake]

On 03/09/2018 11:41 AM, Thomas Huth wrote:

On 09.03.2018 15:36, Eric Blake wrote:

On 03/09/2018 12:13 AM, Thomas Huth wrote:

"-net" is clearly a legacy option. Yet we still use it in almost all
examples in the qemu documentation, and many other spots in the network
chapter. We should make it less prominent that users are not lured into
using it so often anymore. So instead of starting the network chapter
with
"-net nic" and documenting "-net " below "-netdev "
everywhere, all the "-net" related documentation is now moved to the end
of the chapter. The new "--nic" option is moved to the beginning of the
chapter instead, with a new example that should demonstrate how "--nic"
can be used to shortcut "--device" with "--netdev".
And the examples in this chapter are changed to use the "--device" and
"--netdev" options or "--nic" instead of "-net nic -net ".




+@example
+qemu-system-i386 --netdev user,id=n1,ipv6=off
--device=e1000,netdev=n1,mac=52:54:98:76:54:32
+qemu-system-i386 --nic user,ipv6=off,model=e1000,mac=52:54:98:76:54:32
+@end example


Nice example.


... but looks like I even got it wrong - it should be "--device e1000",
without "=". Will fix it.


Really?  As I understand it, both long-opt spellings work ('--long=opt' 
as one arg, and '--long' 'opt' as two args).  So the only reason to drop 
'=' would be consistency with other examples.



@@ -2166,8 +2166,6 @@ or @file{C:\WINNT\SYSTEM32\DRIVERS\ETC\LMHOSTS}
(Windows NT/2000).
   Then @file{@var{dir}} can be accessed in @file{\\smbserver\qemu}.
     Note that a SAMBA server must be installed on the host OS.
-QEMU was tested successfully with smbd versions from Red Hat 9,
-Fedora Core 3 and OpenSUSE 11.x.
   


This change makes sense, but is somewhat unrelated to -net.  Worth
splitting the patch?


Not sure whether it's really worth the effort ... I think I'll just
mention it in the patch description, ok?


Yes, if you don't split, at least mention it (if a reviewer questioned 
it in an earlier revision, then an improved commit message serves as the 
preemptive reason for the next person looking at the next revision).




-qemu-system-i386 -net user,hostfwd=tcp:127.0.0.1:6001-:6000 [...]
+qemu-system-i386 --nic user,hostfwd=tcp:127.0.0.1:6001-:6000


The trailing [...] makes sense here, why did you drop it?


Since we're incredibly inconsistent with that. Some examples do have
that "[...]", many others don't have it. Since the amount of examples
without it is larger than the amount of examples with it, I think it's
more consistent to drop it here.

I guess I should mention this in the commit message, too.


Yes, mention that it's intentional, and I'll go along with it.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] fsl-imx6: Swap Ethernet interrupt defines

[Bill Paul]

Of all the gin joints in all the towns in all the world, Bill Paul had to walk 
into mine at 10:53 on Friday 09 March 2018 and say:

> Of all the gin joints in all the towns in all the world, Guenter Roeck had
> to
> 
> walk into mine at 10:20 on Friday 09 March 2018 and say:
> > On Fri, Mar 09, 2018 at 05:47:16PM +, Peter Maydell wrote:
> > > On 8 March 2018 at 18:28, Bill Paul  wrote:
> > > > Anyway, this means that the only reason older Linux kernels worked in
> > > > QEMU with the broken interrupt configuration is that they also
> > > > registered a handler on vector 151 (119). Even though QEMU could not
> > > > send events via GPIO6, it was mistakenly sending them via vector 151,
> > > > so it looked like things worked. On real hardware, the older kernels
> > > > would have gotten their interrupts via GPIO6 and also worked. The
> > > > older kernels would _not_ work if you fix QEMU because now they would
> > > > never get interrupts on either vector, unless you fudge things so
> > > > that the ENET module triggers both vector 150 and the vector for
> > > > GPIO6 in the GIC or patch them to back out the erratum 6678
> > > > workaround as later kernels do.
> > > 
> > > Thanks for that really useful writeup. So if I understand correctly
> > > 
> > > we have several choices here:
> > >  (1) we could implement a model of the IOMUX block that is at least
> > >  sufficient to support guests that configure it to route the ENET
> > >  interrupt line to a GPIO pin. Then we could apply this patch that
> > >  fixes the ENET line definitions. Old kernels would continue to work
> > >  (for the same reason they worked on hardware), and new ones would
> > >  work now too. This is in some ways the preferred option, but it's
> > >  possibly a lot of code and we're nearly in freeze for 2.12.
> > >  
> > >  (2) we could leave everything as it is for 2.12. This would mean that
> > >  at least we don't regress setups that used to work on older QEMU
> > >  versions. Downside is that we wouldn't be able to run Linux v4.15+, or
> > >  other guest OSes that don't have the bug that older Linux kernels do.
> > >  (Presumably we'd only do this on the understanding that we were going
> > >  to go down route (1) for 2.13.)
> > >  
> > >  (3) we could apply this patch for 2.12. Linux v4.15+ now works, as
> > >  do other guest OSes that use the ENET interrupt. v4.1 and older Linux
> > >  guests that used to boot in QEMU stop doing so, and 4.2-4.9 boot but
> > >  lose the ethernet device support. Perhaps for 2.13 we might
> > >  take route (1) to make those older guests start working again.
> > > 
> > > Do I have that right?
> > 
> > Pretty much.
> 
> There may be a 4th option.
> 
> Since older kernels work because they were looking at vector 151, you could
> patch the imx_fec.c module so that when it signals an event for vector 150,
> it also signals vector 151 too. This would keep newer versions of QEMU
> "bug- compatible" with older versions while also fixing support for newer
> Linux kernels and other guests (e.g. VxWorks) that follow the hardware
> spec correctly.

Oops, just to be clear: I mean that the vector definitions should be fixed and 
this backward-compatibility change should be applied at the same time.

-Bill
 
> I think this would require only a small modification to this function:
> 
> static void imx_eth_update(IMXFECState *s)
> {
> if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] & ENET_INT_TS_TIMER) {
> qemu_set_irq(s->irq[1], 1);
> } else {
> qemu_set_irq(s->irq[1], 0);
> }
> 
> if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] & ENET_INT_MAC) {
> qemu_set_irq(s->irq[0], 1);
> } else {
> qemu_set_irq(s->irq[0], 0);
> }
> }
> 
> (For ENET_INT_MAC events, just signal both s->irq[0] and s->irq[1]).
> 
> This of course means signaling spurious events on vector 151, but you're
> doing that now anyway. :)
> 
> This is much less work than implementing an IOMUX module. Creating an IOMUX
> module for QEMU just for this one fixup would probably be the most elegant
> solution, but adding IOMUX support to QEMU also doesn't make much sense
> since QEMU has no actual pins.
> 
> -Bill
> 
> > > None of these options seems especially palatable to me, so we're
> > > choosing the lesser evil, I think... (unless somebody wants to say
> > > that option (1) would be 20 lines of code and here's the patch :-))
> > > I guess in the absence of (1) that (3) is better than (2) ?
> > 
> > I would prefer (2). This is what I decided to use in my "local"
> > version of qemu. Older versions of Linux can be fixed by applying one
> > (4.2..4.9) or two (4.1 and older) upstream patches; anyone interested
> > running those kernels in qemu with Ethernet working should apply those
> > patches (or, alternatively, provide a patch adding IOMUX support to
> > qemu).
> > 
> > Thanks,
> > Guenter
-- 
=
-Bill Paul

Re: [Qemu-devel] [PATCH] fsl-imx6: Swap Ethernet interrupt defines

[Bill Paul]

Of all the gin joints in all the towns in all the world, Guenter Roeck had to 
walk into mine at 10:20 on Friday 09 March 2018 and say:

> On Fri, Mar 09, 2018 at 05:47:16PM +, Peter Maydell wrote:
> > On 8 March 2018 at 18:28, Bill Paul  wrote:
> > > Anyway, this means that the only reason older Linux kernels worked in
> > > QEMU with the broken interrupt configuration is that they also
> > > registered a handler on vector 151 (119). Even though QEMU could not
> > > send events via GPIO6, it was mistakenly sending them via vector 151,
> > > so it looked like things worked. On real hardware, the older kernels
> > > would have gotten their interrupts via GPIO6 and also worked. The
> > > older kernels would _not_ work if you fix QEMU because now they would
> > > never get interrupts on either vector, unless you fudge things so that
> > > the ENET module triggers both vector 150 and the vector for GPIO6 in
> > > the GIC or patch them to back out the erratum 6678 workaround as later
> > > kernels do.
> > 
> > Thanks for that really useful writeup. So if I understand correctly
> > 
> > we have several choices here:
> >  (1) we could implement a model of the IOMUX block that is at least
> >  sufficient to support guests that configure it to route the ENET
> >  interrupt line to a GPIO pin. Then we could apply this patch that fixes
> >  the ENET line definitions. Old kernels would continue to work (for the
> >  same reason they worked on hardware), and new ones would work now too.
> >  This is in some ways the preferred option, but it's possibly a lot of
> >  code and we're nearly in freeze for 2.12.
> >  
> >  (2) we could leave everything as it is for 2.12. This would mean that
> >  at least we don't regress setups that used to work on older QEMU
> >  versions. Downside is that we wouldn't be able to run Linux v4.15+, or
> >  other guest OSes that don't have the bug that older Linux kernels do.
> >  (Presumably we'd only do this on the understanding that we were going
> >  to go down route (1) for 2.13.)
> >  
> >  (3) we could apply this patch for 2.12. Linux v4.15+ now works, as
> >  do other guest OSes that use the ENET interrupt. v4.1 and older Linux
> >  guests that used to boot in QEMU stop doing so, and 4.2-4.9 boot but
> >  lose the ethernet device support. Perhaps for 2.13 we might
> >  take route (1) to make those older guests start working again.
> > 
> > Do I have that right?
> 
> Pretty much.

There may be a 4th option.

Since older kernels work because they were looking at vector 151, you could 
patch the imx_fec.c module so that when it signals an event for vector 150, it 
also signals vector 151 too. This would keep newer versions of QEMU "bug-
compatible" with older versions while also fixing support for newer Linux 
kernels and other guests (e.g. VxWorks) that follow the hardware spec 
correctly.

I think this would require only a small modification to this function:

static void imx_eth_update(IMXFECState *s)
{
if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] & ENET_INT_TS_TIMER) {
qemu_set_irq(s->irq[1], 1);
} else {
qemu_set_irq(s->irq[1], 0);
}

if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] & ENET_INT_MAC) {
qemu_set_irq(s->irq[0], 1);
} else {
qemu_set_irq(s->irq[0], 0);
}
}

(For ENET_INT_MAC events, just signal both s->irq[0] and s->irq[1]).

This of course means signaling spurious events on vector 151, but you're doing 
that now anyway. :)

This is much less work than implementing an IOMUX module. Creating an IOMUX 
module for QEMU just for this one fixup would probably be the most elegant 
solution, but adding IOMUX support to QEMU also doesn't make much sense since 
QEMU has no actual pins.

-Bill

> > None of these options seems especially palatable to me, so we're
> > choosing the lesser evil, I think... (unless somebody wants to say
> > that option (1) would be 20 lines of code and here's the patch :-))
> > I guess in the absence of (1) that (3) is better than (2) ?
> 
> I would prefer (2). This is what I decided to use in my "local"
> version of qemu. Older versions of Linux can be fixed by applying one
> (4.2..4.9) or two (4.1 and older) upstream patches; anyone interested
> running those kernels in qemu with Ethernet working should apply those
> patches (or, alternatively, provide a patch adding IOMUX support to
> qemu).
> 
> Thanks,
> Guenter
-- 
=
-Bill Paul(510) 749-2329 | Senior Member of Technical Staff,
 wp...@windriver.com | Master of Unix-Fu - Wind River Systems
=
   "I put a dollar in a change machine. Nothing changed." - George Carlin
=

Re: [Qemu-devel] [PATCH 7/9] iotests.py: tiny refactor: move system imports up

[Vladimir Sementsov-Ogievskiy]

16.02.2018 23:44, Eric Blake wrote:

On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote:

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  tests/qemu-iotests/iotests.py | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)


What breaks if they aren't moved?  But stylistically, this looks 
reasonable; and can be merged independently of NBD stuff if someone 
else wants.


This movement also make it a bit nearer to PEP8, as it dislike "E402 
module level import not at top of file", because of "sys.path.append" 
before imports.. So, I'd just saved as many imports as I could)




Reviewed-by: Eric Blake 




--
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH] fsl-imx6: Swap Ethernet interrupt defines

[Peter Maydell]

On 9 March 2018 at 18:20, Guenter Roeck  wrote:
> On Fri, Mar 09, 2018 at 05:47:16PM +, Peter Maydell wrote:
>> Thanks for that really useful writeup. So if I understand correctly
>> we have several choices here:
>>
>>  (1) we could implement a model of the IOMUX block that is at least
>>  sufficient to support guests that configure it to route the ENET interrupt
>>  line to a GPIO pin. Then we could apply this patch that fixes the ENET
>>  line definitions. Old kernels would continue to work (for the same
>>  reason they worked on hardware), and new ones would work now too.
>>  This is in some ways the preferred option, but it's possibly a lot
>>  of code and we're nearly in freeze for 2.12.
>>
>>  (2) we could leave everything as it is for 2.12. This would mean that
>>  at least we don't regress setups that used to work on older QEMU versions.
>>  Downside is that we wouldn't be able to run Linux v4.15+, or other
>>  guest OSes that don't have the bug that older Linux kernels do.
>>  (Presumably we'd only do this on the understanding that we were going
>>  to go down route (1) for 2.13.)
>>
>>  (3) we could apply this patch for 2.12. Linux v4.15+ now works, as
>>  do other guest OSes that use the ENET interrupt. v4.1 and older Linux
>>  guests that used to boot in QEMU stop doing so, and 4.2-4.9 boot but
>>  lose the ethernet device support. Perhaps for 2.13 we might
>>  take route (1) to make those older guests start working again.
>>
>> Do I have that right?
>>
> Pretty much.
>
>> None of these options seems especially palatable to me, so we're
>> choosing the lesser evil, I think... (unless somebody wants to say
>> that option (1) would be 20 lines of code and here's the patch :-))
>> I guess in the absence of (1) that (3) is better than (2) ?
>>
>
> I would prefer (2). This is what I decided to use in my "local"
> version of qemu. Older versions of Linux can be fixed by applying one
> (4.2..4.9) or two (4.1 and older) upstream patches; anyone interested
> running those kernels in qemu with Ethernet working should apply those
> patches (or, alternatively, provide a patch adding IOMUX support to
> qemu).

Did you mean "prefer (3) [apply this patch]" ? The rest of the paragraph
makes more sense if you did.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/7] Block patches

[Peter Maydell]

On 9 March 2018 at 13:19, Stefan Hajnoczi  wrote:
> The following changes since commit 0ab4537f08e09b13788db67efd760592fb7db769:
>
>   Merge remote-tracking branch 
> 'remotes/stefanberger/tags/pull-tpm-2018-03-07-1' into staging (2018-03-08 
> 12:56:39 +)
>
> are available in the Git repository at:
>
>   git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 4486e89c219c0d1b9bd8dfa0b1dd5b0d51ff2268:
>
>   vl: introduce vm_shutdown() (2018-03-08 17:38:51 +)
>
> 
>
> 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH 6/9] nbd: BLOCK_STATUS for standard get_block_status function: client part

[Vladimir Sementsov-Ogievskiy]

16.02.2018 23:40, Eric Blake wrote:

On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote:

Minimal realization: only one extent in server answer is supported.
Flag NBD_CMD_FLAG_REQ_ONE is used to force this behavior.


[...]


+    memcpy(extent, payload, sizeof(*extent));
+    be32_to_cpus(>length);
+    be32_to_cpus(>flags);


Instead of doing a memcpy() and then in-place bit-swizzling, you could 
do the swapping as part of assignment, for one less function call (and 
make the code a bit easier to extend, if we later drop our REQ_ONE 
limitation on only having one extent, because you'll advance payload 
as needed):


extent->length = payload_advance32();
extent->flags = payload_advance32();


Aha, yes. The funny thing is that these are my own helpers.



We should probably validate that the length field is a multiple of 
min_block (if a server tells us that all operations must be 512-byte 
aligned, then reports an extent that is smaller than 512 bytes, we 
have no way to ask for the status of the second half of the sector). 
Probably also something that needs to be explicitly stated in the NBD 
spec. [1]



+
+    if (extent->length > orig_length) {
+    error_setg(errp, "Protocol error: server sent chunk 
exceeding requested"

+ " region");
+    return -EINVAL;


That matches the current spec wording, but I'm not sure I agree with 
it - what's wrong with a server providing a final extent that extends 
beyond the request, if the information was already available for free 
(the classic example: if the server never replies with HOLE or ZERO, 
then the entire file has the same status, so all requests could 
trivially be replied to by taking the starting offset to the end of 
the file as the returned length, rather than just clamping at the 
requested length).


Maybe. But our already released clients not prepared to such change =(

But, on the other hand, this gives us possibility to understand, the the 
whole target (for backup/mirror) is zero in one request, skipping 
target-zeroing loop by 4gb chunks.. What about adding such possibility 
with an additionally negotiated option or something like this? (and 
don't we now have same possibility with something like INFO?)





+    }
+
+    return 0;
+}
+
  /* nbd_parse_error_payload
   * on success @errp contains message describing nbd error reply
   */


--
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v9 09/14] hw/arm/smmuv3: Implement translate callback

[Peter Maydell]

On 17 February 2018 at 18:46, Eric Auger  wrote:
> This patch implements the IOMMU Memory Region translate()
> callback. Most of the code relates to the translation
> configuration decoding and check (STE, CD).
>
> Signed-off-by: Eric Auger 
>
> ---
> v8 -> v9:
> - use SMMU_EVENT_STRING macro
> - get rid of last erro_report's
> - decode asid
> - handle config abort before ptw
> - add 64-bit single-copy atomic comment
>
> v7 -> v8:
> - use address_space_rw
> - s/Ste/STE, s/Cd/CD
> - use dma_memory_read
> - remove everything related to stage 2
> - collect data for both TTx
> - renamings
> - pass the event handle all along the config decoding path
> - decode tbi, ars
> ---
>  hw/arm/smmuv3-internal.h | 146 
>  hw/arm/smmuv3.c  | 341 
> +++
>  hw/arm/trace-events  |   9 ++
>  3 files changed, 496 insertions(+)
>
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index 3929f69..b203426 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -462,4 +462,150 @@ typedef struct SMMUEventInfo {
>
>  void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo *event);
>
> +/* Configuration Data */
> +
> +/* STE Level 1 Descriptor */
> +typedef struct STEDesc {
> +uint32_t word[2];
> +} STEDesc;
> +
> +/* CD Level 1 Descriptor */
> +typedef struct CDDesc {
> +uint32_t word[2];
> +} CDDesc;
> +
> +/* Stream Table Entry(STE) */
> +typedef struct STE {
> +uint32_t word[16];
> +} STE;
> +
> +/* Context Descriptor(CD) */
> +typedef struct CD {
> +uint32_t word[16];
> +} CD;
> +
> +/* STE fields */
> +
> +#define STE_VALID(x)   extract32((x)->word[0], 0, 1) /* 0 */

I'm not sure what the comment is supposed to be telling me ?

> +
> +#define STE_CONFIG(x)  extract32((x)->word[0], 1, 3)
> +#define STE_CFG_S1_ENABLED(config) (config & 0x1)
> +#define STE_CFG_S2_ENABLED(config) (config & 0x2)
> +#define STE_CFG_ABORT(config)  (!(config & 0x4))
> +#define STE_CFG_BYPASS(config) (config == 0x4)
> +
> +#define STE_S1FMT(x)   extract32((x)->word[0], 4 , 2)
> +#define STE_S1CDMAX(x) extract32((x)->word[1], 27, 5)
> +#define STE_EATS(x)extract32((x)->word[2], 28, 2)
> +#define STE_STRW(x)extract32((x)->word[2], 30, 2)
> +#define STE_S2VMID(x)  extract32((x)->word[4], 0 , 16)
> +#define STE_S2T0SZ(x)  extract32((x)->word[5], 0 , 6)
> +#define STE_S2SL0(x)   extract32((x)->word[5], 6 , 2)
> +#define STE_S2TG(x)extract32((x)->word[5], 14, 2)
> +#define STE_S2PS(x)extract32((x)->word[5], 16, 3)
> +#define STE_S2AA64(x)  extract32((x)->word[5], 19, 1)
> +#define STE_S2HD(x)extract32((x)->word[5], 24, 1)
> +#define STE_S2HA(x)extract32((x)->word[5], 25, 1)
> +#define STE_S2S(x) extract32((x)->word[5], 26, 1)
> +#define STE_CTXPTR(x)   \
> +({  \
> +unsigned long addr; \
> +addr = (uint64_t)extract32((x)->word[1], 0, 16) << 32;  \
> +addr |= (uint64_t)((x)->word[0] & 0xffc0);  \
> +addr;   \
> +})
> +
> +#define STE_S2TTB(x)\
> +({  \
> +unsigned long addr; \
> +addr = (uint64_t)extract32((x)->word[7], 0, 16) << 32;  \
> +addr |= (uint64_t)((x)->word[6] & 0xfff0);  \
> +addr;   \
> +})
> +
> +static inline int oas2bits(int oas_field)
> +{
> +switch (oas_field) {
> +case 0b011:
> +return 42;
> +case 0b100:
> +return 44;
> +default:
> +return 32 + (1 << oas_field);

Is this right? For instance OAS == 0b001 should be 36,
but 32 + (1 << 1) == 34 ?  0b110 should be 52, but
32 + (1 << 6) == 96 ?

I'm not sure there's a neat relationship between the field
values and the address sizes here, so maybe we should just
have a switch with a case for each value?

> +   }
> +}
> +
> +static inline int pa_range(STE *ste)
> +{
> +int oas_field = MIN(STE_S2PS(ste), SMMU_IDR5_OAS);
> +
> +if (!STE_S2AA64(ste)) {
> +return 40;
> +}
> +
> +return oas2bits(oas_field);
> +}
> +
> +#define MAX_PA(ste) ((1 << pa_range(ste)) - 1)
> +
> +/* CD fields */
> +
> +#define CD_VALID(x)   extract32((x)->word[0], 30, 1)
> +#define CD_ASID(x)extract32((x)->word[1], 16, 16)
> +#define CD_TTB(x, sel)  \
> +({  \
> +uint64_t hi, lo;\
> +hi = extract32((x)->word[(sel) * 2 + 3], 0, 16);\

Spec says TTB0/1 fields go up to bit 19. You want the whole lot
even if we don't support address sizes that wide 

Re: [Qemu-devel] [edk2] [PATCH v3 0/7] ovmf: preliminary TPM2 support

[Laszlo Ersek]

Hi Marc-André,

On 03/09/18 14:09, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> Hi,
> 
> The following series adds basic TPM2 support for OVMF-on-QEMU (I
> haven't tested TPM1, for lack of interest). It links with the modules
> to initializes the device in PEI phase, and do measurements (both PEI
> and DXE). The Tcg2Dxe module provides the Tcg2 protocol which allows
> the guest to access the measurement log and other facilities.
> 
> DxeTpm2MeasureBootLib seems to do its job at measuring images that are
> not measured in PEI phase (such as PCI PXE rom)
> 
> Tcg2ConfigDxe is not included due to its integration with edk2 own PPI
> implementation which conflicts with qemu design. PPI design is still
> being discussed & experimented at this point.
> 
> Linux guests seem to work fine. But windows guest generally complains
> about the lack of PPI interface (most HLK tests require it, tpm.msc
> admin interactions too). I haven't done "real" use-cases tests, as I
> lack experience with TPM usage. Any help appreciated to test the TPM.
> 
> I build edk2 with:
> 
> $ build -DTPM2_ENABLE -DSECURE_BOOT_ENABLE  -DMEM_VARSTORE_EMU_ENABLE=FALSE
> 
> I test with qemu & swtpm/libtpms (tpm2 branches, swtpm_setup.sh --tpm2 
> --tpm-state tpmstatedir)
> 
> $ swtpm socket --tpmstate tpmstatedir --ctrl type=unixio,path=tpmsock  --tpm2 
> &
> $ qemu .. -chardev socket,id=chrtpm,path=tpmsock -tpmdev 
> emulator,id=tpm0,chardev=chrtpm -device tpm-crb,tpmdev=tpm0
> 
> Thanks
> 
> Github tree:
> https://github.com/elmarco/edk2/tree/tpm2 (tpm2-v2 tag)
> 
> Related bug:
> https://bugzilla.tianocore.org/show_bug.cgi?id=594
> 
> v3:  after Laszlo review
> - many simplifications to "add customized Tcg2ConfigPei clone" patch
> - various move of fdf/dsc sections
> - modify Ia32 & Ia32x64 fdf/dsc too
> - modify commit messages
> - add r-b tags
> 
> v2:
> - the series can now be applied to master directly, thanks to dropping
>   PeiReadOnlyVariable requirement
> - remove the HOB list workaround, the main fix is now upstream. Add a
>   preliminary patch to complete it.
> - removed traces of TPM1.2 support
> - add own OvmfPkg Tcg2ConfigPei, which performs only TPM2 detection
> - make PcdTpmInstanceGuid default all-bits-zero
> - drop unneeded Pcd values
> - explain why SHA1 is still nice to have (for 1.2 log format)
> - drop Tcg2ConfigDxe
> - more detailed commit messages, thanks to Laszlo explanations!
> - rebased
> 
> TODO:
> - modify Ia32 and Ia32X64 builds
> 
> Marc-André Lureau (7):
>   SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex
>   MdeModulePkg/Core/Pei: fix REGISITER -> REGISTER typo
>   OvmfPkg: simplify SecurityStubDxe.inf inclusion
>   OvmfPkg: add customized Tcg2ConfigPei clone
>   OvmfPkg: include Tcg2Pei module
>   OvmfPkg: include Tcg2Dxe module
>   OvmfPkg: plug DxeTpm2MeasureBootLib into SecurityStubDxe
> 
>  MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 18 ++--
>  MdeModulePkg/Core/Pei/Image/Image.c   |  4 +-
>  MdeModulePkg/Core/Pei/PeiMain.h   |  2 +-
>  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c   |  2 +-
>  OvmfPkg/OvmfPkgIa32.dsc   | 49 ++-
>  OvmfPkg/OvmfPkgIa32.fdf   |  9 ++
>  OvmfPkg/OvmfPkgIa32X64.dsc| 49 ++-
>  OvmfPkg/OvmfPkgIa32X64.fdf|  9 ++
>  OvmfPkg/OvmfPkgX64.dsc| 49 ++-
>  OvmfPkg/OvmfPkgX64.fdf|  9 ++
>  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf  | 53 
>  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c   | 84 +++
>  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf   |  1 -
>  13 files changed, 312 insertions(+), 26 deletions(-)
>  create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
>  create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
> 

huge kudos for this work. I've pushed it now: commit range
7878f706e7eb..d5a002aba0aa.

* For future work, please update your git setting so that you include,
in your commit message:

Contributed-under: TianoCore Contribution Agreement 1.1
^^^

The patches in this series vary between 1.0 and 1.1. For now, 1.0 is
acceptable as well (see
 and commit
f2d1b866dfd9, "BaseTools/PatchCheck: Support Contribution Agreement
1.1", 2017-08-03).

The difference between 1.0 and 1.1 is basically "docs files" -- please
see commit b6538c118ae8 ("edk2: Update to TianoCore Contribution
Agreement 1.1", 2017-08-03). You don't touch documentation in this
series, just source code, so 1.0 is fine (wherever you used that); in
the future 1.1 should be used consistently.

* I did some regression testing too, with this set applied, for both
TRUE and FALSE values of TPM2_ENABLE. (Note that I didn't actually set
up swtpm, so my regression-testing for TRUE exercised the "detection
failed" branch.) Normal boot and S3 

[Qemu-devel] [PATCH 2/5] block/quorum: Remove protocol-related fields

[Fabiano Rosas]

The quorum driver is not a protocol so it should implement bdrv_open
instead of bdrv_file_open and not provide a protocol_name.

Attempts to invoke this driver using protocol syntax
(i.e. quorum:) will now fail gracefully:

  $ qemu-img info quorum:foo
  qemu-img: Could not open 'quorum:foo': Unknown protocol 'quorum'

Signed-off-by: Fabiano Rosas 
---
 block/quorum.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/quorum.c b/block/quorum.c
index 14333c18aa..cfe484a945 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1098,11 +1098,10 @@ static void quorum_refresh_filename(BlockDriverState 
*bs, QDict *options)
 
 static BlockDriver bdrv_quorum = {
 .format_name= "quorum",
-.protocol_name  = "quorum",
 
 .instance_size  = sizeof(BDRVQuorumState),
 
-.bdrv_file_open = quorum_open,
+.bdrv_open  = quorum_open,
 .bdrv_close = quorum_close,
 .bdrv_refresh_filename  = quorum_refresh_filename,
 
-- 
2.13.6

Re: [Qemu-devel] [PATCH] vnc: deal with surface NULL pointers

[Christian Borntraeger]

On 03/08/2018 05:18 PM, Gerd Hoffmann wrote:
> Secondary displays in multihead setups are allowed to have a NULL
> DisplaySurface.  Typically user interfaces handle this by hiding the
> window which shows the display in question.
> 
> This isn't an option for vnc though because it simply hasn't a concept
> of windows or outputs.  So handle the situation by showing a placeholder
> DisplaySurface instead.  Also check in console_select whenever a surface
> is preset in the first place before requesting an update.
> 
> This fixes a segfault which can be triggered by switching to an unused
> display (via vtrl-alt-) in a multihead setup, for example using
> -device virtio-vga,max_outputs=2.
> 
> Cc: Christian Borntraeger 
> Signed-off-by: Gerd Hoffmann 
> ---

Tested-by: Christian Borntraeger 

I can see the new placeholder message and the crash is gone.


>  include/ui/console.h |  2 ++
>  ui/console.c | 10 ++
>  ui/vnc.c | 10 ++
>  3 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/include/ui/console.h b/include/ui/console.h
> index aae9e44cb3..5fca9afcbc 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -260,6 +260,8 @@ DisplaySurface *qemu_create_displaysurface_guestmem(int 
> width, int height,
>  pixman_format_code_t 
> format,
>  int linesize,
>  uint64_t addr);
> +DisplaySurface *qemu_create_message_surface(int w, int h,
> +const char *msg);
>  PixelFormat qemu_default_pixelformat(int bpp);
> 
>  DisplaySurface *qemu_create_displaysurface(int width, int height);
> diff --git a/ui/console.c b/ui/console.c
> index 6ab4ff3baf..348610dd43 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1039,8 +1039,10 @@ void console_select(unsigned int index)
>  dcl->ops->dpy_gfx_switch(dcl, s->surface);
>  }
>  }
> -dpy_gfx_update(s, 0, 0, surface_width(s->surface),
> -   surface_height(s->surface));
> +if (s->surface) {
> +dpy_gfx_update(s, 0, 0, surface_width(s->surface),
> +   surface_height(s->surface));
> +}
>  }
>  if (ds->have_text) {
>  dpy_text_resize(s, s->width, s->height);
> @@ -1370,8 +1372,8 @@ DisplaySurface *qemu_create_displaysurface_guestmem(int 
> width, int height,
>  return surface;
>  }
> 
> -static DisplaySurface *qemu_create_message_surface(int w, int h,
> -   const char *msg)
> +DisplaySurface *qemu_create_message_surface(int w, int h,
> +const char *msg)
>  {
>  DisplaySurface *surface = qemu_create_displaysurface(w, h);
>  pixman_color_t bg = color_table_rgb[0][QEMU_COLOR_BLACK];
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 13c28cabb0..e164eb798c 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -746,9 +746,19 @@ static void vnc_update_server_surface(VncDisplay *vd)
>  static void vnc_dpy_switch(DisplayChangeListener *dcl,
> DisplaySurface *surface)
>  {
> +static const char placeholder_msg[] =
> +"Display output is not active.";
> +static DisplaySurface *placeholder;
>  VncDisplay *vd = container_of(dcl, VncDisplay, dcl);
>  VncState *vs;
> 
> +if (surface == NULL) {
> +if (placeholder == NULL) {
> +placeholder = qemu_create_message_surface(640, 480, 
> placeholder_msg);
> +}
> +surface = placeholder;
> +}
> +
>  vnc_abort_display_jobs(vd);
>  vd->ds = surface;
> 

[Qemu-devel] [PULL 6/6] tests: Silence migration-test 'bad' test

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

In 2c9bb29703c I added a migration test that purposely fails;
unfortunately it prints a copy of the failure message to stderr
which makes the output a bit messy.

Hide stderr for that test.

Signed-off-by: Dr. David Alan Gilbert 
Message-Id: <20180306173042.24572-1-dgilb...@redhat.com>
Reviewed-by: Peter Xu 
Tested-by: Peter Xu 
Signed-off-by: Dr. David Alan Gilbert 
---
 tests/migration-test.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 74f9361bdd..422bf1afdf 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -382,7 +382,7 @@ static void migrate_start_postcopy(QTestState *who)
 }
 
 static void test_migrate_start(QTestState **from, QTestState **to,
-   const char *uri)
+   const char *uri, bool hide_stderr)
 {
 gchar *cmd_src, *cmd_dst;
 char *bootpath = g_strdup_printf("%s/bootsect", tmpfs);
@@ -427,6 +427,17 @@ static void test_migrate_start(QTestState **from, 
QTestState **to,
 
 g_free(bootpath);
 
+if (hide_stderr) {
+gchar *tmp;
+tmp = g_strdup_printf("%s 2>/dev/null", cmd_src);
+g_free(cmd_src);
+cmd_src = tmp;
+
+tmp = g_strdup_printf("%s 2>/dev/null", cmd_dst);
+g_free(cmd_dst);
+cmd_dst = tmp;
+}
+
 *from = qtest_start(cmd_src);
 g_free(cmd_src);
 
@@ -518,7 +529,7 @@ static void test_migrate(void)
 char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
 QTestState *from, *to;
 
-test_migrate_start(, , uri);
+test_migrate_start(, , uri, false);
 
 migrate_set_capability(from, "postcopy-ram", "true");
 migrate_set_capability(to, "postcopy-ram", "true");
@@ -560,7 +571,7 @@ static void test_baddest(void)
 const char *status;
 bool failed;
 
-test_migrate_start(, , "tcp:0:0");
+test_migrate_start(, , "tcp:0:0", true);
 migrate(from, "tcp:0:0");
 do {
 rsp = wait_command(from, "{ 'execute': 'query-migrate' }");
-- 
2.14.3

[Qemu-devel] [PULL 5/6] migration: fix applying wrong capabilities

[Dr. David Alan Gilbert (git)]

From: Peter Xu 

When setting migration capabilities via QMP/HMP, we'll apply them even
if the capability check failed.  Fix it.

Fixes: 4a84214ebe ("migration: provide migrate_caps_check()", 2017-07-18)
Signed-off-by: Peter Xu 
Message-Id: <20180305094938.31374-1-pet...@redhat.com>
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Dr. David Alan Gilbert 
---
 migration/migration.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 62c243d2d4..6a4780ef6f 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -747,13 +747,15 @@ void 
qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
 {
 MigrationState *s = migrate_get_current();
 MigrationCapabilityStatusList *cap;
+bool cap_list[MIGRATION_CAPABILITY__MAX];
 
 if (migration_is_setup_or_active(s->state)) {
 error_setg(errp, QERR_MIGRATION_ACTIVE);
 return;
 }
 
-if (!migrate_caps_check(s->enabled_capabilities, params, errp)) {
+memcpy(cap_list, s->enabled_capabilities, sizeof(cap_list));
+if (!migrate_caps_check(cap_list, params, errp)) {
 return;
 }
 
-- 
2.14.3

Re: [Qemu-devel] [PATCH v8 00/23] RISC-V QEMU Port Submission

[Richard W.M. Jones]

On Fri, Mar 09, 2018 at 04:43:34PM +, Peter Maydell wrote:
> On 5 March 2018 at 08:41, Richard W.M. Jones  wrote:
> >
> > The attached patch is also needed to avoid crashes during various
> > math-heavy test suites.
> 
> Applied to master, thanks.
> 
> FYI, sending patches as attachments in the middle of a long
> thread on something else is a good way to cause them to get
> lost. Luckily Michael ran into the same bug again and that
> reminded me to fish this one out.

Yup sorry I thought that the patch had been separately submitted,
but apparently it never was.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

[Qemu-devel] [PATCH 5/5] include/block/block_int: Document protocol related functions

[Fabiano Rosas]

Clarify that for protocols the brdv_file_open function is used instead
of bdrv_open and that protocol_name is expected to be set.

Signed-off-by: Fabiano Rosas 
---
 include/block/block_int.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 64a5700f2b..d5e864c2dc 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -126,6 +126,8 @@ struct BlockDriver {
 
 int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags,
  Error **errp);
+
+/* Protocol drivers should implement this instead of bdrv_open */
 int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
   Error **errp);
 void (*bdrv_close)(BlockDriverState *bs);
@@ -247,6 +249,10 @@ struct BlockDriver {
  */
 int coroutine_fn (*bdrv_co_flush_to_os)(BlockDriverState *bs);
 
+/*
+ * Drivers that set this field should also provide a
+ * bdrv_file_open implementation
+ */
 const char *protocol_name;
 int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset,
  PreallocMode prealloc, Error **errp);
-- 
2.13.6

[Qemu-devel] [PATCH 5/6] luks: Catch integer overflow for huge sizes

[Kevin Wolf]

When you request an image size close to UINT64_MAX, the addition of the
crypto header may cause an integer overflow. Catch it instead of
silently truncating the image size.

Signed-off-by: Kevin Wolf 
---
 block/crypto.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/block/crypto.c b/block/crypto.c
index 4908d8627f..1b46519c53 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -102,6 +102,11 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
 {
 struct BlockCryptoCreateData *data = opaque;
 
+if (headerlen > UINT64_MAX - data->size) {
+error_setg(errp, "The requested file size is too large");
+return -EFBIG;
+}
+
 /* User provided size should reflect amount of space made
  * available to the guest, so we must take account of that
  * which will be used by the crypto header
-- 
2.13.6

[Qemu-devel] [PATCH 1/5] block/replication: Remove protocol_name field

[Fabiano Rosas]

The replication driver is only selected explicitly (via
driver=replication,mode=primary,...) so it is not a protocol driver.

This patch removes the protocol_name field from the brdv_replication
structure so that attempts to invoke this driver using protocol
syntax (i.e. replication:) will fail gracefully:

  $ qemu-img info replication:foo
  qemu-img: Could not open 'replication:': Unknown protocol 'replication'

Buglink: https://bugs.launchpad.net/qemu/+bug/1726733
Signed-off-by: Fabiano Rosas 
---
 block/replication.c | 1 -
 replication.h   | 1 -
 2 files changed, 2 deletions(-)

diff --git a/block/replication.c b/block/replication.c
index f98ef094b9..6c0c7186d9 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -703,7 +703,6 @@ static void replication_stop(ReplicationState *rs, bool 
failover, Error **errp)
 
 BlockDriver bdrv_replication = {
 .format_name= "replication",
-.protocol_name  = "replication",
 .instance_size  = sizeof(BDRVReplicationState),
 
 .bdrv_open  = replication_open,
diff --git a/replication.h b/replication.h
index 8faefe005f..4c8354de23 100644
--- a/replication.h
+++ b/replication.h
@@ -67,7 +67,6 @@ typedef struct ReplicationState ReplicationState;
  *
  * BlockDriver bdrv_replication = {
  * .format_name= "replication",
- * .protocol_name  = "replication",
  * .instance_size  = sizeof(BDRVReplicationState),
  *
  * .bdrv_open  = replication_open,
-- 
2.13.6