Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp
08.03.2020 22:09, Christian Schoenebeck wrote: On Freitag, 6. März 2020 06:15:28 CET Vladimir Sementsov-Ogievskiy wrote: diff --git a/scripts/coccinelle/auto-propagated-errp.cocci b/scripts/coccinelle/auto-propagated-errp.cocci new file mode 100644 index 00..bff274bd6d --- /dev/null +++ b/scripts/coccinelle/auto-propagated-errp.cocci @@ -0,0 +1,231 @@ +// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h) +// +// Copyright (c) 2020 Virtuozzo International GmbH. Just in case: WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? Hmm, seems this, and some other coccinelle scripts should be added to "Error reporting" section. Best regards, Christian Schoenebeck -- Best regards, Vladimir
Re: [PATCH v3 2/3] pci: Honour wmask when resetting PCI_INTERRUPT_LINE
On Mon, Mar 09, 2020 at 09:54:27PM +0100, BALATON Zoltan wrote:
> On Mon, 9 Mar 2020, Michael S. Tsirkin wrote:
> > On Mon, Mar 09, 2020 at 08:18:13PM +0100, BALATON Zoltan wrote:
> > > The pci_do_device_reset() function (called from pci_device_reset)
> > > clears the PCI_INTERRUPT_LINE config reg of devices on the bus but did
> > > this without taking wmask into account. We'll have a device model now
> > > that needs to set a constant value for this reg and this patch allows
> > > to do that without additional workaround in device emulation to
> > > reverse the effect of this PCI bus reset function.
> > >
> > > Suggested-by: Mark Cave-Ayland
> > > Signed-off-by: BALATON Zoltan
> > > ---
> > > hw/pci/pci.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > index e1ed6677e1..d07e4ed9de 100644
> > > --- a/hw/pci/pci.c
> > > +++ b/hw/pci/pci.c
> > > @@ -302,8 +302,10 @@ static void pci_do_device_reset(PCIDevice *dev)
> > > pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
> > > pci_get_word(dev->wmask + PCI_STATUS) |
> > > pci_get_word(dev->w1cmask +
> > > PCI_STATUS));
> > > +pci_word_test_and_clear_mask(dev->config + PCI_INTERRUPT_LINE,
> > > + pci_get_word(dev->wmask +
> > > PCI_INTERRUPT_LINE) |
> > > + pci_get_word(dev->w1cmask +
> > > PCI_INTERRUPT_LINE));
> >
> > PCI spec says:
> >
> > Interrupt Line
> > The Interrupt Line register is an eight-bit register used to communicate
> > interrupt line routing
> > information.
> >
> > I don't see how it makes sense to access it as a word.
>
> Patch actually comes from Mark, I don't know. Should we change it to
> pci_byte_test_and_clear_mask or what's the appropriate way here?
Superficially that makes sense. I don't know if that does what
you want to do.
> >
> > > dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
> > > -dev->config[PCI_INTERRUPT_LINE] = 0x0;
> > > for (r = 0; r < PCI_NUM_REGIONS; ++r) {
> > > PCIIORegion *region = &dev->io_regions[r];
> > > if (!region->size) {
> >
> > Please add comments here explaining that some devices
> > make PCI_INTERRUPT_LINE read-only.
>
> Something like the commit message would be appropriate?
Code comments are more appropriate when we want to describe why code is
the way it is. commit message is there to describe the change, answering
questions like: why we aren't happy with the old code? why is the new
code is better? etc ...
> Regards,
> BALATON Zoltan
Re: [PATCH v3 3/3] via-ide: Also emulate non 100% native mode
On Mon, Mar 09, 2020 at 09:50:57PM +0100, BALATON Zoltan wrote:
> On Mon, 9 Mar 2020, Michael S. Tsirkin wrote:
> > On Mon, Mar 09, 2020 at 08:18:13PM +0100, BALATON Zoltan wrote:
> > > Some machines operate in "non 100% native mode" where interrupts are
> > > fixed at legacy IDE interrupts and some guests expect this behaviour
> > > without checking based on knowledge about hardware. Even Linux has
> > > arch specific workarounds for this that are activated on such boards
> > > so this needs to be emulated as well.
> > >
> > > Signed-off-by: BALATON Zoltan
> > > ---
> > > v2: Don't use PCI_INTERRUPT_LINE in via_ide_set_irq()
> > > v3: Patch pci.c instead of local workaround for PCI reset clearing
> > > PCI_INTERRUPT_PIN config reg
> > >
> > > hw/ide/via.c| 37 +
> > > hw/mips/mips_fulong2e.c | 2 +-
> > > include/hw/ide.h| 3 ++-
> > > 3 files changed, 32 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/hw/ide/via.c b/hw/ide/via.c
> > > index 096de8dba0..02d29809f2 100644
> > > --- a/hw/ide/via.c
> > > +++ b/hw/ide/via.c
> > > @@ -1,9 +1,10 @@
> > > /*
> > > - * QEMU IDE Emulation: PCI VIA82C686B support.
> > > + * QEMU VIA southbridge IDE emulation (VT82C686B, VT8231)
> > > *
> > > * Copyright (c) 2003 Fabrice Bellard
> > > * Copyright (c) 2006 Openedhand Ltd.
> > > * Copyright (c) 2010 Huacai Chen
> > > + * Copyright (c) 2019-2020 BALATON Zoltan
> > > *
> > > * Permission is hereby granted, free of charge, to any person obtaining
> > > a copy
> > > * of this software and associated documentation files (the "Software"),
> > > to deal
> > > @@ -25,6 +26,8 @@
> > > */
> > >
> > > #include "qemu/osdep.h"
> > > +#include "qemu/range.h"
> > > +#include "hw/qdev-properties.h"
> > > #include "hw/pci/pci.h"
> > > #include "migration/vmstate.h"
> > > #include "qemu/module.h"
> > > @@ -111,11 +114,18 @@ static void via_ide_set_irq(void *opaque, int n,
> > > int level)
> > > } else {
> > > d->config[0x70 + n * 8] &= ~0x80;
> > > }
> > > -
> > > level = (d->config[0x70] & 0x80) || (d->config[0x78] & 0x80);
> > > -n = pci_get_byte(d->config + PCI_INTERRUPT_LINE);
> > > -if (n) {
> > > -qemu_set_irq(isa_get_irq(NULL, n), level);
> > > +
> > > +/*
> > > + * Some machines operate in "non 100% native mode" where
> > > PCI_INTERRUPT_LINE
> > > + * is not used but IDE always uses ISA IRQ 14 and 15 even in native
> > > mode.
> > > + * Some guest drivers expect this, often without checking.
> > > + */
> > > +if (!(pci_get_byte(d->config + PCI_CLASS_PROG) & (n ? 4 : 1)) ||
> > > +PCI_IDE(d)->flags & BIT(PCI_IDE_LEGACY_IRQ)) {
> > > +qemu_set_irq(isa_get_irq(NULL, (n ? 15 : 14)), level);
> > > +} else {
> > > +qemu_set_irq(isa_get_irq(NULL, 14), level);
> > > }
> > > }
> > >
> > > @@ -169,7 +179,8 @@ static void via_ide_realize(PCIDevice *dev, Error
> > > **errp)
> > >
> > > pci_config_set_prog_interface(pci_conf, 0x8f); /* native PCI ATA
> > > mode */
> > > pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x00c0);
> > > -dev->wmask[PCI_INTERRUPT_LINE] = 0xf;
> > > +dev->wmask[PCI_CLASS_PROG] = 5;
> >
> > What's the story here? Why is class suddenly writeable?
>
> The via-ide (function 1 of some VIA southbridge chips) use bits in this reg
> to set legacy compatibility mode as described in VT82C686B and VT8231
> datasheets and Linux writes this on pegasos2 board I'm emulating. See longer
> description in this message:
>
> https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg00019.html
>
> Regards,
> BALATON Zoltan
Pls add a code comment so people don't have to dig through mailing list
archives.
--
MST
Re: [PATCH] qcow2: remove QCowL2Meta parameter from handle_copied
CC qemu-block and qcow2 maintainers
(Use scripts/get_maintainer.pl to identify maintainers.)
On 3/9/20 12:35 PM, Yi Li wrote:
> The QCowL2Meta **m parameter is not used
>
> Signed-off-by: Yi Li
> ---
> block/qcow2-cluster.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 17f1363..db9efa5 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1154,7 +1154,7 @@ static int handle_dependencies(BlockDriverState *bs,
> uint64_t guest_offset,
> * -errno: in error cases
> */
> static int handle_copied(BlockDriverState *bs, uint64_t guest_offset,
> -uint64_t *host_offset, uint64_t *bytes, QCowL2Meta **m)
> +uint64_t *host_offset, uint64_t *bytes)
> {
> BDRVQcow2State *s = bs->opaque;
> int l2_index;
> @@ -1567,7 +1567,7 @@ again:
> /*
> * 2. Count contiguous COPIED clusters.
> */
> -ret = handle_copied(bs, start, &cluster_offset, &cur_bytes, m);
> +ret = handle_copied(bs, start, &cluster_offset, &cur_bytes);
> if (ret < 0) {
> return ret;
> } else if (ret) {
>
Seems OK to me -- it is definitely unused.
(Is _that_ a problem? For qcow2 maintainers to know.)
Reviewed-by: John Snow
Re: [PATCH v3 0/3] Implement "non 100% native mode" in via-ide
Patchew URL: https://patchew.org/QEMU/cover.1583781493.git.bala...@eik.bme.hu/
Hi,
This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==6206==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="test-coroutine"
==6249==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 1 test-coroutine /basic/no-dangling-access
PASS 2 test-coroutine /basic/lifecycle
PASS 3 test-coroutine /basic/yield
==6249==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top:
0x7fff1325c000; bottom 0x7f172b0bd000; size: 0x00e7e819f000 (996031459328)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 4 test-coroutine /basic/nesting
---
PASS 11 test-aio /aio/event/wait
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
==6264==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img
tests/qtest/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="ide-test"
==6272==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 28 test-aio /aio-gsource/timer/schedule
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-aio-multithread -m=quick -k --tap < /dev/null |
./scripts/tap-driver.pl --test-name="test-aio-multithread"
PASS 1 ide-test /x86_64/ide/identify
==6279==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
==6281==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 2 ide-test /x86_64/ide/flush
PASS 2 test-aio-multithread /aio/multi/schedule
==6298==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
==6309==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 4 ide-test /x86_64/ide/bmdma/trim
PASS 3 test-aio-multithread /aio/multi/mutex/contended
==6315==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="test-throttle"
==6337==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 1 test-throttle /throttle/leak_bucket
PASS 2 test-throttle /throttle/compute_wait
PASS 3 test-throttle /throttle/init
---
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="test-thread-pool"
==6343==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
PASS 3 test-thread-pool /thread-pool/submit-co
PASS 4 test-thread-pool /thread-pool/submit-many
==6339==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 5 test-thread-pool /thread-pool/cancel
PASS 6
Re: [PATCH v3 3/3] via-ide: Also emulate non 100% native mode
On Mon, 9 Mar 2020, BALATON Zoltan wrote:
On Mon, 9 Mar 2020, Michael S. Tsirkin wrote:
On Mon, Mar 09, 2020 at 08:18:13PM +0100, BALATON Zoltan wrote:
Some machines operate in "non 100% native mode" where interrupts are
fixed at legacy IDE interrupts and some guests expect this behaviour
without checking based on knowledge about hardware. Even Linux has
arch specific workarounds for this that are activated on such boards
so this needs to be emulated as well.
Signed-off-by: BALATON Zoltan
---
v2: Don't use PCI_INTERRUPT_LINE in via_ide_set_irq()
v3: Patch pci.c instead of local workaround for PCI reset clearing
PCI_INTERRUPT_PIN config reg
hw/ide/via.c| 37 +
hw/mips/mips_fulong2e.c | 2 +-
include/hw/ide.h| 3 ++-
3 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 096de8dba0..02d29809f2 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -1,9 +1,10 @@
/*
- * QEMU IDE Emulation: PCI VIA82C686B support.
+ * QEMU VIA southbridge IDE emulation (VT82C686B, VT8231)
*
* Copyright (c) 2003 Fabrice Bellard
* Copyright (c) 2006 Openedhand Ltd.
* Copyright (c) 2010 Huacai Chen
+ * Copyright (c) 2019-2020 BALATON Zoltan
*
* Permission is hereby granted, free of charge, to any person obtaining
a copy
* of this software and associated documentation files (the "Software"),
to deal
@@ -25,6 +26,8 @@
*/
#include "qemu/osdep.h"
+#include "qemu/range.h"
+#include "hw/qdev-properties.h"
#include "hw/pci/pci.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
@@ -111,11 +114,18 @@ static void via_ide_set_irq(void *opaque, int n, int
level)
} else {
d->config[0x70 + n * 8] &= ~0x80;
}
-
level = (d->config[0x70] & 0x80) || (d->config[0x78] & 0x80);
-n = pci_get_byte(d->config + PCI_INTERRUPT_LINE);
-if (n) {
-qemu_set_irq(isa_get_irq(NULL, n), level);
+
+/*
+ * Some machines operate in "non 100% native mode" where
PCI_INTERRUPT_LINE
+ * is not used but IDE always uses ISA IRQ 14 and 15 even in native
mode.
+ * Some guest drivers expect this, often without checking.
+ */
+if (!(pci_get_byte(d->config + PCI_CLASS_PROG) & (n ? 4 : 1)) ||
+PCI_IDE(d)->flags & BIT(PCI_IDE_LEGACY_IRQ)) {
+qemu_set_irq(isa_get_irq(NULL, (n ? 15 : 14)), level);
+} else {
+qemu_set_irq(isa_get_irq(NULL, 14), level);
}
}
@@ -169,7 +179,8 @@ static void via_ide_realize(PCIDevice *dev, Error
**errp)
pci_config_set_prog_interface(pci_conf, 0x8f); /* native PCI ATA mode
*/
pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x00c0);
-dev->wmask[PCI_INTERRUPT_LINE] = 0xf;
+dev->wmask[PCI_CLASS_PROG] = 5;
What's the story here? Why is class suddenly writeable?
The via-ide (function 1 of some VIA southbridge chips) use bits in this reg
to set legacy compatibility mode as described in VT82C686B and VT8231
datasheets and Linux writes this on pegasos2 board I'm emulating. See longer
description in this message:
https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg00019.html
And before we go through all previous discussions again I'd like to add
that the comment in Linux fixup function saying that firmware sets
conroller in legacy mode is wrong it does actually set it in native mode
but on this hardware IRQs are hardcoded to legacy interrupts for some
reason even in native mode. Most guest OSes just know this and expect that
without looking at config regs but Linux uses this reg to force its driver
to legacy mode despite using io addresses from PCI BARs which Linux calls
non-100% native mode. This probably happens on some platforms but looks
like pegasos2 will be the first in QEMU.
Regards,
BALATON Zoltan
Re: [PATCH v3 2/3] pci: Honour wmask when resetting PCI_INTERRUPT_LINE
On Mon, 9 Mar 2020, Michael S. Tsirkin wrote:
On Mon, Mar 09, 2020 at 08:18:13PM +0100, BALATON Zoltan wrote:
The pci_do_device_reset() function (called from pci_device_reset)
clears the PCI_INTERRUPT_LINE config reg of devices on the bus but did
this without taking wmask into account. We'll have a device model now
that needs to set a constant value for this reg and this patch allows
to do that without additional workaround in device emulation to
reverse the effect of this PCI bus reset function.
Suggested-by: Mark Cave-Ayland
Signed-off-by: BALATON Zoltan
---
hw/pci/pci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index e1ed6677e1..d07e4ed9de 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -302,8 +302,10 @@ static void pci_do_device_reset(PCIDevice *dev)
pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
pci_get_word(dev->wmask + PCI_STATUS) |
pci_get_word(dev->w1cmask + PCI_STATUS));
+pci_word_test_and_clear_mask(dev->config + PCI_INTERRUPT_LINE,
+ pci_get_word(dev->wmask + PCI_INTERRUPT_LINE) |
+ pci_get_word(dev->w1cmask + PCI_INTERRUPT_LINE));
PCI spec says:
Interrupt Line
The Interrupt Line register is an eight-bit register used to communicate
interrupt line routing
information.
I don't see how it makes sense to access it as a word.
Patch actually comes from Mark, I don't know. Should we change it to
pci_byte_test_and_clear_mask or what's the appropriate way here?
dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
-dev->config[PCI_INTERRUPT_LINE] = 0x0;
for (r = 0; r < PCI_NUM_REGIONS; ++r) {
PCIIORegion *region = &dev->io_regions[r];
if (!region->size) {
Please add comments here explaining that some devices
make PCI_INTERRUPT_LINE read-only.
Something like the commit message would be appropriate?
Regards,
BALATON Zoltan
Re: [PATCH v3 3/3] via-ide: Also emulate non 100% native mode
On Mon, 9 Mar 2020, Michael S. Tsirkin wrote:
On Mon, Mar 09, 2020 at 08:18:13PM +0100, BALATON Zoltan wrote:
Some machines operate in "non 100% native mode" where interrupts are
fixed at legacy IDE interrupts and some guests expect this behaviour
without checking based on knowledge about hardware. Even Linux has
arch specific workarounds for this that are activated on such boards
so this needs to be emulated as well.
Signed-off-by: BALATON Zoltan
---
v2: Don't use PCI_INTERRUPT_LINE in via_ide_set_irq()
v3: Patch pci.c instead of local workaround for PCI reset clearing
PCI_INTERRUPT_PIN config reg
hw/ide/via.c| 37 +
hw/mips/mips_fulong2e.c | 2 +-
include/hw/ide.h| 3 ++-
3 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 096de8dba0..02d29809f2 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -1,9 +1,10 @@
/*
- * QEMU IDE Emulation: PCI VIA82C686B support.
+ * QEMU VIA southbridge IDE emulation (VT82C686B, VT8231)
*
* Copyright (c) 2003 Fabrice Bellard
* Copyright (c) 2006 Openedhand Ltd.
* Copyright (c) 2010 Huacai Chen
+ * Copyright (c) 2019-2020 BALATON Zoltan
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
deal
@@ -25,6 +26,8 @@
*/
#include "qemu/osdep.h"
+#include "qemu/range.h"
+#include "hw/qdev-properties.h"
#include "hw/pci/pci.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
@@ -111,11 +114,18 @@ static void via_ide_set_irq(void *opaque, int n, int
level)
} else {
d->config[0x70 + n * 8] &= ~0x80;
}
-
level = (d->config[0x70] & 0x80) || (d->config[0x78] & 0x80);
-n = pci_get_byte(d->config + PCI_INTERRUPT_LINE);
-if (n) {
-qemu_set_irq(isa_get_irq(NULL, n), level);
+
+/*
+ * Some machines operate in "non 100% native mode" where PCI_INTERRUPT_LINE
+ * is not used but IDE always uses ISA IRQ 14 and 15 even in native mode.
+ * Some guest drivers expect this, often without checking.
+ */
+if (!(pci_get_byte(d->config + PCI_CLASS_PROG) & (n ? 4 : 1)) ||
+PCI_IDE(d)->flags & BIT(PCI_IDE_LEGACY_IRQ)) {
+qemu_set_irq(isa_get_irq(NULL, (n ? 15 : 14)), level);
+} else {
+qemu_set_irq(isa_get_irq(NULL, 14), level);
}
}
@@ -169,7 +179,8 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
pci_config_set_prog_interface(pci_conf, 0x8f); /* native PCI ATA mode */
pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x00c0);
-dev->wmask[PCI_INTERRUPT_LINE] = 0xf;
+dev->wmask[PCI_CLASS_PROG] = 5;
What's the story here? Why is class suddenly writeable?
The via-ide (function 1 of some VIA southbridge chips) use bits in this
reg to set legacy compatibility mode as described in VT82C686B and VT8231
datasheets and Linux writes this on pegasos2 board I'm emulating. See
longer description in this message:
https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg00019.html
Regards,
BALATON Zoltan
Re: [PATCH v3 2/3] pci: Honour wmask when resetting PCI_INTERRUPT_LINE
On Mon, Mar 09, 2020 at 08:18:13PM +0100, BALATON Zoltan wrote:
> The pci_do_device_reset() function (called from pci_device_reset)
> clears the PCI_INTERRUPT_LINE config reg of devices on the bus but did
> this without taking wmask into account. We'll have a device model now
> that needs to set a constant value for this reg and this patch allows
> to do that without additional workaround in device emulation to
> reverse the effect of this PCI bus reset function.
>
> Suggested-by: Mark Cave-Ayland
> Signed-off-by: BALATON Zoltan
> ---
> hw/pci/pci.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index e1ed6677e1..d07e4ed9de 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -302,8 +302,10 @@ static void pci_do_device_reset(PCIDevice *dev)
> pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
> pci_get_word(dev->wmask + PCI_STATUS) |
> pci_get_word(dev->w1cmask + PCI_STATUS));
> +pci_word_test_and_clear_mask(dev->config + PCI_INTERRUPT_LINE,
> + pci_get_word(dev->wmask + PCI_INTERRUPT_LINE) |
> + pci_get_word(dev->w1cmask +
> PCI_INTERRUPT_LINE));
PCI spec says:
Interrupt Line
The Interrupt Line register is an eight-bit register used to communicate
interrupt line routing
information.
I don't see how it makes sense to access it as a word.
> dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
> -dev->config[PCI_INTERRUPT_LINE] = 0x0;
> for (r = 0; r < PCI_NUM_REGIONS; ++r) {
> PCIIORegion *region = &dev->io_regions[r];
> if (!region->size) {
Please add comments here explaining that some devices
make PCI_INTERRUPT_LINE read-only.
> --
> 2.21.1
>
>
Re: [PATCH v3 3/3] via-ide: Also emulate non 100% native mode
On Mon, Mar 09, 2020 at 08:18:13PM +0100, BALATON Zoltan wrote:
> Some machines operate in "non 100% native mode" where interrupts are
> fixed at legacy IDE interrupts and some guests expect this behaviour
> without checking based on knowledge about hardware. Even Linux has
> arch specific workarounds for this that are activated on such boards
> so this needs to be emulated as well.
>
> Signed-off-by: BALATON Zoltan
> ---
> v2: Don't use PCI_INTERRUPT_LINE in via_ide_set_irq()
> v3: Patch pci.c instead of local workaround for PCI reset clearing
> PCI_INTERRUPT_PIN config reg
>
> hw/ide/via.c| 37 +
> hw/mips/mips_fulong2e.c | 2 +-
> include/hw/ide.h| 3 ++-
> 3 files changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index 096de8dba0..02d29809f2 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -1,9 +1,10 @@
> /*
> - * QEMU IDE Emulation: PCI VIA82C686B support.
> + * QEMU VIA southbridge IDE emulation (VT82C686B, VT8231)
> *
> * Copyright (c) 2003 Fabrice Bellard
> * Copyright (c) 2006 Openedhand Ltd.
> * Copyright (c) 2010 Huacai Chen
> + * Copyright (c) 2019-2020 BALATON Zoltan
> *
> * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> * of this software and associated documentation files (the "Software"), to
> deal
> @@ -25,6 +26,8 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/range.h"
> +#include "hw/qdev-properties.h"
> #include "hw/pci/pci.h"
> #include "migration/vmstate.h"
> #include "qemu/module.h"
> @@ -111,11 +114,18 @@ static void via_ide_set_irq(void *opaque, int n, int
> level)
> } else {
> d->config[0x70 + n * 8] &= ~0x80;
> }
> -
> level = (d->config[0x70] & 0x80) || (d->config[0x78] & 0x80);
> -n = pci_get_byte(d->config + PCI_INTERRUPT_LINE);
> -if (n) {
> -qemu_set_irq(isa_get_irq(NULL, n), level);
> +
> +/*
> + * Some machines operate in "non 100% native mode" where
> PCI_INTERRUPT_LINE
> + * is not used but IDE always uses ISA IRQ 14 and 15 even in native mode.
> + * Some guest drivers expect this, often without checking.
> + */
> +if (!(pci_get_byte(d->config + PCI_CLASS_PROG) & (n ? 4 : 1)) ||
> +PCI_IDE(d)->flags & BIT(PCI_IDE_LEGACY_IRQ)) {
> +qemu_set_irq(isa_get_irq(NULL, (n ? 15 : 14)), level);
> +} else {
> +qemu_set_irq(isa_get_irq(NULL, 14), level);
> }
> }
>
> @@ -169,7 +179,8 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
>
> pci_config_set_prog_interface(pci_conf, 0x8f); /* native PCI ATA mode */
> pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x00c0);
> -dev->wmask[PCI_INTERRUPT_LINE] = 0xf;
> +dev->wmask[PCI_CLASS_PROG] = 5;
What's the story here? Why is class suddenly writeable?
> +dev->wmask[PCI_INTERRUPT_LINE] = 0;
>
> memory_region_init_io(&d->data_bar[0], OBJECT(d), &pci_ide_data_le_ops,
>&d->bus[0], "via-ide0-data", 8);
> @@ -213,14 +224,23 @@ static void via_ide_exitfn(PCIDevice *dev)
> }
> }
>
> -void via_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
> +void via_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn,
> + bool legacy_irq)
> {
> PCIDevice *dev;
>
> -dev = pci_create_simple(bus, devfn, "via-ide");
> +dev = pci_create(bus, devfn, "via-ide");
> +qdev_prop_set_bit(&dev->qdev, "legacy-irq", legacy_irq);
> +qdev_init_nofail(&dev->qdev);
> pci_ide_create_devs(dev, hd_table);
> }
>
> +static Property via_ide_properties[] = {
> +DEFINE_PROP_BIT("legacy-irq", PCIIDEState, flags, PCI_IDE_LEGACY_IRQ,
> +false),
> +DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void via_ide_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -233,6 +253,7 @@ static void via_ide_class_init(ObjectClass *klass, void
> *data)
> k->device_id = PCI_DEVICE_ID_VIA_IDE;
> k->revision = 0x06;
> k->class_id = PCI_CLASS_STORAGE_IDE;
> +device_class_set_props(dc, via_ide_properties);
> set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> }
>
> diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
> index 4727b1d3a4..150182c62a 100644
> --- a/hw/mips/mips_fulong2e.c
> +++ b/hw/mips/mips_fulong2e.c
> @@ -257,7 +257,7 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus,
> int slot, qemu_irq intc,
> isa_create_simple(isa_bus, TYPE_VT82C686B_SUPERIO);
>
> ide_drive_get(hd, ARRAY_SIZE(hd));
> -via_ide_init(pci_bus, hd, PCI_DEVFN(slot, 1));
> +via_ide_init(pci_bus, hd, PCI_DEVFN(slot, 1), false);
>
> pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
> pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
> diff --git a/include/hw/ide.h b/include/hw/ide.h
> index d88c5ee695..2a7001ccba 100644
> --- a/in
Re: [PATCH v2 2/2] via-ide: Also emulate non 100% native mode
On Mon, 9 Mar 2020, Mark Cave-Ayland wrote:
On 09/03/2020 00:42, BALATON Zoltan wrote:
Some machines operate in "non 100% native mode" where interrupts are
fixed at legacy IDE interrupts and some guests expect this behaviour
without checking based on knowledge about hardware. Even Linux has
arch specific workarounds for this that are activated on such boards
so this needs to be emulated as well.
Signed-off-by: BALATON Zoltan
---
v2: Don't use PCI_INTERRUPT_LINE in via_ide_set_irq()
hw/ide/via.c| 57 +++--
hw/mips/mips_fulong2e.c | 2 +-
include/hw/ide.h| 3 ++-
3 files changed, 52 insertions(+), 10 deletions(-)
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 096de8dba0..44ecc2af29 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -1,9 +1,10 @@
/*
- * QEMU IDE Emulation: PCI VIA82C686B support.
+ * QEMU VIA southbridge IDE emulation (VT82C686B, VT8231)
*
* Copyright (c) 2003 Fabrice Bellard
* Copyright (c) 2006 Openedhand Ltd.
* Copyright (c) 2010 Huacai Chen
+ * Copyright (c) 2019-2020 BALATON Zoltan
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
deal
@@ -25,6 +26,8 @@
*/
#include "qemu/osdep.h"
+#include "qemu/range.h"
+#include "hw/qdev-properties.h"
#include "hw/pci/pci.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
@@ -111,14 +114,40 @@ static void via_ide_set_irq(void *opaque, int n, int
level)
} else {
d->config[0x70 + n * 8] &= ~0x80;
}
-
level = (d->config[0x70] & 0x80) || (d->config[0x78] & 0x80);
-n = pci_get_byte(d->config + PCI_INTERRUPT_LINE);
-if (n) {
-qemu_set_irq(isa_get_irq(NULL, n), level);
+
+/*
+ * Some machines operate in "non 100% native mode" where PCI_INTERRUPT_LINE
+ * is not used but IDE always uses ISA IRQ 14 and 15 even in native mode.
+ * Some guest drivers expect this, often without checking.
+ */
+if (!(pci_get_byte(d->config + PCI_CLASS_PROG) & (n ? 4 : 1)) ||
One other thing whilst I'm here: the above line is quite cryptic to read unless
you're very familiar with the PCI IDE specifications. How about something like
this
towards the top of the function:
int native = n ? pci_get_byte(d->config + PCI_CLASS_PROG) & 4 :
pci_get_byte(d->config + PCI_CLASS_PROG) & 1;
and change the if() accordingly:
if (native) {
...
} else {
...
}
With that you can could probably drop the comment since it's really obvious
what the
code is doing when reading it against the datasheet.
Too late, already sent v3, may consider later but the comment is actually
for the line after the || not the one checking CLASS_PROG.
Regards,
BALATON Zoltan
Re: [PATCH v2 2/2] via-ide: Also emulate non 100% native mode
On Mon, 9 Mar 2020, Mark Cave-Ayland wrote:
On 09/03/2020 00:42, BALATON Zoltan wrote:
Some machines operate in "non 100% native mode" where interrupts are
fixed at legacy IDE interrupts and some guests expect this behaviour
without checking based on knowledge about hardware. Even Linux has
arch specific workarounds for this that are activated on such boards
so this needs to be emulated as well.
Signed-off-by: BALATON Zoltan
---
v2: Don't use PCI_INTERRUPT_LINE in via_ide_set_irq()
hw/ide/via.c| 57 +++--
hw/mips/mips_fulong2e.c | 2 +-
include/hw/ide.h| 3 ++-
3 files changed, 52 insertions(+), 10 deletions(-)
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 096de8dba0..44ecc2af29 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -1,9 +1,10 @@
/*
- * QEMU IDE Emulation: PCI VIA82C686B support.
+ * QEMU VIA southbridge IDE emulation (VT82C686B, VT8231)
*
* Copyright (c) 2003 Fabrice Bellard
* Copyright (c) 2006 Openedhand Ltd.
* Copyright (c) 2010 Huacai Chen
+ * Copyright (c) 2019-2020 BALATON Zoltan
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
deal
@@ -25,6 +26,8 @@
*/
#include "qemu/osdep.h"
+#include "qemu/range.h"
+#include "hw/qdev-properties.h"
#include "hw/pci/pci.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
@@ -111,14 +114,40 @@ static void via_ide_set_irq(void *opaque, int n, int
level)
} else {
d->config[0x70 + n * 8] &= ~0x80;
}
-
level = (d->config[0x70] & 0x80) || (d->config[0x78] & 0x80);
-n = pci_get_byte(d->config + PCI_INTERRUPT_LINE);
-if (n) {
-qemu_set_irq(isa_get_irq(NULL, n), level);
+
+/*
+ * Some machines operate in "non 100% native mode" where PCI_INTERRUPT_LINE
+ * is not used but IDE always uses ISA IRQ 14 and 15 even in native mode.
+ * Some guest drivers expect this, often without checking.
+ */
+if (!(pci_get_byte(d->config + PCI_CLASS_PROG) & (n ? 4 : 1)) ||
+PCI_IDE(d)->flags & BIT(PCI_IDE_LEGACY_IRQ)) {
+qemu_set_irq(isa_get_irq(NULL, (n ? 15 : 14)), level);
+} else {
+qemu_set_irq(isa_get_irq(NULL, 14), level);
}
}
There's still the need to convert this to qdev gpio, but for now I'll review the
updated version of this patch (and provide an example once the rest of the
patchset
is okay).
There's no need to do that now. I think it only makes sense to do that
when the 686B and VT8231 models are also qdevified (which I'll plan to do
when cleaning up my pegasos2 patches later) since that may change how this
should be qdevified. But I don't have time to fully do it now so don't
even ask. This will have to do for now as it's not worse than it is
already and does fix clients so I see no immediate need to force more
clean ups upon me.
This looks much closer to what I was expecting with the fixed IRQs, and in fact
doesn't it make the feature bit requirement obsolete? The PCI_CLASS_PROG bit is
now
the single source of truth as to whether native or legacy IRQs are used, and
the code
routes the IRQ accordingly.
No, the feature bit is still needed to flag if this device should work
with 100% native mode as on fulong2e or with forced legacy IRQ non-100%
native mode as on pegasos2. In both cases PCI_CLASS_PROG is actually set
to native mode (most of the time, Linux fixes this up on pegasos2 for it's
own convenience but other OSes don't care about it but we still need to
know to use legacy interrupts) so the feature bit is needed to know when
to use legacy and when native interrupts.
The hardcoded IRQ14 in native mode is also wrong, If you check VT8231
datasheet it clearly says that the IRQ raised is selected by
PCI_INTERRUPT_LINE so I think my previous version was correct but this
still works because we fixed PCI_INTERRUPT_LINE to 14 so we know here it's
14 without looking at the config reg that you forbid me to do. But due to
coincidence it still works and matches your ideals about PCI specs that I
don't think apply for this device but I could not convince you about that
so if this is what it takes then so be it. As long as it works with
clients I don't care, we can always clean this up later.
+static uint32_t via_ide_config_read(PCIDevice *d, uint32_t address, int len)
+{
+/*
+ * The pegasos2 firmware writes to PCI_INTERRUPT_LINE but on real
+ * hardware it's fixed at 14 and won't change. Some guests also expect
+ * legacy interrupts, without reading PCI_INTERRUPT_LINE but Linux
+ * depends on this to read 14. We set it to 14 in the reset method and
+ * also set the wmask to 0 to emulate this but that turns out to be not
+ * enough. QEMU resets the PCI bus after this device and
+ * pci_do_device_reset() called from pci_device_reset() will zero
+ * PCI_INTERRUPT_LINE so this config_read function is to counter t
Re: [PATCH v2 2/2] via-ide: Also emulate non 100% native mode
On 09/03/2020 00:42, BALATON Zoltan wrote:
> Some machines operate in "non 100% native mode" where interrupts are
> fixed at legacy IDE interrupts and some guests expect this behaviour
> without checking based on knowledge about hardware. Even Linux has
> arch specific workarounds for this that are activated on such boards
> so this needs to be emulated as well.
>
> Signed-off-by: BALATON Zoltan
> ---
> v2: Don't use PCI_INTERRUPT_LINE in via_ide_set_irq()
>
> hw/ide/via.c| 57 +++--
> hw/mips/mips_fulong2e.c | 2 +-
> include/hw/ide.h| 3 ++-
> 3 files changed, 52 insertions(+), 10 deletions(-)
>
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index 096de8dba0..44ecc2af29 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -1,9 +1,10 @@
> /*
> - * QEMU IDE Emulation: PCI VIA82C686B support.
> + * QEMU VIA southbridge IDE emulation (VT82C686B, VT8231)
> *
> * Copyright (c) 2003 Fabrice Bellard
> * Copyright (c) 2006 Openedhand Ltd.
> * Copyright (c) 2010 Huacai Chen
> + * Copyright (c) 2019-2020 BALATON Zoltan
> *
> * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> * of this software and associated documentation files (the "Software"), to
> deal
> @@ -25,6 +26,8 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/range.h"
> +#include "hw/qdev-properties.h"
> #include "hw/pci/pci.h"
> #include "migration/vmstate.h"
> #include "qemu/module.h"
> @@ -111,14 +114,40 @@ static void via_ide_set_irq(void *opaque, int n, int
> level)
> } else {
> d->config[0x70 + n * 8] &= ~0x80;
> }
> -
> level = (d->config[0x70] & 0x80) || (d->config[0x78] & 0x80);
> -n = pci_get_byte(d->config + PCI_INTERRUPT_LINE);
> -if (n) {
> -qemu_set_irq(isa_get_irq(NULL, n), level);
> +
> +/*
> + * Some machines operate in "non 100% native mode" where
> PCI_INTERRUPT_LINE
> + * is not used but IDE always uses ISA IRQ 14 and 15 even in native mode.
> + * Some guest drivers expect this, often without checking.
> + */
> +if (!(pci_get_byte(d->config + PCI_CLASS_PROG) & (n ? 4 : 1)) ||
One other thing whilst I'm here: the above line is quite cryptic to read unless
you're very familiar with the PCI IDE specifications. How about something like
this
towards the top of the function:
int native = n ? pci_get_byte(d->config + PCI_CLASS_PROG) & 4 :
pci_get_byte(d->config + PCI_CLASS_PROG) & 1;
and change the if() accordingly:
if (native) {
...
} else {
...
}
With that you can could probably drop the comment since it's really obvious
what the
code is doing when reading it against the datasheet.
> +PCI_IDE(d)->flags & BIT(PCI_IDE_LEGACY_IRQ)) {
> +qemu_set_irq(isa_get_irq(NULL, (n ? 15 : 14)), level);
> +} else {
> +qemu_set_irq(isa_get_irq(NULL, 14), level);
> }
> }
>
> +static uint32_t via_ide_config_read(PCIDevice *d, uint32_t address, int len)
> +{
> +/*
> + * The pegasos2 firmware writes to PCI_INTERRUPT_LINE but on real
> + * hardware it's fixed at 14 and won't change. Some guests also expect
> + * legacy interrupts, without reading PCI_INTERRUPT_LINE but Linux
> + * depends on this to read 14. We set it to 14 in the reset method and
> + * also set the wmask to 0 to emulate this but that turns out to be not
> + * enough. QEMU resets the PCI bus after this device and
> + * pci_do_device_reset() called from pci_device_reset() will zero
> + * PCI_INTERRUPT_LINE so this config_read function is to counter that and
> + * restore the correct value, otherwise this should not be needed.
> + */
> +if (range_covers_byte(address, len, PCI_INTERRUPT_LINE)) {
> +pci_set_byte(d->config + PCI_INTERRUPT_LINE, 14);
> +}
> +return pci_default_read_config(d, address, len);
> +}
> +
> static void via_ide_reset(DeviceState *dev)
> {
> PCIIDEState *d = PCI_IDE(dev);
> @@ -169,7 +198,8 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
>
> pci_config_set_prog_interface(pci_conf, 0x8f); /* native PCI ATA mode */
> pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x00c0);
> -dev->wmask[PCI_INTERRUPT_LINE] = 0xf;
> +dev->wmask[PCI_CLASS_PROG] = 5;
> +dev->wmask[PCI_INTERRUPT_LINE] = 0;
>
> memory_region_init_io(&d->data_bar[0], OBJECT(d), &pci_ide_data_le_ops,
>&d->bus[0], "via-ide0-data", 8);
> @@ -213,14 +243,23 @@ static void via_ide_exitfn(PCIDevice *dev)
> }
> }
>
> -void via_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
> +void via_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn,
> + bool legacy_irq)
> {
> PCIDevice *dev;
>
> -dev = pci_create_simple(bus, devfn, "via-ide");
> +dev = pci_create(bus, devfn, "via-ide");
> +qdev_prop_set_bit(&dev->qdev, "legacy-irq", legacy_irq);
> +qdev_init_nofail(&dev->qdev);
>
[PATCH v3 2/3] pci: Honour wmask when resetting PCI_INTERRUPT_LINE
The pci_do_device_reset() function (called from pci_device_reset)
clears the PCI_INTERRUPT_LINE config reg of devices on the bus but did
this without taking wmask into account. We'll have a device model now
that needs to set a constant value for this reg and this patch allows
to do that without additional workaround in device emulation to
reverse the effect of this PCI bus reset function.
Suggested-by: Mark Cave-Ayland
Signed-off-by: BALATON Zoltan
---
hw/pci/pci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index e1ed6677e1..d07e4ed9de 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -302,8 +302,10 @@ static void pci_do_device_reset(PCIDevice *dev)
pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
pci_get_word(dev->wmask + PCI_STATUS) |
pci_get_word(dev->w1cmask + PCI_STATUS));
+pci_word_test_and_clear_mask(dev->config + PCI_INTERRUPT_LINE,
+ pci_get_word(dev->wmask + PCI_INTERRUPT_LINE) |
+ pci_get_word(dev->w1cmask + PCI_INTERRUPT_LINE));
dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
-dev->config[PCI_INTERRUPT_LINE] = 0x0;
for (r = 0; r < PCI_NUM_REGIONS; ++r) {
PCIIORegion *region = &dev->io_regions[r];
if (!region->size) {
--
2.21.1
[PATCH v3 0/3] Implement "non 100% native mode" in via-ide
This small series implements "non-100% native mode" of via-ide found at least on pegasos2 where io addresses come from PCI BARs but interrupts are hard coded to legacy IRQ14 and 15. This is needed for guests that expect it and activate work arounds on that platform and don't work unless this is emulated. (Symptom is missing IDE IRQs after enabling BMDMA and boot freezes waiting for interrupt.) We need a flag to turn this mode on or off so the first patch repurposes the last remaining CMD646 specific field in PCIIDEState to allow more flags and make room for the new legacy-irq flag there. (The CMD646 may need similar mode or something else may need more flags in the future.) Boards using CMD646 and VIA IDE are updated for the above changes. Second patch fixes up PCI reset to not clear value set by device emulation on bus reset when wmask does not allow that. Tested with Linux and MorphOS on pegasos2 and a Gentoo live CD kernel for mips_fulong2e that's the only one I could find but being beta not sure if that fully works on real hardware. (The mips_fulong2e also seems to have problems with pci devices so to boot Linux you need -net none -vga none and use serial console otherwise the kernel panics.) Regards, BALATON Zoltan BALATON Zoltan (3): ide: Make room for flags in PCIIDEState and add one for legacy IRQ routing pci: Honour wmask when resetting PCI_INTERRUPT_LINE via-ide: Also emulate non 100% native mode hw/alpha/dp264.c| 2 +- hw/ide/cmd646.c | 12 ++-- hw/ide/via.c| 37 + hw/mips/mips_fulong2e.c | 2 +- hw/pci/pci.c| 4 +++- hw/sparc64/sun4u.c | 9 ++--- include/hw/ide.h| 7 --- include/hw/ide/pci.h| 7 ++- 8 files changed, 52 insertions(+), 28 deletions(-) -- 2.21.1
[PATCH v3 1/3] ide: Make room for flags in PCIIDEState and add one for legacy IRQ routing
We'll need a flag for implementing some device specific behaviour in
via-ide but we already have a currently CMD646 specific field that can
be repurposed for this and leave room for furhter flags if needed in
the future. This patch changes the "secondary" field to "flags" and
define the flags for CMD646 and via-ide and change CMD646 and its
users accordingly.
Signed-off-by: BALATON Zoltan
---
hw/alpha/dp264.c | 2 +-
hw/ide/cmd646.c | 12 ++--
hw/sparc64/sun4u.c | 9 ++---
include/hw/ide.h | 4 ++--
include/hw/ide/pci.h | 7 ++-
5 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index e5350a287f..f104e5dfa4 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -103,7 +103,7 @@ static void clipper_init(MachineState *machine)
DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
ide_drive_get(hd, ARRAY_SIZE(hd));
-pci_cmd646_ide_init(pci_bus, hd, 0);
+pci_cmd646_ide_init(pci_bus, hd, -1, false);
}
/* Load PALcode. Given that this is not "real" cpu palcode,
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 335c060673..0be650791f 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -256,7 +256,7 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error
**errp)
pci_conf[PCI_CLASS_PROG] = 0x8f;
pci_conf[CNTRL] = CNTRL_EN_CH0; // enable IDE0
-if (d->secondary) {
+if (d->flags & BIT(PCI_IDE_SECONDARY)) {
/* XXX: if not enabled, really disable the seconday IDE controller */
pci_conf[CNTRL] |= CNTRL_EN_CH1; /* enable IDE1 */
}
@@ -317,20 +317,20 @@ static void pci_cmd646_ide_exitfn(PCIDevice *dev)
}
}
-void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
- int secondary_ide_enabled)
+void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn,
+ bool secondary_ide_enabled)
{
PCIDevice *dev;
-dev = pci_create(bus, -1, "cmd646-ide");
-qdev_prop_set_uint32(&dev->qdev, "secondary", secondary_ide_enabled);
+dev = pci_create(bus, devfn, "cmd646-ide");
+qdev_prop_set_bit(&dev->qdev, "secondary", secondary_ide_enabled);
qdev_init_nofail(&dev->qdev);
pci_ide_create_devs(dev, hd_table);
}
static Property cmd646_ide_properties[] = {
-DEFINE_PROP_UINT32("secondary", PCIIDEState, secondary, 0),
+DEFINE_PROP_BIT("secondary", PCIIDEState, flags, PCI_IDE_SECONDARY, false),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index d33e84f831..fbe6790847 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -50,8 +50,7 @@
#include "hw/sparc/sparc64.h"
#include "hw/nvram/fw_cfg.h"
#include "hw/sysbus.h"
-#include "hw/ide.h"
-#include "hw/ide/pci.h"
+#include "hw/ide/internal.h"
#include "hw/loader.h"
#include "hw/fw-path-provider.h"
#include "elf.h"
@@ -664,11 +663,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
}
ide_drive_get(hd, ARRAY_SIZE(hd));
-
-pci_dev = pci_create(pci_busA, PCI_DEVFN(3, 0), "cmd646-ide");
-qdev_prop_set_uint32(&pci_dev->qdev, "secondary", 1);
-qdev_init_nofail(&pci_dev->qdev);
-pci_ide_create_devs(pci_dev, hd);
+pci_cmd646_ide_init(pci_busA, hd, PCI_DEVFN(3, 0), true);
/* Map NVRAM into I/O (ebus) space */
nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
diff --git a/include/hw/ide.h b/include/hw/ide.h
index 28d8a06439..d88c5ee695 100644
--- a/include/hw/ide.h
+++ b/include/hw/ide.h
@@ -12,8 +12,8 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2,
int isairq,
DriveInfo *hd0, DriveInfo *hd1);
/* ide-pci.c */
-void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
- int secondary_ide_enabled);
+void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn,
+ bool secondary_ide_enabled);
PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int
devfn);
PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index a9f2c33e68..21075edf16 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -40,6 +40,11 @@ typedef struct BMDMAState {
#define TYPE_PCI_IDE "pci-ide"
#define PCI_IDE(obj) OBJECT_CHECK(PCIIDEState, (obj), TYPE_PCI_IDE)
+enum {
+PCI_IDE_SECONDARY, /* used only for cmd646 */
+PCI_IDE_LEGACY_IRQ
+};
+
typedef struct PCIIDEState {
/*< private >*/
PCIDevice parent_obj;
@@ -47,7 +52,7 @@ typedef struct PCIIDEState {
IDEBus bus[2];
BMDMAState bmdma[2];
-uint32_t secondary; /* used only for cmd646 */
+uint32_t flags;
MemoryRegion bmdma_bar;
MemoryRegion cmd_bar[2];
MemoryRegion data_bar[2];
--
2.21.1
[PATCH v3 3/3] via-ide: Also emulate non 100% native mode
Some machines operate in "non 100% native mode" where interrupts are
fixed at legacy IDE interrupts and some guests expect this behaviour
without checking based on knowledge about hardware. Even Linux has
arch specific workarounds for this that are activated on such boards
so this needs to be emulated as well.
Signed-off-by: BALATON Zoltan
---
v2: Don't use PCI_INTERRUPT_LINE in via_ide_set_irq()
v3: Patch pci.c instead of local workaround for PCI reset clearing
PCI_INTERRUPT_PIN config reg
hw/ide/via.c| 37 +
hw/mips/mips_fulong2e.c | 2 +-
include/hw/ide.h| 3 ++-
3 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 096de8dba0..02d29809f2 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -1,9 +1,10 @@
/*
- * QEMU IDE Emulation: PCI VIA82C686B support.
+ * QEMU VIA southbridge IDE emulation (VT82C686B, VT8231)
*
* Copyright (c) 2003 Fabrice Bellard
* Copyright (c) 2006 Openedhand Ltd.
* Copyright (c) 2010 Huacai Chen
+ * Copyright (c) 2019-2020 BALATON Zoltan
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
deal
@@ -25,6 +26,8 @@
*/
#include "qemu/osdep.h"
+#include "qemu/range.h"
+#include "hw/qdev-properties.h"
#include "hw/pci/pci.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
@@ -111,11 +114,18 @@ static void via_ide_set_irq(void *opaque, int n, int
level)
} else {
d->config[0x70 + n * 8] &= ~0x80;
}
-
level = (d->config[0x70] & 0x80) || (d->config[0x78] & 0x80);
-n = pci_get_byte(d->config + PCI_INTERRUPT_LINE);
-if (n) {
-qemu_set_irq(isa_get_irq(NULL, n), level);
+
+/*
+ * Some machines operate in "non 100% native mode" where PCI_INTERRUPT_LINE
+ * is not used but IDE always uses ISA IRQ 14 and 15 even in native mode.
+ * Some guest drivers expect this, often without checking.
+ */
+if (!(pci_get_byte(d->config + PCI_CLASS_PROG) & (n ? 4 : 1)) ||
+PCI_IDE(d)->flags & BIT(PCI_IDE_LEGACY_IRQ)) {
+qemu_set_irq(isa_get_irq(NULL, (n ? 15 : 14)), level);
+} else {
+qemu_set_irq(isa_get_irq(NULL, 14), level);
}
}
@@ -169,7 +179,8 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
pci_config_set_prog_interface(pci_conf, 0x8f); /* native PCI ATA mode */
pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x00c0);
-dev->wmask[PCI_INTERRUPT_LINE] = 0xf;
+dev->wmask[PCI_CLASS_PROG] = 5;
+dev->wmask[PCI_INTERRUPT_LINE] = 0;
memory_region_init_io(&d->data_bar[0], OBJECT(d), &pci_ide_data_le_ops,
&d->bus[0], "via-ide0-data", 8);
@@ -213,14 +224,23 @@ static void via_ide_exitfn(PCIDevice *dev)
}
}
-void via_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
+void via_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn,
+ bool legacy_irq)
{
PCIDevice *dev;
-dev = pci_create_simple(bus, devfn, "via-ide");
+dev = pci_create(bus, devfn, "via-ide");
+qdev_prop_set_bit(&dev->qdev, "legacy-irq", legacy_irq);
+qdev_init_nofail(&dev->qdev);
pci_ide_create_devs(dev, hd_table);
}
+static Property via_ide_properties[] = {
+DEFINE_PROP_BIT("legacy-irq", PCIIDEState, flags, PCI_IDE_LEGACY_IRQ,
+false),
+DEFINE_PROP_END_OF_LIST(),
+};
+
static void via_ide_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -233,6 +253,7 @@ static void via_ide_class_init(ObjectClass *klass, void
*data)
k->device_id = PCI_DEVICE_ID_VIA_IDE;
k->revision = 0x06;
k->class_id = PCI_CLASS_STORAGE_IDE;
+device_class_set_props(dc, via_ide_properties);
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
}
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 4727b1d3a4..150182c62a 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -257,7 +257,7 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int
slot, qemu_irq intc,
isa_create_simple(isa_bus, TYPE_VT82C686B_SUPERIO);
ide_drive_get(hd, ARRAY_SIZE(hd));
-via_ide_init(pci_bus, hd, PCI_DEVFN(slot, 1));
+via_ide_init(pci_bus, hd, PCI_DEVFN(slot, 1), false);
pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
diff --git a/include/hw/ide.h b/include/hw/ide.h
index d88c5ee695..2a7001ccba 100644
--- a/include/hw/ide.h
+++ b/include/hw/ide.h
@@ -18,7 +18,8 @@ PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo
**hd_table, int devfn);
PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux);
-void via_ide_init(PCIBus *bus, D
Re: [PULL 00/33] Trivial branch patches
On 3/9/20 8:17 PM, Michael S. Tsirkin wrote: On Mon, Mar 09, 2020 at 04:08:04PM +0100, Laurent Vivier wrote: The following changes since commit 7a5853cec479a448edae0fb2aaf4e2f78c9c774d: Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2020-03-09 10:32:53 +) are available in the Git repository at: git://github.com/vivier/qemu.git tags/trivial-branch-pull-request for you to fetch changes up to 916c92503bd5348a33e561db600d8894bde636bb: monitor/hmp-cmds: Remove redundant statement in hmp_rocker_of_dpa_groups() (2020-03-09 15:59:31 +0100) Could you avoid CC everyone involved on the whole PULL req? I was involved in a patch or two and don't really need to see the whole series. Just the cover and the relevant patches would be enough - if I do want it there's always lore. I suppose Laurent used git-publish, which has this limitation. - includes cleanup - reduce .data footprint - fix warnings reported by Clang static code analyzer - fix dp8393x part lost in merge - update git.orderfile and rules.mak Chen Qun (7): block/stream: Remove redundant statement in stream_run() block/file-posix: Remove redundant statement in raw_handle_perm_lock() dma/xlnx-zdma: Remove redundant statement in zdma_write_dst() scsi/scsi-disk: Remove redundant statement in scsi_disk_emulate_command() display/pxa2xx_lcd: Remove redundant statement in pxa2xx_palette_parse() display/exynos4210_fimd: Remove redundant statement in exynos4210_fimd_update() monitor/hmp-cmds: Remove redundant statement in hmp_rocker_of_dpa_groups() Eric Blake (1): maint: Include top-level *.rst files early in git diff Finn Thain (1): dp8393x: Mask EOL bit from descriptor addresses, take 2 Pan Nengyuan (1): core/qdev: fix memleak in qdev_get_gpio_out_connector() Philippe Mathieu-Daudé (23): build-sys: Move the print-variable rule to rules.mak hw/audio/fmopl: Fix a typo twice hw/net/e1000: Add readops/writeops typedefs hw/net/e1000: Move macreg[] arrays to .rodata to save 1MiB of .data virtfs-proxy-helper: Make the helper_opts[] array const vl: Add missing "hw/boards.h" include hw/southbridge/ich9: Removed unused headers hw/i386/ioapic_internal: Remove unused "hw/i386/ioapic.h" header hw/timer: Remove unused "ui/console.h" header hw/usb/dev-storage: Remove unused "ui/console.h" header hw/i386/intel_iommu: Remove unused includes hw/alpha/alpha_sys: Remove unused "hw/ide.h" header hw/alpha/dp264: Include "net/net.h" hw/hppa/machine: Include "net/net.h" hw/acpi/cpu_hotplug: Include "hw/pci/pci.h" hw/timer/hpet: Include "exec/address-spaces.h" hw/pci-host/q35: Include "qemu/range.h" hw/i2c/smbus_ich9: Include "qemu/range.h" hw/pci-host/piix: Include "qemu/range.h" hw/acpi: Include "hw/mem/nvdimm.h" hw/i386: Include "hw/mem/nvdimm.h" hw/pci-host/q35: Remove unused includes hw/i386/pc: Clean up includes Makefile | 3 --- block/file-posix.c| 1 - block/stream.c| 3 +-- fsdev/virtfs-proxy-helper.c | 2 +- hw/acpi/cpu_hotplug.c | 1 + hw/acpi/ich9.c| 2 +- hw/acpi/piix4.c | 1 + hw/alpha/alpha_sys.h | 1 - hw/alpha/dp264.c | 1 + hw/audio/fmopl.c | 4 ++-- hw/core/qdev.c| 2 +- hw/display/exynos4210_fimd.c | 1 - hw/display/pxa2xx_lcd.c | 1 - hw/dma/xlnx-zdma.c| 10 +- hw/hppa/machine.c | 1 + hw/i2c/smbus_ich9.c | 1 + hw/i386/acpi-build.c | 1 + hw/i386/pc.c | 1 + hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c | 1 + hw/isa/lpc_ich9.c | 1 - hw/net/dp8393x.c | 4 ++-- hw/net/e1000.c| 6 -- hw/net/e1000e_core.c | 6 -- hw/pci-host/i440fx.c | 1 + hw/pci-host/q35.c | 1 + hw/rtc/twl92230.c | 1 - hw/scsi/scsi-disk.c | 1 - hw/timer/hpet.c | 2 +- hw/usb/dev-storage.c | 1 - include/hw/i386/ich9.h| 1 - include/hw/i386/intel_iommu.h | 4 include/hw/i386/ioapic_internal.h | 1 - include/hw/i386/pc.h | 11 +++ include/hw/pci-host/q35.h | 8 +--- monitor/hmp-cmds.c| 5 + rules.mak | 3 +++ scripts/git.orderfile | 1 + softmmu/vl.c | 1 + 39 files changed, 43 insertions(+), 55 deletions(-) -- 2.24.1
Re: [PULL 00/33] Trivial branch patches
On Mon, Mar 09, 2020 at 04:08:04PM +0100, Laurent Vivier wrote: > The following changes since commit 7a5853cec479a448edae0fb2aaf4e2f78c9c774d: > > Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging > (2020-03-09 10:32:53 +) > > are available in the Git repository at: > > git://github.com/vivier/qemu.git tags/trivial-branch-pull-request > > for you to fetch changes up to 916c92503bd5348a33e561db600d8894bde636bb: > > monitor/hmp-cmds: Remove redundant statement in hmp_rocker_of_dpa_groups() > (2020-03-09 15:59:31 +0100) Could you avoid CC everyone involved on the whole PULL req? I was involved in a patch or two and don't really need to see the whole series. Just the cover and the relevant patches would be enough - if I do want it there's always lore. > > - includes cleanup > - reduce .data footprint > - fix warnings reported by Clang static code analyzer > - fix dp8393x part lost in merge > - update git.orderfile and rules.mak > > > > Chen Qun (7): > block/stream: Remove redundant statement in stream_run() > block/file-posix: Remove redundant statement in raw_handle_perm_lock() > dma/xlnx-zdma: Remove redundant statement in zdma_write_dst() > scsi/scsi-disk: Remove redundant statement in > scsi_disk_emulate_command() > display/pxa2xx_lcd: Remove redundant statement in > pxa2xx_palette_parse() > display/exynos4210_fimd: Remove redundant statement in > exynos4210_fimd_update() > monitor/hmp-cmds: Remove redundant statement in > hmp_rocker_of_dpa_groups() > > Eric Blake (1): > maint: Include top-level *.rst files early in git diff > > Finn Thain (1): > dp8393x: Mask EOL bit from descriptor addresses, take 2 > > Pan Nengyuan (1): > core/qdev: fix memleak in qdev_get_gpio_out_connector() > > Philippe Mathieu-Daudé (23): > build-sys: Move the print-variable rule to rules.mak > hw/audio/fmopl: Fix a typo twice > hw/net/e1000: Add readops/writeops typedefs > hw/net/e1000: Move macreg[] arrays to .rodata to save 1MiB of .data > virtfs-proxy-helper: Make the helper_opts[] array const > vl: Add missing "hw/boards.h" include > hw/southbridge/ich9: Removed unused headers > hw/i386/ioapic_internal: Remove unused "hw/i386/ioapic.h" header > hw/timer: Remove unused "ui/console.h" header > hw/usb/dev-storage: Remove unused "ui/console.h" header > hw/i386/intel_iommu: Remove unused includes > hw/alpha/alpha_sys: Remove unused "hw/ide.h" header > hw/alpha/dp264: Include "net/net.h" > hw/hppa/machine: Include "net/net.h" > hw/acpi/cpu_hotplug: Include "hw/pci/pci.h" > hw/timer/hpet: Include "exec/address-spaces.h" > hw/pci-host/q35: Include "qemu/range.h" > hw/i2c/smbus_ich9: Include "qemu/range.h" > hw/pci-host/piix: Include "qemu/range.h" > hw/acpi: Include "hw/mem/nvdimm.h" > hw/i386: Include "hw/mem/nvdimm.h" > hw/pci-host/q35: Remove unused includes > hw/i386/pc: Clean up includes > > Makefile | 3 --- > block/file-posix.c| 1 - > block/stream.c| 3 +-- > fsdev/virtfs-proxy-helper.c | 2 +- > hw/acpi/cpu_hotplug.c | 1 + > hw/acpi/ich9.c| 2 +- > hw/acpi/piix4.c | 1 + > hw/alpha/alpha_sys.h | 1 - > hw/alpha/dp264.c | 1 + > hw/audio/fmopl.c | 4 ++-- > hw/core/qdev.c| 2 +- > hw/display/exynos4210_fimd.c | 1 - > hw/display/pxa2xx_lcd.c | 1 - > hw/dma/xlnx-zdma.c| 10 +- > hw/hppa/machine.c | 1 + > hw/i2c/smbus_ich9.c | 1 + > hw/i386/acpi-build.c | 1 + > hw/i386/pc.c | 1 + > hw/i386/pc_piix.c | 1 + > hw/i386/pc_q35.c | 1 + > hw/isa/lpc_ich9.c | 1 - > hw/net/dp8393x.c | 4 ++-- > hw/net/e1000.c| 6 -- > hw/net/e1000e_core.c | 6 -- > hw/pci-host/i440fx.c | 1 + > hw/pci-host/q35.c | 1 + > hw/rtc/twl92230.c | 1 - > hw/scsi/scsi-disk.c | 1 - > hw/timer/hpet.c | 2 +- > hw/usb/dev-storage.c | 1 - > include/hw/i386/ich9.h| 1 - > include/hw/i386/intel_iommu.h | 4 > include/hw/i386/ioapic_internal.h | 1 - > include/hw/i386/pc.h | 11 +++ > include/hw/pci-host/q35.h | 8 +--- > monitor/hmp-cmds.c| 5 + > rules.mak | 3 +++ > scripts/git.orderfile | 1 + > softmmu/vl.c | 1 + > 39 files changed, 43 insertions(+), 55 deletions(-) > > -- > 2.24.1
Re: [PATCH] block/qcow2-threads: fix qcow2_decompress
On 02.03.20 16:09, Vladimir Sementsov-Ogievskiy wrote: > On success path we return what inflate() returns instead of 0. And it > most probably works for Z_STREAM_END as it is positive, but is > definitely broken for Z_BUF_ERROR. > > While being here, switch to errno return code, to be closer to > qcow2_compress API (and usual expectations). > > Revert condition in if to be more positive. Drop dead initialization of > ret. > > Cc: qemu-sta...@nongnu.org # v4.0 > Fixes: 341926ab83e2b > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > > Hi! > > Reviewing Den's series about zstd in qcow2 support, I found an existing > bug. Let's fix it. This is to be a new base of zstd series. > > block/qcow2-threads.c | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) Thanks, applied to my block branch: https://git.xanclic.moe/XanClic/qemu/commits/branch/block Max signature.asc Description: OpenPGP digital signature
Re: [PATCH v5 00/11] HMP monitor handlers refactoring
On Mon, 2020-03-09 at 18:30 +, Dr. David Alan Gilbert wrote: > * Maxim Levitsky (mlevi...@redhat.com) wrote: > > This patch series is bunch of cleanups to the hmp monitor code. > > It mostly moves the blockdev related hmp handlers to its own file, > > and does some minor refactoring. > > > > No functional changes expected. > > Queued for HMP, with the commit message fix up in 05. Thanks a million! Best regards, Maxim Levitsky > > Dave > > > Changes from V1: > >* move the handlers to block/monitor/block-hmp-cmds.c > >* tiny cleanup for the commit messages > > > > Changes from V2: > >* Moved all the function prototypes to new header (blockdev-hmp-cmds.h) > >* Set the license of blockdev-hmp-cmds.c to GPLv2+ > >* Moved hmp_snapshot_* functions to blockdev-hmp-cmds.c > >* Moved hmp_drive_add_node to blockdev-hmp-cmds.c > > (this change needed some new exports, thus in separate new patch) > >* Moved hmp_qemu_io and hmp_eject to blockdev-hmp-cmds.c > >* Added 'error:' prefix to vreport, and updated the iotests > > This is invasive change, but really feels like the right one > >* Added minor refactoring patch that drops an unused #include > > > > Changes from V3: > >* Dropped the error prefix patches for now due to fact that it seems > > that libvirt doesn't need that after all. Oh well... > > I'll send them in a separate series. > > > >* Hopefully correctly merged the copyright info the new files > > Both files are GPLv2 now (due to code from hmp.h/hmp-cmds.c) > > > >* Addressed review feedback > >* Renamed the added header to block-hmp-cmds.h > > > >* Got rid of checkpatch.pl warnings in the moved code > > (cosmetic code changes only) > > > >* I kept the reviewed-by tags, since the changes I did are minor. > > I hope that this is right thing to do. > > > > Changes from V4: > >* Rebase with recent changes > >* Fixed review feedback > > > > Best regards, > > Maxim Levitsky > > > > Maxim Levitsky (11): > > usb/dev-storage: remove unused include > > monitor/hmp: inline add_init_drive > > monitor/hmp: rename device-hotplug.c to block/monitor/block-hmp-cmds.c > > monitor/hmp: move hmp_drive_del and hmp_commit to block-hmp-cmds.c > > monitor/hmp: move hmp_drive_mirror and hmp_drive_backup to > > block-hmp-cmds.c Moved code was added after 2012-01-13, thus under > > GPLv2+ > > monitor/hmp: move hmp_block_job* to block-hmp-cmds.c > > monitor/hmp: move hmp_snapshot_* to block-hmp-cmds.c > > monitor/hmp: move hmp_nbd_server* to block-hmp-cmds.c > > monitor/hmp: move remaining hmp_block* functions to block-hmp-cmds.c > > monitor/hmp: move hmp_info_block* to block-hmp-cmds.c > > monitor/hmp: Move hmp_drive_add_node to block-hmp-cmds.c > > > > MAINTAINERS|1 + > > Makefile.objs |2 +- > > block/Makefile.objs|1 + > > block/monitor/Makefile.objs|1 + > > block/monitor/block-hmp-cmds.c | 1015 > > blockdev.c | 137 + > > device-hotplug.c | 91 --- > > hw/usb/dev-storage.c |1 - > > include/block/block-hmp-cmds.h | 54 ++ > > include/block/block_int.h |5 +- > > include/monitor/hmp.h | 24 - > > include/sysemu/blockdev.h |4 - > > include/sysemu/sysemu.h|3 - > > monitor/hmp-cmds.c | 782 > > monitor/misc.c |1 + > > 15 files changed, 1085 insertions(+), 1037 deletions(-) > > create mode 100644 block/monitor/Makefile.objs > > create mode 100644 block/monitor/block-hmp-cmds.c > > delete mode 100644 device-hotplug.c > > create mode 100644 include/block/block-hmp-cmds.h > > > > -- > > 2.17.2 > > > > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH v5 00/11] HMP monitor handlers refactoring
* Maxim Levitsky (mlevi...@redhat.com) wrote: > This patch series is bunch of cleanups to the hmp monitor code. > It mostly moves the blockdev related hmp handlers to its own file, > and does some minor refactoring. > > No functional changes expected. Queued for HMP, with the commit message fix up in 05. Dave > Changes from V1: >* move the handlers to block/monitor/block-hmp-cmds.c >* tiny cleanup for the commit messages > > Changes from V2: >* Moved all the function prototypes to new header (blockdev-hmp-cmds.h) >* Set the license of blockdev-hmp-cmds.c to GPLv2+ >* Moved hmp_snapshot_* functions to blockdev-hmp-cmds.c >* Moved hmp_drive_add_node to blockdev-hmp-cmds.c > (this change needed some new exports, thus in separate new patch) >* Moved hmp_qemu_io and hmp_eject to blockdev-hmp-cmds.c >* Added 'error:' prefix to vreport, and updated the iotests > This is invasive change, but really feels like the right one >* Added minor refactoring patch that drops an unused #include > > Changes from V3: >* Dropped the error prefix patches for now due to fact that it seems > that libvirt doesn't need that after all. Oh well... > I'll send them in a separate series. > >* Hopefully correctly merged the copyright info the new files > Both files are GPLv2 now (due to code from hmp.h/hmp-cmds.c) > >* Addressed review feedback >* Renamed the added header to block-hmp-cmds.h > >* Got rid of checkpatch.pl warnings in the moved code > (cosmetic code changes only) > >* I kept the reviewed-by tags, since the changes I did are minor. > I hope that this is right thing to do. > > Changes from V4: >* Rebase with recent changes >* Fixed review feedback > > Best regards, > Maxim Levitsky > > Maxim Levitsky (11): > usb/dev-storage: remove unused include > monitor/hmp: inline add_init_drive > monitor/hmp: rename device-hotplug.c to block/monitor/block-hmp-cmds.c > monitor/hmp: move hmp_drive_del and hmp_commit to block-hmp-cmds.c > monitor/hmp: move hmp_drive_mirror and hmp_drive_backup to > block-hmp-cmds.c Moved code was added after 2012-01-13, thus under > GPLv2+ > monitor/hmp: move hmp_block_job* to block-hmp-cmds.c > monitor/hmp: move hmp_snapshot_* to block-hmp-cmds.c > monitor/hmp: move hmp_nbd_server* to block-hmp-cmds.c > monitor/hmp: move remaining hmp_block* functions to block-hmp-cmds.c > monitor/hmp: move hmp_info_block* to block-hmp-cmds.c > monitor/hmp: Move hmp_drive_add_node to block-hmp-cmds.c > > MAINTAINERS|1 + > Makefile.objs |2 +- > block/Makefile.objs|1 + > block/monitor/Makefile.objs|1 + > block/monitor/block-hmp-cmds.c | 1015 > blockdev.c | 137 + > device-hotplug.c | 91 --- > hw/usb/dev-storage.c |1 - > include/block/block-hmp-cmds.h | 54 ++ > include/block/block_int.h |5 +- > include/monitor/hmp.h | 24 - > include/sysemu/blockdev.h |4 - > include/sysemu/sysemu.h|3 - > monitor/hmp-cmds.c | 782 > monitor/misc.c |1 + > 15 files changed, 1085 insertions(+), 1037 deletions(-) > create mode 100644 block/monitor/Makefile.objs > create mode 100644 block/monitor/block-hmp-cmds.c > delete mode 100644 device-hotplug.c > create mode 100644 include/block/block-hmp-cmds.h > > -- > 2.17.2 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: Regarding SESparse support in QEMU
Hi, This is regarding the new VMDK format of SESparse snapshots which is available since ESXi 6.5 (for disks > 2TB) and is default since ESXi 6.7 (for all disks). Unlike the previous format (VMFSSparse), SESparse's format is not disclosed by VMware. Even though, I believe that the format itself is not too complicated and I'll try to give out pointers on how it is possible to implement a read-write support for it. In commit 98eb9733f (vmdk: Add read-only support for seSparse snapshots) I have added a read-only support. As stated in the commit message - I did not implement the following features which are implemented in VMware: * read-write * journal replay * space reclamation * unmap support I don't fully understand what you're trying to implement when you say "read-write" since there are some scenarios for which you will not need to implement a fully compatible SESparse image. After creating an SESparse snapshot (by invoking "Take Snapshot" in VMware), the following file: /var/log/hostd.log Contains messages of the like: [...] Const Header: [...] constMagic = 0xcafebabe [...] version= 2.1 [...] capacity = 204800 [...] grainSize = 8 [...] grainTableSize = 64 [...] flags = 0 [...] Extents: [...] Header : <1 : 1> [...] JournalHdr : <2 : 2> [...] Journal: <2048 : 2048> [...] GrainDirectory : <4096 : 2048> [...] GrainTables: <6144 : 2048> [...] FreeBitmap : <8192 : 2048> [...] BackMap: <10240 : 2048> [...] Grain : <12288 : 204800> [...] Volatile Header: [...] volatileMagic = 0xcafecafe [...] FreeGTNumber = 0 [...] nextTxnSeqNumber = 0 [...] replayJournal = 0 The sizes that are seen in the log file are in sectors. Extents are of the following format: Just from the strings in this message and by information publicly disclosed by VMware it is obvious how SESparse is implemented: The problem with VMFSSparse (previous format) is that given an offset to a grain in the file - you don't know if that grain is allocated or not - you must scan all grain tables to perform space reclamation, and even if somehow you know that the grain is not allocated - you don't have the offset to the grain table that references it - to unmap it from there. This was obviously solved using the structures "FreeBitmap" and "BackMap". FreeBitmap is simply a very big bitmap that tells you if the grain at the offset of the bit is allocated. Meaning: If bit X is set to 1 then Grain[X] is allocated (Notice that each grain is of the size "grainSize" sectors - usually 4K bytes). Now, we can simply iterate through FreeBitmap to find grains we can reclaim. Problem - we don't know where is the grain table to update, scanning all grain tables is slow. Solution: BackMap: For Grain[X] look at BackMap[X] - BackMap[X] contains an index to GrainTables at which the grain table resides (Multiply by the size of a grain table in order to get the real offset). (Notice that BackMap scales with the size of the VMDK - but only 8 bytes per entry instead 4K bytes) Problem - when we now write to BackMap & FreeBitmap structures too - we might experience data corruption of the SESparse format. Solution: Journal Write data (write what where) to the journal, mark journal dirty, execute instructions in journal, clean dirty mark - if you crash in the middle - you will reexecute the instructions when opening the VMDK file. (Notice this was not a problem in VMFSSprase since we could first write the data in the grain, then update the grain table to the grain, then update the grain directory to the grain table - so the data is actually written on the last operation and the format structure is not corrupted) In any case, now we need to implement the following: When opening the file: * Execute journal (not implemented) When reading * Follow Grain Directory -> Grain Table -> Grain (This is implemented now) For writing (not implemented, probably something like this) * Follow Grain Directory * If grain table allocated -> get it * Otherwise, allocate new grain table (this probably uses FreeGTNumber in the volatile header to know where to do so) * Extend the file by the size needed to store all the grains in the grain table (I think this is 16MB by default) * Follow Grain Table * If grain allocated, get it * Otherwise, allocate it * Write to grain (you may need to read it if you write only to some of it, since now grains are 4K - it is possible to read/write half a grain) * Update BackMap and FreeBitmap accordingly. * All of the above writes should go through the Journal for consistency. When unmapping: * Update bits in FreeBitmap (1 -> 0) When reclaiming space: Go through FreeBitmaps, decide if should perform cleaning (must have a complete empty grain table), if so, reclaim (You can only clean when a full Grain table is unmapped, copy another grain table in its stead, you'll have to update Grains, and the Grain d
Re: [PATCH 0/7] aio-posix: polling scalability improvements
On Thu, Mar 05, 2020 at 05:07:59PM +, Stefan Hajnoczi wrote: > A guest with 100 virtio-blk-pci,num-queues=32 devices only reaches 10k IOPS > while a guest with a single device reaches 105k IOPS > (rw=randread,bs=4k,iodepth=1,ioengine=libaio). > > The bottleneck is that aio_poll() userspace polling iterates over all > AioHandlers to invoke their ->io_poll() callbacks. All AioHandlers are polled > even if only one of them was recently active. Therefore a guest with many > disks is slower than a guest with a single disk even when the workload only > accesses a single disk. > > This patch series solves this scalability problem so that IOPS is unaffected > by > the number of devices. The trick is to poll only AioHandlers that were > recently active so that userspace polling scales well. > > Unfortunately it's not possible to accomplish this with the existing epoll(7) > fd monitoring implementation. This patch series adds a Linux io_uring fd > monitoring implementation. The critical feature is that io_uring can check > the > readiness of file descriptors through userspace polling. This makes it > possible to safely poll a subset of AioHandlers from userspace without risk of > starving the other AioHandlers. > > Stefan Hajnoczi (7): > aio-posix: completely stop polling when disabled > aio-posix: move RCU_READ_LOCK() into run_poll_handlers() > aio-posix: extract ppoll(2) and epoll(7) fd monitoring > aio-posix: simplify FDMonOps->update() prototype > aio-posix: add io_uring fd monitoring implementation > aio-posix: support userspace polling of fd monitoring > aio-posix: remove idle poll handlers to improve scalability > > MAINTAINERS | 2 + > configure | 5 + > include/block/aio.h | 70 ++- > util/Makefile.objs| 3 + > util/aio-posix.c | 449 ++ > util/aio-posix.h | 81 > util/fdmon-epoll.c| 155 +++ > util/fdmon-io_uring.c | 332 +++ > util/fdmon-poll.c | 107 ++ > util/trace-events | 2 + > 10 files changed, 898 insertions(+), 308 deletions(-) > create mode 100644 util/aio-posix.h > create mode 100644 util/fdmon-epoll.c > create mode 100644 util/fdmon-io_uring.c > create mode 100644 util/fdmon-poll.c > > -- > 2.24.1 > Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan signature.asc Description: PGP signature
Re: [PATCH v2 2/2] via-ide: Also emulate non 100% native mode
On 09/03/2020 00:42, BALATON Zoltan wrote:
> Some machines operate in "non 100% native mode" where interrupts are
> fixed at legacy IDE interrupts and some guests expect this behaviour
> without checking based on knowledge about hardware. Even Linux has
> arch specific workarounds for this that are activated on such boards
> so this needs to be emulated as well.
>
> Signed-off-by: BALATON Zoltan
> ---
> v2: Don't use PCI_INTERRUPT_LINE in via_ide_set_irq()
>
> hw/ide/via.c| 57 +++--
> hw/mips/mips_fulong2e.c | 2 +-
> include/hw/ide.h| 3 ++-
> 3 files changed, 52 insertions(+), 10 deletions(-)
>
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index 096de8dba0..44ecc2af29 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -1,9 +1,10 @@
> /*
> - * QEMU IDE Emulation: PCI VIA82C686B support.
> + * QEMU VIA southbridge IDE emulation (VT82C686B, VT8231)
> *
> * Copyright (c) 2003 Fabrice Bellard
> * Copyright (c) 2006 Openedhand Ltd.
> * Copyright (c) 2010 Huacai Chen
> + * Copyright (c) 2019-2020 BALATON Zoltan
> *
> * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> * of this software and associated documentation files (the "Software"), to
> deal
> @@ -25,6 +26,8 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/range.h"
> +#include "hw/qdev-properties.h"
> #include "hw/pci/pci.h"
> #include "migration/vmstate.h"
> #include "qemu/module.h"
> @@ -111,14 +114,40 @@ static void via_ide_set_irq(void *opaque, int n, int
> level)
> } else {
> d->config[0x70 + n * 8] &= ~0x80;
> }
> -
> level = (d->config[0x70] & 0x80) || (d->config[0x78] & 0x80);
> -n = pci_get_byte(d->config + PCI_INTERRUPT_LINE);
> -if (n) {
> -qemu_set_irq(isa_get_irq(NULL, n), level);
> +
> +/*
> + * Some machines operate in "non 100% native mode" where
> PCI_INTERRUPT_LINE
> + * is not used but IDE always uses ISA IRQ 14 and 15 even in native mode.
> + * Some guest drivers expect this, often without checking.
> + */
> +if (!(pci_get_byte(d->config + PCI_CLASS_PROG) & (n ? 4 : 1)) ||
> +PCI_IDE(d)->flags & BIT(PCI_IDE_LEGACY_IRQ)) {
> +qemu_set_irq(isa_get_irq(NULL, (n ? 15 : 14)), level);
> +} else {
> +qemu_set_irq(isa_get_irq(NULL, 14), level);
> }
> }
There's still the need to convert this to qdev gpio, but for now I'll review the
updated version of this patch (and provide an example once the rest of the
patchset
is okay).
This looks much closer to what I was expecting with the fixed IRQs, and in fact
doesn't it make the feature bit requirement obsolete? The PCI_CLASS_PROG bit is
now
the single source of truth as to whether native or legacy IRQs are used, and
the code
routes the IRQ accordingly.
> +static uint32_t via_ide_config_read(PCIDevice *d, uint32_t address, int len)
> +{
> +/*
> + * The pegasos2 firmware writes to PCI_INTERRUPT_LINE but on real
> + * hardware it's fixed at 14 and won't change. Some guests also expect
> + * legacy interrupts, without reading PCI_INTERRUPT_LINE but Linux
> + * depends on this to read 14. We set it to 14 in the reset method and
> + * also set the wmask to 0 to emulate this but that turns out to be not
> + * enough. QEMU resets the PCI bus after this device and
> + * pci_do_device_reset() called from pci_device_reset() will zero
> + * PCI_INTERRUPT_LINE so this config_read function is to counter that and
> + * restore the correct value, otherwise this should not be needed.
> + */
> +if (range_covers_byte(address, len, PCI_INTERRUPT_LINE)) {
> +pci_set_byte(d->config + PCI_INTERRUPT_LINE, 14);
> +}
> +return pci_default_read_config(d, address, len);
> +}
The comment here is interesting so I had a quick look at pci_do_device_reset()
to see
what is happening there, and to me it seems that the real bug is
pci_do_device_reset() doesn't honour wmask for PCI_INTERRUPT_LINE. This is
because
normally the BIOS would write this value long after the PCI bus has been
physically
reset and since via-ide is the first device to hardwire a value here, this
wouldn't
have been needed up until now.
Fortunately it seems that there is already precedent for this so does the
following
diff work?
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index e1ed6677e1..4ae0e0e90f 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -302,8 +302,10 @@ static void pci_do_device_reset(PCIDevice *dev)
pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
pci_get_word(dev->wmask + PCI_STATUS) |
pci_get_word(dev->w1cmask + PCI_STATUS));
+pci_word_test_and_clear_mask(dev->config + PCI_INTERRUPT_LINE,
+ pci_get_word(dev->wmask + PCI_INTERRUPT_LINE)
|
+ pci_get_word(dev->w1cmask +
PCI_INTERRUPT_LINE));
dev->conf
Re: [PULL 00/33] Trivial branch patches
On Mon, 9 Mar 2020 at 15:09, Laurent Vivier wrote: > > The following changes since commit 7a5853cec479a448edae0fb2aaf4e2f78c9c774d: > > Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging > (2020-03-09 10:32:53 +) > > are available in the Git repository at: > > git://github.com/vivier/qemu.git tags/trivial-branch-pull-request > > for you to fetch changes up to 916c92503bd5348a33e561db600d8894bde636bb: > > monitor/hmp-cmds: Remove redundant statement in hmp_rocker_of_dpa_groups() > (2020-03-09 15:59:31 +0100) > > > - includes cleanup > - reduce .data footprint > - fix warnings reported by Clang static code analyzer > - fix dp8393x part lost in merge > - update git.orderfile and rules.mak > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0 for any user-visible changes. -- PMM
Re: [PATCH 0/2] qemu/queue.h: clear linked list pointers on remove
On Mon, Feb 24, 2020 at 10:34:04AM +, Stefan Hajnoczi wrote:
> QLIST_REMOVE() and friends leave dangling linked list pointers in the node
> that
> was removed. This makes it impossible to decide whether a node is currently
> in
> a list or not. It also makes debugging harder.
>
> Based-on: 20200222085030.1760640-1-stefa...@redhat.com
> ("[PULL 00/31] Block patches")
>
> Stefan Hajnoczi (2):
> qemu/queue.h: clear linked list pointers on remove
> aio-posix: remove confusing QLIST_SAFE_REMOVE()
>
> include/qemu/queue.h | 19 +++
> util/aio-posix.c | 2 +-
> 2 files changed, 16 insertions(+), 5 deletions(-)
>
> --
> 2.24.1
>
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
signature.asc
Description: PGP signature
Re: [PATCH 7/7] aio-posix: remove idle poll handlers to improve scalability
On Fri, Mar 06, 2020 at 03:17:40PM +0100, Paolo Bonzini wrote: > On 06/03/20 14:50, Stefan Hajnoczi wrote: > >> Not sure I understand the "almost" part. If it's accessed only from > >> aio_poll() it is protected via either AIO_WAIT_WHILE or the BQL, not by > >> ctx->list_lock; if it's protected by ctx->list_lock (using > >> qemu_lockcnt_inc in readers), it is an RCU list. > > aio_remove_fd_handler() removes nodes from the list during > > aio_set_fd_handler(), but only while holding ctx->list_lock and the > > count is zero (no readers). > > > > All other access is done from with ctx->list_lock incremented. This > > code needs to be reentrant in case of nested aio_poll() but nothing else > > will access the list at the same time. > > Oh, I see, adds are only done under ctx->list_lock and those are the > part that need the write barriers in the RCU iterators. I'll update the comment when merging this series. Stefan signature.asc Description: PGP signature
Re: [PATCH v5 05/11] monitor/hmp: move hmp_drive_mirror and hmp_drive_backup to block-hmp-cmds.c Moved code was added after 2012-01-13, thus under GPLv2+
On Mon, 2020-03-09 at 16:31 +, Dr. David Alan Gilbert wrote:
> * Maxim Levitsky (mlevi...@redhat.com) wrote:
> >
> > I see that I have the same issue of long subject line here.
> > Its because I forgot the space after first line, when adding this.
> > If I need to resend another version of this patchset I'll fix this,
> > but otherwise maybe that can be fixed when applying this to one of
> > maintainer's
> > trees.
> >
> > Sorry for noise.
>
> I can just fix the commit message.
Thank you!!
Best regards,
Maxim Levitsky
>
> Dave
>
> > Best regards,
> > Maxim Levitsky
> >
> > On Sun, 2020-03-08 at 11:24 +0200, Maxim Levitsky wrote:
> > > Signed-off-by: Maxim Levitsky
> > > Reviewed-by: Dr. David Alan Gilbert
> > > ---
> > > block/monitor/block-hmp-cmds.c | 60 ++
> > > include/block/block-hmp-cmds.h | 12 +--
> > > include/monitor/hmp.h | 2 --
> > > monitor/hmp-cmds.c | 58
> > > 4 files changed, 69 insertions(+), 63 deletions(-)
> > >
> > > diff --git a/block/monitor/block-hmp-cmds.c
> > > b/block/monitor/block-hmp-cmds.c
> > > index ad727a6b08..d6dd5d97f7 100644
> > > --- a/block/monitor/block-hmp-cmds.c
> > > +++ b/block/monitor/block-hmp-cmds.c
> > > @@ -37,10 +37,12 @@
> > > #include "qapi/qapi-commands-block.h"
> > > #include "qapi/qmp/qdict.h"
> > > #include "qapi/error.h"
> > > +#include "qapi/qmp/qerror.h"
> > > #include "qemu/config-file.h"
> > > #include "qemu/option.h"
> > > #include "sysemu/sysemu.h"
> > > #include "monitor/monitor.h"
> > > +#include "monitor/hmp.h"
> > > #include "block/block_int.h"
> > > #include "block/block-hmp-cmds.h"
> > >
> > > @@ -187,3 +189,61 @@ void hmp_commit(Monitor *mon, const QDict *qdict)
> > > error_report("'commit' error for '%s': %s", device,
> > > strerror(-ret));
> > > }
> > > }
> > > +
> > > +void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
> > > +{
> > > +const char *filename = qdict_get_str(qdict, "target");
> > > +const char *format = qdict_get_try_str(qdict, "format");
> > > +bool reuse = qdict_get_try_bool(qdict, "reuse", false);
> > > +bool full = qdict_get_try_bool(qdict, "full", false);
> > > +Error *err = NULL;
> > > +DriveMirror mirror = {
> > > +.device = (char *)qdict_get_str(qdict, "device"),
> > > +.target = (char *)filename,
> > > +.has_format = !!format,
> > > +.format = (char *)format,
> > > +.sync = full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
> > > +.has_mode = true,
> > > +.mode = reuse ? NEW_IMAGE_MODE_EXISTING :
> > > NEW_IMAGE_MODE_ABSOLUTE_PATHS,
> > > +.unmap = true,
> > > +};
> > > +
> > > +if (!filename) {
> > > +error_setg(&err, QERR_MISSING_PARAMETER, "target");
> > > +hmp_handle_error(mon, err);
> > > +return;
> > > +}
> > > +qmp_drive_mirror(&mirror, &err);
> > > +hmp_handle_error(mon, err);
> > > +}
> > > +
> > > +void hmp_drive_backup(Monitor *mon, const QDict *qdict)
> > > +{
> > > +const char *device = qdict_get_str(qdict, "device");
> > > +const char *filename = qdict_get_str(qdict, "target");
> > > +const char *format = qdict_get_try_str(qdict, "format");
> > > +bool reuse = qdict_get_try_bool(qdict, "reuse", false);
> > > +bool full = qdict_get_try_bool(qdict, "full", false);
> > > +bool compress = qdict_get_try_bool(qdict, "compress", false);
> > > +Error *err = NULL;
> > > +DriveBackup backup = {
> > > +.device = (char *)device,
> > > +.target = (char *)filename,
> > > +.has_format = !!format,
> > > +.format = (char *)format,
> > > +.sync = full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
> > > +.has_mode = true,
> > > +.mode = reuse ? NEW_IMAGE_MODE_EXISTING :
> > > NEW_IMAGE_MODE_ABSOLUTE_PATHS,
> > > +.has_compress = !!compress,
> > > +.compress = compress,
> > > +};
> > > +
> > > +if (!filename) {
> > > +error_setg(&err, QERR_MISSING_PARAMETER, "target");
> > > +hmp_handle_error(mon, err);
> > > +return;
> > > +}
> > > +
> > > +qmp_drive_backup(&backup, &err);
> > > +hmp_handle_error(mon, err);
> > > +}
> > > diff --git a/include/block/block-hmp-cmds.h
> > > b/include/block/block-hmp-cmds.h
> > > index 30b0f56415..a64b737b3a 100644
> > > --- a/include/block/block-hmp-cmds.h
> > > +++ b/include/block/block-hmp-cmds.h
> > > @@ -3,10 +3,13 @@
> > > *
> > > * Copyright (c) 2003-2008 Fabrice Bellard
> > > * Copyright (c) 2020 Red Hat, Inc.
> > > + * Copyright IBM, Corp. 2011
> > > *
> > > - * This work is licensed under the terms of the GNU GPL, version 2.
> > > - * or (at your option) any later version.
> > > - * See the COPYING file in the top-level directory.
> > > + * Authors:
> > > + * Anthony Liguori
> > > + *
> > > + * This work is licensed under the terms of
Re: [PULL 00/33] Trivial branch patches
Patchew URL: https://patchew.org/QEMU/20200309150837.3193387-1-laur...@vivier.eu/ Hi, This series failed the docker-clang@ubuntu build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-ubuntu V=1 NETWORK=1 time make docker-test-clang@ubuntu SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === LINKqemu-bridge-helper LINKvirtiofsd LINKvhost-user-input /usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6 /usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6 /usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6 /usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6 GEN cris-softmmu/hmp-commands.h GEN lm32-softmmu/hmp-commands.h GEN m68k-softmmu/hmp-commands.h --- CC mips64-softmmu/hw/misc/mips_cmgcr.o CC mips64-softmmu/hw/misc/mips_cpc.o CC mips64-softmmu/hw/misc/mips_itu.o /usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6 /usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6 CC mips64-softmmu/hw/net/virtio-net.o CC mips64-softmmu/hw/net/rocker/qmp-norocker.o /usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6 CC arm-softmmu/hw/arm/integratorcp.o CC mips64el-softmmu/hw/block/virtio-blk.o CC i386-softmmu/hw/virtio/vhost-user-fs-pci.o --- CC mips64-softmmu/hw/scsi/virtio-scsi-dataplane.o CC i386-softmmu/hw/virtio/virtio-iommu.o CC i386-softmmu/hw/virtio/vhost-vsock.o /usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6 CC aarch64-softmmu/hw/arm/integratorcp.o CC aarch64-softmmu/hw/arm/mainstone.o CC mips64el-softmmu/hw/block/dataplane/virtio-blk.o --- CC i386-softmmu/hw/virtio/vhost-user-scsi-pci.o CC aarch64-softmmu/hw/arm/netduino2.o CC aarch64-softmmu/hw/arm/netduinoplus2.o /usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6 CC aarch64-softmmu/hw/arm/nseries.o CC i386-softmmu/hw/virtio/vhost-scsi-pci.o CC mipsel-softmmu/disas.o --- CC mips64el-softmmu/qapi/qapi-events-machine-target.o CC mips-softmmu/target/mips/dsp_helper.o CC mips-softmmu/target/mips/lmi_helper.o /usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6 CC arm-softmmu/softmmu/vl.o CC nios2-softmmu/tcg/tcg-op-vec.o CC mips64el-softmmu/qapi/qapi-events-misc-target.o --- CC moxie-softmmu/accel/tcg/cpu-exec.o CC mipsel-softmmu/hw/virtio/virtio-balloon-pci.o CC moxie-softmmu/accel/tcg/cpu-exec-common.o /usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6 CC moxie-softmmu/accel/tcg/translate-all.o CC mips64-softmmu/target/mips/dsp_helper.o CC arm-softmmu/target/arm/cpu.o --- CC nios2-softmmu/accel/stubs/whpx-stub.o CC mipsel-softmmu/hw/virtio/virtio-scsi-pci.o CC or1k-softmmu/accel/tcg/cputlb.o /usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6 CC mips64-softmmu/target/mips/mips-semi.o CC ppc-softmmu/accel/qtest.o CC i386-softmmu/hw/i386/vmport.o --- CC or1k-softmmu/softmmu/vl.o CC arm-softmmu/trace/generated-helpers.o CC i386-softmmu/hw/i386/kvm/clock.o /usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6 CC ppc64-softmmu/exec-vary.o CC ppc64-softmmu/tcg/tcg.o CC i386-softmmu/hw/i386/kvm/apic.o --- CC moxie-softmmu/trace/generated-helpers.o CC ppc64-softmmu/balloon.o CC ppc64-softmmu/ioport.o /usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of
Re: [PATCH v5 05/11] monitor/hmp: move hmp_drive_mirror and hmp_drive_backup to block-hmp-cmds.c Moved code was added after 2012-01-13, thus under GPLv2+
* Maxim Levitsky (mlevi...@redhat.com) wrote:
>
> I see that I have the same issue of long subject line here.
> Its because I forgot the space after first line, when adding this.
> If I need to resend another version of this patchset I'll fix this,
> but otherwise maybe that can be fixed when applying this to one of
> maintainer's
> trees.
>
> Sorry for noise.
I can just fix the commit message.
Dave
> Best regards,
> Maxim Levitsky
>
> On Sun, 2020-03-08 at 11:24 +0200, Maxim Levitsky wrote:
> > Signed-off-by: Maxim Levitsky
> > Reviewed-by: Dr. David Alan Gilbert
> > ---
> > block/monitor/block-hmp-cmds.c | 60 ++
> > include/block/block-hmp-cmds.h | 12 +--
> > include/monitor/hmp.h | 2 --
> > monitor/hmp-cmds.c | 58
> > 4 files changed, 69 insertions(+), 63 deletions(-)
> >
> > diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
> > index ad727a6b08..d6dd5d97f7 100644
> > --- a/block/monitor/block-hmp-cmds.c
> > +++ b/block/monitor/block-hmp-cmds.c
> > @@ -37,10 +37,12 @@
> > #include "qapi/qapi-commands-block.h"
> > #include "qapi/qmp/qdict.h"
> > #include "qapi/error.h"
> > +#include "qapi/qmp/qerror.h"
> > #include "qemu/config-file.h"
> > #include "qemu/option.h"
> > #include "sysemu/sysemu.h"
> > #include "monitor/monitor.h"
> > +#include "monitor/hmp.h"
> > #include "block/block_int.h"
> > #include "block/block-hmp-cmds.h"
> >
> > @@ -187,3 +189,61 @@ void hmp_commit(Monitor *mon, const QDict *qdict)
> > error_report("'commit' error for '%s': %s", device,
> > strerror(-ret));
> > }
> > }
> > +
> > +void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
> > +{
> > +const char *filename = qdict_get_str(qdict, "target");
> > +const char *format = qdict_get_try_str(qdict, "format");
> > +bool reuse = qdict_get_try_bool(qdict, "reuse", false);
> > +bool full = qdict_get_try_bool(qdict, "full", false);
> > +Error *err = NULL;
> > +DriveMirror mirror = {
> > +.device = (char *)qdict_get_str(qdict, "device"),
> > +.target = (char *)filename,
> > +.has_format = !!format,
> > +.format = (char *)format,
> > +.sync = full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
> > +.has_mode = true,
> > +.mode = reuse ? NEW_IMAGE_MODE_EXISTING :
> > NEW_IMAGE_MODE_ABSOLUTE_PATHS,
> > +.unmap = true,
> > +};
> > +
> > +if (!filename) {
> > +error_setg(&err, QERR_MISSING_PARAMETER, "target");
> > +hmp_handle_error(mon, err);
> > +return;
> > +}
> > +qmp_drive_mirror(&mirror, &err);
> > +hmp_handle_error(mon, err);
> > +}
> > +
> > +void hmp_drive_backup(Monitor *mon, const QDict *qdict)
> > +{
> > +const char *device = qdict_get_str(qdict, "device");
> > +const char *filename = qdict_get_str(qdict, "target");
> > +const char *format = qdict_get_try_str(qdict, "format");
> > +bool reuse = qdict_get_try_bool(qdict, "reuse", false);
> > +bool full = qdict_get_try_bool(qdict, "full", false);
> > +bool compress = qdict_get_try_bool(qdict, "compress", false);
> > +Error *err = NULL;
> > +DriveBackup backup = {
> > +.device = (char *)device,
> > +.target = (char *)filename,
> > +.has_format = !!format,
> > +.format = (char *)format,
> > +.sync = full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
> > +.has_mode = true,
> > +.mode = reuse ? NEW_IMAGE_MODE_EXISTING :
> > NEW_IMAGE_MODE_ABSOLUTE_PATHS,
> > +.has_compress = !!compress,
> > +.compress = compress,
> > +};
> > +
> > +if (!filename) {
> > +error_setg(&err, QERR_MISSING_PARAMETER, "target");
> > +hmp_handle_error(mon, err);
> > +return;
> > +}
> > +
> > +qmp_drive_backup(&backup, &err);
> > +hmp_handle_error(mon, err);
> > +}
> > diff --git a/include/block/block-hmp-cmds.h b/include/block/block-hmp-cmds.h
> > index 30b0f56415..a64b737b3a 100644
> > --- a/include/block/block-hmp-cmds.h
> > +++ b/include/block/block-hmp-cmds.h
> > @@ -3,10 +3,13 @@
> > *
> > * Copyright (c) 2003-2008 Fabrice Bellard
> > * Copyright (c) 2020 Red Hat, Inc.
> > + * Copyright IBM, Corp. 2011
> > *
> > - * This work is licensed under the terms of the GNU GPL, version 2.
> > - * or (at your option) any later version.
> > - * See the COPYING file in the top-level directory.
> > + * Authors:
> > + * Anthony Liguori
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2. See
> > + * the COPYING file in the top-level directory.
> > */
> >
> > #ifndef BLOCK_HMP_COMMANDS_H
> > @@ -17,4 +20,7 @@ void hmp_drive_add(Monitor *mon, const QDict *qdict);
> > void hmp_commit(Monitor *mon, const QDict *qdict);
> > void hmp_drive_del(Monitor *mon, const QDict *qdict);
> >
> > +void hmp_drive_mirror(Monitor *mon, const QDict
Re: [PATCH v3 1/4] block: Add trivial backing_fmt support to qcow, sheepdog, vmdk
Am 09.03.2020 um 16:32 hat Eric Blake geschrieben: > On 3/9/20 10:21 AM, Kevin Wolf wrote: > > Am 06.03.2020 um 23:51 hat Eric Blake geschrieben: > > > For qcow2 and qed, we want to encourage the use of -F always, as these > > > formats can suffer from data corruption or security holes if backing > > > format is probed. But for other formats, the backing format cannot be > > > recorded. Making the user decide on a per-format basis whether to > > > supply a backing format string is awkward, better is to just blindly > > > accept a backing format argument even if it is ignored by the > > > contraints of the format at hand. > > > > > > Signed-off-by: Eric Blake > > > > I'm not sure if I agree with this reasoning. Accepting and silently > > ignoring -F could give users a false sense of security. If I specify a > > -F raw and QEMU later probes qcow2, that would be very surprising. > > Do we know what formats qcow, sheepdog, and vmdk expect to probe? I'm > wondering if we can compromise by checking that the requested backing image > has the specified format, and error if it is not, rather than completely > ignoring it - but at the same time, the image formats have no where to > record a backing format. The important distinction (and in fact the only one that qed makes) is raw and non-raw. Problems only arise if a guest can write an image header to a raw file and get it probed as non-raw when opening the image the next time. If you start with a non-raw format, at least the first 512 bytes (which are used for probing) are used for metadata and not accessible for the guest. > I'm guessing that qcow works with either raw or qcow as backing format (and > anything else is odd - a qcow2 backing to a qcow is unusual, and would be > better to reject). I'm not sure if sheepdog can be backed by anything but > another sheepdog, similarly, I'm not sure if a vmdk can be backed by > anything but another vmdk. I think vmdk only expects vmdk as backing files, even though QEMU supports everything else, too. However, this is a format for compatibility with another hypervisor and you're unlikely to find QEMU-only VMDK images, so requiring non-raw unconditionally might make sense. I have no idea about how backing files in Sheepdog are used in practice. However, QEMU is a primary target for Sheepdog. It wouldn't surprise me if it's used for both raw and non-raw. qcow is definitely used for both, as you already said. Allowing only raw and qcow and forbidding other formats doesn't improve the situation because the problem is with supporting raw and non-raw at the same time and you would still have this. > > If so, it should be simple enough to do a v4 of > this patch which requires -F to be a known-acceptable probe type for these > images. > > Still, the point of this patch is that I want to add -F into all the > iotests, and without something along the lines of this patch, all of those > iotests are broken for these image formats. Patch 2 is a lot harder to > write if we have to make our use of -F conditional on the image format in > question. Hm... Maybe _make_test_img can insert/filter out -F depending on $IMGFMT? Kevin
Re: [PATCH v3 1/4] block: Add trivial backing_fmt support to qcow, sheepdog, vmdk
Am 09.03.2020 um 16:44 hat Daniel P. Berrangé geschrieben: > We could support "-F ..." and validate any non-raw formats, while raising a > runtime error in the case of "-F raw", as only the "raw" backing format has > the probing security risk. > > Users who need to use qcow, with a backing file, without a format can > just not pass "-F" and in doing so will be insecure. Hm, this is actually an interesting option. We wouldn't lose features compared to today without -F, but we would allow -F when we can verify that the operation is safe (the image is already non-raw). > We could take this opportunity to deprecate 'qcow' perhaps, declare it > a read-only format, restricted to qemu-img/qemu-io for purpose of data > liberation ? I'm against making any format read-only because that immediately means that it becomes untestable. > For sheepdog, if it is something we genuinely still care about, then > adding a backing file format record seems neccessary, unless we either > forbid use of raw backing files, or forbid use of non-raw backing files, > either way would be safe. In case of doubt, we can use the same logic as you suggested for qcow (accept only non-raw with -F, but no restrictions without -F). Kevin
Re: [PATCH v3 1/4] block: Add trivial backing_fmt support to qcow, sheepdog, vmdk
On 3/9/20 10:48 AM, Kevin Wolf wrote: Still, the point of this patch is that I want to add -F into all the iotests, and without something along the lines of this patch, all of those iotests are broken for these image formats. Patch 2 is a lot harder to write if we have to make our use of -F conditional on the image format in question. Hm... Maybe _make_test_img can insert/filter out -F depending on $IMGFMT? I was hoping to avoid that, but yes, if that's what we have to do... :( The complication is that even if I filter out -F from the command line based on $IMGFMT, then I have conditional output (whether backing_fmt= or the warning message is output), which means doubling the number of expected output cases, or else adding a multi-line filter which is also smart enough based on $IMGFMT to translate a warning on one line into an addition of a faked backing_fmt= on the next line. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v3 1/4] block: Add trivial backing_fmt support to qcow, sheepdog, vmdk
On 3/9/20 10:44 AM, Daniel P. Berrangé wrote: Consider the user creates an image with "-F raw". We can validate the backing image is raw, and so our check succeeds. Later the malicious can write a qcow header into this raw file and QEMU will thereafter probe the image as qcow, not raw. IOW, in the case of "-F raw", even if we immediately check the format, we're still not offering the protection promised by the "-F" flag, because that promise refers to the runtime behaviour of the QEMU emulator, not the immediate qemu-img cmd. We could support "-F ..." and validate any non-raw formats, while raising a runtime error in the case of "-F raw", as only the "raw" backing format has the probing security risk. Users who need to use qcow, with a backing file, without a format can just not pass "-F" and in doing so will be insecure. And the warning will remind them of that. We could take this opportunity to deprecate 'qcow' perhaps, declare it a read-only format, restricted to qemu-img/qemu-io for purpose of data liberation ? I'm fine with that, although it makes for a bigger task. For sheepdog, if it is something we genuinely still care about, then adding a backing file format record seems neccessary, unless we either forbid use of raw backing files, or forbid use of non-raw backing files, either way would be safe. I concur - as long as you either have ONLY non-raw (in which case probing is safe), or ONLY raw (in which case no probing is necessary), then not recording the backing format is safe. It is only for formats that allow both raw and non-raw backing, but which do not have space in the image to document which of the two backing formats is expected, where we have problems. I'm guessing that qcow works with either raw or qcow as backing format (and anything else is odd - a qcow2 backing to a qcow is unusual, and would be better to reject). I'm not sure if sheepdog can be backed by anything but another sheepdog, similarly, I'm not sure if a vmdk can be backed by anything but another vmdk. If so, it should be simple enough to do a v4 of this patch which requires -F to be a known-acceptable probe type for these images. Still, the point of this patch is that I want to add -F into all the iotests, and without something along the lines of this patch, all of those iotests are broken for these image formats. Patch 2 is a lot harder to write if we have to make our use of -F conditional on the image format in question. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org Regards, Daniel -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v3 1/4] block: Add trivial backing_fmt support to qcow, sheepdog, vmdk
On 3/9/20 10:36 AM, Daniel P. Berrangé wrote: On Mon, Mar 09, 2020 at 04:21:12PM +0100, Kevin Wolf wrote: Am 06.03.2020 um 23:51 hat Eric Blake geschrieben: For qcow2 and qed, we want to encourage the use of -F always, as these formats can suffer from data corruption or security holes if backing format is probed. But for other formats, the backing format cannot be recorded. Making the user decide on a per-format basis whether to supply a backing format string is awkward, better is to just blindly accept a backing format argument even if it is ignored by the contraints of the format at hand. Signed-off-by: Eric Blake I'm not sure if I agree with this reasoning. Accepting and silently ignoring -F could give users a false sense of security. If I specify a -F raw and QEMU later probes qcow2, that would be very surprising. And if the user specifies "-F raw" and we probe qcow2, and the user does not realize this, they can become silently reliant on always probing qcow2. If we then honour the "-F raw" option in a later QEMU release, we'll break the behaviour they've relied on. IMHO, we must not accept "-F fmt" unless we're in a position to honour it. So I'm thinking: qemu-img create -f qcow -b backing.qcow -F qcow img.qcow => okay qemu-img create -f qcow -b backing.raw -F raw img.qcow => okay, slightly risky (if backing.raw is ever changed to be non-raw), but then again, backing files tend to be read-only (do we even support commit on qcow images, or do we limit that to qcow2?) qemu-img create -f qcow -b backing.qcow -F raw img.qcow => fails, due to mismatch qemu-img create -u -f qcow -b anything -F anything img.qcow $size => fails: we can't write -F into the image, nor can we open anything to probe its type to check that -F was correct qemu-img create -f qcow -b backing.qcow img.qcow => warns, but okay (we did not get -F, but the probe works out) qemu-img create -f qcow -b backing.raw img.qcow=> likewise warns qemu-img create -f qcow -b backing.qcow2 img.qcow => error; new qcow images (which you should avoid where possible anyways) must be backed by only raw or qcow, going forward Other scenarios? Do the above ideas look reasonable? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v3 1/4] block: Add trivial backing_fmt support to qcow, sheepdog, vmdk
On Mon, Mar 09, 2020 at 10:32:52AM -0500, Eric Blake wrote: > On 3/9/20 10:21 AM, Kevin Wolf wrote: > > Am 06.03.2020 um 23:51 hat Eric Blake geschrieben: > > > For qcow2 and qed, we want to encourage the use of -F always, as these > > > formats can suffer from data corruption or security holes if backing > > > format is probed. But for other formats, the backing format cannot be > > > recorded. Making the user decide on a per-format basis whether to > > > supply a backing format string is awkward, better is to just blindly > > > accept a backing format argument even if it is ignored by the > > > contraints of the format at hand. > > > > > > Signed-off-by: Eric Blake > > > > I'm not sure if I agree with this reasoning. Accepting and silently > > ignoring -F could give users a false sense of security. If I specify a > > -F raw and QEMU later probes qcow2, that would be very surprising. > > Do we know what formats qcow, sheepdog, and vmdk expect to probe? I'm > wondering if we can compromise by checking that the requested backing image > has the specified format, and error if it is not, rather than completely > ignoring it - but at the same time, the image formats have no where to > record a backing format. Consider the user creates an image with "-F raw". We can validate the backing image is raw, and so our check succeeds. Later the malicious can write a qcow header into this raw file and QEMU will thereafter probe the image as qcow, not raw. IOW, in the case of "-F raw", even if we immediately check the format, we're still not offering the protection promised by the "-F" flag, because that promise refers to the runtime behaviour of the QEMU emulator, not the immediate qemu-img cmd. We could support "-F ..." and validate any non-raw formats, while raising a runtime error in the case of "-F raw", as only the "raw" backing format has the probing security risk. Users who need to use qcow, with a backing file, without a format can just not pass "-F" and in doing so will be insecure. We could take this opportunity to deprecate 'qcow' perhaps, declare it a read-only format, restricted to qemu-img/qemu-io for purpose of data liberation ? For sheepdog, if it is something we genuinely still care about, then adding a backing file format record seems neccessary, unless we either forbid use of raw backing files, or forbid use of non-raw backing files, either way would be safe. > I'm guessing that qcow works with either raw or qcow as backing format (and > anything else is odd - a qcow2 backing to a qcow is unusual, and would be > better to reject). I'm not sure if sheepdog can be backed by anything but > another sheepdog, similarly, I'm not sure if a vmdk can be backed by > anything but another vmdk. If so, it should be simple enough to do a v4 of > this patch which requires -F to be a known-acceptable probe type for these > images. > > Still, the point of this patch is that I want to add -F into all the > iotests, and without something along the lines of this patch, all of those > iotests are broken for these image formats. Patch 2 is a lot harder to > write if we have to make our use of -F conditional on the image format in > question. > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3226 > Virtualization: qemu.org | libvirt.org > Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v3 1/4] block: Add trivial backing_fmt support to qcow, sheepdog, vmdk
Am 06.03.2020 um 23:51 hat Eric Blake geschrieben: > For qcow2 and qed, we want to encourage the use of -F always, as these > formats can suffer from data corruption or security holes if backing > format is probed. But for other formats, the backing format cannot be > recorded. Making the user decide on a per-format basis whether to > supply a backing format string is awkward, better is to just blindly > accept a backing format argument even if it is ignored by the > contraints of the format at hand. > > Signed-off-by: Eric Blake I'm not sure if I agree with this reasoning. Accepting and silently ignoring -F could give users a false sense of security. If I specify a -F raw and QEMU later probes qcow2, that would be very surprising. Kevin
Re: [PATCH v3 4/4] qemu-img: Deprecate use of -b without -F
On 3/9/20 10:31 AM, Kashyap Chamarthy wrote: After (with the patch series applied to QEMU Git): $> git describe v4.2.0-2204-gd6c7830114 # Create; *without* specifying "-F raw" $> ~/build/qemu/qemu-img create -f qcow2 -b ./base.raw ./overlay2.qcow2 qemu-img: warning: Deprecated use of backing file without explicit backing format (detected format of raw) Formatting './overlay2.qcow2', fmt=qcow2 size=4294967296 backing_file=./base.raw backing_fmt=raw cluster_size=65536 lazy_refcounts=off refcount_bits=16 If you'll note, this case _did_ write an implied backing_fmt=raw into the image. Constrast that with creating an image on a qcow2 backing file, which tells you it detected a format of qcow2, but does NOT write backing_fmt=qcow2 into the image (this was a change from v2, at Peter's request). Thus, when the backing is raw, we warn but future use of the image is now safe where it previously was not; when the backing file is non-raw, we warn but do not change our behavior, but because the backing file is non-raw any future probes will not be any less safe than before. # Rebase; *without* specifying "-F raw" $> ~/build/qemu/qemu-img rebase -b base.raw overlay1.qcow2 qemu-img: warning: Deprecated use of backing file without explicit backing format, use of this image requires potentially unsafe format probing However, for the "Convert" case, is it correct that no warning is thrown for the below? $> ~/build/qemu/qemu-img info overlay1.qcow2 image: overlay1.qcow2 file format: qcow2 virtual size: 4 GiB (4294967296 bytes) disk size: 196 KiB cluster_size: 65536 backing file: base.raw Format specific information: compat: 1.1 lazy refcounts: false refcount bits: 16 corrupt: false We have an image with no backing format, so we had to probe. This patch series did not change the behavior of opening an existing image, only for creating a new image (or amending an image in-place). So the lack of a warning on opening the unsafe image may be desirable, but it would be via even more patches. $> ~/build/qemu/qemu-img convert -f qcow2 -O qcow2 overlay1.qcow2 flattened.raw Ouch - you are creating a qcow2 destination file named 'flattened.raw', which is rather confusing on your part. However, as your destination file is being created without a backing image, it is to be expected that there is no warning (when there is no backing file, -F makes no sense). To provoke the warning during convert, you'll have to also pass -B (or -o backing_file), without -o backing_fmt (since convert lacks the -F shorthand). $> echo $? 0 diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 6c1d9034d9e3..a8ffacf54a52 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -376,6 +376,25 @@ The above, converted to the current supported format:: Related binaries +qemu-img backing file without format (since 5.0.0) +'' + +The use of ``qemu-img create``, ``qemu-img rebase``, ``qemu-img +convert``, or ``qemu-img amend`` to create or modify an image that +depends on a backing file now recommends that an explicit backing +format be provided. This is for safety: if qemu probes a different +format than what you thought, the data presented to the guest will be +corrupt; similarly, presenting a raw image to a guest allows a +potential security exploit if a future probe sees a non-raw image +based on guest writes. To avoid the warning message, or even future +refusal to create an unsafe image, you must pass ``-o backing_fmt=`` +(or the shorthand ``-F`` during create) to specify the intended +backing format. You may use ``qemu-img rebase -u`` to retroactively +add a backing format to an existing image. However, be aware that +there are already potential security risks to blindly using ``qemu-img +info`` to probe the format of an untrusted backing image, when +deciding what format to add into an existing image. Nit: s/qemu/QEMU/g/ Ultra Nit: should this paragraph be broken down into two? Experience tells people usually feel deterred read "substantial paragraphs" :-) Could do, right before 'To avoid the warning'. I'll report back the Amend case. (And once I get clarification on the Convert scenario, I'll be happy to give Tested-by.) [...] -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v3 1/4] block: Add trivial backing_fmt support to qcow, sheepdog, vmdk
On Mon, Mar 09, 2020 at 04:21:12PM +0100, Kevin Wolf wrote: > Am 06.03.2020 um 23:51 hat Eric Blake geschrieben: > > For qcow2 and qed, we want to encourage the use of -F always, as these > > formats can suffer from data corruption or security holes if backing > > format is probed. But for other formats, the backing format cannot be > > recorded. Making the user decide on a per-format basis whether to > > supply a backing format string is awkward, better is to just blindly > > accept a backing format argument even if it is ignored by the > > contraints of the format at hand. > > > > Signed-off-by: Eric Blake > > I'm not sure if I agree with this reasoning. Accepting and silently > ignoring -F could give users a false sense of security. If I specify a > -F raw and QEMU later probes qcow2, that would be very surprising. And if the user specifies "-F raw" and we probe qcow2, and the user does not realize this, they can become silently reliant on always probing qcow2. If we then honour the "-F raw" option in a later QEMU release, we'll break the behaviour they've relied on. IMHO, we must not accept "-F fmt" unless we're in a position to honour it. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PULL 23/33] hw/i386: Include "hw/mem/nvdimm.h"
From: Philippe Mathieu-Daudé
All this files use methods/definitions declared in the NVDIMM
device header. Include it.
This fixes (when modifying unrelated headers):
hw/i386/acpi-build.c:2733:9: error: implicit declaration of function
'nvdimm_build_acpi' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
^
hw/i386/pc.c:1996:61: error: use of undeclared identifier 'TYPE_NVDIMM'
const bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
^
hw/i386/pc.c:2032:55: error: use of undeclared identifier 'TYPE_NVDIMM'
bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
^
hw/i386/pc.c:2040:9: error: implicit declaration of function 'nvdimm_plug' is
invalid in C99 [-Werror,-Wimplicit-function-declaration]
nvdimm_plug(ms->nvdimms_state);
^
hw/i386/pc.c:2040:9: error: this function declaration is not a prototype
[-Werror,-Wstrict-prototypes]
nvdimm_plug(ms->nvdimms_state);
^
hw/i386/pc.c:2065:42: error: use of undeclared identifier 'TYPE_NVDIMM'
if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) {
^
hw/i386/pc_i440fx.c:307:9: error: implicit declaration of function
'nvdimm_init_acpi_state' is invalid in C99
[-Werror,-Wimplicit-function-declaration]
nvdimm_init_acpi_state(machine->nvdimms_state, system_io,
^
hw/i386/pc_q35.c:332:9: error: implicit declaration of function
'nvdimm_init_acpi_state' is invalid in C99
[-Werror,-Wimplicit-function-declaration]
nvdimm_init_acpi_state(machine->nvdimms_state, system_io,
^
Acked-by: John Snow
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-Id: <20200228114649.12818-17-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/i386/acpi-build.c | 1 +
hw/i386/pc.c | 1 +
hw/i386/pc_piix.c| 1 +
hw/i386/pc_q35.c | 1 +
4 files changed, 4 insertions(+)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 26777f882844..9a19c14e661b 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -47,6 +47,7 @@
#include "hw/rtc/mc146818rtc_regs.h"
#include "migration/vmstate.h"
#include "hw/mem/memory-device.h"
+#include "hw/mem/nvdimm.h"
#include "sysemu/numa.h"
#include "sysemu/reset.h"
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6ab4acb0c62e..362eb2a180ff 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -76,6 +76,7 @@
#include "hw/boards.h"
#include "acpi-build.h"
#include "hw/mem/pc-dimm.h"
+#include "hw/mem/nvdimm.h"
#include "qapi/error.h"
#include "qapi/qapi-visit-common.h"
#include "qapi/visitor.h"
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9088db8fb601..e2d98243bc64 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -60,6 +60,7 @@
#include "migration/global_state.h"
#include "migration/misc.h"
#include "sysemu/numa.h"
+#include "hw/mem/nvdimm.h"
#define MAX_IDE_BUS 2
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 84cf925cf43a..d37c425e2236 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -53,6 +53,7 @@
#include "qapi/error.h"
#include "qemu/error-report.h"
#include "sysemu/numa.h"
+#include "hw/mem/nvdimm.h"
/* ICH9 AHCI has 6 ports */
#define MAX_SATA_PORTS 6
--
2.24.1
[PULL 29/33] dma/xlnx-zdma: Remove redundant statement in zdma_write_dst()
From: Chen Qun
Clang static code analyzer show warning:
hw/dma/xlnx-zdma.c:399:13: warning: Value stored to 'dst_type' is never read
dst_type = FIELD_EX32(s->dsc_dst.words[3], ZDMA_CH_DST_DSCR_WORD3,
^ ~~~
Reported-by: Euler Robot
Signed-off-by: Chen Qun
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Francisco Iglesias
Reviewed-by: Alistair Francis
Reviewed-by: Edgar E. Iglesias
Message-Id: <20200302130715.29440-11-kuhn.chen...@huawei.com>
Signed-off-by: Laurent Vivier
---
hw/dma/xlnx-zdma.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c
index 1c1b142293a6..2d9c0a0d5e19 100644
--- a/hw/dma/xlnx-zdma.c
+++ b/hw/dma/xlnx-zdma.c
@@ -372,7 +372,7 @@ static uint64_t zdma_update_descr_addr(XlnxZDMA *s, bool
type,
static void zdma_write_dst(XlnxZDMA *s, uint8_t *buf, uint32_t len)
{
uint32_t dst_size, dlen;
-bool dst_intr, dst_type;
+bool dst_intr;
unsigned int ptype = ARRAY_FIELD_EX32(s->regs, ZDMA_CH_CTRL0, POINT_TYPE);
unsigned int rw_mode = ARRAY_FIELD_EX32(s->regs, ZDMA_CH_CTRL0, MODE);
unsigned int burst_type = ARRAY_FIELD_EX32(s->regs, ZDMA_CH_DATA_ATTR,
@@ -386,17 +386,17 @@ static void zdma_write_dst(XlnxZDMA *s, uint8_t *buf,
uint32_t len)
while (len) {
dst_size = FIELD_EX32(s->dsc_dst.words[2], ZDMA_CH_DST_DSCR_WORD2,
SIZE);
-dst_type = FIELD_EX32(s->dsc_dst.words[3], ZDMA_CH_DST_DSCR_WORD3,
- TYPE);
if (dst_size == 0 && ptype == PT_MEM) {
uint64_t next;
+bool dst_type = FIELD_EX32(s->dsc_dst.words[3],
+ ZDMA_CH_DST_DSCR_WORD3,
+ TYPE);
+
next = zdma_update_descr_addr(s, dst_type,
R_ZDMA_CH_DST_CUR_DSCR_LSB);
zdma_load_descriptor(s, next, &s->dsc_dst);
dst_size = FIELD_EX32(s->dsc_dst.words[2], ZDMA_CH_DST_DSCR_WORD2,
SIZE);
-dst_type = FIELD_EX32(s->dsc_dst.words[3], ZDMA_CH_DST_DSCR_WORD3,
- TYPE);
}
/* Match what hardware does by ignoring the dst_size and only using
--
2.24.1
Re: [PATCH v3 1/4] block: Add trivial backing_fmt support to qcow, sheepdog, vmdk
On 3/9/20 10:21 AM, Kevin Wolf wrote: Am 06.03.2020 um 23:51 hat Eric Blake geschrieben: For qcow2 and qed, we want to encourage the use of -F always, as these formats can suffer from data corruption or security holes if backing format is probed. But for other formats, the backing format cannot be recorded. Making the user decide on a per-format basis whether to supply a backing format string is awkward, better is to just blindly accept a backing format argument even if it is ignored by the contraints of the format at hand. Signed-off-by: Eric Blake I'm not sure if I agree with this reasoning. Accepting and silently ignoring -F could give users a false sense of security. If I specify a -F raw and QEMU later probes qcow2, that would be very surprising. Do we know what formats qcow, sheepdog, and vmdk expect to probe? I'm wondering if we can compromise by checking that the requested backing image has the specified format, and error if it is not, rather than completely ignoring it - but at the same time, the image formats have no where to record a backing format. I'm guessing that qcow works with either raw or qcow as backing format (and anything else is odd - a qcow2 backing to a qcow is unusual, and would be better to reject). I'm not sure if sheepdog can be backed by anything but another sheepdog, similarly, I'm not sure if a vmdk can be backed by anything but another vmdk. If so, it should be simple enough to do a v4 of this patch which requires -F to be a known-acceptable probe type for these images. Still, the point of this patch is that I want to add -F into all the iotests, and without something along the lines of this patch, all of those iotests are broken for these image formats. Patch 2 is a lot harder to write if we have to make our use of -F conditional on the image format in question. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v3 4/4] qemu-img: Deprecate use of -b without -F
On Fri, Mar 06, 2020 at 04:51:21PM -0600, Eric Blake wrote: > Creating an image that requires format probing of the backing image is > inherently unsafe (we've had several CVEs over the years based on > probes leaking information to the guest on a subsequent boot, although > these days tools like libvirt are aware of the issue enough to prevent > the worst effects). However, if our probing algorithm ever changes, > or if other tools like libvirt determine a different probe result than > we do, then subsequent use of that backing file under a different > format will present corrupted data to the guest. Start a deprecation > clock so that future qemu-img can refuse to create unsafe backing > chains that would rely on probing. The warnings are intentionally > emitted from the block layer rather than qemu-img (thus, all paths > into image creation or rewriting perform the check). I happily welcome this change. I'm going around and fixing various docs of differnt projects that create overlays without explicitly spelling out backing files. (FWIW, I also make sure to mention this, in context, in all QEMU-related talks I give publicliy.) This proactive action from QEMU will definitely help. > However, there is one time where probing is safe: if we probe raw, > then it is safe to record that implicitly in the image (but we still > warn, as it's better to teach the user to supply -F always than to > make them guess when it is safe). > > iotest 114 specifically wants to create an unsafe image for later > amendment rather than defaulting to our new default of recording a > probed format, so it needs an update. While touching it, expand it to > cover all of the various warnings enabled by this patch. > > Signed-off-by: Eric Blake > --- > docs/system/deprecated.rst | 19 +++ > block.c| 21 - > qemu-img.c | 2 +- > tests/qemu-iotests/114 | 11 +++ > tests/qemu-iotests/114.out | 8 > 5 files changed, 59 insertions(+), 2 deletions(-) Before (with: qemu-4.2.0-2.fc30): $> qemu-img create -f qcow2 -b ./base.raw ./overlay1.qcow2 Formatting './overlay1.qcow2', fmt=qcow2 size=4294967296 backing_file=./base.raw cluster_size=65536 lazy_refcounts=off refcount_bits=16 After (with the patch series applied to QEMU Git): $> git describe v4.2.0-2204-gd6c7830114 # Create; *without* specifying "-F raw" $> ~/build/qemu/qemu-img create -f qcow2 -b ./base.raw ./overlay2.qcow2 qemu-img: warning: Deprecated use of backing file without explicit backing format (detected format of raw) Formatting './overlay2.qcow2', fmt=qcow2 size=4294967296 backing_file=./base.raw backing_fmt=raw cluster_size=65536 lazy_refcounts=off refcount_bits=16 # Rebase; *without* specifying "-F raw" $> ~/build/qemu/qemu-img rebase -b base.raw overlay1.qcow2 qemu-img: warning: Deprecated use of backing file without explicit backing format, use of this image requires potentially unsafe format probing However, for the "Convert" case, is it correct that no warning is thrown for the below? $> ~/build/qemu/qemu-img info overlay1.qcow2 image: overlay1.qcow2 file format: qcow2 virtual size: 4 GiB (4294967296 bytes) disk size: 196 KiB cluster_size: 65536 backing file: base.raw Format specific information: compat: 1.1 lazy refcounts: false refcount bits: 16 corrupt: false $> ~/build/qemu/qemu-img convert -f qcow2 -O qcow2 overlay1.qcow2 flattened.raw $> echo $? 0 > diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst > index 6c1d9034d9e3..a8ffacf54a52 100644 > --- a/docs/system/deprecated.rst > +++ b/docs/system/deprecated.rst > @@ -376,6 +376,25 @@ The above, converted to the current supported format:: > Related binaries > > > +qemu-img backing file without format (since 5.0.0) > +'' > + > +The use of ``qemu-img create``, ``qemu-img rebase``, ``qemu-img > +convert``, or ``qemu-img amend`` to create or modify an image that > +depends on a backing file now recommends that an explicit backing > +format be provided. This is for safety: if qemu probes a different > +format than what you thought, the data presented to the guest will be > +corrupt; similarly, presenting a raw image to a guest allows a > +potential security exploit if a future probe sees a non-raw image > +based on guest writes. To avoid the warning message, or even future > +refusal to create an unsafe image, you must pass ``-o backing_fmt=`` > +(or the shorthand ``-F`` during create) to specify the intended > +backing format. You may use ``qemu-img rebase -u`` to retroactively > +add a backing format to an existing image. However, be aware that > +there are already potential security risks to blindly using ``qemu-img > +info`` to probe the format of an untrusted backing image, when
[PULL 28/33] block/file-posix: Remove redundant statement in raw_handle_perm_lock()
From: Chen Qun Clang static code analyzer show warning: block/file-posix.c:891:9: warning: Value stored to 'op' is never read op = RAW_PL_ABORT; ^ Reported-by: Euler Robot Signed-off-by: Chen Qun Reviewed-by: Kevin Wolf Message-Id: <20200302130715.29440-5-kuhn.chen...@huawei.com> Signed-off-by: Laurent Vivier --- block/file-posix.c | 1 - 1 file changed, 1 deletion(-) diff --git a/block/file-posix.c b/block/file-posix.c index 634547711297..0f77447a25df 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -888,7 +888,6 @@ static int raw_handle_perm_lock(BlockDriverState *bs, "Is another process using the image [%s]?\n", bs->filename); } -op = RAW_PL_ABORT; /* fall through to unlock bytes. */ case RAW_PL_ABORT: raw_apply_lock_bytes(s, s->fd, s->perm, ~s->shared_perm, -- 2.24.1
[PULL 32/33] display/exynos4210_fimd: Remove redundant statement in exynos4210_fimd_update()
From: Chen Qun Clang static code analyzer show warning: hw/display/exynos4210_fimd.c:1313:17: warning: Value stored to 'is_dirty' is never read is_dirty = false; Reported-by: Euler Robot Signed-off-by: Chen Qun Reviewed-by: Laurent Vivier Message-Id: <20200302130715.29440-9-kuhn.chen...@huawei.com> Signed-off-by: Laurent Vivier --- hw/display/exynos4210_fimd.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c index ec6776680e41..1c0266ce9f2d 100644 --- a/hw/display/exynos4210_fimd.c +++ b/hw/display/exynos4210_fimd.c @@ -1311,7 +1311,6 @@ static void exynos4210_fimd_update(void *opaque) } host_fb_addr += inc_size; fb_line_addr += inc_size; -is_dirty = false; } g_free(snap); blend = true; -- 2.24.1
[PULL 24/33] hw/pci-host/q35: Remove unused includes
From: Philippe Mathieu-Daudé Only q35.c requires declarations from "hw/i386/pc.h", move it there. Remove all the includes not used by "q35.h". Acked-by: John Snow Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200228114649.12818-18-phi...@redhat.com> Signed-off-by: Laurent Vivier --- hw/pci-host/q35.c | 1 + include/hw/pci-host/q35.h | 7 --- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index a9b9ccc87657..993f467668dc 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -29,6 +29,7 @@ */ #include "qemu/osdep.h" +#include "hw/i386/pc.h" #include "hw/pci-host/q35.h" #include "hw/qdev-properties.h" #include "migration/vmstate.h" diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h index 47086c645e9f..070305f83dfd 100644 --- a/include/hw/pci-host/q35.h +++ b/include/hw/pci-host/q35.h @@ -22,16 +22,9 @@ #ifndef HW_Q35_H #define HW_Q35_H -#include "hw/isa/isa.h" -#include "hw/sysbus.h" -#include "hw/i386/pc.h" -#include "hw/isa/apm.h" #include "hw/pci/pci.h" #include "hw/pci/pcie_host.h" -#include "hw/acpi/acpi.h" -#include "hw/acpi/ich9.h" #include "hw/pci-host/pam.h" -#include "hw/i386/intel_iommu.h" #include "qemu/units.h" #include "qemu/range.h" -- 2.24.1
[PULL 33/33] monitor/hmp-cmds: Remove redundant statement in hmp_rocker_of_dpa_groups()
From: Chen Qun
Clang static code analyzer show warning:
monitor/hmp-cmds.c:2867:17: warning: Value stored to 'set' is never read
set = true;
^
Reported-by: Euler Robot
Signed-off-by: Chen Qun
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Laurent Vivier
Message-Id: <20200302130715.29440-14-kuhn.chen...@huawei.com>
Signed-off-by: Laurent Vivier
---
monitor/hmp-cmds.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index fb4c2fd2a875..6fd7aca5007b 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -2842,7 +2842,6 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict
*qdict)
const char *name = qdict_get_str(qdict, "name");
uint8_t type = qdict_get_try_int(qdict, "type", 9);
Error *err = NULL;
-bool set = false;
list = qmp_query_rocker_of_dpa_groups(name, type != 9, type, &err);
if (err != NULL) {
@@ -2854,6 +2853,7 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict
*qdict)
for (g = list; g; g = g->next) {
RockerOfDpaGroup *group = g->value;
+bool set = false;
monitor_printf(mon, "0x%08x", group->id);
@@ -2898,14 +2898,11 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict
*qdict)
if (group->has_set_eth_dst) {
if (!set) {
-set = true;
monitor_printf(mon, " set");
}
monitor_printf(mon, " dst %s", group->set_eth_dst);
}
-set = false;
-
if (group->has_ttl_check && group->ttl_check) {
monitor_printf(mon, " check TTL");
}
--
2.24.1
[PULL 31/33] display/pxa2xx_lcd: Remove redundant statement in pxa2xx_palette_parse()
From: Chen Qun Clang static code analyzer show warning: hw/display/pxa2xx_lcd.c:596:9: warning: Value stored to 'format' is never read format = 0; ^~ Reported-by: Euler Robot Signed-off-by: Chen Qun Reviewed-by: Laurent Vivier Message-Id: <20200302130715.29440-8-kuhn.chen...@huawei.com> Signed-off-by: Laurent Vivier --- hw/display/pxa2xx_lcd.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/display/pxa2xx_lcd.c b/hw/display/pxa2xx_lcd.c index 05f5f8467123..464e93161a21 100644 --- a/hw/display/pxa2xx_lcd.c +++ b/hw/display/pxa2xx_lcd.c @@ -593,7 +593,6 @@ static void pxa2xx_palette_parse(PXA2xxLCDState *s, int ch, int bpp) n = 256; break; default: -format = 0; return; } -- 2.24.1
[PULL 22/33] hw/acpi: Include "hw/mem/nvdimm.h"
From: Philippe Mathieu-Daudé
Both ich9.c and piix4.c use methods/definitions declared in the
NVDIMM device header. Include it.
This fixes (when modifying unrelated headers):
hw/acpi/ich9.c:507:46: error: use of undeclared identifier 'TYPE_NVDIMM'
if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) {
^
hw/acpi/ich9.c:508:13: error: implicit declaration of function
'nvdimm_acpi_plug_cb' is invalid in C99
[-Werror,-Wimplicit-function-declaration]
nvdimm_acpi_plug_cb(hotplug_dev, dev);
^
hw/acpi/piix4.c:403:46: error: use of undeclared identifier 'TYPE_NVDIMM'
if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) {
^
hw/acpi/piix4.c:404:13: error: implicit declaration of function
'nvdimm_acpi_plug_cb' is invalid in C99
[-Werror,-Wimplicit-function-declaration]
nvdimm_acpi_plug_cb(hotplug_dev, dev);
^
Acked-by: John Snow
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-Id: <20200228114649.12818-16-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/acpi/ich9.c | 1 +
hw/acpi/piix4.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index fdd0a6c79e11..4e74284b65b7 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -39,6 +39,7 @@
#include "hw/i386/ich9.h"
#include "hw/mem/pc-dimm.h"
+#include "hw/mem/nvdimm.h"
//#define DEBUG
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 6d621c31e751..b84dbba2c3e8 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -38,6 +38,7 @@
#include "hw/acpi/cpu.h"
#include "hw/hotplug.h"
#include "hw/mem/pc-dimm.h"
+#include "hw/mem/nvdimm.h"
#include "hw/acpi/memory_hotplug.h"
#include "hw/acpi/acpi_dev_interface.h"
#include "hw/xen/xen.h"
--
2.24.1
[PULL 27/33] block/stream: Remove redundant statement in stream_run()
From: Chen Qun
Clang static code analyzer show warning:
block/stream.c:186:9: warning: Value stored to 'ret' is never read
ret = 0;
^ ~
Reported-by: Euler Robot
Signed-off-by: Chen Qun
Reviewed-by: John Snow
Reviewed-by: Kevin Wolf
Message-Id: <20200302130715.29440-3-kuhn.chen...@huawei.com>
Signed-off-by: Laurent Vivier
---
block/stream.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/block/stream.c b/block/stream.c
index 5562ccbf577a..aa2e7af98e37 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -114,7 +114,6 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
int64_t offset = 0;
uint64_t delay_ns = 0;
int error = 0;
-int ret = 0;
int64_t n = 0; /* bytes */
if (bs == s->bottom) {
@@ -139,6 +138,7 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
for ( ; offset < len; offset += n) {
bool copy;
+int ret;
/* Note that even when no rate limit is applied we need to yield
* with no pending I/O here so that bdrv_drain_all() returns.
@@ -183,7 +183,6 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
break;
}
}
-ret = 0;
/* Publish progress */
job_progress_update(&s->common.job, n);
--
2.24.1
[PULL 30/33] scsi/scsi-disk: Remove redundant statement in scsi_disk_emulate_command()
From: Chen Qun
Clang static code analyzer show warning:
scsi/scsi-disk.c:1918:5: warning: Value stored to 'buflen' is never read
buflen = req->cmd.xfer;
^~
Reported-by: Euler Robot
Signed-off-by: Chen Qun
Reviewed-by: Laurent Vivier
Message-Id: <20200302130715.29440-7-kuhn.chen...@huawei.com>
Signed-off-by: Laurent Vivier
---
hw/scsi/scsi-disk.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 10d0794d60f1..1c0cb63a6fe0 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1915,7 +1915,6 @@ static int32_t scsi_disk_emulate_command(SCSIRequest
*req, uint8_t *buf)
r->iov.iov_base = blk_blockalign(s->qdev.conf.blk, r->buflen);
}
-buflen = req->cmd.xfer;
outbuf = r->iov.iov_base;
memset(outbuf, 0, r->buflen);
switch (req->cmd.buf[0]) {
--
2.24.1
[PULL 10/33] hw/i386/ioapic_internal: Remove unused "hw/i386/ioapic.h" header
From: Philippe Mathieu-Daudé The "ioapic_internal.h" does not use anything from "hw/i386/ioapic.h", remove it. Acked-by: John Snow Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200228114649.12818-4-phi...@redhat.com> Signed-off-by: Laurent Vivier --- include/hw/i386/ioapic_internal.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h index d46c87c51030..fe06938bda47 100644 --- a/include/hw/i386/ioapic_internal.h +++ b/include/hw/i386/ioapic_internal.h @@ -23,7 +23,6 @@ #define QEMU_IOAPIC_INTERNAL_H #include "exec/memory.h" -#include "hw/i386/ioapic.h" #include "hw/sysbus.h" #include "qemu/notify.h" -- 2.24.1
[PULL 26/33] core/qdev: fix memleak in qdev_get_gpio_out_connector()
From: Pan Nengyuan
Fix a memory leak in qdev_get_gpio_out_connector().
Reported-by: Euler Robot
Signed-off-by: Pan Nengyuan
Reviewed-by: Marc-André Lureau
Message-Id: <20200307030756.5913-1-pannengy...@huawei.com>
Signed-off-by: Laurent Vivier
---
hw/core/qdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 3937d1eb1a5f..85f062def72b 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -557,7 +557,7 @@ void qdev_connect_gpio_out_named(DeviceState *dev, const
char *name, int n,
qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n)
{
-char *propname = g_strdup_printf("%s[%d]",
+g_autofree char *propname = g_strdup_printf("%s[%d]",
name ? name : "unnamed-gpio-out", n);
qemu_irq ret = (qemu_irq)object_property_get_link(OBJECT(dev), propname,
--
2.24.1
[PULL 16/33] hw/hppa/machine: Include "net/net.h"
From: Philippe Mathieu-Daudé
hw/hppa/machine.c uses NICInfo variables which are declared in
"net/net.h". Include it.
This fixes (when modifying unrelated headers):
hw/hppa/machine.c:126:21: error: use of undeclared identifier 'nb_nics'
for (i = 0; i < nb_nics; i++) {
^
hw/hppa/machine.c:127:30: error: use of undeclared identifier 'nd_table'
pci_nic_init_nofail(&nd_table[i], pci_bus, "e1000", NULL);
^
Acked-by: John Snow
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-Id: <20200228114649.12818-10-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/hppa/machine.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index bf18767e2494..9175f4b790f5 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -22,6 +22,7 @@
#include "qapi/error.h"
#include "net/net.h"
#include "qemu/log.h"
+#include "net/net.h"
#define MAX_IDE_BUS 2
--
2.24.1
[PULL 17/33] hw/acpi/cpu_hotplug: Include "hw/pci/pci.h"
From: Philippe Mathieu-Daudé hw/acpi/cpu_hotplug.c calls pci_address_space_io(). Include "hw/pci/pci.h" which declares it. This fixes (when modifying unrelated headers): hw/acpi/cpu_hotplug.c:103:28: error: implicit declaration of function 'pci_address_space_io' is invalid in C99 [-Werror,-Wimplicit-function-declaration] MemoryRegion *parent = pci_address_space_io(PCI_DEVICE(gpe_cpu->device)); ^ Acked-by: John Snow Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200228114649.12818-11-phi...@redhat.com> Signed-off-by: Laurent Vivier --- hw/acpi/cpu_hotplug.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c index 9c3bcc84de56..3e687d227a65 100644 --- a/hw/acpi/cpu_hotplug.c +++ b/hw/acpi/cpu_hotplug.c @@ -14,6 +14,7 @@ #include "qapi/error.h" #include "hw/core/cpu.h" #include "hw/i386/pc.h" +#include "hw/pci/pci.h" #include "qemu/error-report.h" #define CPU_EJECT_METHOD "CPEJ" -- 2.24.1
[PULL 09/33] hw/southbridge/ich9: Removed unused headers
From: Philippe Mathieu-Daudé The ICH9 chipset is not X86/PC specific. These files don't use anything declared by the "hw/i386/pc.h" or "hw/i386/ioapic.h" headers. Remove them. Reviewed-by: John Snow Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200228114649.12818-3-phi...@redhat.com> Signed-off-by: Laurent Vivier --- hw/acpi/ich9.c | 1 - hw/isa/lpc_ich9.c | 1 - include/hw/i386/ich9.h | 1 - 3 files changed, 3 deletions(-) diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c index 2034dd749edc..fdd0a6c79e11 100644 --- a/hw/acpi/ich9.c +++ b/hw/acpi/ich9.c @@ -27,7 +27,6 @@ #include "qemu/osdep.h" #include "qapi/error.h" #include "qapi/visitor.h" -#include "hw/i386/pc.h" #include "hw/pci/pci.h" #include "migration/vmstate.h" #include "qemu/timer.h" diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index f85b484eac63..cb79616cede8 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -37,7 +37,6 @@ #include "migration/vmstate.h" #include "hw/irq.h" #include "hw/isa/apm.h" -#include "hw/i386/ioapic.h" #include "hw/pci/pci.h" #include "hw/pci/pci_bridge.h" #include "hw/i386/ich9.h" diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h index 72e803f6e2e0..a98d10b252df 100644 --- a/include/hw/i386/ich9.h +++ b/include/hw/i386/ich9.h @@ -5,7 +5,6 @@ #include "hw/sysbus.h" #include "hw/i386/pc.h" #include "hw/isa/apm.h" -#include "hw/i386/ioapic.h" #include "hw/pci/pci.h" #include "hw/pci/pcie_host.h" #include "hw/pci/pci_bridge.h" -- 2.24.1
[PULL 15/33] hw/alpha/dp264: Include "net/net.h"
From: Philippe Mathieu-Daudé
hw/alpha/dp264.c uses NICInfo variables which are declared in
"net/net.h". Include it.
This fixes (when modifying unrelated headers):
hw/alpha/dp264.c:89:21: error: use of undeclared identifier 'nb_nics'
for (i = 0; i < nb_nics; i++) {
^
hw/alpha/dp264.c:90:30: error: use of undeclared identifier 'nd_table'
pci_nic_init_nofail(&nd_table[i], pci_bus, "e1000", NULL);
^
Acked-by: John Snow
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-Id: <20200228114649.12818-9-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/alpha/dp264.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index d28f57199fa1..e5350a287f73 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -21,6 +21,7 @@
#include "hw/dma/i8257.h"
#include "net/net.h"
#include "qemu/cutils.h"
+#include "net/net.h"
#define MAX_IDE_BUS 2
--
2.24.1
[PULL 25/33] hw/i386/pc: Clean up includes
From: Philippe Mathieu-Daudé Various headers are not required by hw/i386/pc.h: - "qemu/range.h" - "qemu/bitmap.h" - "qemu/module.h" - "exec/memory.h" - "hw/pci/pci.h" - "hw/mem/pc-dimm.h" - "hw/mem/nvdimm.h" - "net/net.h" Remove them. Add 3 headers that were missing: - "hw/hotplug.h" PCMachineState::acpi_dev is of type HotplugHandler - "qemu/notify.h" PCMachineState::machine_done is of type Notifier - "qapi/qapi-types-common.h" PCMachineState::vmport/smm is of type OnOffAuto Acked-by: John Snow Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200228114649.12818-19-phi...@redhat.com> Signed-off-by: Laurent Vivier --- include/hw/i386/pc.h | 11 +++ 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index d5ac76d54e1f..6ab6eda046fd 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -1,20 +1,15 @@ #ifndef HW_PC_H #define HW_PC_H -#include "exec/memory.h" +#include "qemu/notify.h" +#include "qapi/qapi-types-common.h" #include "hw/boards.h" #include "hw/block/fdc.h" #include "hw/block/flash.h" -#include "net/net.h" #include "hw/i386/x86.h" -#include "qemu/range.h" -#include "qemu/bitmap.h" -#include "qemu/module.h" -#include "hw/pci/pci.h" -#include "hw/mem/pc-dimm.h" -#include "hw/mem/nvdimm.h" #include "hw/acpi/acpi_dev_interface.h" +#include "hw/hotplug.h" #define HPET_INTCAP "hpet-intcap" -- 2.24.1
[PULL 20/33] hw/i2c/smbus_ich9: Include "qemu/range.h"
From: Philippe Mathieu-Daudé
hw/i2c/smbus_ich9.c calls range_covers_byte(). Include "qemu/range.h"
which declares it.
This fixes (when modifying unrelated headers):
hw/i2c/smbus_ich9.c:66:9: error: implicit declaration of function
'range_covers_byte' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
if (range_covers_byte(address, len, ICH9_SMB_HOSTC)) {
^
Acked-by: John Snow
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-Id: <20200228114649.12818-14-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/i2c/smbus_ich9.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
index fd50fb851af4..48f1ff419174 100644
--- a/hw/i2c/smbus_ich9.c
+++ b/hw/i2c/smbus_ich9.c
@@ -21,6 +21,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/range.h"
#include "hw/i2c/pm_smbus.h"
#include "hw/pci/pci.h"
#include "migration/vmstate.h"
--
2.24.1
[PULL 02/33] maint: Include top-level *.rst files early in git diff
From: Eric Blake We are converting more doc files to *.rst rather than *.texi. Most doc files are already listed early in diffs due to our catchall docs/*, but a few top-level files get missed by that glob. Signed-off-by: Eric Blake Reviewed-by: Stefano Garzarella Message-Id: <20200220162214.3474280-1-ebl...@redhat.com> Signed-off-by: Laurent Vivier --- scripts/git.orderfile | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/git.orderfile b/scripts/git.orderfile index 7cf22e0bf546..73fd818d7f3e 100644 --- a/scripts/git.orderfile +++ b/scripts/git.orderfile @@ -11,6 +11,7 @@ # Documentation docs/* +*.rst *.texi # build system -- 2.24.1
[PULL 13/33] hw/i386/intel_iommu: Remove unused includes
From: Philippe Mathieu-Daudé intel_iommu.h does not use any of these includes, remove them. Acked-by: John Snow Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200228114649.12818-7-phi...@redhat.com> Signed-off-by: Laurent Vivier --- include/hw/i386/intel_iommu.h | 4 1 file changed, 4 deletions(-) diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h index 66b931e52628..a1c4afcda5c9 100644 --- a/include/hw/i386/intel_iommu.h +++ b/include/hw/i386/intel_iommu.h @@ -22,11 +22,7 @@ #ifndef INTEL_IOMMU_H #define INTEL_IOMMU_H -#include "sysemu/dma.h" #include "hw/i386/x86-iommu.h" -#include "hw/i386/ioapic.h" -#include "hw/pci/msi.h" -#include "hw/sysbus.h" #include "qemu/iova-tree.h" #define TYPE_INTEL_IOMMU_DEVICE "intel-iommu" -- 2.24.1
[PULL 14/33] hw/alpha/alpha_sys: Remove unused "hw/ide.h" header
From: Philippe Mathieu-Daudé alpha_sys.h does not use anything from the "hw/ide.h" header. Remove it. Acked-by: John Snow Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200228114649.12818-8-phi...@redhat.com> Signed-off-by: Laurent Vivier --- hw/alpha/alpha_sys.h | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h index bc0a286226f1..e2c02e2bbe1d 100644 --- a/hw/alpha/alpha_sys.h +++ b/hw/alpha/alpha_sys.h @@ -6,7 +6,6 @@ #include "target/alpha/cpu-qom.h" #include "hw/pci/pci.h" #include "hw/pci/pci_host.h" -#include "hw/ide.h" #include "hw/boards.h" #include "hw/intc/i8259.h" -- 2.24.1
[PULL 21/33] hw/pci-host/piix: Include "qemu/range.h"
From: Philippe Mathieu-Daudé
hw/pci-host/piix.c calls various functions from the Range API.
Include "qemu/range.h" which declares them.
This fixes (when modifying unrelated headers):
hw/pci-host/i440fx.c:54:11: error: field has incomplete type 'Range' (aka
'struct Range')
Range pci_hole;
^
include/qemu/typedefs.h:116:16: note: forward declaration of 'struct Range'
typedef struct Range Range;
^
hw/pci-host/i440fx.c:126:9: error: implicit declaration of function
'ranges_overlap' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
if (ranges_overlap(address, len, I440FX_PAM, I440FX_PAM_SIZE) ||
^
hw/pci-host/i440fx.c:126:9: error: this function declaration is not a
prototype [-Werror,-Wstrict-prototypes]
hw/pci-host/i440fx.c:127:9: error: implicit declaration of function
'range_covers_byte' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
range_covers_byte(address, len, I440FX_SMRAM)) {
^
hw/pci-host/i440fx.c:127:9: error: this function declaration is not a
prototype [-Werror,-Wstrict-prototypes]
hw/pci-host/i440fx.c:189:13: error: implicit declaration of function
'range_is_empty' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
val64 = range_is_empty(&s->pci_hole) ? 0 : range_lob(&s->pci_hole);
^
Acked-by: John Snow
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-Id: <20200228114649.12818-15-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/pci-host/i440fx.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 11050a0f8bb9..d980c9704906 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -23,6 +23,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/range.h"
#include "hw/i386/pc.h"
#include "hw/pci/pci.h"
#include "hw/pci/pci_host.h"
--
2.24.1
[PULL 12/33] hw/usb/dev-storage: Remove unused "ui/console.h" header
From: Philippe Mathieu-Daudé The USB models related to storage don't need anything from "ui/console.h". Remove it. Acked-by: John Snow Signed-off-by: Philippe Mathieu-Daudé Acked-by: Gerd Hoffmann Reviewed-by: Richard Henderson Message-Id: <20200228114649.12818-6-phi...@redhat.com> Signed-off-by: Laurent Vivier --- hw/usb/dev-storage.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index 90da008df18c..4883c1d89e0c 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -17,7 +17,6 @@ #include "desc.h" #include "hw/qdev-properties.h" #include "hw/scsi/scsi.h" -#include "ui/console.h" #include "migration/vmstate.h" #include "monitor/monitor.h" #include "sysemu/sysemu.h" -- 2.24.1
[PULL 18/33] hw/timer/hpet: Include "exec/address-spaces.h"
From: Philippe Mathieu-Daudé hw/timer/hpet.c calls address_space_stl_le() declared in "exec/address-spaces.h". Include it. This fixes (when modifying unrelated headers): hw/timer/hpet.c:210:31: error: use of undeclared identifier 'address_space_memory' address_space_stl_le(&address_space_memory, timer->fsb >> 32, ^~~~ Acked-by: John Snow Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200228114649.12818-12-phi...@redhat.com> Signed-off-by: Laurent Vivier --- hw/timer/hpet.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c index 4f30dd50a40a..380acfa7c8a5 100644 --- a/hw/timer/hpet.c +++ b/hw/timer/hpet.c @@ -36,6 +36,7 @@ #include "hw/rtc/mc146818rtc_regs.h" #include "migration/vmstate.h" #include "hw/timer/i8254.h" +#include "exec/address-spaces.h" //#define HPET_DEBUG #ifdef HPET_DEBUG -- 2.24.1
[PULL 04/33] hw/audio/fmopl: Fix a typo twice
From: Philippe Mathieu-Daudé
Reviewed-by: Laurent Vivier
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Stefano Garzarella
Message-Id: <20200305124525.14555-2-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/audio/fmopl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/audio/fmopl.c b/hw/audio/fmopl.c
index 9f50a89b4a88..173a7521f2a7 100644
--- a/hw/audio/fmopl.c
+++ b/hw/audio/fmopl.c
@@ -1066,7 +1066,7 @@ static void OPLResetChip(FM_OPL *OPL)
}
}
-/* -- Create one of vietual YM3812 -- */
+/* -- Create one of virtual YM3812 -- */
/* 'rate' is sampling rate and 'bufsiz' is the size of the */
FM_OPL *OPLCreate(int clock, int rate)
{
@@ -1115,7 +1115,7 @@ FM_OPL *OPLCreate(int clock, int rate)
return OPL;
}
-/* -- Destroy one of vietual YM3812 -- */
+/* -- Destroy one of virtual YM3812 -- */
void OPLDestroy(FM_OPL *OPL)
{
#ifdef OPL_OUTPUT_LOG
--
2.24.1
[PULL 07/33] virtfs-proxy-helper: Make the helper_opts[] array const
From: Philippe Mathieu-Daudé
Reduce a bit the memory footprint by making the helper_opts[]
array const.
Signed-off-by: Philippe Mathieu-Daudé
Acked-by: Greg Kurz
Acked-by: Paolo Bonzini
Reviewed-by: Stefano Garzarella
Message-Id: <20200305010446.17029-4-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
fsdev/virtfs-proxy-helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index aa1ab2590d42..de061a8a0eaa 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -43,7 +43,7 @@
#define BTRFS_SUPER_MAGIC 0x9123683E
#endif
-static struct option helper_opts[] = {
+static const struct option helper_opts[] = {
{"fd", required_argument, NULL, 'f'},
{"path", required_argument, NULL, 'p'},
{"nodaemon", no_argument, NULL, 'n'},
--
2.24.1
[PULL 19/33] hw/pci-host/q35: Include "qemu/range.h"
From: Philippe Mathieu-Daudé The MCHPCIState structure uses the Range type which is declared in "qemu/range.h". Include it. This fixes (when modifying unrelated headers): In file included from hw/pci-host/q35.c:32: include/hw/pci-host/q35.h:57:11: error: field has incomplete type 'Range' (aka 'struct Range') Range pci_hole; ^ include/qemu/typedefs.h:116:16: note: forward declaration of 'struct Range' typedef struct Range Range; ^ Acked-by: John Snow Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200228114649.12818-13-phi...@redhat.com> Signed-off-by: Laurent Vivier --- include/hw/pci-host/q35.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h index 976fbae5996b..47086c645e9f 100644 --- a/include/hw/pci-host/q35.h +++ b/include/hw/pci-host/q35.h @@ -33,6 +33,7 @@ #include "hw/pci-host/pam.h" #include "hw/i386/intel_iommu.h" #include "qemu/units.h" +#include "qemu/range.h" #define TYPE_Q35_HOST_DEVICE "q35-pcihost" #define Q35_HOST_DEVICE(obj) \ -- 2.24.1
[PULL 00/33] Trivial branch patches
The following changes since commit 7a5853cec479a448edae0fb2aaf4e2f78c9c774d: Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2020-03-09 10:32:53 +) are available in the Git repository at: git://github.com/vivier/qemu.git tags/trivial-branch-pull-request for you to fetch changes up to 916c92503bd5348a33e561db600d8894bde636bb: monitor/hmp-cmds: Remove redundant statement in hmp_rocker_of_dpa_groups() (2020-03-09 15:59:31 +0100) - includes cleanup - reduce .data footprint - fix warnings reported by Clang static code analyzer - fix dp8393x part lost in merge - update git.orderfile and rules.mak Chen Qun (7): block/stream: Remove redundant statement in stream_run() block/file-posix: Remove redundant statement in raw_handle_perm_lock() dma/xlnx-zdma: Remove redundant statement in zdma_write_dst() scsi/scsi-disk: Remove redundant statement in scsi_disk_emulate_command() display/pxa2xx_lcd: Remove redundant statement in pxa2xx_palette_parse() display/exynos4210_fimd: Remove redundant statement in exynos4210_fimd_update() monitor/hmp-cmds: Remove redundant statement in hmp_rocker_of_dpa_groups() Eric Blake (1): maint: Include top-level *.rst files early in git diff Finn Thain (1): dp8393x: Mask EOL bit from descriptor addresses, take 2 Pan Nengyuan (1): core/qdev: fix memleak in qdev_get_gpio_out_connector() Philippe Mathieu-Daudé (23): build-sys: Move the print-variable rule to rules.mak hw/audio/fmopl: Fix a typo twice hw/net/e1000: Add readops/writeops typedefs hw/net/e1000: Move macreg[] arrays to .rodata to save 1MiB of .data virtfs-proxy-helper: Make the helper_opts[] array const vl: Add missing "hw/boards.h" include hw/southbridge/ich9: Removed unused headers hw/i386/ioapic_internal: Remove unused "hw/i386/ioapic.h" header hw/timer: Remove unused "ui/console.h" header hw/usb/dev-storage: Remove unused "ui/console.h" header hw/i386/intel_iommu: Remove unused includes hw/alpha/alpha_sys: Remove unused "hw/ide.h" header hw/alpha/dp264: Include "net/net.h" hw/hppa/machine: Include "net/net.h" hw/acpi/cpu_hotplug: Include "hw/pci/pci.h" hw/timer/hpet: Include "exec/address-spaces.h" hw/pci-host/q35: Include "qemu/range.h" hw/i2c/smbus_ich9: Include "qemu/range.h" hw/pci-host/piix: Include "qemu/range.h" hw/acpi: Include "hw/mem/nvdimm.h" hw/i386: Include "hw/mem/nvdimm.h" hw/pci-host/q35: Remove unused includes hw/i386/pc: Clean up includes Makefile | 3 --- block/file-posix.c| 1 - block/stream.c| 3 +-- fsdev/virtfs-proxy-helper.c | 2 +- hw/acpi/cpu_hotplug.c | 1 + hw/acpi/ich9.c| 2 +- hw/acpi/piix4.c | 1 + hw/alpha/alpha_sys.h | 1 - hw/alpha/dp264.c | 1 + hw/audio/fmopl.c | 4 ++-- hw/core/qdev.c| 2 +- hw/display/exynos4210_fimd.c | 1 - hw/display/pxa2xx_lcd.c | 1 - hw/dma/xlnx-zdma.c| 10 +- hw/hppa/machine.c | 1 + hw/i2c/smbus_ich9.c | 1 + hw/i386/acpi-build.c | 1 + hw/i386/pc.c | 1 + hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c | 1 + hw/isa/lpc_ich9.c | 1 - hw/net/dp8393x.c | 4 ++-- hw/net/e1000.c| 6 -- hw/net/e1000e_core.c | 6 -- hw/pci-host/i440fx.c | 1 + hw/pci-host/q35.c | 1 + hw/rtc/twl92230.c | 1 - hw/scsi/scsi-disk.c | 1 - hw/timer/hpet.c | 2 +- hw/usb/dev-storage.c | 1 - include/hw/i386/ich9.h| 1 - include/hw/i386/intel_iommu.h | 4 include/hw/i386/ioapic_internal.h | 1 - include/hw/i386/pc.h | 11 +++ include/hw/pci-host/q35.h | 8 +--- monitor/hmp-cmds.c| 5 + rules.mak | 3 +++ scripts/git.orderfile | 1 + softmmu/vl.c | 1 + 39 files changed, 43 insertions(+), 55 deletions(-) -- 2.24.1
[PULL 01/33] dp8393x: Mask EOL bit from descriptor addresses, take 2
From: Finn Thain
A portion of a recent patch got lost due to a merge snafu. That patch is
now commit 88f632fbb1 ("dp8393x: Mask EOL bit from descriptor addresses").
This patch restores the portion that got lost.
Signed-off-by: Finn Thain
Reviewed-by: Laurent Vivier
Reviewed-by: Philippe Mathieu-Daudé
Message-Id:
Signed-off-by: Laurent Vivier
---
hw/net/dp8393x.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 8a3504d9628e..81fc13ee9fa7 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -525,8 +525,8 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
* (4 + 3 * s->regs[SONIC_TFC]),
MEMTXATTRS_UNSPECIFIED, s->data,
size);
-s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0) & ~0x1;
-if (dp8393x_get(s, width, 0) & SONIC_DESC_EOL) {
+s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0);
+if (s->regs[SONIC_CTDA] & SONIC_DESC_EOL) {
/* EOL detected */
break;
}
--
2.24.1
[PULL 08/33] vl: Add missing "hw/boards.h" include
From: Philippe Mathieu-Daudé
vl.c calls machine_usb() declared in "hw/boards.h". Include it.
This fixes (when modifying unrelated headers):
vl.c:1283:10: error: implicit declaration of function 'machine_usb' is
invalid in C99 [-Werror,-Wimplicit-function-declaration]
if (!machine_usb(current_machine)) {
^
vl.c:1283:10: error: this function declaration is not a prototype
[-Werror,-Wstrict-prototypes]
vl.c:1283:22: error: use of undeclared identifier 'current_machine'
if (!machine_usb(current_machine)) {
^
Acked-by: John Snow
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-Id: <20200228114649.12818-2-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
softmmu/vl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 5549f4b61986..ff2685dff845 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -25,6 +25,7 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
#include "qemu/units.h"
+#include "hw/boards.h"
#include "hw/qdev-properties.h"
#include "qapi/error.h"
#include "qemu-version.h"
--
2.24.1
[PULL 05/33] hw/net/e1000: Add readops/writeops typedefs
From: Philippe Mathieu-Daudé
Express the macreg[] arrays using typedefs.
No logical changes introduced here.
Signed-off-by: Philippe Mathieu-Daudé
Acked-by: Paolo Bonzini
Reviewed-by: Dmitry Fleytman
Reviewed-by: Stefano Garzarella
Message-Id: <20200305010446.17029-2-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/net/e1000.c | 6 --
hw/net/e1000e_core.c | 6 --
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 0b833d5a152e..972d9b508399 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1150,7 +1150,8 @@ set_ims(E1000State *s, int index, uint32_t val)
}
#define getreg(x)[x] = mac_readreg
-static uint32_t (*macreg_readops[])(E1000State *, int) = {
+typedef uint32_t (*readops)(E1000State *, int);
+static readops macreg_readops[] = {
getreg(PBA), getreg(RCTL), getreg(TDH), getreg(TXDCTL),
getreg(WUFC), getreg(TDT), getreg(CTRL), getreg(LEDCTL),
getreg(MANC), getreg(MDIC), getreg(SWSM), getreg(STATUS),
@@ -1205,7 +1206,8 @@ static uint32_t (*macreg_readops[])(E1000State *, int) = {
enum { NREADOPS = ARRAY_SIZE(macreg_readops) };
#define putreg(x)[x] = mac_writereg
-static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
+typedef void (*writeops)(E1000State *, int, uint32_t);
+static writeops macreg_writeops[] = {
putreg(PBA), putreg(EERD), putreg(SWSM), putreg(WUFC),
putreg(TDBAL),putreg(TDBAH),putreg(TXDCTL), putreg(RDBAH),
putreg(RDBAL),putreg(LEDCTL), putreg(VET), putreg(FCRUC),
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 94ea34dca56d..38bdb90114c6 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -2855,7 +2855,8 @@ e1000e_set_gcr(E1000ECore *core, int index, uint32_t val)
}
#define e1000e_getreg(x)[x] = e1000e_mac_readreg
-static uint32_t (*e1000e_macreg_readops[])(E1000ECore *, int) = {
+typedef uint32_t (*readops)(E1000ECore *, int);
+static readops e1000e_macreg_readops[] = {
e1000e_getreg(PBA),
e1000e_getreg(WUFC),
e1000e_getreg(MANC),
@@ -3061,7 +3062,8 @@ static uint32_t (*e1000e_macreg_readops[])(E1000ECore *,
int) = {
enum { E1000E_NREADOPS = ARRAY_SIZE(e1000e_macreg_readops) };
#define e1000e_putreg(x)[x] = e1000e_mac_writereg
-static void (*e1000e_macreg_writeops[])(E1000ECore *, int, uint32_t) = {
+typedef void (*writeops)(E1000ECore *, int, uint32_t);
+static writeops e1000e_macreg_writeops[] = {
e1000e_putreg(PBA),
e1000e_putreg(SWSM),
e1000e_putreg(WUFC),
--
2.24.1
[PULL 03/33] build-sys: Move the print-variable rule to rules.mak
From: Philippe Mathieu-Daudé Currently the print-variable rule can only be used in the root directory: $ make print-vhost-user-json-y vhost-user-json-y= contrib/vhost-user-gpu/50-qemu-gpu.json tools/virtiofsd/50-qemu-virtiofsd.json $ make -C i386-softmmu print-obj-y make: Entering directory 'build/i386-softmmu' make: *** No rule to make target 'print-obj-y'. Stop. make: Leaving directory 'build/i386-softmmu' Move it to rules.mak so we can use it from other directories: $ make -C i386-softmmu print-obj-y make: Entering directory 'build/i386-softmmu' obj-y=qapi-introspect.o qapi-types-machine-target.o qapi-types-misc-target.o qapi-types.o qapi-visit-machine-target.o qapi-visit-misc-target.o qapi-visit.o qapi-events-machine-target.o qapi-events-misc-target.o qapi-events.o qapi-commands-machine-target.o qapi-commands-misc-target.o qapi-commands.o qapi-init-commands.o make: Leaving directory 'build/i386-softmmu' Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Marc-André Lureau Message-Id: <20200306170456.21977-1-phi...@redhat.com> Signed-off-by: Laurent Vivier --- Makefile | 3 --- rules.mak | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 2e930688942b..37aed4a24422 100644 --- a/Makefile +++ b/Makefile @@ -15,9 +15,6 @@ UNCHECKED_GOALS := %clean TAGS cscope ctags dist \ help check-help print-% \ docker docker-% vm-help vm-test vm-build-% -print-%: - @echo '$*=$($*)' - # All following code might depend on configuration variables ifneq ($(wildcard config-host.mak),) # Put the all: rule here so that config-host.mak can contain dependencies. diff --git a/rules.mak b/rules.mak index e39b073d4648..694865b63ee8 100644 --- a/rules.mak +++ b/rules.mak @@ -435,3 +435,6 @@ sentinel = .$(subst $(SPACE),_,$(subst /,_,$1)).sentinel. atomic = $(eval $1: $(call sentinel,$1) ; @:) \ $(call sentinel,$1) : $2 ; @touch $$@ \ $(foreach t,$1,$(if $(wildcard $t),,$(shell rm -f $(call sentinel,$1 + +print-%: + @echo '$*=$($*)' -- 2.24.1
[PULL 11/33] hw/timer: Remove unused "ui/console.h" header
From: Philippe Mathieu-Daudé The timer models don't need anything from "ui/console.h". Remove it. Acked-by: John Snow Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20200228114649.12818-5-phi...@redhat.com> Signed-off-by: Laurent Vivier --- hw/rtc/twl92230.c | 1 - hw/timer/hpet.c | 1 - 2 files changed, 2 deletions(-) diff --git a/hw/rtc/twl92230.c b/hw/rtc/twl92230.c index 63bd13d2caa4..d0011be89eef 100644 --- a/hw/rtc/twl92230.c +++ b/hw/rtc/twl92230.c @@ -27,7 +27,6 @@ #include "migration/qemu-file-types.h" #include "migration/vmstate.h" #include "sysemu/sysemu.h" -#include "ui/console.h" #include "qemu/bcd.h" #include "qemu/module.h" diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c index 8dbcbdca16a6..4f30dd50a40a 100644 --- a/hw/timer/hpet.c +++ b/hw/timer/hpet.c @@ -27,7 +27,6 @@ #include "qemu/osdep.h" #include "hw/i386/pc.h" #include "hw/irq.h" -#include "ui/console.h" #include "qapi/error.h" #include "qemu/error-report.h" #include "qemu/timer.h" -- 2.24.1
[PULL 06/33] hw/net/e1000: Move macreg[] arrays to .rodata to save 1MiB of .data
From: Philippe Mathieu-Daudé
Each array consumes 256KiB of .data. As we do not reassign entries,
we can move it to the .rodata section, and save a total of 1MiB of
.data (size reported on x86_64 host).
Signed-off-by: Philippe Mathieu-Daudé
Acked-by: Paolo Bonzini
Reviewed-by: Dmitry Fleytman
Reviewed-by: Stefano Garzarella
Message-Id: <20200305010446.17029-3-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/net/e1000.c | 4 ++--
hw/net/e1000e_core.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 972d9b508399..9233248c9af0 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1151,7 +1151,7 @@ set_ims(E1000State *s, int index, uint32_t val)
#define getreg(x)[x] = mac_readreg
typedef uint32_t (*readops)(E1000State *, int);
-static readops macreg_readops[] = {
+static const readops macreg_readops[] = {
getreg(PBA), getreg(RCTL), getreg(TDH), getreg(TXDCTL),
getreg(WUFC), getreg(TDT), getreg(CTRL), getreg(LEDCTL),
getreg(MANC), getreg(MDIC), getreg(SWSM), getreg(STATUS),
@@ -1207,7 +1207,7 @@ enum { NREADOPS = ARRAY_SIZE(macreg_readops) };
#define putreg(x)[x] = mac_writereg
typedef void (*writeops)(E1000State *, int, uint32_t);
-static writeops macreg_writeops[] = {
+static const writeops macreg_writeops[] = {
putreg(PBA), putreg(EERD), putreg(SWSM), putreg(WUFC),
putreg(TDBAL),putreg(TDBAH),putreg(TXDCTL), putreg(RDBAH),
putreg(RDBAL),putreg(LEDCTL), putreg(VET), putreg(FCRUC),
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 38bdb90114c6..df957e0c1a09 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -2856,7 +2856,7 @@ e1000e_set_gcr(E1000ECore *core, int index, uint32_t val)
#define e1000e_getreg(x)[x] = e1000e_mac_readreg
typedef uint32_t (*readops)(E1000ECore *, int);
-static readops e1000e_macreg_readops[] = {
+static const readops e1000e_macreg_readops[] = {
e1000e_getreg(PBA),
e1000e_getreg(WUFC),
e1000e_getreg(MANC),
@@ -3063,7 +3063,7 @@ enum { E1000E_NREADOPS =
ARRAY_SIZE(e1000e_macreg_readops) };
#define e1000e_putreg(x)[x] = e1000e_mac_writereg
typedef void (*writeops)(E1000ECore *, int, uint32_t);
-static writeops e1000e_macreg_writeops[] = {
+static const writeops e1000e_macreg_writeops[] = {
e1000e_putreg(PBA),
e1000e_putreg(SWSM),
e1000e_putreg(WUFC),
--
2.24.1
Re: [PATCH 00/20] hw: Clean up hw/i386 headers (and few alpha/hppa)
Le 27/02/2020 à 14:28, Paolo Bonzini a écrit : > On 26/10/19 15:32, Laurent Vivier wrote: >> Le 26/10/2019 à 14:20, Philippe Mathieu-Daudé a écrit : >>> Hi, >>> >>> On 10/14/19 4:22 PM, Philippe Mathieu-Daudé wrote: This is a follow-up of Markus's cleanup series: Tame a few "touch this, recompile the world" https://www.mail-archive.com/qemu-devel@nongnu.org/msg635748.html This part is mostly restricted to X86, but since some file from the Alpha/PA-RISC machines include "hw/i386/pc.h" I had to fix them too. Eventually I'll succeed at removing hw/i386/ dependency on non-X86 platforms (Quest I started 2 years ago...). Regards, Phil. Philippe Mathieu-Daudé (20): vl: Add missing "hw/boards.h" include hw/southbridge/ich9: Removed unused headers hw/input/pckbd: Remove unused "hw/i386/pc.h" header hw/i386/ioapic_internal: Remove unused "hw/i386/ioapic.h" header hw/timer: Remove unused "ui/console.h" header hw/usb/dev-storage: Remove unused "ui/console.h" header hw/i386/intel_iommu: Remove unused includes hw/xen/xen_pt_load_rom: Remove unused includes hw/alpha/alpha_sys: Remove unused "hw/ide.h" header hw/alpha/dp264: Include "net/net.h" hw/hppa/machine: Include "net/net.h" hw/acpi/cpu_hotplug: Include "hw/pci/pci.h" hw/timer/hpet: Include "exec/address-spaces.h" hw/pci-host/q35: Include "qemu/range.h" hw/i2c/smbus_ich9: Include "qemu/range.h" hw/pci-host/piix: Include "qemu/range.h" hw/acpi: Include "hw/mem/nvdimm.h" hw/i386: Include "hw/mem/nvdimm.h" hw/pci-host/q35: Remove unused includes hw/i386/pc: Clean up includes >>> Laurent, since this series is fully reviewed, can it go via >>> your qemu-trivial tree? >> >> I'll try but I'm not sure to have the time to do that before the softfreeze. > > Ping :) Applied v2 to my trivial-patches branch. Thanks, Laurent
Re: [PATCH] tests/qemu-iotests: Fix socket_scm_helper build path
Le 06/03/2020 à 17:57, Philippe Mathieu-Daudé a écrit :
> The socket_scm_helper path got corrupted during the mechanical
> refactor moving the qtests files into their own sub-directory.
>
> Fixes: 1e8a1fae7 ("test: Move qtests to a separate directory")
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> tests/Makefile.include | 1 +
> tests/qtest/Makefile.include | 1 -
> 2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index edcbd475aa..67e8fcddda 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -589,6 +589,7 @@ include $(SRC_PATH)/tests/qtest/Makefile.include
> tests/test-qga$(EXESUF): qemu-ga$(EXESUF)
> tests/test-qga$(EXESUF): tests/test-qga.o $(qtest-obj-y)
> tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o
> $(test-util-obj-y) libvhost-user.a
> +tests/qemu-iotests/socket_scm_helper$(EXESUF):
> tests/qemu-iotests/socket_scm_helper.o
>
> SPEED = quick
>
> diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
> index 383b0ab217..76672990a7 100644
> --- a/tests/qtest/Makefile.include
> +++ b/tests/qtest/Makefile.include
> @@ -287,7 +287,6 @@ tests/qtest/usb-hcd-ehci-test$(EXESUF):
> tests/qtest/usb-hcd-ehci-test.o $(libqos
> tests/qtest/usb-hcd-xhci-test$(EXESUF): tests/qtest/usb-hcd-xhci-test.o
> $(libqos-usb-obj-y)
> tests/qtest/cpu-plug-test$(EXESUF): tests/qtest/cpu-plug-test.o
> tests/qtest/migration-test$(EXESUF): tests/qtest/migration-test.o
> tests/qtest/migration-helpers.o
> -tests/qtest/qemu-iotests/qtest/socket_scm_helper$(EXESUF):
> tests/qtest/qemu-iotests/qtest/socket_scm_helper.o
> tests/qtest/test-netfilter$(EXESUF): tests/qtest/test-netfilter.o
> $(qtest-obj-y)
> tests/qtest/test-filter-mirror$(EXESUF): tests/qtest/test-filter-mirror.o
> $(qtest-obj-y)
> tests/qtest/test-filter-redirector$(EXESUF):
> tests/qtest/test-filter-redirector.o $(qtest-obj-y)
>
Reviewed-by: Laurent Vivier
Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp
Suggest
scripts: Coccinelle script to use auto-propagated errp
or
scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE()
Vladimir Sementsov-Ogievskiy writes:
> Script adds ERRP_AUTO_PROPAGATE macro invocation where appropriate and
> does corresponding changes in code (look for details in
> include/qapi/error.h)
>
> Usage example:
> spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
> --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
> blockdev-nbd.c qemu-nbd.c {block/nbd*,nbd/*,include/block/nbd*}.[hc]
Suggest FILES... instead of a specific set of files.
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
>
> Cc: Eric Blake
> Cc: Kevin Wolf
> Cc: Max Reitz
> Cc: Greg Kurz
> Cc: Christian Schoenebeck
> Cc: Stefano Stabellini
> Cc: Anthony Perard
> Cc: Paul Durrant
> Cc: Stefan Hajnoczi
> Cc: "Philippe Mathieu-Daudé"
> Cc: Laszlo Ersek
> Cc: Gerd Hoffmann
> Cc: Stefan Berger
> Cc: Markus Armbruster
> Cc: Michael Roth
> Cc: qemu-block@nongnu.org
> Cc: qemu-de...@nongnu.org
> Cc: xen-de...@lists.xenproject.org
>
> include/qapi/error.h | 3 +
> scripts/coccinelle/auto-propagated-errp.cocci | 231 ++
> 2 files changed, 234 insertions(+)
> create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci
>
> diff --git a/include/qapi/error.h b/include/qapi/error.h
> index bb9bcf02fb..fbfc6f1c0b 100644
> --- a/include/qapi/error.h
> +++ b/include/qapi/error.h
> @@ -211,6 +211,9 @@
> * }
> * ...
> * }
> + *
> + * For mass conversion use script
mass-conversion (we're not converting mass, we're converting en masse)
> + * scripts/coccinelle/auto-propagated-errp.cocci
> */
>
> #ifndef ERROR_H
> diff --git a/scripts/coccinelle/auto-propagated-errp.cocci
> b/scripts/coccinelle/auto-propagated-errp.cocci
> new file mode 100644
> index 00..bff274bd6d
> --- /dev/null
> +++ b/scripts/coccinelle/auto-propagated-errp.cocci
Preface to my review of this script: may aim isn't to make it
bullet-proof. I want to (1) make it good enough (explained in a
jiffie), and (2) automatically identify the spots where it still isn't
obviously safe for manual review.
The latter may involve additional scripting. That's okay.
The script is good enough when the number of possibly unsafe spots is
low enough for careful manual review.
When I ask for improvements that, in your opinion, go beyond "good
enough", please push back. I'm sure we can work it out together.
> @@ -0,0 +1,231 @@
> +// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
> +//
> +// Copyright (c) 2020 Virtuozzo International GmbH.
> +//
> +// This program is free software; you can redistribute it and/or modify
> +// it under the terms of the GNU General Public License as published by
> +// the Free Software Foundation; either version 2 of the License, or
> +// (at your option) any later version.
> +//
> +// This program is distributed in the hope that 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
