Re: [PATCH 4/7] block: Drop detached child from ignore list
On 10.11.21 14:38, Vladimir Sementsov-Ogievskiy wrote:
04.11.2021 13:38, Hanna Reitz wrote:
bdrv_attach_child_common_abort() restores the parent's AioContext. To
do so, the child (which was supposed to be attached, but is now detached
again by this abort handler) is added to the ignore list for the
AioContext changing functions.
However, since we modify a BDS's children list in the BdrvChildClass's
.attach and .detach handlers, the child is already effectively detached
from the parent by this point. We do not need to put it into the ignore
list.
Use this opportunity to clean up the empty line structure: Keep setting
the ignore list, invoking the AioContext function, and freeing the
ignore list in blocks separated by empty lines.
Signed-off-by: Hanna Reitz
Reviewed-by: Vladimir Sementsov-Ogievskiy
What comes in my mind, is that now bdrv_replace_child_noperm() is more
strong in detaching.. But may be not enough strong: we still have link
from the child to parent (child->opaque).. Maybe more correct for
BdrvChild object to have no relation with parent after detaching. But
than we'll need some GenericParent object (as child->class is mostly
about parent, not about child and even not about the edge).. Now
GenericParent is "included" into BdrvChild, which represents both
GenericParent and Edge objects..
Yes, I thought about this in exactly this function here
(bdrv_attach_child_common_abort()). I was wondering whether I could
save a pointer to the BdrvChildClass, then just let
bdrv_replace_child_noperm() free the child, and then invoke the
BdrvChildClass methods when the child is already gone. As you say, it’s
really just about the parent, and child->opaque is effectively just a
pointer to the parent object, so all of the methods that only use
child->opaque and nothing else from the BdrvChild object can be invoked
even after the child is gone.
But doing something about that (like changing some methods like the
AioContext methods to only take the opaque pointer) was too invasive for
me for this series, so in case of this function I decided to just
begrudgingly keep the BdrvChild object around, including its
back-connection to the parent (via child->opaque), and free it at the
end of the function.
Besides that back-connection, there’s also to consider that some block
drivers can keep pointers to special children in their BDS
“subclasses”. For example, qcow2 keeps s->data_file. That’s a problem
just like bs->backing or bs->file, except that it’s much more unlikely
to cause problems in practice, and that it would probably be even more
invasive to fix...
Hanna
---
block.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index b95f8dcf8f..6d230ad3d1 100644
--- a/block.c
+++ b/block.c
@@ -2774,14 +2774,16 @@ static void
bdrv_attach_child_common_abort(void *opaque)
}
if (bdrv_child_get_parent_aio_context(child) !=
s->old_parent_ctx) {
- GSList *ignore = g_slist_prepend(NULL, child);
+ GSList *ignore;
+ /* No need to ignore `child`, because it has been detached
already */
+ ignore = NULL;
child->klass->can_set_aio_ctx(child, s->old_parent_ctx,
,
_abort);
g_slist_free(ignore);
- ignore = g_slist_prepend(NULL, child);
- child->klass->set_aio_ctx(child, s->old_parent_ctx, );
+ ignore = NULL;
+ child->klass->set_aio_ctx(child, s->old_parent_ctx, );
g_slist_free(ignore);
}
Re: [PATCH 1/3] qdict: make available dump_qobject(), dump_qdict(), dump_qlist()
On 10/11/2021 17:17, Markus Armbruster wrote: Laurent Vivier writes: move them from block/qapi.c to qobject/qdict.c, qobject/qlist.c, qobject/qobject.c This is useful to debug qobjects Signed-off-by: Laurent Vivier I think qobject_to_json_pretty() is better suited to debugging, because it preserves differences like the one between the string "1" and the number 1. Yes, you're right. I didn't think about this solution. So we can drop this patch. Thanks, Laurent
Re: [PATCH 05/15] hw/nvme: Add support for SR-IOV
On Nov 10 14:42, Lukasz Maniak wrote:
> On Mon, Nov 08, 2021 at 08:56:43AM +0100, Klaus Jensen wrote:
> > On Nov 4 15:30, Lukasz Maniak wrote:
> > > On Tue, Nov 02, 2021 at 06:33:31PM +0100, Lukasz Maniak wrote:
> > > > On Tue, Nov 02, 2021 at 03:33:15PM +0100, Klaus Jensen wrote:
> > > > > On Oct 7 18:23, Lukasz Maniak wrote:
> > > > > > This patch implements initial support for Single Root I/O
> > > > > > Virtualization
> > > > > > on an NVMe device.
> > > > > >
> > > > > > Essentially, it allows to define the maximum number of virtual
> > > > > > functions
> > > > > > supported by the NVMe controller via sriov_max_vfs parameter.
> > > > > >
> > > > > > Passing a non-zero value to sriov_max_vfs triggers reporting of
> > > > > > SR-IOV
> > > > > > capability by a physical controller and ARI capability by both the
> > > > > > physical and virtual function devices.
> > > > > >
> > > > > > NVMe controllers created via virtual functions mirror functionally
> > > > > > the physical controller, which may not entirely be the case, thus
> > > > > > consideration would be needed on the way to limit the capabilities
> > > > > > of
> > > > > > the VF.
> > > > > >
> > > > > > NVMe subsystem is required for the use of SR-IOV.
> > > > > >
> > > > > > Signed-off-by: Lukasz Maniak
> > > > > > ---
> > > > > > hw/nvme/ctrl.c | 74
> > > > > > ++--
> > > > > > hw/nvme/nvme.h | 1 +
> > > > > > include/hw/pci/pci_ids.h | 1 +
> > > > > > 3 files changed, 73 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> > > > > > index 6a571d18cf..ad79ff0c00 100644
> > > > > > --- a/hw/nvme/ctrl.c
> > > > > > +++ b/hw/nvme/ctrl.c
> > > > > > @@ -6361,8 +6406,12 @@ static int nvme_init_pci(NvmeCtrl *n,
> > > > > > PCIDevice *pci_dev, Error **errp)
> > > > > >n->reg_size);
> > > > > > memory_region_add_subregion(>bar0, 0, >iomem);
> > > > > >
> > > > > > -pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
> > > > > > - PCI_BASE_ADDRESS_MEM_TYPE_64, >bar0);
> > > > > > +if (pci_is_vf(pci_dev)) {
> > > > > > +pcie_sriov_vf_register_bar(pci_dev, 0, >bar0);
> > > > > > +} else {
> > > > > > +pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY
> > > > > > |
> > > > > > + PCI_BASE_ADDRESS_MEM_TYPE_64, >bar0);
> > > > > > +}
> > > > >
> > > > > I assume that the assert we are seeing means that the
> > > > > pci_register_bars
> > > > > in nvme_init_cmb and nvme_init_pmr must be changed similarly to this.
> > > >
> > > > Assert will only arise for CMB as VF params are initialized with PF
> > > > params.
> > > >
> > > > @@ -6532,6 +6585,15 @@ static void nvme_realize(PCIDevice *pci_dev,
> > > > Error **errp)
> > > > NvmeCtrl *n = NVME(pci_dev);
> > > > NvmeNamespace *ns;
> > > > Error *local_err = NULL;
> > > > +NvmeCtrl *pn = NVME(pcie_sriov_get_pf(pci_dev));
> > > > +
> > > > +if (pci_is_vf(pci_dev)) {
> > > > +/* VFs derive settings from the parent. PF's lifespan exceeds
> > > > + * that of VF's, so it's safe to share params.serial.
> > > > + */
> > > > +memcpy(>params, >params, sizeof(NvmeParams));
> > > > +n->subsys = pn->subsys;
> > > > +}
> > > >
> > > > nvme_check_constraints(n, _err);
> > > > if (local_err) {
> > > >
> > > > The following simple fix will both fix assert and also allow
> > > > each VF to have its own CMB of the size defined for PF.
> > > >
> > > > ---
> > > > hw/nvme/ctrl.c | 13 +
> > > > 1 file changed, 9 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> > > > index 19b32dd4da..99daa6290c 100644
> > > > --- a/hw/nvme/ctrl.c
> > > > +++ b/hw/nvme/ctrl.c
> > > > @@ -6837,10 +6837,15 @@ static void nvme_init_cmb(NvmeCtrl *n,
> > > > PCIDevice *pci_dev)
> > > > n->cmb.buf = g_malloc0(cmb_size);
> > > > memory_region_init_io(>cmb.mem, OBJECT(n), _cmb_ops, n,
> > > >"nvme-cmb", cmb_size);
> > > > -pci_register_bar(pci_dev, NVME_CMB_BIR,
> > > > - PCI_BASE_ADDRESS_SPACE_MEMORY |
> > > > - PCI_BASE_ADDRESS_MEM_TYPE_64 |
> > > > - PCI_BASE_ADDRESS_MEM_PREFETCH, >cmb.mem);
> > > > +
> > > > +if (pci_is_vf(pci_dev)) {
> > > > +pcie_sriov_vf_register_bar(pci_dev, NVME_CMB_BIR, >cmb.mem);
> > > > +} else {
> > > > +pci_register_bar(pci_dev, NVME_CMB_BIR,
> > > > +PCI_BASE_ADDRESS_SPACE_MEMORY |
> > > > +PCI_BASE_ADDRESS_MEM_TYPE_64 |
> > > > +PCI_BASE_ADDRESS_MEM_PREFETCH, >cmb.mem);
> > > > +}
> > > >
> > > > NVME_CAP_SET_CMBS(cap, 1);
> > > > stq_le_p(>bar.cap, cap);
> > > >
> > > > As for PMR, it is currently only available on PF,
Re: [PATCH 1/3] qdict: make available dump_qobject(), dump_qdict(), dump_qlist()
Laurent Vivier writes: > move them from block/qapi.c to qobject/qdict.c, qobject/qlist.c, > qobject/qobject.c > > This is useful to debug qobjects > > Signed-off-by: Laurent Vivier I think qobject_to_json_pretty() is better suited to debugging, because it preserves differences like the one between the string "1" and the number 1.
Re: [PATCH v2 0/3] virtio: increase VIRTQUEUE_MAX_SIZE to 32k
On Mittwoch, 10. November 2021 16:14:19 CET Stefan Hajnoczi wrote: > On Wed, Nov 10, 2021 at 02:14:43PM +0100, Christian Schoenebeck wrote: > > On Mittwoch, 10. November 2021 11:05:50 CET Stefan Hajnoczi wrote: > > As you are apparently reluctant for changing the virtio specs, what about > > introducing those discussed virtio capabalities either as experimental > > ones > > without specs changes, or even just as 9p specific device capabilities for > > now. I mean those could be revoked on both sides at any time anyway. > > I would like to understand the root cause before making changes. > > "It's faster when I do X" is useful information but it doesn't > necessarily mean doing X is the solution. The "it's faster when I do X > because Y" part is missing in my mind. Once there is evidence that shows > Y then it will be clearer if X is a good solution, if there's a more > general solution, or if it was just a side-effect. I think I made it clear that the root cause of the observed performance gain with rising transmission size is latency (and also that performance is not the only reason for addressing this queue size issue). Each request roundtrip has a certain minimum latency, the virtio ring alone has its latency, plus latency of the controller portion of the file server (e.g. permissions, sandbox checks, file IDs) that is executed with *every* request, plus latency of dispatching the request handling between threads several times back and forth (also for each request). Therefore when you split large payloads (e.g. reading a large file) into smaller n amount of chunks, then that individual latency per request accumulates to n times the individual latency, eventually leading to degraded transmission speed as those requests are serialized. > I'm sorry for frustrating your efforts here. We have discussed a lot of > different ideas and maybe our perspectives are not that far apart > anymore. > > Keep going with what you think is best. If I am still skeptical we can > ask someone else to review the patches instead of me so you have a > second opinion. > > Stefan Thanks Stefan! In the meantime I try to address your objections as far as I can. If there is more I can do (with reasonable effort) to resolve your doubts, just let me know. Best regards, Christian Schoenebeck
Re: Poking around bdrv_is_inserted()
Am 09.11.2021 um 16:20 hat Markus Armbruster geschrieben:
> Kevin Wolf writes:
>
> > Am 09.11.2021 um 07:44 hat Markus Armbruster geschrieben:
> >> Screwed up qemu-de...@nongnu.org, sorry for the inconvenience.
> >>
> >> Markus Armbruster writes:
> >>
> >> > bdrv_is_inserted() returns false when:
> >> >
> >> > /**
> >> > * Return TRUE if the media is present
> >> > */
> >> > bool bdrv_is_inserted(BlockDriverState *bs)
> >> > {
> >> > BlockDriver *drv = bs->drv;
> >> > BdrvChild *child;
> >> >
> >> > if (!drv) {
> >> > return false;
> >> >
> >> > 1. @bs has no driver (this is how we represent "no medium").
> >
> > Not really any more. "No medium" is blk->root == NULL.
>
> Uh, blk_is_inserted() does *not* check blk->root:
>
> bool blk_is_inserted(BlockBackend *blk)
> {
> BlockDriverState *bs = blk_bs(blk);
>
> return bs && bdrv_is_inserted(bs);
> }
>
> Now I'm confused.
It does. blk_bs(blk) returns NULL for blk->root == NULL.
> >These days
> > bs->drv == NULL basically means "the backend is broken". This happens
> > after qcow2_signal_corruption(), and I'm not sure if we have more
> > circumstances like it.
>
> I'm not sure having bdrv_is_inserted() return true for "broken"
> backends makes sense.
I wonder if bdrv_is_inserted() makes sense at all (why not just do
whatever you were planning to do if it returns true, and handle the
error?).
But anyway, it returns false for broken backends.
Callers might commonly not be interested in "is a medium inserted?", but
more in "can I access the medium?". In this case, returning false
provides the right answer.
> >> > }
> >> > if (drv->bdrv_is_inserted) {
> >> > return drv->bdrv_is_inserted(bs);
> >> >
> >> > 2. Its driver's ->bdrv_is_inserted() returns false. This is how
> >> > passthrough block backends signal "host device has no medium". Right
> >> > now, the only user is "host_cdrom".
> >> >
> >> > }
> >> > QLIST_FOREACH(child, >children, next) {
> >> > if (!bdrv_is_inserted(child->bs)) {
> >> > return false;
> >> >
> >> > 3. Any of its children has no medium. Common use looking through
> >> > filters, which have a single child.
> >> >
> >> > }
> >> > }
> >> > return true;
> >> > }
> >> >
> >> > Makes sense.
> >> >
> >> > Now look at the uses of QERR_DEVICE_HAS_NO_MEDIUM.
> >> >
> >> > * external_snapshot_prepare() in blockdev.c:
> >> >
> >> > if (!bdrv_is_inserted(state->old_bs)) {
> >> > error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
> >> > goto out;
> >> > }
> >> >
> >> > where @device is the device name, i.e. BlockdevSnapshot member @node
> >> > or BlockdevSnapshotSync member @device. Uh-oh: the latter can be
> >> > null. If we can reach the error_setg() then, we crash on some
> >> > systems.
> >
> > Sounds like we should write a test case and then fix it.
> >
> >> > * bdrv_snapshot_delete() and bdrv_snapshot_load_tmp() in
> >> > block/snaphot.c:
> >> >
> >> > if (!drv) {
> >> > error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM,
> >> > bdrv_get_device_name(bs));
> >> > return -ENOMEDIUM;
> >> > }
> >> >
> >> > where @drv is bs->drv.
> >> >
> >> > Why do we check only for 1. here instead of calling
> >> > bdrv_is_inserted()?
> >
> > I guess we could philosophise about the theoretically right thing to do,
> > but last time I checked, host_cdrom didn't support snapshots, so it
> > probably doesn't matter either way.
>
> We could also philosophize about "any of its children has no medium".
> As far as I know, nothing stops me from using a host_cdrom as a backing
> file for a QCOW2, and that I *can* snapshot.
I'm surprised to learn that host_device actually implements
.bdrv_co_pwritev.
So yes, I suppose if you have a qcow2 formatted CD with a snapshot in
it, and you insert it into your physical drive and somehow convince the
kernel to let us open it read-write, and then you eject the CD while the
guest is running and try to delete the snapshot, then you might get the
wrong error message.
I think this is still deep in "then don't do that" territory, but if you
feel like slapping a bdrv_is_inserted() on it, feel free.
> Functions (and methods) bdrv_is_inserted(), bdrv_eject(), and
> bdrv_lock_medium() are related. block_int.h groups them under
> /* removable device specific */, and block.c under /* removable device
> support */. But only bdrv_is_inserted() recurses into children. Is
> this how it should be?
We don't actually have checks to prevent it, but I doubt you can build
anything meaningful with the combination of removable media and non-raw
drivers.
I know qcow2 will be horribly confused if you swap out the file under
its feet. If you must, you can change bs->file (even without host_cdrom,
blockdev-reopen should be enough).
Re: [PATCH 2/7] block: Manipulate children list in .attach/.detach
On 10.11.21 13:51, Vladimir Sementsov-Ogievskiy wrote: 04.11.2021 13:38, Hanna Reitz wrote: The children list is specific to BDS parents. We should not modify it in the general children modification code, but let BDS parents deal with it in their .attach() and .detach() methods. This also has the advantage that a BdrvChild is removed from the children list before its .bs pointer can become NULL. BDS parents generally assume that their children's .bs pointer is never NULL, so this is actually a bug fix. Signed-off-by: Hanna Reitz Interesting that nor child_root neither child_job do similar things in .attach / .detach ... Should we do something with it? Well, it’s up to them, I thought. :) A BB only has a single child, so it doesn’t need a list. Jobs do have their own child list (BlockJob.nodes). I thought a bit about this when writing this series, and I figured perhaps they don’t need to care about that in .attach() and .detach(), because they don’t really expect nodes to be detached or attached anyway; child_job.stay_at_node is true, after all. Hanna
Re: [PATCH v2 0/3] virtio: increase VIRTQUEUE_MAX_SIZE to 32k
On Wed, Nov 10, 2021 at 02:14:43PM +0100, Christian Schoenebeck wrote: > On Mittwoch, 10. November 2021 11:05:50 CET Stefan Hajnoczi wrote: > As you are apparently reluctant for changing the virtio specs, what about > introducing those discussed virtio capabalities either as experimental ones > without specs changes, or even just as 9p specific device capabilities for > now. I mean those could be revoked on both sides at any time anyway. I would like to understand the root cause before making changes. "It's faster when I do X" is useful information but it doesn't necessarily mean doing X is the solution. The "it's faster when I do X because Y" part is missing in my mind. Once there is evidence that shows Y then it will be clearer if X is a good solution, if there's a more general solution, or if it was just a side-effect. I'm sorry for frustrating your efforts here. We have discussed a lot of different ideas and maybe our perspectives are not that far apart anymore. Keep going with what you think is best. If I am still skeptical we can ask someone else to review the patches instead of me so you have a second opinion. Stefan signature.asc Description: PGP signature
Re: [PATCH 2/7] block: Manipulate children list in .attach/.detach
On 10.11.21 13:46, Vladimir Sementsov-Ogievskiy wrote:
04.11.2021 13:38, Hanna Reitz wrote:
The children list is specific to BDS parents. We should not modify it
in the general children modification code, but let BDS parents deal with
it in their .attach() and .detach() methods.
This also has the advantage that a BdrvChild is removed from the
children list before its .bs pointer can become NULL. BDS parents
generally assume that their children's .bs pointer is never NULL, so
this is actually a bug fix.
Signed-off-by: Hanna Reitz
---
block.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index 580cb77a70..243ae206b5 100644
--- a/block.c
+++ b/block.c
@@ -1387,6 +1387,8 @@ static void bdrv_child_cb_attach(BdrvChild *child)
{
BlockDriverState *bs = child->opaque;
+ QLIST_INSERT_HEAD(>children, child, next);
+
if (child->role & BDRV_CHILD_COW) {
bdrv_backing_attach(child);
}
@@ -1403,6 +1405,8 @@ static void bdrv_child_cb_detach(BdrvChild *child)
}
bdrv_unapply_subtree_drain(child, bs);
+
+ QLIST_REMOVE(child, next);
}
static int bdrv_child_cb_update_filename(BdrvChild *c,
BlockDriverState *base,
@@ -2747,7 +2751,7 @@ static void bdrv_child_free(void *opaque)
static void bdrv_remove_empty_child(BdrvChild *child)
{
assert(!child->bs);
- QLIST_SAFE_REMOVE(child, next);
+ assert(!child->next.le_prev); /* not in children list */
bdrv_child_free(child);
}
@@ -2913,7 +2917,6 @@ static int
bdrv_attach_child_noperm(BlockDriverState *parent_bs,
return ret;
}
- QLIST_INSERT_HEAD(_bs->children, *child, next);
The following comment become stale. We should remove it too. With
comment removed:
Ah, right, thanks!
Reviewed-by: Vladimir Sementsov-Ogievskiy
Re: qemu-img.c possibly overflowing shifts by BDRV_SECTOR_BITS
On Wed, 10 Nov 2021 at 11:36, Kevin Wolf wrote: > > Am 09.11.2021 um 20:07 hat Peter Maydell geschrieben: > > Hi; Coverity is complaining about some of the places in qemu-img.c > > where it takes a 32-bit variable and shifts it left by BDRV_SECTOR_BITS > > to convert a sector count to a byte count, because it's doing the > > shift in 32-bits rather than 64 and so Coverity thinks there might > > be overflow (CID 1465221, 1465219). Is it right and we need extra > > casts to force the shift to be done in 64 bits, or is there some > > constraint that means we know the sector counts are always small > > enough that the byte count is 2GB or less ? > > These are false positives. n is limited to BDRV_REQUEST_MAX_SECTORS > already when it starts out in convert_iteration_sectors() (which is > enough to make the calculation safe), but for the specific code path, I > think it's even guaranteed to be further limited to s->buf_sectors which > is 16 MB at most (MAX_BUF_SECTORS in qemu-img.c). Thanks. I've marked them as false-positives in the coverity UI. -- PMM
[PATCH 3/3] tests/qtest: add some tests for virtio-net failover
Add test cases to test several error cases that must be
generated by invalid failover configuration.
Add a combination of coldplug and hotplug test cases to be
sure the primary is correctly managed according the
presence or not of the STANDBY feature.
Signed-off-by: Laurent Vivier
---
tests/qtest/meson.build | 3 +
tests/qtest/virtio-net-failover.c | 567 ++
2 files changed, 570 insertions(+)
create mode 100644 tests/qtest/virtio-net-failover.c
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index c9d8458062ff..6d66bf522156 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -22,6 +22,9 @@ qtests_generic = \
(config_all_devices.has_key('CONFIG_VIRTIO_SCSI') ?
['fuzz-virtio-scsi-test'] : []) + \
(config_all_devices.has_key('CONFIG_SB16') ? ['fuzz-sb16-test'] : []) + \
(config_all_devices.has_key('CONFIG_SDHCI_PCI') ? ['fuzz-sdcard-test'] : [])
+ \
+ (config_all_devices.has_key('CONFIG_VIRTIO_NET') and \
+ config_all_devices.has_key('CONFIG_Q35') and \
+ config_all_devices.has_key('CONFIG_VIRTIO_PCI') ? ['virtio-net-failover'] :
[]) + \
[
'cdrom-test',
'device-introspect-test',
diff --git a/tests/qtest/virtio-net-failover.c
b/tests/qtest/virtio-net-failover.c
new file mode 100644
index ..c7b03b887179
--- /dev/null
+++ b/tests/qtest/virtio-net-failover.c
@@ -0,0 +1,567 @@
+#include "qemu/osdep.h"
+#include "libqos/libqtest.h"
+#include "libqos/pci.h"
+#include "libqos/pci-pc.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qlist.h"
+#include "qapi/qmp/qjson.h"
+#include "libqos/malloc-pc.h"
+#include "libqos/virtio-pci.h"
+#include "hw/pci/pci.h"
+
+#define BASE_MACHINE "-M q35 -nodefaults " \
+"-device pcie-root-port,id=root0,addr=0x1,bus=pcie.0,chassis=1 " \
+"-device pcie-root-port,id=root1,addr=0x2,bus=pcie.0,chassis=2 "
+
+#define MAC_PRIMARY "52:54:00:11:11:11"
+#define MAC_STANDBY "52:54:00:22:22:22"
+
+static QGuestAllocator guest_malloc;
+static QPCIBus *pcibus;
+
+static QTestState *machine_start(const char *args)
+{
+QTestState *qts;
+QPCIDevice *dev;
+
+qts = qtest_init(args);
+
+pc_alloc_init(_malloc, qts, 0);
+pcibus = qpci_new_pc(qts, _malloc);
+g_assert(qpci_secondary_buses_init(pcibus) == 2);
+
+dev = qpci_device_find(pcibus, QPCI_DEVFN(1, 0)); /* root0 */
+g_assert_nonnull(dev);
+
+qpci_device_enable(dev);
+qpci_iomap(dev, 4, NULL);
+
+g_free(dev);
+
+dev = qpci_device_find(pcibus, QPCI_DEVFN(2, 0)); /* root1 */
+g_assert_nonnull(dev);
+
+qpci_device_enable(dev);
+qpci_iomap(dev, 4, NULL);
+
+g_free(dev);
+
+return qts;
+}
+
+static void machine_stop(QTestState *qts)
+{
+qpci_free_pc(pcibus);
+alloc_destroy(_malloc);
+qtest_quit(qts);
+}
+
+static void test_error_id(void)
+{
+QTestState *qts;
+QDict *resp;
+QDict *err;
+
+qts = machine_start(BASE_MACHINE
+"-device
virtio-net,bus=root0,id=standby0,failover=on");
+
+resp = qtest_qmp(qts, "{'execute': 'device_add',"
+ "'arguments': {"
+ "'driver': 'virtio-net',"
+ "'bus': 'root1',"
+ "'failover_pair_id': 'standby0'"
+ "} }");
+g_assert(qdict_haskey(resp, "error"));
+
+err = qdict_get_qdict(resp, "error");
+g_assert(qdict_haskey(err, "desc"));
+
+g_assert_cmpstr(qdict_get_str(err, "desc"), ==,
+"Device with failover_pair_id needs to have id");
+
+qobject_unref(resp);
+
+machine_stop(qts);
+}
+
+static void test_error_pcie(void)
+{
+QTestState *qts;
+QDict *resp;
+QDict *err;
+
+qts = machine_start(BASE_MACHINE
+"-device
virtio-net,bus=root0,id=standby0,failover=on");
+
+resp = qtest_qmp(qts, "{'execute': 'device_add',"
+ "'arguments': {"
+ "'driver': 'virtio-net',"
+ "'id': 'primary0',"
+ "'bus': 'pcie.0',"
+ "'failover_pair_id': 'standby0'"
+ "} }");
+g_assert(qdict_haskey(resp, "error"));
+
+err = qdict_get_qdict(resp, "error");
+g_assert(qdict_haskey(err, "desc"));
+
+g_assert_cmpstr(qdict_get_str(err, "desc"), ==,
+"Bus 'pcie.0' does not support hotplugging");
+
+qobject_unref(resp);
+
+machine_stop(qts);
+}
+
+static QDict *find_device(QDict *bus, const char *name)
+{
+const QObject *obj;
+QList *devices;
+QList *list;
+
+devices = qdict_get_qlist(bus, "devices");
+if (devices == NULL) {
+return NULL;
+}
+
+list = qlist_copy(devices);
+while ((obj = qlist_pop(list))) {
+QDict *device;
+
+device = qobject_to(QDict, obj);
+
+if (qdict_haskey(device, "pci_bridge")) {
+QDict *bridge;
+QDict *bridge_device;
+
[PATCH 2/3] qtest/libqos: add a function to initialize secondary PCI buses
Scan the PCI devices to find bridge and set PCI_SECONDARY_BUS and
PCI_SUBORDINATE_BUS (algorithm from seabios)
Signed-off-by: Laurent Vivier
---
include/hw/pci/pci_bridge.h | 8 +++
tests/qtest/libqos/pci.c| 118
tests/qtest/libqos/pci.h| 1 +
3 files changed, 127 insertions(+)
diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
index a94d350034bf..30691a6e5728 100644
--- a/include/hw/pci/pci_bridge.h
+++ b/include/hw/pci/pci_bridge.h
@@ -138,6 +138,7 @@ typedef struct PCIBridgeQemuCap {
uint64_t mem_pref_64; /* Prefetchable memory to reserve (64-bit MMIO) */
} PCIBridgeQemuCap;
+#define REDHAT_PCI_CAP_TYPE_OFFSET 3
#define REDHAT_PCI_CAP_RESOURCE_RESERVE 1
/*
@@ -152,6 +153,13 @@ typedef struct PCIResReserve {
uint64_t mem_pref_64;
} PCIResReserve;
+#define REDHAT_PCI_CAP_RES_RESERVE_BUS_RES 4
+#define REDHAT_PCI_CAP_RES_RESERVE_IO 8
+#define REDHAT_PCI_CAP_RES_RESERVE_MEM 16
+#define REDHAT_PCI_CAP_RES_RESERVE_PREF_MEM_32 20
+#define REDHAT_PCI_CAP_RES_RESERVE_PREF_MEM_64 24
+#define REDHAT_PCI_CAP_RES_RESERVE_CAP_SIZE32
+
int pci_bridge_qemu_reserve_cap_init(PCIDevice *dev, int cap_offset,
PCIResReserve res_reserve, Error **errp);
diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
index e1e96189c821..3f0b18f4750b 100644
--- a/tests/qtest/libqos/pci.c
+++ b/tests/qtest/libqos/pci.c
@@ -13,6 +13,8 @@
#include "qemu/osdep.h"
#include "pci.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_bridge.h"
#include "hw/pci/pci_regs.h"
#include "qemu/host-utils.h"
#include "qgraph.h"
@@ -99,6 +101,122 @@ void qpci_device_init(QPCIDevice *dev, QPCIBus *bus,
QPCIAddress *addr)
g_assert(!addr->device_id || device_id == addr->device_id);
}
+static uint8_t qpci_find_resource_reserve_capability(QPCIDevice *dev)
+{
+uint16_t device_id;
+uint8_t cap = 0;
+
+if (qpci_config_readw(dev, PCI_VENDOR_ID) != PCI_VENDOR_ID_REDHAT) {
+return 0;
+}
+
+device_id = qpci_config_readw(dev, PCI_DEVICE_ID);
+
+if (device_id != PCI_DEVICE_ID_REDHAT_PCIE_RP &&
+device_id != PCI_DEVICE_ID_REDHAT_BRIDGE) {
+return 0;
+}
+
+do {
+cap = qpci_find_capability(dev, PCI_CAP_ID_VNDR, cap);
+} while (cap &&
+ qpci_config_readb(dev, cap + REDHAT_PCI_CAP_TYPE_OFFSET) !=
+ REDHAT_PCI_CAP_RESOURCE_RESERVE);
+if (cap) {
+uint8_t cap_len = qpci_config_readb(dev, cap + PCI_CAP_FLAGS);
+if (cap_len < REDHAT_PCI_CAP_RES_RESERVE_CAP_SIZE) {
+return 0;
+}
+}
+return cap;
+}
+
+static void qpci_secondary_buses_rec(QPCIBus *qbus, int bus, int *pci_bus)
+{
+QPCIDevice *dev;
+uint16_t class;
+uint8_t pribus, secbus, subbus;
+int i;
+
+for (i = 0; i < 32; i++) {
+dev = qpci_device_find(qbus, QPCI_DEVFN(bus + i, 0));
+if (dev == NULL) {
+continue;
+}
+class = qpci_config_readw(dev, PCI_CLASS_DEVICE);
+if (class == PCI_CLASS_BRIDGE_PCI) {
+qpci_config_writeb(dev, PCI_SECONDARY_BUS, 255);
+qpci_config_writeb(dev, PCI_SUBORDINATE_BUS, 0);
+}
+g_free(dev);
+}
+
+for (i = 0; i < 32; i++) {
+dev = qpci_device_find(qbus, QPCI_DEVFN(bus + i, 0));
+if (dev == NULL) {
+continue;
+}
+class = qpci_config_readw(dev, PCI_CLASS_DEVICE);
+if (class != PCI_CLASS_BRIDGE_PCI) {
+continue;
+}
+
+pribus = qpci_config_readb(dev, PCI_PRIMARY_BUS);
+if (pribus != bus) {
+qpci_config_writeb(dev, PCI_PRIMARY_BUS, bus);
+}
+
+secbus = qpci_config_readb(dev, PCI_SECONDARY_BUS);
+(*pci_bus)++;
+if (*pci_bus != secbus) {
+secbus = *pci_bus;
+qpci_config_writeb(dev, PCI_SECONDARY_BUS, secbus);
+}
+
+subbus = qpci_config_readb(dev, PCI_SUBORDINATE_BUS);
+qpci_config_writeb(dev, PCI_SUBORDINATE_BUS, 255);
+
+qpci_secondary_buses_rec(qbus, secbus << 5, pci_bus);
+
+if (subbus != *pci_bus) {
+uint8_t res_bus = *pci_bus;
+uint8_t cap = qpci_find_resource_reserve_capability(dev);
+
+if (cap) {
+uint32_t tmp_res_bus;
+
+tmp_res_bus = qpci_config_readl(dev, cap +
+
REDHAT_PCI_CAP_RES_RESERVE_BUS_RES);
+if (tmp_res_bus != (uint32_t)-1) {
+res_bus = tmp_res_bus & 0xFF;
+if ((uint8_t)(res_bus + secbus) < secbus ||
+(uint8_t)(res_bus + secbus) < res_bus) {
+res_bus = 0;
+}
+if (secbus + res_bus > *pci_bus) {
+res_bus = secbus + res_bus;
+}
+}
+}
+
Re: [PATCH v9 7/8] qmp: add QMP command x-query-virtio-queue-element
Jonah Palmer writes:
> From: Laurent Vivier
>
> This new command shows the information of a VirtQueue element.
>
> Signed-off-by: Jonah Palmer
[...]
> diff --git a/qapi/virtio.json b/qapi/virtio.json
> index 0f65044..c57fbc5 100644
> --- a/qapi/virtio.json
> +++ b/qapi/virtio.json
> @@ -1061,3 +1061,180 @@
> { 'command': 'x-query-virtio-vhost-queue-status',
>'data': { 'path': 'str', 'queue': 'uint16' },
>'returns': 'VirtVhostQueueStatus', 'features': [ 'unstable' ] }
> +
> +##
> +# @VirtioRingDescFlags:
> +#
> +# An enumeration of the virtio ring descriptor flags
> +#
> +# Since: 6.3
> +#
> +##
> +
> +{ 'enum': 'VirtioRingDescFlags',
> + 'data': [ 'next', 'write', 'indirect', 'avail', 'used' ]
> +}
> +
> +##
> +# @VirtioRingDesc:
> +#
> +# Information regarding the VRing descriptor area
> +#
> +# @addr: guest physical address of the descriptor data
> +#
> +# @len: length of the descriptor data
> +#
> +# @flags: list of descriptor flags
> +#
> +# Since: 6.3
> +#
> +##
> +
> +{ 'struct': 'VirtioRingDesc',
> + 'data': { 'addr': 'uint64',
> +'len': 'uint32',
> +'flags': [ 'VirtioRingDescFlags' ] } }
> +
> +##
> +# @VirtioRingAvail:
> +#
> +# Information regarding the avail VRing (also known as the driver
> +# area)
> +#
> +# @flags: VRingAvail flags
> +#
> +# @idx: VRingAvail index
> +#
> +# @ring: VRingAvail ring[] entry at provided index
> +#
> +# Since: 6.3
> +#
> +##
> +
> +{ 'struct': 'VirtioRingAvail',
> + 'data': { 'flags': 'uint16',
> +'idx': 'uint16',
> +'ring': 'uint16' } }
> +
> +##
> +# @VirtioRingUsed:
> +#
> +# Information regarding the used VRing (also known as the device
> +# area)
> +#
> +# @flags: VRingUsed flags
> +#
> +# @idx: VRingUsed index
> +#
> +# Since: 6.3
> +#
> +##
> +
> +{ 'struct': 'VirtioRingUsed',
> + 'data': { 'flags': 'uint16',
> +'idx': 'uint16' } }
> +
> +##
> +# @VirtioQueueElement:
> +#
> +# Information regarding a VirtQueue VirtQueueElement including
> +# descriptor, driver, and device areas
> +#
> +# @device-name: name of the VirtIODevice which this VirtQueue belongs
> +# to (for reference)
> +#
> +# @index: index of the element in the queue
> +#
> +# @ndescs: number of descriptors
> +#
> +# @descs: list of the descriptors
Can @ndescs ever be not equal to the length of @descs?
If no, it's redundant.
> +#
> +# @avail: VRingAvail info
> +#
> +# @used: VRingUsed info
> +#
> +# Since: 6.3
> +#
> +##
> +
> +{ 'struct': 'VirtioQueueElement',
> + 'data': { 'device-name': 'str',
> +'index': 'uint32',
> +'ndescs': 'uint32',
> +'descs': [ 'VirtioRingDesc' ],
> +'avail': 'VirtioRingAvail',
> +'used': 'VirtioRingUsed' } }
> +
> +##
> +# @x-query-virtio-queue-element:
> +#
> +# Return the information about a VirtQueue VirtQueueElement (by
> +# default looks at the head of the queue)
> +#
> +# @path: VirtIODevice canonical QOM path
> +#
> +# @queue: VirtQueue index to examine
> +#
> +# @index: the index in the queue, by default head
> +#
> +# Features:
> +# @unstable: This command is meant for debugging.
> +#
> +# Returns: VirtioQueueElement information
> +#
> +# Since: 6.3
> +#
> +# Examples:
> +#
> +# 1. Introspect on virtio-net virtqueue 0 at index 5
> +#
> +# -> { "execute": "x-query-virtio-queue-element",
> +# "arguments": { "path":
> "/machine/peripheral-anon/device[1]/virtio-backend",
> +# "queue": 0,
> +# "index": 5 }
> +#}
> +# <- { "return": {
> +# "index": 5,
> +# "ndescs": 1,
> +# "device-name": "virtio-net",
> +# "descs": [ { "flags": ["write"], "len": 1536, "addr": 5257305600 }
> ],
> +# "avail": { "idx": 256, "flags": 0, "ring": 5 },
> +# "used": { "idx": 13, "flags": 0 } }
> +#}
> +#
> +# 2. Introspect on virtio-crypto virtqueue 1 at head
> +#
> +# -> { "execute": "x-query-virtio-queue-element",
> +# "arguments": { "path": "/machine/peripheral/crypto0/virtio-backend",
> +# "queue": 1 }
> +#}
> +# <- { "return": {
> +# "index": 0,
> +# "ndescs": 1,
> +# "device-name": "virtio-crypto",
> +# "descs": [ { "flags": [], "len": 0, "addr": 8080268923184214134 }
> ],
> +# "avail": { "idx": 280, "flags": 0, "ring": 0 },
> +# "used": { "idx": 280, "flags": 0 } }
> +#}
> +#
> +# 3. Introspect on virtio-scsi virtqueue 2 at head
> +#
> +# -> { "execute": "x-query-virtio-queue-element",
> +# "arguments": { "path":
> "/machine/peripheral-anon/device[2]/virtio-backend",
> +# "queue": 2 }
> +#}
> +# <- { "return": {
> +# "index": 19,
> +# "ndescs": 1,
> +# "device-name": "virtio-scsi",
> +# "descs": [ { "flags": ["used", "indirect", "write"], "len":
> 4099327944,
> +# "addr": 12055409292258155293 } ],
> +# "avail": { "idx": 1147, "flags": 0, "ring": 19 },
>
[PATCH 1/3] qdict: make available dump_qobject(), dump_qdict(), dump_qlist()
move them from block/qapi.c to qobject/qdict.c, qobject/qlist.c,
qobject/qobject.c
This is useful to debug qobjects
Signed-off-by: Laurent Vivier
---
block/qapi.c | 82 +-
include/qapi/qmp/qdict.h | 2 +
include/qapi/qmp/qlist.h | 1 +
include/qapi/qmp/qobject.h | 1 +
qobject/qdict.c| 25
qobject/qlist.c| 17
qobject/qobject.c | 35
7 files changed, 82 insertions(+), 81 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index cf557e3aea7c..26bbc45a67f5 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -697,86 +697,6 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
g_free(sizing);
}
-static void dump_qdict(int indentation, QDict *dict);
-static void dump_qlist(int indentation, QList *list);
-
-static void dump_qobject(int comp_indent, QObject *obj)
-{
-switch (qobject_type(obj)) {
-case QTYPE_QNUM: {
-QNum *value = qobject_to(QNum, obj);
-char *tmp = qnum_to_string(value);
-qemu_printf("%s", tmp);
-g_free(tmp);
-break;
-}
-case QTYPE_QSTRING: {
-QString *value = qobject_to(QString, obj);
-qemu_printf("%s", qstring_get_str(value));
-break;
-}
-case QTYPE_QDICT: {
-QDict *value = qobject_to(QDict, obj);
-dump_qdict(comp_indent, value);
-break;
-}
-case QTYPE_QLIST: {
-QList *value = qobject_to(QList, obj);
-dump_qlist(comp_indent, value);
-break;
-}
-case QTYPE_QBOOL: {
-QBool *value = qobject_to(QBool, obj);
-qemu_printf("%s", qbool_get_bool(value) ? "true" : "false");
-break;
-}
-default:
-abort();
-}
-}
-
-static void dump_qlist(int indentation, QList *list)
-{
-const QListEntry *entry;
-int i = 0;
-
-for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
-QType type = qobject_type(entry->value);
-bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
-qemu_printf("%*s[%i]:%c", indentation * 4, "", i,
-composite ? '\n' : ' ');
-dump_qobject(indentation + 1, entry->value);
-if (!composite) {
-qemu_printf("\n");
-}
-}
-}
-
-static void dump_qdict(int indentation, QDict *dict)
-{
-const QDictEntry *entry;
-
-for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) {
-QType type = qobject_type(entry->value);
-bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
-char *key = g_malloc(strlen(entry->key) + 1);
-int i;
-
-/* replace dashes with spaces in key (variable) names */
-for (i = 0; entry->key[i]; i++) {
-key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
-}
-key[i] = 0;
-qemu_printf("%*s%s:%c", indentation * 4, "", key,
-composite ? '\n' : ' ');
-dump_qobject(indentation + 1, entry->value);
-if (!composite) {
-qemu_printf("\n");
-}
-g_free(key);
-}
-}
-
void bdrv_image_info_specific_dump(ImageInfoSpecific *info_spec)
{
QObject *obj, *data;
@@ -785,7 +705,7 @@ void bdrv_image_info_specific_dump(ImageInfoSpecific
*info_spec)
visit_type_ImageInfoSpecific(v, NULL, _spec, _abort);
visit_complete(v, );
data = qdict_get(qobject_to(QDict, obj), "data");
-dump_qobject(1, data);
+dump_qobject(1, data, qemu_printf);
qobject_unref(obj);
visit_free(v);
}
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index d5b5430e21a9..e529a9a9a29c 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -67,4 +67,6 @@ QDict *qdict_clone_shallow(const QDict *src);
QObject *qdict_crumple(const QDict *src, Error **errp);
void qdict_flatten(QDict *qdict);
+void dump_qdict(int indentation, QDict *dict, int (*qemu_printf)(const char
*fmt, ...));
+
#endif /* QDICT_H */
diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index 06e98ad5f498..4af55d82d3e4 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -62,4 +62,5 @@ static inline const QListEntry *qlist_next(const QListEntry
*entry)
return QTAILQ_NEXT(entry, next);
}
+void dump_qlist(int indentation, QList *list, int (*qemu_printf)(const char
*fmt, ...));
#endif /* QLIST_H */
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index 9003b71fd32d..bf893b146217 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -135,4 +135,5 @@ static inline QObject *qobject_check_type(const QObject
*obj, QType type)
}
}
+void dump_qobject(int comp_indent, QObject *obj, int (*qemu_printf)(const char
*fmt, ...));
#endif /* QOBJECT_H */
diff --git a/qobject/qdict.c b/qobject/qdict.c
[PATCH 0/3] tests/qtest: add some tests for virtio-net failover
This series adds a qtest entry to test virtio-net failover feature. We check following error cases: - check missing id on device with failover_pair_id triggers an error - check a primary device plugged on a bus that doesn't support hotplug triggers an error We check the status of the machine before and after hotplugging cards and feature negotiation: - check we don't see the primary device at boot if failover is on - check we see the primary device at boot if failover is off - check we don't see the primary device if failover is on but failover_pair_id is not the one with on (I think this should be changed) - check the primary device is plugged after the feature negotiation - check the result if the primary device is plugged before standby device and vice-versa - check the if the primary device is coldplugged and the standy device hotplugged and vice-versa - check the migration triggers the unplug -> this one needs to be improved as we can't actualy unplug the card as the qtest framework doesn't allow to really do the OS level unplug. So we receive the UNPLUG_PRIMARY event but nothing more. There are two preliminary patches in the series: - PATCH 1 makes available functions that helped me to debug the qmp command result. I think it's a good point to have them available widely - PATCH 2 introduces a function to enable PCI bridge. Failover needs to be plugged on a pcie-root-port and while the root port is not configured the cards behind it are not available Laurent Vivier (3): qdict: make available dump_qobject(), dump_qdict(), dump_qlist() qtest/libqos: add a function to initialize secondary PCI buses tests/qtest: add some tests for virtio-net failover block/qapi.c | 82 + include/hw/pci/pci_bridge.h | 8 + include/qapi/qmp/qdict.h | 2 + include/qapi/qmp/qlist.h | 1 + include/qapi/qmp/qobject.h| 1 + qobject/qdict.c | 25 ++ qobject/qlist.c | 17 + qobject/qobject.c | 35 ++ tests/qtest/libqos/pci.c | 118 +++ tests/qtest/libqos/pci.h | 1 + tests/qtest/meson.build | 3 + tests/qtest/virtio-net-failover.c | 567 ++ 12 files changed, 779 insertions(+), 81 deletions(-) create mode 100644 tests/qtest/virtio-net-failover.c -- 2.31.1
Re: [PATCH v9 5/8] qmp: decode feature & status bits in virtio-status
Jonah Palmer writes:
> From: Laurent Vivier
>
> Display feature names instead of bitmaps for host, guest, and
> backend for VirtIODevice.
>
> Display status names instead of bitmaps for VirtIODevice.
>
> Display feature names instead of bitmaps for backend, protocol,
> acked, and features (hdev->features) for vhost devices.
>
> Decode features according to device type. Decode status
> according to configuration status bitmap (config_status_map).
> Decode vhost user protocol features according to vhost user
> protocol bitmap (vhost_user_protocol_map).
>
> Transport features are on the first line. Undecoded bits
> (if any) are stored in a separate field. Vhost device field
> wont show if there's no vhost active for a given VirtIODevice.
>
> Signed-off-by: Jonah Palmer
[...]
> diff --git a/qapi/virtio.json b/qapi/virtio.json
> index 54212f2..6b11d52 100644
> --- a/qapi/virtio.json
> +++ b/qapi/virtio.json
> @@ -67,6 +67,466 @@
> }
>
> ##
> +# @VirtioType:
> +#
> +# An enumeration of Virtio device types (or names)
> +#
> +# Since: 6.3
> +##
> +
> +{ 'enum': 'VirtioType',
> + 'data': [ 'virtio-net', 'virtio-blk', 'virtio-serial', 'virtio-rng',
> +'virtio-balloon', 'virtio-iomem', 'virtio-rpmsg',
> +'virtio-scsi', 'virtio-9p', 'virtio-mac-wlan',
> +'virtio-rproc-serial', 'virtio-caif', 'virtio-mem-balloon',
> +'virtio-gpu', 'virtio-clk', 'virtio-input', 'vhost-vsock',
> +'virtio-crypto', 'virtio-signal', 'virtio-pstore',
> +'virtio-iommu', 'virtio-mem', 'virtio-sound', 'vhost-user-fs',
> +'virtio-pmem', 'virtio-mac-hwsim', 'vhost-user-i2c',
> +'virtio-bluetooth' ]
> +}
> +
> +##
> +# @VirtioConfigStatus:
> +#
> +# An enumeration of Virtio device configuration statuses
> +#
> +# Since: 6.3
> +##
> +
> +{ 'enum': 'VirtioConfigStatus',
> + 'data': [ 'driver-ok', 'features-ok', 'driver', 'needs-reset',
> +'failed', 'acknowledge' ]
> +}
> +
> +##
> +# @VirtioDeviceStatus:
> +#
> +# A structure defined to list the configuration statuses of a virtio
> +# device
> +#
> +# @dev-status: List of decoded configuration statuses of the virtio
> +# device
> +#
> +# @unknown-statuses: virtio device statuses bitmap that have not been decoded
Why is @dev-status singular, and @unknown-statuses plural?
> +#
> +# Since: 6.3
> +##
> +
> +{ 'struct': 'VirtioDeviceStatus',
> + 'data': { 'dev-status': [ 'VirtioConfigStatus' ],
> +'*unknown-statuses': 'uint8' } }
> +
> +##
> +# @VhostProtocolFeature:
> +#
> +# An enumeration of Vhost User protocol features
> +#
> +# Since: 6.3
> +##
> +
> +{ 'enum': 'VhostProtocolFeature',
> + 'data': [ 'mq', 'log-shmfd', 'rarp', 'reply-ack', 'net-mtu',
> +'slave-req', 'cross-endian', 'crypto-session', 'pagefault',
> +'config', 'slave-send-fd', 'host-notifier',
> +'inflight-shmfd', 'reset-device', 'inband-notifications',
> +'configure-mem-slots' ]
> +}
> +
> +##
> +# @VhostDeviceProtocols:
> +#
> +# A structure defined to list the vhost user protocol features of a
> +# Vhost User device
> +#
> +# @features: List of decoded vhost user protocol features of a vhost
> +#user device
> +#
> +# @unknown-protocols: vhost user device protocol features bitmap that
> +# have not been decoded
Why are the known protocol features called @features, and the unknown
ones @unknown-protocols?
> +#
> +# Since: 6.3
> +##
> +
> +{ 'struct': 'VhostDeviceProtocols',
> + 'data': { 'features': [ 'VhostProtocolFeature' ],
> +'*unknown-protocols': 'uint64' } }
> +
> +##
> +# @VirtioTransportFeature:
> +#
> +# An enumeration of Virtio device transport features, including virtio-ring
> +#
> +# Since: 6.3
> +##
> +
> +{ 'enum': 'VirtioTransportFeature',
> + 'data': [ 'notify-on-empty', 'any-layout', 'protocol-features',
> +'version-1', 'iommu-platform', 'ring-packed', 'order-platform',
> +'sr-iov', 'indirect-desc', 'event-idx' ]
> +}
> +
> +##
> +# @VirtioMemFeature:
> +#
> +# An enumeration of Virtio mem features
> +#
> +# Since: 6.3
> +##
> +
> +{ 'enum': 'VirtioMemFeature',
> + 'data': [ 'acpi-pxm' ]
> +}
> +
> +##
> +# @VirtioSerialFeature:
> +#
> +# An enumeration of Virtio serial/console features
> +#
> +# Since: 6.3
> +##
> +
> +{ 'enum': 'VirtioSerialFeature',
> + 'data': [ 'size', 'multiport', 'emerg-write' ]
> +}
> +
> +##
> +# @VirtioBlkFeature:
> +#
> +# An enumeration of Virtio block features
> +#
> +# Since: 6.3
> +##
> +
> +{ 'enum': 'VirtioBlkFeature',
> + 'data': [ 'size-max', 'seg-max', 'geometry', 'ro', 'blk-size',
> +'topology', 'mq', 'discard', 'write-zeroes', 'barrier',
> +'scsi', 'flush', 'config-wce', 'log-all' ]
> +}
> +
> +##
> +# @VirtioGpuFeature:
> +#
> +# An enumeration of Virtio gpu features
> +#
> +# Since: 6.3
> +##
> +
> +{ 'enum': 'VirtioGpuFeature',
> + 'data': [ 'virgl', 'edid', 'resource-uuid',
Re: [PATCH 05/15] hw/nvme: Add support for SR-IOV
On Mon, Nov 08, 2021 at 08:56:43AM +0100, Klaus Jensen wrote:
> On Nov 4 15:30, Lukasz Maniak wrote:
> > On Tue, Nov 02, 2021 at 06:33:31PM +0100, Lukasz Maniak wrote:
> > > On Tue, Nov 02, 2021 at 03:33:15PM +0100, Klaus Jensen wrote:
> > > > On Oct 7 18:23, Lukasz Maniak wrote:
> > > > > This patch implements initial support for Single Root I/O
> > > > > Virtualization
> > > > > on an NVMe device.
> > > > >
> > > > > Essentially, it allows to define the maximum number of virtual
> > > > > functions
> > > > > supported by the NVMe controller via sriov_max_vfs parameter.
> > > > >
> > > > > Passing a non-zero value to sriov_max_vfs triggers reporting of SR-IOV
> > > > > capability by a physical controller and ARI capability by both the
> > > > > physical and virtual function devices.
> > > > >
> > > > > NVMe controllers created via virtual functions mirror functionally
> > > > > the physical controller, which may not entirely be the case, thus
> > > > > consideration would be needed on the way to limit the capabilities of
> > > > > the VF.
> > > > >
> > > > > NVMe subsystem is required for the use of SR-IOV.
> > > > >
> > > > > Signed-off-by: Lukasz Maniak
> > > > > ---
> > > > > hw/nvme/ctrl.c | 74
> > > > > ++--
> > > > > hw/nvme/nvme.h | 1 +
> > > > > include/hw/pci/pci_ids.h | 1 +
> > > > > 3 files changed, 73 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> > > > > index 6a571d18cf..ad79ff0c00 100644
> > > > > --- a/hw/nvme/ctrl.c
> > > > > +++ b/hw/nvme/ctrl.c
> > > > > @@ -6361,8 +6406,12 @@ static int nvme_init_pci(NvmeCtrl *n,
> > > > > PCIDevice *pci_dev, Error **errp)
> > > > >n->reg_size);
> > > > > memory_region_add_subregion(>bar0, 0, >iomem);
> > > > >
> > > > > -pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
> > > > > - PCI_BASE_ADDRESS_MEM_TYPE_64, >bar0);
> > > > > +if (pci_is_vf(pci_dev)) {
> > > > > +pcie_sriov_vf_register_bar(pci_dev, 0, >bar0);
> > > > > +} else {
> > > > > +pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
> > > > > + PCI_BASE_ADDRESS_MEM_TYPE_64, >bar0);
> > > > > +}
> > > >
> > > > I assume that the assert we are seeing means that the pci_register_bars
> > > > in nvme_init_cmb and nvme_init_pmr must be changed similarly to this.
> > >
> > > Assert will only arise for CMB as VF params are initialized with PF
> > > params.
> > >
> > > @@ -6532,6 +6585,15 @@ static void nvme_realize(PCIDevice *pci_dev, Error
> > > **errp)
> > > NvmeCtrl *n = NVME(pci_dev);
> > > NvmeNamespace *ns;
> > > Error *local_err = NULL;
> > > +NvmeCtrl *pn = NVME(pcie_sriov_get_pf(pci_dev));
> > > +
> > > +if (pci_is_vf(pci_dev)) {
> > > +/* VFs derive settings from the parent. PF's lifespan exceeds
> > > + * that of VF's, so it's safe to share params.serial.
> > > + */
> > > +memcpy(>params, >params, sizeof(NvmeParams));
> > > +n->subsys = pn->subsys;
> > > +}
> > >
> > > nvme_check_constraints(n, _err);
> > > if (local_err) {
> > >
> > > The following simple fix will both fix assert and also allow
> > > each VF to have its own CMB of the size defined for PF.
> > >
> > > ---
> > > hw/nvme/ctrl.c | 13 +
> > > 1 file changed, 9 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> > > index 19b32dd4da..99daa6290c 100644
> > > --- a/hw/nvme/ctrl.c
> > > +++ b/hw/nvme/ctrl.c
> > > @@ -6837,10 +6837,15 @@ static void nvme_init_cmb(NvmeCtrl *n, PCIDevice
> > > *pci_dev)
> > > n->cmb.buf = g_malloc0(cmb_size);
> > > memory_region_init_io(>cmb.mem, OBJECT(n), _cmb_ops, n,
> > >"nvme-cmb", cmb_size);
> > > -pci_register_bar(pci_dev, NVME_CMB_BIR,
> > > - PCI_BASE_ADDRESS_SPACE_MEMORY |
> > > - PCI_BASE_ADDRESS_MEM_TYPE_64 |
> > > - PCI_BASE_ADDRESS_MEM_PREFETCH, >cmb.mem);
> > > +
> > > +if (pci_is_vf(pci_dev)) {
> > > +pcie_sriov_vf_register_bar(pci_dev, NVME_CMB_BIR, >cmb.mem);
> > > +} else {
> > > +pci_register_bar(pci_dev, NVME_CMB_BIR,
> > > +PCI_BASE_ADDRESS_SPACE_MEMORY |
> > > +PCI_BASE_ADDRESS_MEM_TYPE_64 |
> > > +PCI_BASE_ADDRESS_MEM_PREFETCH, >cmb.mem);
> > > +}
> > >
> > > NVME_CAP_SET_CMBS(cap, 1);
> > > stq_le_p(>bar.cap, cap);
> > >
> > > As for PMR, it is currently only available on PF, as only PF is capable
> > > of specifying the memory-backend-file object to use with PMR.
> > > Otherwise, either VFs would have to share the PMR with its PF, or there
> > > would be a requirement to define a memory-backend-file object for each VF.
> >
> > Hi Klaus,
> >
> > After some discussion, we
Re: [PATCH 4/7] block: Drop detached child from ignore list
04.11.2021 13:38, Hanna Reitz wrote:
bdrv_attach_child_common_abort() restores the parent's AioContext. To
do so, the child (which was supposed to be attached, but is now detached
again by this abort handler) is added to the ignore list for the
AioContext changing functions.
However, since we modify a BDS's children list in the BdrvChildClass's
.attach and .detach handlers, the child is already effectively detached
from the parent by this point. We do not need to put it into the ignore
list.
Use this opportunity to clean up the empty line structure: Keep setting
the ignore list, invoking the AioContext function, and freeing the
ignore list in blocks separated by empty lines.
Signed-off-by: Hanna Reitz
Reviewed-by: Vladimir Sementsov-Ogievskiy
What comes in my mind, is that now bdrv_replace_child_noperm() is more strong in detaching..
But may be not enough strong: we still have link from the child to parent (child->opaque)..
Maybe more correct for BdrvChild object to have no relation with parent after detaching. But
than we'll need some GenericParent object (as child->class is mostly about parent, not about
child and even not about the edge).. Now GenericParent is "included" into BdrvChild,
which represents both GenericParent and Edge objects..
---
block.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index b95f8dcf8f..6d230ad3d1 100644
--- a/block.c
+++ b/block.c
@@ -2774,14 +2774,16 @@ static void bdrv_attach_child_common_abort(void *opaque)
}
if (bdrv_child_get_parent_aio_context(child) != s->old_parent_ctx) {
-GSList *ignore = g_slist_prepend(NULL, child);
+GSList *ignore;
+/* No need to ignore `child`, because it has been detached already */
+ignore = NULL;
child->klass->can_set_aio_ctx(child, s->old_parent_ctx, ,
_abort);
g_slist_free(ignore);
-ignore = g_slist_prepend(NULL, child);
-child->klass->set_aio_ctx(child, s->old_parent_ctx, );
+ignore = NULL;
+child->klass->set_aio_ctx(child, s->old_parent_ctx, );
g_slist_free(ignore);
}
--
Best regards,
Vladimir
Re: [PATCH v9 8/8] hmp: add virtio commands
Jonah Palmer writes:
> From: Laurent Vivier
>
> This patch implements the HMP versions of the virtio QMP commands.
>
> Signed-off-by: Jonah Palmer
> ---
> hmp-commands-info.hx | 218 ++
> include/monitor/hmp.h | 5 +
> monitor/hmp-cmds.c| 358
> ++
> 3 files changed, 581 insertions(+)
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 407a1da..6bf7359 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -877,3 +877,221 @@ SRST
>``info sgx``
> Show intel SGX information.
> ERST
> +
> +{
> +.name = "virtio",
> +.args_type = "",
> +.params= "",
> +.help = "List all available virtio devices",
> +.cmd = hmp_virtio_query,
> +.flags = "p",
> +},
> +
> +SRST
> + ``info virtio``
> +List all available virtio devices
> +
> +Example:
> +
> +List all available virtio devices in the machine::
> +
> +(qemu) info virtio
> +/machine/peripheral/vsock0/virtio-backend [vhost-vsock]
I get
docs/../hmp-commands-info.hx:899:Inconsistent literal block quoting.
This is from Sphinx. I can't see what's wrong.
> +/machine/peripheral/crypto0/virtio-backend [virtio-crypto]
> +/machine/peripheral-anon/device[2]/virtio-backend [virtio-scsi]
> +/machine/peripheral-anon/device[1]/virtio-backend [virtio-net]
> +/machine/peripheral-anon/device[0]/virtio-backend [virtio-serial]
> +
> +ERST
[...]
Re: [PATCH v2 0/3] virtio: increase VIRTQUEUE_MAX_SIZE to 32k
On Mittwoch, 10. November 2021 11:05:50 CET Stefan Hajnoczi wrote: > > > > So looks like it was probably still easier and realistic to just add > > > > virtio > > > > capabilities for now for allowing to exceed current descriptor limit. > > > > > > I'm still not sure why virtio-net, virtio-blk, virtio-fs, etc perform > > > fine under today's limits while virtio-9p needs a much higher limit to > > > achieve good performance. Maybe there is an issue in a layer above the > > > vring that's causing the virtio-9p performance you've observed? > > > > Are you referring to (somewhat) recent benchmarks when saying those would > > all still perform fine today? > > I'm not referring to specific benchmark results. Just that none of those > devices needed to raise the descriptor chain length, so I'm surprised > that virtio-9p needs it because it's conceptually similar to these > devices. I would not say virtio-net and virtio-blk were comparable with virtio-9p and virtio-fs. virtio-9p and virtio-fs are fully fledged file servers which must perform various controller tasks before handling the actually requested I/O task, which inevitably adds latency to each request, whereas virtio-net and virtio-blk are just very thin layers that do not have that controller task overhead per request. And a network device only needs to handle very small messages in the first place. > > Vivek was running detailed benchmarks for virtiofs vs. 9p: > > https://lists.gnu.org/archive/html/qemu-devel/2020-12/msg02704.html > > > > For the virtio aspect discussed here, only the benchmark configurations > > without cache are relevant (9p-none, vtfs-none) and under this aspect the > > situation seems to be quite similar between 9p and virtio-fs. You'll also > > note once DAX is enabled (vtfs-none-dax) that apparently boosts virtio-fs > > performance significantly, which however seems to corelate to numbers > > when I am running 9p with msize > 300k. Note: Vivek was presumably > > running 9p effecively with msize=300k, as this was the kernel limitation > > at that time. > Agreed, virtio-9p and virtiofs are similar without caching. > > I think we shouldn't consider DAX here since it bypasses the virtqueue. DAX bypasses virtio, sure, but the performance boost you get with DAX actually shows the limiting factor with virtio pretty well. > > To bring things into relation: there are known performance aspects in 9p > > that can be improved, yes, both on Linux kernel side and on 9p server > > side in QEMU. For instance 9p server uses coroutines [1] and currently > > dispatches between worker thread(s) and main thread too often per request > > (partly addressed already [2], but still WIP), which accumulates to > > overall latency. But Vivek was actually using a 9p patch here which > > disabled coroutines entirely, which suggests that the virtio transmission > > size limit still represents a bottleneck. > > These results were collected with 4k block size. Neither msize nor the > descriptor chain length limits will be stressed, so I don't think these > results are relevant here. > > Maybe a more relevant comparison would be virtio-9p, virtiofs, and > virtio-blk when block size is large (e.g. 1M). The Linux block layer in > the guest will split virtio-blk requests when they exceed the block > queue limits. I am sorry, I cannot spend time for more benchmarks like that. For really making fair comparisons I would need to review all their code on both ends, adjust configuration/sources, etc. I do think that I performed enough benchmarks and tests to show that increasing the transmission size can significantly improve performance with 9p, and that allowing to exceed the queue size does make sense even for small transmission sizes (e.g. max. active requests on 9p server side vs. max. transmission size per request). The reason for the performance gain is the minimum latency involved per request, and like I said, that can be improved to a certain extent with 9p, but that will take a long time and it could not be eliminated entirely. As you are apparently reluctant for changing the virtio specs, what about introducing those discussed virtio capabalities either as experimental ones without specs changes, or even just as 9p specific device capabilities for now. I mean those could be revoked on both sides at any time anyway. Best regards, Christian Schoenebeck
Re: [PATCH v9 4/8] qmp: add QMP command x-query-virtio-status
Jonah Palmer writes:
> From: Laurent Vivier
>
> This new command shows the status of a VirtIODevice, including
> its corresponding vhost device status (if active).
>
> Next patch will improve output by decoding feature bits, including
> vhost device's feature bits (backend, protocol, acked, and features).
> Also will decode status bits of a VirtIODevice.
>
> Next patch will also suppress the vhost device field from displaying
> if no vhost device is active for a given VirtIODevice.
>
> Signed-off-by: Jonah Palmer
[...]
> diff --git a/qapi/virtio.json b/qapi/virtio.json
> index 324ba8c..54212f2 100644
> --- a/qapi/virtio.json
> +++ b/qapi/virtio.json
> @@ -53,3 +53,249 @@
>
> { 'command': 'x-query-virtio', 'returns': ['VirtioInfo'],
>'features': [ 'unstable' ] }
> +
> +##
> +# @VirtioStatusEndianness:
> +#
> +# Enumeration of endianness for VirtioDevice
> +#
> +# Since: 6.3
> +##
> +
> +{ 'enum': 'VirtioStatusEndianness',
> + 'data': [ 'unknown', 'little', 'big' ]
> +}
> +
> +##
> +# @VhostStatus:
> +#
> +# Information about a vhost device. This information will only be
> +# displayed if the vhost device is active.
> +#
> +# @n-mem-sections: vhost_dev n_mem_sections
> +#
> +# @n-tmp-sections: vhost_dev n_tmp_sections
> +#
> +# @nvqs: vhost_dev nvqs. This is the number of virtqueues being used
> +#by the vhost device.
> +#
> +# @vq-index: vhost_dev vq_index
> +#
> +# @features: vhost_dev features
> +#
> +# @acked-features: vhost_dev acked_features
> +#
> +# @backend-features: vhost_dev backend_features
> +#
> +# @protocol-features: vhost_dev protocol_features
> +#
> +# @max-queues: vhost_dev max_queues
> +#
> +# @backend-cap: vhost_dev backend_cap
> +#
> +# @log-enabled: vhost_dev log_enabled flag
> +#
> +# @log-size: vhost_dev log_size
> +#
> +# Since: 6.3
> +#
> +##
> +
> +{ 'struct': 'VhostStatus',
> + 'data': { 'n-mem-sections': 'int',
> +'n-tmp-sections': 'int',
> +'nvqs': 'uint32',
> +'vq-index': 'int',
> +'features': 'uint64',
> +'acked-features': 'uint64',
> +'backend-features': 'uint64',
> +'protocol-features': 'uint64',
> +'max-queues': 'uint64',
> +'backend-cap': 'uint64',
> +'log-enabled': 'bool',
> +'log-size': 'uint64' } }
> +
> +##
> +# @VirtioStatus:
> +#
> +# Full status of the virtio device with most VirtIODevice members.
> +# Also includes the full status of the corresponding vhost device
> +# if the vhost device is active.
> +#
> +# @name: VirtIODevice name
> +#
> +# @device-id: VirtIODevice ID
> +#
> +# @vhost-started: VirtIODevice vhost_started flag
> +#
> +# @guest-features: VirtIODevice guest_features
> +#
> +# @host-features: VirtIODevice host_features
> +#
> +# @backend-features: VirtIODevice backend_features
> +#
> +# @device-endian: VirtIODevice device_endian
> +#
> +# @num-vqs: VirtIODevice virtqueue count. This is the number of active
> +# virtqueues being used by the VirtIODevice.
> +#
> +# @status: VirtIODevice configuration status (e.g. DRIVER_OK,
> +# FEATURES_OK, DRIVER, etc.)
> +#
> +# @isr: VirtIODevice ISR
> +#
> +# @queue-sel: VirtIODevice queue_sel
> +#
> +# @vm-running: VirtIODevice vm_running flag
> +#
> +# @broken: VirtIODevice broken flag
> +#
> +# @disabled: VirtIODevice disabled flag
> +#
> +# @use-started: VirtIODevice use_started flag
> +#
> +# @started: VirtIODevice started flag
> +#
> +# @start-on-kick: VirtIODevice start_on_kick flag
> +#
> +# @disable-legacy-check: VirtIODevice disabled_legacy_check flag
> +#
> +# @bus-name: VirtIODevice bus_name
> +#
> +# @use-guest-notifier-mask: VirtIODevice use_guest_notifier_mask flag
> +#
> +# @vhost-dev: corresponding vhost device info for a given VirtIODevice
> +#
> +# Since: 6.3
> +#
> +##
> +
> +{ 'struct': 'VirtioStatus',
> + 'data': { 'name': 'str',
> +'device-id': 'uint16',
> +'vhost-started': 'bool',
> +'guest-features': 'uint64',
> +'host-features': 'uint64',
> +'backend-features': 'uint64',
> +'device-endian': 'VirtioStatusEndianness',
> +'num-vqs': 'int',
> +'status': 'uint8',
> +'isr': 'uint8',
> +'queue-sel': 'uint16',
> +'vm-running': 'bool',
> +'broken': 'bool',
> +'disabled': 'bool',
> +'use-started': 'bool',
> +'started': 'bool',
> +'start-on-kick': 'bool',
> +'disable-legacy-check': 'bool',
> +'bus-name': 'str',
> +'use-guest-notifier-mask': 'bool',
> +'vhost-dev': 'VhostStatus' } }
> +
> +##
> +# @x-query-virtio-status:
> +#
> +# Poll for a comprehensive status of a given virtio device
> +#
> +# @path: Canonical QOM path of the VirtIODevice
> +#
> +# Features:
> +# @unstable: This command is meant for debugging.
> +#
> +# Returns: VirtioStatus of the virtio device
> +#
> +# Since: 6.3
> +#
>
Re: [PATCH 3/7] block: Unite remove_empty_child and child_free
04.11.2021 13:38, Hanna Reitz wrote: Now that bdrv_remove_empty_child() no longer removes the child from the parent's children list but only checks that it is not in such a list, it is only a wrapper around bdrv_child_free() that checks that the child is empty and unused. That should apply to all children that we free, so put those checks into bdrv_child_free() and drop bdrv_remove_empty_child(). Signed-off-by: Hanna Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
Re: [PATCH 2/7] block: Manipulate children list in .attach/.detach
04.11.2021 13:38, Hanna Reitz wrote: The children list is specific to BDS parents. We should not modify it in the general children modification code, but let BDS parents deal with it in their .attach() and .detach() methods. This also has the advantage that a BdrvChild is removed from the children list before its .bs pointer can become NULL. BDS parents generally assume that their children's .bs pointer is never NULL, so this is actually a bug fix. Signed-off-by: Hanna Reitz Interesting that nor child_root neither child_job do similar things in .attach / .detach ... Should we do something with it? -- Best regards, Vladimir
Re: [PATCH 2/7] block: Manipulate children list in .attach/.detach
04.11.2021 13:38, Hanna Reitz wrote:
The children list is specific to BDS parents. We should not modify it
in the general children modification code, but let BDS parents deal with
it in their .attach() and .detach() methods.
This also has the advantage that a BdrvChild is removed from the
children list before its .bs pointer can become NULL. BDS parents
generally assume that their children's .bs pointer is never NULL, so
this is actually a bug fix.
Signed-off-by: Hanna Reitz
---
block.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index 580cb77a70..243ae206b5 100644
--- a/block.c
+++ b/block.c
@@ -1387,6 +1387,8 @@ static void bdrv_child_cb_attach(BdrvChild *child)
{
BlockDriverState *bs = child->opaque;
+QLIST_INSERT_HEAD(>children, child, next);
+
if (child->role & BDRV_CHILD_COW) {
bdrv_backing_attach(child);
}
@@ -1403,6 +1405,8 @@ static void bdrv_child_cb_detach(BdrvChild *child)
}
bdrv_unapply_subtree_drain(child, bs);
+
+QLIST_REMOVE(child, next);
}
static int bdrv_child_cb_update_filename(BdrvChild *c, BlockDriverState *base,
@@ -2747,7 +2751,7 @@ static void bdrv_child_free(void *opaque)
static void bdrv_remove_empty_child(BdrvChild *child)
{
assert(!child->bs);
-QLIST_SAFE_REMOVE(child, next);
+assert(!child->next.le_prev); /* not in children list */
bdrv_child_free(child);
}
@@ -2913,7 +2917,6 @@ static int bdrv_attach_child_noperm(BlockDriverState *parent_bs,
return ret;
}
-QLIST_INSERT_HEAD(_bs->children, *child, next);
The following comment become stale. We should remove it too. With comment
removed:
Reviewed-by: Vladimir Sementsov-Ogievskiy
/*
* child is removed in bdrv_attach_child_common_abort(), so don't care to
* abort this change separately.
@@ -4851,7 +4854,6 @@ static void bdrv_remove_filter_or_cow_child_abort(void
*opaque)
BdrvRemoveFilterOrCowChild *s = opaque;
BlockDriverState *parent_bs = s->child->opaque;
-QLIST_INSERT_HEAD(_bs->children, s->child, next);
if (s->is_backing) {
parent_bs->backing = s->child;
} else {
@@ -4906,7 +4908,6 @@ static void
bdrv_remove_file_or_backing_child(BlockDriverState *bs,
};
tran_add(tran, _remove_filter_or_cow_child_drv, s);
-QLIST_SAFE_REMOVE(child, next);
if (s->is_backing) {
bs->backing = NULL;
} else {
--
Best regards,
Vladimir
Re: [PATCH 1/7] stream: Traverse graph after modification
04.11.2021 13:38, Hanna Reitz wrote:
bdrv_cor_filter_drop() modifies the block graph. That means that other
parties can also modify the block graph before it returns. Therefore,
we cannot assume that the result of a graph traversal we did before
remains valid afterwards.
We should thus fetch `base` and `unfiltered_base` afterwards instead of
before.
Signed-off-by: Hanna Reitz
The fact that other parties can modify graph during our graph modification is a
global problem.. The patch doesn't fix it, but reduces its effect in specific
case.. OK.
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
block/stream.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/block/stream.c b/block/stream.c
index 97bee482dc..e45113aed6 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -54,8 +54,8 @@ static int stream_prepare(Job *job)
{
StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
BlockDriverState *unfiltered_bs = bdrv_skip_filters(s->target_bs);
-BlockDriverState *base = bdrv_filter_or_cow_bs(s->above_base);
-BlockDriverState *unfiltered_base = bdrv_skip_filters(base);
+BlockDriverState *base;
+BlockDriverState *unfiltered_base;
Error *local_err = NULL;
int ret = 0;
@@ -63,6 +63,9 @@ static int stream_prepare(Job *job)
bdrv_cor_filter_drop(s->cor_filter_bs);
s->cor_filter_bs = NULL;
+base = bdrv_filter_or_cow_bs(s->above_base);
+unfiltered_base = bdrv_skip_filters(base);
+
if (bdrv_cow_child(unfiltered_bs)) {
const char *base_id = NULL, *base_fmt = NULL;
if (unfiltered_base) {
--
Best regards,
Vladimir
Re: [PATCH v9 3/8] qmp: add QMP command x-query-virtio
Jonah Palmer writes:
> From: Laurent Vivier
>
> This new command lists all the instances of VirtIODevice with
> their QOM paths and virtio type/name.
>
> Signed-off-by: Jonah Palmer
[...]
> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> index 4912b97..1512ada 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -93,3 +93,4 @@
> { 'include': 'audio.json' }
> { 'include': 'acpi.json' }
> { 'include': 'pci.json' }
> +{ 'include': 'virtio.json' }
> diff --git a/qapi/virtio.json b/qapi/virtio.json
> new file mode 100644
> index 000..324ba8c
> --- /dev/null
> +++ b/qapi/virtio.json
> @@ -0,0 +1,55 @@
> +# -*- Mode: Python -*-
> +# vim: filetype=python
> +#
> +
> +##
> +# = Virtio devices
> +##
> +
> +##
> +# @VirtioInfo:
> +#
> +# Basic information about a given VirtIODevice
> +#
> +# @path: the device's canonical QOM path
> +#
> +# @type: VirtIO device name
> +#
> +# Since: 6.3
I expect the next release to be numbered 7.0.
> +#
> +##
> +{ 'struct': 'VirtioInfo',
> + 'data': { 'path': 'str',
> +'type': 'str' } }
> +
> +##
> +# @x-query-virtio:
> +#
> +# Returns a list of all realized VirtIO devices
> +#
> +# Features:
> +# @unstable: This command is meant for debugging.
> +#
> +# Returns: list of gathered @VirtioInfo devices
> +#
> +# Since: 6.3
> +#
> +# Example:
> +#
> +# -> { "execute": "x-query-virtio" }
> +# <- { "return": [ { "path":
> "/machine/peripheral-anon/device[4]/virtio-backend",
> +#"type": "virtio-input" },
> +# { "path": "/machine/peripheral/crypto0/virtio-backend",
> +#"type": "virtio-crypto" },
> +# { "path":
> "/machine/peripheral-anon/device[2]/virtio-backend",
> +#"type": "virtio-scsi" },
> +# { "path":
> "/machine/peripheral-anon/device[1]/virtio-backend",
> +#"type": "virtio-net" },
> +# { "path":
> "/machine/peripheral-anon/device[0]/virtio-backend",
> +#"type": "virtio-serial" }
> +#] }
Any particular reason for reformatting the example? For what it's
worth, I'd prefer the previous version.
Aside: consistent formatting of examples would be nice. Not in this
series.
> +#
> +##
> +
> +{ 'command': 'x-query-virtio', 'returns': ['VirtioInfo'],
> + 'features': [ 'unstable' ] }
> diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
> index 7f103ea..fd00ee2 100644
> --- a/tests/qtest/qmp-cmd-test.c
> +++ b/tests/qtest/qmp-cmd-test.c
> @@ -103,6 +103,7 @@ static bool query_is_ignored(const char *cmd)
> "query-gic-capabilities", /* arm */
> /* Success depends on target-specific build configuration: */
> "query-pci", /* CONFIG_PCI */
> +"x-query-virtio", /* CONFIG_VIRTIO */
> /* Success depends on launching SEV guest */
> "query-sev-launch-measure",
> /* Success depends on Host or Hypervisor SEV support */
Re: qemu-img.c possibly overflowing shifts by BDRV_SECTOR_BITS
Am 09.11.2021 um 20:07 hat Peter Maydell geschrieben: > Hi; Coverity is complaining about some of the places in qemu-img.c > where it takes a 32-bit variable and shifts it left by BDRV_SECTOR_BITS > to convert a sector count to a byte count, because it's doing the > shift in 32-bits rather than 64 and so Coverity thinks there might > be overflow (CID 1465221, 1465219). Is it right and we need extra > casts to force the shift to be done in 64 bits, or is there some > constraint that means we know the sector counts are always small > enough that the byte count is 2GB or less ? These are false positives. n is limited to BDRV_REQUEST_MAX_SECTORS already when it starts out in convert_iteration_sectors() (which is enough to make the calculation safe), but for the specific code path, I think it's even guaranteed to be further limited to s->buf_sectors which is 16 MB at most (MAX_BUF_SECTORS in qemu-img.c). Kevin
[RFC PATCH v3] hw/nvme:Adding Support for namespace management
From: Naveen Nagar
This patch supports namespace management : create and delete operations
This patch has been tested with the following command and size of image
file for unallocated namespaces is taken as 0GB. ns_create will look into
the list of unallocated namespaces and it will initialize the same and
return the nsid of the same. A new mandatory field has been added called
tnvmcap and we have ensured that the total capacity of namespace created
does not exceed tnvmcap
-device nvme-subsys,id=subsys0,tnvmcap=8
-device nvme,serial=foo,id=nvme0,subsys=subsys0
-device nvme,serial=bar,id=nvme1,subsys=subsys0
-drive id=ns1,file=ns1.img,if=none
-device nvme-ns,drive=ns1,bus=nvme0,nsid=1,zoned=false,shared=true
-drive id=ns2,file=ns2.img,if=none
-device nvme-ns,drive=ns2,bus=nvme0,nsid=2,zoned=false,shared=true
-drive id=ns3,file=ns3.img,if=none
-device nvme-ns,drive=ns3,bus=nvme0,nsid=3,zoned=false,shared=true
-drive id=ns4,file=ns4.img,if=none
-device nvme-ns,drive=ns4,bus=nvme0,nsid=4,zoned=false,shared=true
Please review and suggest if any changes are required.
Signed-off-by: Naveen Nagar
Since v2:
-Lukasz Maniak found a bug in namespace attachment and proposed
solution is added
---
hw/nvme/ctrl.c | 241 ---
hw/nvme/ns.c | 61 ++-
hw/nvme/nvme.h | 7 +-
hw/nvme/subsys.c | 3 +
include/block/nvme.h | 18 +++-
5 files changed, 288 insertions(+), 42 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 6baf9e0420..63ea2fcb14 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -219,6 +219,7 @@ static const uint32_t nvme_cse_acs[256] = {
[NVME_ADM_CMD_SET_FEATURES] = NVME_CMD_EFF_CSUPP,
[NVME_ADM_CMD_GET_FEATURES] = NVME_CMD_EFF_CSUPP,
[NVME_ADM_CMD_ASYNC_EV_REQ] = NVME_CMD_EFF_CSUPP,
+[NVME_ADM_CMD_NS_MANAGEMENT]= NVME_CMD_EFF_CSUPP | NVME_CMD_EFF_NIC,
[NVME_ADM_CMD_NS_ATTACHMENT]= NVME_CMD_EFF_CSUPP | NVME_CMD_EFF_NIC,
[NVME_ADM_CMD_FORMAT_NVM] = NVME_CMD_EFF_CSUPP | NVME_CMD_EFF_LBCC,
};
@@ -4450,11 +4451,19 @@ static uint16_t nvme_identify_ns(NvmeCtrl *n,
NvmeRequest *req, bool active)
NvmeNamespace *ns;
NvmeIdentify *c = (NvmeIdentify *)>cmd;
uint32_t nsid = le32_to_cpu(c->nsid);
+NvmeIdNs *id_ns = NULL;
+uint16_t ret;
trace_pci_nvme_identify_ns(nsid);
-if (!nvme_nsid_valid(n, nsid) || nsid == NVME_NSID_BROADCAST) {
+if (!nvme_nsid_valid(n, nsid)) {
return NVME_INVALID_NSID | NVME_DNR;
+} else if (nsid == NVME_NSID_BROADCAST) {
+id_ns = g_new0(NvmeIdNs, 1);
+nvme_ns_identify_common(id_ns);
+ret = nvme_c2h(n, (uint8_t *)id_ns, sizeof(NvmeIdNs), req);
+g_free(id_ns);
+return ret;
}
ns = nvme_ns(n, nsid);
@@ -5184,9 +5193,204 @@ static void nvme_select_iocs_ns(NvmeCtrl *n,
NvmeNamespace *ns)
}
}
+static int nvme_blk_truncate(BlockBackend *blk, size_t len, Error **errp)
+{
+int ret;
+uint64_t perm, shared_perm;
+
+blk_get_perm(blk, , _perm);
+
+ret = blk_set_perm(blk, perm | BLK_PERM_RESIZE, shared_perm, errp);
+if (ret < 0) {
+return ret;
+}
+
+ret = blk_truncate(blk, len, false, PREALLOC_MODE_OFF, 0, errp);
+if (ret < 0) {
+return ret;
+}
+
+ret = blk_set_perm(blk, perm, shared_perm, errp);
+if (ret < 0) {
+return ret;
+}
+
+return 0;
+}
+
+static uint32_t nvme_allocate_nsid(NvmeCtrl *n)
+{
+uint32_t nsid = 0;
+for (int i = 1; i <= NVME_MAX_NAMESPACES; i++) {
+if (nvme_ns(n, i) || nvme_subsys_ns(n->subsys, i)) {
+continue;
+}
+
+nsid = i;
+return nsid;
+}
+return nsid;
+}
+
+static uint16_t nvme_namespace_create(NvmeCtrl *n, NvmeRequest *req)
+{
+uint32_t ret;
+NvmeIdNs id_ns_host;
+NvmeSubsystem *subsys = n->subsys;
+Error *err = NULL;
+uint8_t flbas_host;
+uint64_t ns_size;
+int lba_index;
+NvmeNamespace *ns;
+NvmeCtrl *ctrl;
+NvmeIdNs *id_ns;
+
+ret = nvme_h2c(n, (uint8_t *)_ns_host, sizeof(id_ns_host), req);
+if (ret) {
+return ret;
+}
+
+if (id_ns_host.ncap < id_ns_host.nsze) {
+return NVME_THIN_PROVISION_NO_SUPP | NVME_DNR;
+} else if (id_ns_host.ncap > id_ns_host.nsze) {
+return NVME_INVALID_FIELD | NVME_DNR;
+}
+
+if (!id_ns_host.nsze) {
+return NVME_INVALID_FIELD | NVME_DNR;
+}
+
+if (QSLIST_EMPTY(>unallocated_namespaces)) {
+return NVME_NS_ID_UNAVAILABLE;
+}
+
+ns = QSLIST_FIRST(>unallocated_namespaces);
+id_ns = >id_ns;
+flbas_host = (id_ns_host.flbas) & (0xF);
+
+if (flbas_host > id_ns->nlbaf) {
+return NVME_INVALID_FORMAT | NVME_DNR;
+}
+
+ret = nvme_ns_setup(ns, );
+if (ret) {
+return ret;
+}
+
+id_ns->flbas = id_ns_host.flbas;
+id_ns->dps = id_ns_host.dps;
+id_ns->nmic = id_ns_host.nmic;
+
+lba_index =
[PATCH v9 7/8] qmp: add QMP command x-query-virtio-queue-element
From: Laurent Vivier
This new command shows the information of a VirtQueue element.
Signed-off-by: Jonah Palmer
---
hw/virtio/virtio-stub.c | 9 +++
hw/virtio/virtio.c | 154 +
qapi/virtio.json| 177
3 files changed, 340 insertions(+)
diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
index 3484b1f..810317c 100644
--- a/hw/virtio/virtio-stub.c
+++ b/hw/virtio/virtio-stub.c
@@ -31,3 +31,12 @@ VirtQueueStatus *qmp_x_query_virtio_queue_status(const char
*path,
{
return qmp_virtio_unsupported(errp);
}
+
+VirtioQueueElement *qmp_x_query_virtio_queue_element(const char *path,
+ uint16_t queue,
+ bool has_index,
+ uint16_t index,
+ Error **errp)
+{
+return qmp_virtio_unsupported(errp);
+}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 160cc90..6df6a85 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -480,6 +480,19 @@ static inline void vring_used_write(VirtQueue *vq,
VRingUsedElem *uelem,
address_space_cache_invalidate(>used, pa, sizeof(VRingUsedElem));
}
+/* Called within rcu_read_lock(). */
+static inline uint16_t vring_used_flags(VirtQueue *vq)
+{
+VRingMemoryRegionCaches *caches = vring_get_region_caches(vq);
+hwaddr pa = offsetof(VRingUsed, flags);
+
+if (!caches) {
+return 0;
+}
+
+return virtio_lduw_phys_cached(vq->vdev, >used, pa);
+}
+
/* Called within rcu_read_lock(). */
static uint16_t vring_used_idx(VirtQueue *vq)
{
@@ -4392,6 +4405,147 @@ VirtQueueStatus *qmp_x_query_virtio_queue_status(const
char *path,
return status;
}
+static VirtioRingDescFlagsList *qmp_decode_vring_desc_flags(uint16_t flags)
+{
+VirtioRingDescFlagsList *list = NULL;
+VirtioRingDescFlagsList *node;
+int i;
+
+struct {
+uint16_t flag;
+VirtioRingDescFlags value;
+} map[] = {
+{ VRING_DESC_F_NEXT, VIRTIO_RING_DESC_FLAGS_NEXT },
+{ VRING_DESC_F_WRITE, VIRTIO_RING_DESC_FLAGS_WRITE },
+{ VRING_DESC_F_INDIRECT, VIRTIO_RING_DESC_FLAGS_INDIRECT },
+{ 1 << VRING_PACKED_DESC_F_AVAIL, VIRTIO_RING_DESC_FLAGS_AVAIL },
+{ 1 << VRING_PACKED_DESC_F_USED, VIRTIO_RING_DESC_FLAGS_USED },
+{ 0, -1 }
+};
+
+for (i = 0; map[i].flag; i++) {
+if ((map[i].flag & flags) == 0) {
+continue;
+}
+node = g_malloc0(sizeof(VirtioRingDescFlagsList));
+node->value = map[i].value;
+node->next = list;
+list = node;
+}
+
+return list;
+}
+
+VirtioQueueElement *qmp_x_query_virtio_queue_element(const char *path,
+ uint16_t queue,
+ bool has_index,
+ uint16_t index,
+ Error **errp)
+{
+VirtIODevice *vdev;
+VirtQueue *vq;
+VirtioQueueElement *element = NULL;
+
+vdev = virtio_device_find(path);
+if (vdev == NULL) {
+error_setg(errp, "Path %s is not a VirtIO device", path);
+return NULL;
+}
+
+if (queue >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev, queue)) {
+error_setg(errp, "Invalid virtqueue number %d", queue);
+return NULL;
+}
+vq = >vq[queue];
+
+if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
+error_setg(errp, "Packed ring not supported");
+return NULL;
+} else {
+unsigned int head, i, max;
+VRingMemoryRegionCaches *caches;
+MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
+MemoryRegionCache *desc_cache;
+VRingDesc desc;
+VirtioRingDescList *list = NULL;
+VirtioRingDescList *node;
+int rc;
+
+RCU_READ_LOCK_GUARD();
+
+max = vq->vring.num;
+
+if (!has_index) {
+head = vring_avail_ring(vq, vq->last_avail_idx % vq->vring.num);
+} else {
+head = vring_avail_ring(vq, index % vq->vring.num);
+}
+i = head;
+
+caches = vring_get_region_caches(vq);
+if (!caches) {
+error_setg(errp, "Region caches not initialized");
+return NULL;
+}
+if (caches->desc.len < max * sizeof(VRingDesc)) {
+error_setg(errp, "Cannot map descriptor ring");
+return NULL;
+}
+
+desc_cache = >desc;
+vring_split_desc_read(vdev, , desc_cache, i);
+if (desc.flags & VRING_DESC_F_INDIRECT) {
+int64_t len;
+len = address_space_cache_init(_desc_cache, vdev->dma_as,
+ desc.addr, desc.len, false);
+
[PATCH v9 5/8] qmp: decode feature & status bits in virtio-status
From: Laurent Vivier
Display feature names instead of bitmaps for host, guest, and
backend for VirtIODevice.
Display status names instead of bitmaps for VirtIODevice.
Display feature names instead of bitmaps for backend, protocol,
acked, and features (hdev->features) for vhost devices.
Decode features according to device type. Decode status
according to configuration status bitmap (config_status_map).
Decode vhost user protocol features according to vhost user
protocol bitmap (vhost_user_protocol_map).
Transport features are on the first line. Undecoded bits
(if any) are stored in a separate field. Vhost device field
wont show if there's no vhost active for a given VirtIODevice.
Signed-off-by: Jonah Palmer
---
hw/block/virtio-blk.c | 28 ++
hw/char/virtio-serial-bus.c| 11 +
hw/display/virtio-gpu-base.c | 18 +-
hw/input/virtio-input.c| 11 +-
hw/net/virtio-net.c| 47
hw/scsi/virtio-scsi.c | 17 ++
hw/virtio/vhost-user-fs.c | 10 +
hw/virtio/vhost-vsock-common.c | 10 +
hw/virtio/virtio-balloon.c | 14 +
hw/virtio/virtio-crypto.c | 10 +
hw/virtio/virtio-iommu.c | 14 +
hw/virtio/virtio-mem.c | 11 +
hw/virtio/virtio.c | 278 +++-
include/hw/virtio/vhost.h | 3 +
include/hw/virtio/virtio.h | 18 ++
qapi/virtio.json | 580 ++---
16 files changed, 1035 insertions(+), 45 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 505e574..c2e901f 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -13,6 +13,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
+#include "qapi/qapi-visit-virtio.h"
#include "qemu/iov.h"
#include "qemu/module.h"
#include "qemu/error-report.h"
@@ -32,6 +33,7 @@
#include "hw/virtio/virtio-bus.h"
#include "migration/qemu-file-types.h"
#include "hw/virtio/virtio-access.h"
+#include "standard-headers/linux/vhost_types.h"
/* Config size before the discard support (hide associated config fields) */
#define VIRTIO_BLK_CFG_SIZE offsetof(struct virtio_blk_config, \
@@ -48,6 +50,32 @@ static const VirtIOFeature feature_sizes[] = {
{}
};
+qmp_virtio_feature_map_t blk_map[] = {
+#define FEATURE_ENTRY(name) \
+{ VIRTIO_BLK_F_##name, VIRTIO_BLK_FEATURE_##name }
+FEATURE_ENTRY(SIZE_MAX),
+FEATURE_ENTRY(SEG_MAX),
+FEATURE_ENTRY(GEOMETRY),
+FEATURE_ENTRY(RO),
+FEATURE_ENTRY(BLK_SIZE),
+FEATURE_ENTRY(TOPOLOGY),
+FEATURE_ENTRY(MQ),
+FEATURE_ENTRY(DISCARD),
+FEATURE_ENTRY(WRITE_ZEROES),
+#ifndef VIRTIO_BLK_NO_LEGACY
+FEATURE_ENTRY(BARRIER),
+FEATURE_ENTRY(SCSI),
+FEATURE_ENTRY(FLUSH),
+FEATURE_ENTRY(CONFIG_WCE),
+#endif /* !VIRTIO_BLK_NO_LEGACY */
+#undef FEATURE_ENTRY
+#define FEATURE_ENTRY(name) \
+{ VHOST_F_##name, VIRTIO_BLK_FEATURE_##name }
+FEATURE_ENTRY(LOG_ALL),
+#undef FEATURE_ENTRY
+{ -1, -1 }
+};
+
static void virtio_blk_set_config_size(VirtIOBlock *s, uint64_t host_features)
{
s->config_size = MAX(VIRTIO_BLK_CFG_SIZE,
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 232f4c9..fa57059 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -20,6 +20,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
+#include "qapi/qapi-visit-virtio.h"
#include "qemu/iov.h"
#include "qemu/main-loop.h"
#include "qemu/module.h"
@@ -32,6 +33,16 @@
#include "hw/virtio/virtio-serial.h"
#include "hw/virtio/virtio-access.h"
+qmp_virtio_feature_map_t serial_map[] = {
+#define FEATURE_ENTRY(name) \
+{ VIRTIO_CONSOLE_F_##name, VIRTIO_SERIAL_FEATURE_##name }
+FEATURE_ENTRY(SIZE),
+FEATURE_ENTRY(MULTIPORT),
+FEATURE_ENTRY(EMERG_WRITE),
+#undef FEATURE_ENTRY
+{ -1, -1 }
+};
+
static struct VirtIOSerialDevices {
QLIST_HEAD(, VirtIOSerial) devices;
} vserdevices;
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 5411a7b..a322349 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -12,13 +12,29 @@
*/
#include "qemu/osdep.h"
-
+#include "standard-headers/linux/vhost_types.h"
#include "hw/virtio/virtio-gpu.h"
#include "migration/blocker.h"
#include "qapi/error.h"
+#include "qapi/qapi-visit-virtio.h"
#include "qemu/error-report.h"
#include "trace.h"
+qmp_virtio_feature_map_t gpu_map[] = {
+#define FEATURE_ENTRY(name) \
+{ VIRTIO_GPU_F_##name, VIRTIO_GPU_FEATURE_##name }
+FEATURE_ENTRY(VIRGL),
+FEATURE_ENTRY(EDID),
+FEATURE_ENTRY(RESOURCE_UUID),
+FEATURE_ENTRY(RESOURCE_BLOB),
+#undef FEATURE_ENTRY
+#define FEATURE_ENTRY(name) \
+{ VHOST_F_##name, VIRTIO_GPU_FEATURE_##name }
+FEATURE_ENTRY(LOG_ALL),
+#undef FEATURE_ENTRY
+{ -1, -1 }
+};
+
void
virtio_gpu_base_reset(VirtIOGPUBase *g)
{
diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
index 5b5398b..b4562a3 100644
--- a/hw/input/virtio-input.c
+++
[PATCH v9 4/8] qmp: add QMP command x-query-virtio-status
From: Laurent Vivier
This new command shows the status of a VirtIODevice, including
its corresponding vhost device status (if active).
Next patch will improve output by decoding feature bits, including
vhost device's feature bits (backend, protocol, acked, and features).
Also will decode status bits of a VirtIODevice.
Next patch will also suppress the vhost device field from displaying
if no vhost device is active for a given VirtIODevice.
Signed-off-by: Jonah Palmer
---
hw/virtio/virtio-stub.c | 5 +
hw/virtio/virtio.c | 96 +++
qapi/virtio.json| 246
3 files changed, 347 insertions(+)
diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
index 05a81ed..acd4148 100644
--- a/hw/virtio/virtio-stub.c
+++ b/hw/virtio/virtio-stub.c
@@ -12,3 +12,8 @@ VirtioInfoList *qmp_x_query_virtio(Error **errp)
{
return qmp_virtio_unsupported(errp);
}
+
+VirtioStatus *qmp_x_query_virtio_status(const char* path, Error **errp)
+{
+return qmp_virtio_unsupported(errp);
+}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index aad554b..580d9a8 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3933,6 +3933,102 @@ VirtioInfoList *qmp_x_query_virtio(Error **errp)
return list;
}
+static VirtIODevice *virtio_device_find(const char *path)
+{
+VirtIODevice *vdev;
+
+QTAILQ_FOREACH(vdev, _list, next) {
+DeviceState *dev = DEVICE(vdev);
+
+if (strcmp(dev->canonical_path, path) != 0) {
+continue;
+}
+return vdev;
+}
+
+return NULL;
+}
+
+VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
+{
+VirtIODevice *vdev;
+VirtioStatus *status;
+
+vdev = virtio_device_find(path);
+if (vdev == NULL) {
+error_setg(errp, "Path %s is not a VirtIO device", path);
+return NULL;
+}
+
+status = g_new0(VirtioStatus, 1);
+status->vhost_dev = g_new0(VhostStatus, 1);
+status->name = g_strdup(vdev->name);
+status->device_id = vdev->device_id;
+status->vhost_started = vdev->vhost_started;
+status->guest_features = vdev->guest_features;
+status->host_features = vdev->host_features;
+status->backend_features = vdev->backend_features;
+
+switch (vdev->device_endian) {
+case VIRTIO_DEVICE_ENDIAN_LITTLE:
+status->device_endian = VIRTIO_STATUS_ENDIANNESS_LITTLE;
+break;
+case VIRTIO_DEVICE_ENDIAN_BIG:
+status->device_endian = VIRTIO_STATUS_ENDIANNESS_BIG;
+break;
+default:
+status->device_endian = VIRTIO_STATUS_ENDIANNESS_UNKNOWN;
+break;
+}
+
+status->num_vqs = virtio_get_num_queues(vdev);
+status->status = vdev->status;
+status->isr = vdev->isr;
+status->queue_sel = vdev->queue_sel;
+status->vm_running = vdev->vm_running;
+status->broken = vdev->broken;
+status->disabled = vdev->disabled;
+status->use_started = vdev->use_started;
+status->started = vdev->started;
+status->start_on_kick = vdev->start_on_kick;
+status->disable_legacy_check = vdev->disable_legacy_check;
+status->bus_name = g_strdup(vdev->bus_name);
+status->use_guest_notifier_mask = vdev->use_guest_notifier_mask;
+
+if (vdev->vhost_started) {
+VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+struct vhost_dev *hdev = vdc->get_vhost(vdev);
+
+status->vhost_dev->n_mem_sections = hdev->n_mem_sections;
+status->vhost_dev->n_tmp_sections = hdev->n_tmp_sections;
+status->vhost_dev->nvqs = hdev->nvqs;
+status->vhost_dev->vq_index = hdev->vq_index;
+status->vhost_dev->features = hdev->features;
+status->vhost_dev->acked_features = hdev->acked_features;
+status->vhost_dev->backend_features = hdev->backend_features;
+status->vhost_dev->protocol_features = hdev->protocol_features;
+status->vhost_dev->max_queues = hdev->max_queues;
+status->vhost_dev->backend_cap = hdev->backend_cap;
+status->vhost_dev->log_enabled = hdev->log_enabled;
+status->vhost_dev->log_size = hdev->log_size;
+} else {
+status->vhost_dev->n_mem_sections = 0;
+status->vhost_dev->n_tmp_sections = 0;
+status->vhost_dev->nvqs = 0;
+status->vhost_dev->vq_index = 0;
+status->vhost_dev->features = 0;
+status->vhost_dev->acked_features = 0;
+status->vhost_dev->backend_features = 0;
+status->vhost_dev->protocol_features = 0;
+status->vhost_dev->max_queues = 0;
+status->vhost_dev->backend_cap = 0;
+status->vhost_dev->log_enabled = false;
+status->vhost_dev->log_size = 0;
+}
+
+return status;
+}
+
static const TypeInfo virtio_device_info = {
.name = TYPE_VIRTIO_DEVICE,
.parent = TYPE_DEVICE,
diff --git a/qapi/virtio.json b/qapi/virtio.json
index 324ba8c..54212f2 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@
[PATCH v9 8/8] hmp: add virtio commands
From: Laurent Vivier
This patch implements the HMP versions of the virtio QMP commands.
Signed-off-by: Jonah Palmer
---
hmp-commands-info.hx | 218 ++
include/monitor/hmp.h | 5 +
monitor/hmp-cmds.c| 358 ++
3 files changed, 581 insertions(+)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 407a1da..6bf7359 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -877,3 +877,221 @@ SRST
``info sgx``
Show intel SGX information.
ERST
+
+{
+.name = "virtio",
+.args_type = "",
+.params= "",
+.help = "List all available virtio devices",
+.cmd = hmp_virtio_query,
+.flags = "p",
+},
+
+SRST
+ ``info virtio``
+List all available virtio devices
+
+Example:
+
+List all available virtio devices in the machine::
+
+(qemu) info virtio
+/machine/peripheral/vsock0/virtio-backend [vhost-vsock]
+/machine/peripheral/crypto0/virtio-backend [virtio-crypto]
+/machine/peripheral-anon/device[2]/virtio-backend [virtio-scsi]
+/machine/peripheral-anon/device[1]/virtio-backend [virtio-net]
+/machine/peripheral-anon/device[0]/virtio-backend [virtio-serial]
+
+ERST
+
+{
+.name = "virtio-status",
+.args_type = "path:s",
+.params= "path",
+.help = "Display status of a given virtio device",
+.cmd = hmp_virtio_status,
+.flags = "p",
+},
+
+SRST
+ ``info virtio-status`` *path*
+Display status of a given virtio device
+
+Example:
+
+Dump the status of virtio-net (vhost on)::
+
+(qemu) info virtio-status /machine/peripheral-anon/device[1]/virtio-backend
+/machine/peripheral-anon/device[1]/virtio-backend:
+device_name: virtio-net (vhost)
+device_id: 1
+vhost_started: true
+bus_name:(null)
+broken: false
+disabled:false
+disable_legacy_check:false
+started: true
+use_started: true
+start_on_kick: false
+use_guest_notifier_mask: true
+vm_running: true
+num_vqs: 3
+queue_sel: 2
+isr: 1
+endianness: little
+status: acknowledge, driver, features-ok, driver-ok
+Guest features: event-idx, indirect-desc, version-1
+ ctrl-mac-addr, guest-announce, ctrl-vlan, ctrl-rx,
ctrl-vq, status, mrg-rxbuf,
+ host-ufo, host-ecn, host-tso6, host-tso4, guest-ufo,
guest-ecn, guest-tso6,
+ guest-tso4, mac, ctrl-guest-offloads, guest-csum,
csum
+Host features:protocol-features, event-idx, indirect-desc,
version-1, any-layout,
+ notify-on-empty, gso, ctrl-mac-addr, guest-announce,
ctrl-rx-extra, ctrl-vlan,
+ ctrl-rx, ctrl-vq, status, mrg-rxbuf, host-ufo,
host-ecn, host-tso6, host-tso4,
+ guest-ufo, guest-ecn, guest-tso6, guest-tso4, mac,
ctrl-guest-offloads,
+ guest-csum, csum
+Backend features: protocol-features, event-idx, indirect-desc,
version-1, any-layout,
+ notify-on-empty, gso, ctrl-mac-addr, guest-announce,
ctrl-rx-extra, ctrl-vlan,
+ ctrl-rx, ctrl-vq, status, mrg-rxbuf, host-ufo,
host-ecn, host-tso6, host-tso4,
+ guest-ufo, guest-ecn, guest-tso6, guest-tso4, mac,
ctrl-guest-offloads,
+ guest-csum, csum
+VHost:
+ nvqs: 2
+ vq_index: 0
+ max_queues: 1
+ n_mem_sections: 4
+ n_tmp_sections: 4
+ backend_cap:2
+ log_enabled:false
+ log_size: 0
+ Features: event-idx, indirect-desc, iommu-platform, version-1,
any-layout,
+ notify-on-empty, log-all, mrg-rxbuf
+Acked features: event-idx, indirect-desc, version-1, mrg-rxbuf
+Backend features:
+Protocol features:
+
+ERST
+
+{
+.name = "virtio-queue-status",
+.args_type = "path:s,queue:i",
+.params= "path queue",
+.help = "Display status of a given virtio queue",
+.cmd = hmp_virtio_queue_status,
+.flags = "p",
+},
+
+SRST
+ ``info virtio-queue-status`` *path* *queue*
+Display status of a given virtio queue
+
+Example:
+
+Dump the status of the 6th queue of virtio-scsi::
+
+(qemu) info virtio-queue-status
/machine/peripheral-anon/device[2]/virtio-backend 5
+/machine/peripheral-anon/device[2]/virtio-backend:
+device_name: virtio-scsi
+queue_index:
[PATCH v9 2/8] virtio: add vhost support for virtio devices
This patch adds a get_vhost() callback function for VirtIODevices that
returns the device's corresponding vhost_dev structure if the vhost
device is running. This patch also adds a vhost_started flag for VirtIODevices.
Previously, a VirtIODevice wouldn't be able to tell if its corresponding
vhost device was active or not.
Signed-off-by: Jonah Palmer
---
hw/block/vhost-user-blk.c | 7 +++
hw/display/vhost-user-gpu.c| 7 +++
hw/input/vhost-user-input.c| 7 +++
hw/net/virtio-net.c| 9 +
hw/scsi/vhost-scsi.c | 8
hw/virtio/vhost-user-fs.c | 7 +++
hw/virtio/vhost-user-rng.c | 7 +++
hw/virtio/vhost-vsock-common.c | 7 +++
hw/virtio/vhost.c | 3 +++
hw/virtio/virtio-crypto.c | 10 ++
hw/virtio/virtio.c | 1 +
include/hw/virtio/virtio.h | 3 +++
12 files changed, 76 insertions(+)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index f61f8c1..b059da1 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -568,6 +568,12 @@ static void vhost_user_blk_instance_init(Object *obj)
"/disk@0,0", DEVICE(obj));
}
+static struct vhost_dev *vhost_user_blk_get_vhost(VirtIODevice *vdev)
+{
+VHostUserBlk *s = VHOST_USER_BLK(vdev);
+return >dev;
+}
+
static const VMStateDescription vmstate_vhost_user_blk = {
.name = "vhost-user-blk",
.minimum_version_id = 1,
@@ -602,6 +608,7 @@ static void vhost_user_blk_class_init(ObjectClass *klass,
void *data)
vdc->get_features = vhost_user_blk_get_features;
vdc->set_status = vhost_user_blk_set_status;
vdc->reset = vhost_user_blk_reset;
+vdc->get_vhost = vhost_user_blk_get_vhost;
}
static const TypeInfo vhost_user_blk_info = {
diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index 49df56c..6e93b46 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -565,6 +565,12 @@ vhost_user_gpu_device_realize(DeviceState *qdev, Error
**errp)
g->vhost_gpu_fd = -1;
}
+static struct vhost_dev *vhost_user_gpu_get_vhost(VirtIODevice *vdev)
+{
+VhostUserGPU *g = VHOST_USER_GPU(vdev);
+return >vhost->dev;
+}
+
static Property vhost_user_gpu_properties[] = {
VIRTIO_GPU_BASE_PROPERTIES(VhostUserGPU, parent_obj.conf),
DEFINE_PROP_END_OF_LIST(),
@@ -586,6 +592,7 @@ vhost_user_gpu_class_init(ObjectClass *klass, void *data)
vdc->guest_notifier_pending = vhost_user_gpu_guest_notifier_pending;
vdc->get_config = vhost_user_gpu_get_config;
vdc->set_config = vhost_user_gpu_set_config;
+vdc->get_vhost = vhost_user_gpu_get_vhost;
device_class_set_props(dc, vhost_user_gpu_properties);
}
diff --git a/hw/input/vhost-user-input.c b/hw/input/vhost-user-input.c
index 273e96a..43d2ff3 100644
--- a/hw/input/vhost-user-input.c
+++ b/hw/input/vhost-user-input.c
@@ -79,6 +79,12 @@ static void vhost_input_set_config(VirtIODevice *vdev,
virtio_notify_config(vdev);
}
+static struct vhost_dev *vhost_input_get_vhost(VirtIODevice *vdev)
+{
+VHostUserInput *vhi = VHOST_USER_INPUT(vdev);
+return >vhost->dev;
+}
+
static const VMStateDescription vmstate_vhost_input = {
.name = "vhost-user-input",
.unmigratable = 1,
@@ -93,6 +99,7 @@ static void vhost_input_class_init(ObjectClass *klass, void
*data)
dc->vmsd = _vhost_input;
vdc->get_config = vhost_input_get_config;
vdc->set_config = vhost_input_set_config;
+vdc->get_vhost = vhost_input_get_vhost;
vic->realize = vhost_input_realize;
vic->change_active = vhost_input_change_active;
}
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index b275acf..2449b9c 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3610,6 +3610,14 @@ static bool dev_unplug_pending(void *opaque)
return vdc->primary_unplug_pending(dev);
}
+static struct vhost_dev *virtio_net_get_vhost(VirtIODevice *vdev)
+{
+VirtIONet *n = VIRTIO_NET(vdev);
+NetClientState *nc = qemu_get_queue(n->nic);
+struct vhost_net *net = get_vhost_net(nc->peer);
+return >dev;
+}
+
static const VMStateDescription vmstate_virtio_net = {
.name = "virtio-net",
.minimum_version_id = VIRTIO_NET_VM_VERSION,
@@ -3712,6 +3720,7 @@ static void virtio_net_class_init(ObjectClass *klass,
void *data)
vdc->post_load = virtio_net_post_load_virtio;
vdc->vmsd = _virtio_net_device;
vdc->primary_unplug_pending = primary_unplug_pending;
+vdc->get_vhost = virtio_net_get_vhost;
}
static const TypeInfo virtio_net_info = {
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 039caf2..b0a9c45 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -264,6 +264,13 @@ static void vhost_scsi_unrealize(DeviceState *dev)
virtio_scsi_common_unrealize(dev);
}
+static struct vhost_dev *vhost_scsi_get_vhost(VirtIODevice *vdev)
+{
+VHostSCSI *s = VHOST_SCSI(vdev);
+
[PATCH v9 1/8] virtio: drop name parameter for virtio_init()
This patch drops the name parameter for the virtio_init function.
The pair between the numeric device ID and the string device ID
(name) of a virtio device already exists, but not in a way that
lets us map between them.
This patch will let us do this and removes the need for the name
parameter in virtio_init().
Signed-off-by: Jonah Palmer
---
hw/9pfs/virtio-9p-device.c | 2 +-
hw/block/vhost-user-blk.c | 2 +-
hw/block/virtio-blk.c | 2 +-
hw/char/virtio-serial-bus.c | 4 +--
hw/display/virtio-gpu-base.c| 2 +-
hw/input/virtio-input.c | 3 +-
hw/net/virtio-net.c | 2 +-
hw/scsi/virtio-scsi.c | 3 +-
hw/virtio/vhost-user-fs.c | 3 +-
hw/virtio/vhost-user-i2c.c | 6 +---
hw/virtio/vhost-user-rng.c | 2 +-
hw/virtio/vhost-user-vsock.c| 2 +-
hw/virtio/vhost-vsock-common.c | 4 +--
hw/virtio/vhost-vsock.c | 2 +-
hw/virtio/virtio-balloon.c | 3 +-
hw/virtio/virtio-crypto.c | 2 +-
hw/virtio/virtio-iommu.c| 3 +-
hw/virtio/virtio-mem.c | 3 +-
hw/virtio/virtio-pmem.c | 3 +-
hw/virtio/virtio-rng.c | 2 +-
hw/virtio/virtio.c | 45 +++--
include/hw/virtio/vhost-vsock-common.h | 2 +-
include/hw/virtio/virtio-gpu.h | 3 +-
include/hw/virtio/virtio.h | 3 +-
include/standard-headers/linux/virtio_ids.h | 1 +
25 files changed, 68 insertions(+), 41 deletions(-)
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 54ee93b..5f522e6 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -216,7 +216,7 @@ static void virtio_9p_device_realize(DeviceState *dev,
Error **errp)
}
v->config_size = sizeof(struct virtio_9p_config) + strlen(s->fsconf.tag);
-virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P, v->config_size);
+virtio_init(vdev, VIRTIO_ID_9P, v->config_size);
v->vq = virtio_add_queue(vdev, MAX_REQ, handle_9p_output);
}
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index ba13cb8..f61f8c1 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -490,7 +490,7 @@ static void vhost_user_blk_device_realize(DeviceState *dev,
Error **errp)
return;
}
-virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
+virtio_init(vdev, VIRTIO_ID_BLOCK,
sizeof(struct virtio_blk_config));
s->virtqs = g_new(VirtQueue *, s->num_queues);
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index f139cd7..505e574 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1213,7 +1213,7 @@ static void virtio_blk_device_realize(DeviceState *dev,
Error **errp)
virtio_blk_set_config_size(s, s->host_features);
-virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, s->config_size);
+virtio_init(vdev, VIRTIO_ID_BLOCK, s->config_size);
s->blk = conf->conf.blk;
s->rq = NULL;
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index f01ec21..232f4c9 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -1044,8 +1044,8 @@ static void virtio_serial_device_realize(DeviceState
*dev, Error **errp)
VIRTIO_CONSOLE_F_EMERG_WRITE)) {
config_size = offsetof(struct virtio_console_config, emerg_wr);
}
-virtio_init(vdev, "virtio-serial", VIRTIO_ID_CONSOLE,
-config_size);
+
+virtio_init(vdev, VIRTIO_ID_CONSOLE, config_size);
/* Spawn a new virtio-serial bus on which the ports will ride as devices */
qbus_init(>bus, sizeof(vser->bus), TYPE_VIRTIO_SERIAL_BUS,
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index c8da480..5411a7b 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -170,7 +170,7 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
}
g->virtio_config.num_scanouts = cpu_to_le32(g->conf.max_outputs);
-virtio_init(VIRTIO_DEVICE(g), "virtio-gpu", VIRTIO_ID_GPU,
+virtio_init(VIRTIO_DEVICE(g), VIRTIO_ID_GPU,
sizeof(struct virtio_gpu_config));
if (virtio_gpu_virgl_enabled(g->conf)) {
diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
index 54bcb46..5b5398b 100644
--- a/hw/input/virtio-input.c
+++ b/hw/input/virtio-input.c
@@ -257,8 +257,7 @@ static void virtio_input_device_realize(DeviceState *dev,
Error **errp)
vinput->cfg_size += 8;
assert(vinput->cfg_size <= sizeof(virtio_input_config));
-virtio_init(vdev, "virtio-input", VIRTIO_ID_INPUT,
-vinput->cfg_size);
+virtio_init(vdev, VIRTIO_ID_INPUT, vinput->cfg_size);
vinput->evt =
[PATCH v9 0/8] hmp,qmp: Add commands to introspect virtio devices
This series introduces new QMP/HMP commands to dump the status of a
virtio device at different levels.
[Jonah: Rebasing previous patchset from Oct. 27 (v8). Original patches
are from Laurent Vivier from May 2020.
Rebase from v8 to v9 includes renaming of 'x-' prefixed QMP commands,
adding the 'unstable' feature to 'x-' prefixed QMP commands, fixes to
odd indentation in qapi/virtio.json, support for virtio-mem features,
and moving HMP 'virtio' sub-commands to 'info' sub-commands.]
1. List available virtio devices in the machine
HMP Form:
info virtio
Example:
(qemu) info virtio
/machine/peripheral/vsock0/virtio-backend [vhost-vsock]
/machine/peripheral/crypto0/virtio-backend [virtio-crypto]
/machine/peripheral-anon/device[2]/virtio-backend [virtio-scsi]
/machine/peripheral-anon/device[1]/virtio-backend [virtio-net]
/machine/peripheral-anon/device[0]/virtio-backend [virtio-serial]
QMP Form:
{ 'command': 'x-query-virtio', 'returns': ['VirtioInfo'] }
Example:
-> { "execute": "x-query-virtio" }
<- { "return": [
{
"path": "/machine/peripheral/vsock0/virtio-backend",
"type": "vhost-vsock"
},
{
"path": "/machine/peripheral/crypto0/virtio-backend",
"type": "virtio-crypto"
},
{
"path": "/machine/peripheral-anon/device[2]/virtio-backend",
"type": "virtio-scsi"
},
{
"path": "/machine/peripheral-anon/device[1]/virtio-backend",
"type": "virtio-net"
},
{
"path": "/machine/peripheral-anon/device[0]/virtio-backend",
"type": "virtio-serial"
}
]
}
2. Display status of a given virtio device
HMP Form:
info virtio-status
Example:
(qemu) info virtio-status /machine/peripheral/vsock0/virtio-backend
/machine/peripheral/vsock0/virtio-backend:
device_name: vhost-vsock (vhost)
device_id: 19
vhost_started: true
bus_name:(null)
broken: false
disabled:false
disable_legacy_check:false
started: true
use_started: true
start_on_kick: false
use_guest_notifier_mask: true
vm_running: true
num_vqs: 3
queue_sel: 2
isr: 0
endianness: little
status: acknowledge, driver, features-ok, driver-ok
Guest features: event-idx, indirect-desc, version-1
Host features:protocol-features, event-idx, indirect-desc,
version-1, any-layout,
notify-on-empty
Backend features:
VHost:
nvqs: 2
vq_index: 0
max_queues: 0
n_mem_sections: 4
n_tmp_sections: 4
backend_cap:0
log_enabled:false
log_size: 0
Features: event-idx, indirect-desc, version-1,
any-layout, notify-on-empty,
log-all
Acked features:event-idx, indirect-desc, version-1
Backend features:
Protocol features:
QMP Form:
{ 'command': 'x-query-virtio-status',
'data': { 'path': 'str' },
'returns': 'VirtioStatus'
}
Example:
-> { "execute": "x-query-virtio-status",
"arguments": {
"path": "/machine/peripheral/vsock0/virtio-backend"
}
}
<- { "return": {
"device-endian": "little",
"bus-name": "",
"disable-legacy-check": false,
"name": "vhost-vsock",
"started": true,
"device-id": 19,
"vhost-dev": {
"n-tmp-sections": 4,
"n-mem-sections": 4,
"max-queues": 0,
"backend-cap": 0,
"log-size": 0,
"backend-features": {
"transport": [],
"type": "vhost-vsock",
"features": []
},
"nvqs": 2,
"protocol-features": {
"features": []
},
"vq-index": 0,
"log-enabled": false,
"acked-features": {
"transport":
[PATCH v9 3/8] qmp: add QMP command x-query-virtio
From: Laurent Vivier
This new command lists all the instances of VirtIODevice with
their QOM paths and virtio type/name.
Signed-off-by: Jonah Palmer
---
hw/virtio/meson.build | 2 ++
hw/virtio/virtio-stub.c| 14
hw/virtio/virtio.c | 27 +++
include/hw/virtio/virtio.h | 1 +
qapi/meson.build | 1 +
qapi/qapi-schema.json | 1 +
qapi/virtio.json | 55 ++
tests/qtest/qmp-cmd-test.c | 1 +
8 files changed, 102 insertions(+)
create mode 100644 hw/virtio/virtio-stub.c
create mode 100644 qapi/virtio.json
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 521f7d6..d893f5f 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -6,8 +6,10 @@ softmmu_virtio_ss.add(when: 'CONFIG_VHOST', if_false:
files('vhost-stub.c'))
softmmu_ss.add_all(when: 'CONFIG_VIRTIO', if_true: softmmu_virtio_ss)
softmmu_ss.add(when: 'CONFIG_VIRTIO', if_false: files('vhost-stub.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO', if_false: files('virtio-stub.c'))
softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('vhost-stub.c'))
+softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('virtio-stub.c'))
virtio_ss = ss.source_set()
virtio_ss.add(files('virtio.c'))
diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
new file mode 100644
index 000..05a81ed
--- /dev/null
+++ b/hw/virtio/virtio-stub.c
@@ -0,0 +1,14 @@
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-virtio.h"
+
+static void *qmp_virtio_unsupported(Error **errp)
+{
+error_setg(errp, "Virtio is disabled");
+return NULL;
+}
+
+VirtioInfoList *qmp_x_query_virtio(Error **errp)
+{
+return qmp_virtio_unsupported(errp);
+}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 7050bd5..aad554b 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -13,6 +13,8 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
+#include "qapi/qapi-commands-virtio.h"
+#include "qapi/qapi-visit-virtio.h"
#include "cpu.h"
#include "trace.h"
#include "qemu/error-report.h"
@@ -29,6 +31,9 @@
#include "sysemu/runstate.h"
#include "standard-headers/linux/virtio_ids.h"
+/* QAPI list of VirtIODevices */
+static QTAILQ_HEAD(, VirtIODevice) virtio_list;
+
/*
* The alignment to use between consumer and producer parts of vring.
* x86 pagesize again. This is the default, used by transports like PCI
@@ -3709,6 +3714,7 @@ static void virtio_device_realize(DeviceState *dev, Error
**errp)
vdev->listener.commit = virtio_memory_listener_commit;
vdev->listener.name = "virtio";
memory_listener_register(>listener, vdev->dma_as);
+QTAILQ_INSERT_TAIL(_list, vdev, next);
}
static void virtio_device_unrealize(DeviceState *dev)
@@ -3723,6 +3729,7 @@ static void virtio_device_unrealize(DeviceState *dev)
vdc->unrealize(dev);
}
+QTAILQ_REMOVE(_list, vdev, next);
g_free(vdev->bus_name);
vdev->bus_name = NULL;
}
@@ -3896,6 +3903,8 @@ static void virtio_device_class_init(ObjectClass *klass,
void *data)
vdc->stop_ioeventfd = virtio_device_stop_ioeventfd_impl;
vdc->legacy_features |= VIRTIO_LEGACY_FEATURES;
+
+QTAILQ_INIT(_list);
}
bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev)
@@ -3906,6 +3915,24 @@ bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev)
return virtio_bus_ioeventfd_enabled(vbus);
}
+VirtioInfoList *qmp_x_query_virtio(Error **errp)
+{
+VirtioInfoList *list = NULL;
+VirtioInfoList *node;
+VirtIODevice *vdev;
+
+QTAILQ_FOREACH(vdev, _list, next) {
+DeviceState *dev = DEVICE(vdev);
+node = g_new0(VirtioInfoList, 1);
+node->value = g_new(VirtioInfo, 1);
+node->value->path = g_strdup(dev->canonical_path);
+node->value->type = g_strdup(vdev->name);
+QAPI_LIST_PREPEND(list, node->value);
+}
+
+return list;
+}
+
static const TypeInfo virtio_device_info = {
.name = TYPE_VIRTIO_DEVICE,
.parent = TYPE_DEVICE,
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 105b98c..eceaafc 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -110,6 +110,7 @@ struct VirtIODevice
bool use_guest_notifier_mask;
AddressSpace *dma_as;
QLIST_HEAD(, VirtQueue) *vector_queues;
+QTAILQ_ENTRY(VirtIODevice) next;
};
struct VirtioDeviceClass {
diff --git a/qapi/meson.build b/qapi/meson.build
index c0c49c1..e332f28 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -48,6 +48,7 @@ qapi_all_modules = [
'sockets',
'trace',
'transaction',
+ 'virtio',
'yank',
]
if have_system
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 4912b97..1512ada 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -93,3 +93,4 @@
{ 'include': 'audio.json' }
{ 'include': 'acpi.json' }
{ 'include': 'pci.json' }
+{ 'include': 'virtio.json' }
diff --git
[PATCH v9 6/8] qmp: add QMP commands for virtio/vhost queue-status
From: Laurent Vivier
These new commands show the internal status of a VirtIODevice's
VirtQueue and a vhost device's vhost_virtqueue (if active).
Signed-off-by: Jonah Palmer
---
hw/virtio/virtio-stub.c | 14 +++
hw/virtio/virtio.c | 103
qapi/virtio.json| 250
3 files changed, 367 insertions(+)
diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
index acd4148..3484b1f 100644
--- a/hw/virtio/virtio-stub.c
+++ b/hw/virtio/virtio-stub.c
@@ -17,3 +17,17 @@ VirtioStatus *qmp_x_query_virtio_status(const char* path,
Error **errp)
{
return qmp_virtio_unsupported(errp);
}
+
+VirtVhostQueueStatus *qmp_x_query_virtio_vhost_queue_status(const char *path,
+uint16_t queue,
+Error **errp)
+{
+return qmp_virtio_unsupported(errp);
+}
+
+VirtQueueStatus *qmp_x_query_virtio_queue_status(const char *path,
+ uint16_t queue,
+ Error **errp)
+{
+return qmp_virtio_unsupported(errp);
+}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index b3b3578..160cc90 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -4289,6 +4289,109 @@ VirtioStatus *qmp_x_query_virtio_status(const char
*path, Error **errp)
return status;
}
+VirtVhostQueueStatus *qmp_x_query_virtio_vhost_queue_status(const char *path,
+uint16_t queue,
+Error **errp)
+{
+VirtIODevice *vdev;
+VirtVhostQueueStatus *status;
+
+vdev = virtio_device_find(path);
+if (vdev == NULL) {
+error_setg(errp, "Path %s is not a VirtIODevice", path);
+return NULL;
+}
+
+if (!vdev->vhost_started) {
+error_setg(errp, "Error: vhost device has not started yet");
+return NULL;
+}
+
+VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+struct vhost_dev *hdev = vdc->get_vhost(vdev);
+
+if (queue < hdev->vq_index || queue >= hdev->vq_index + hdev->nvqs) {
+error_setg(errp, "Invalid vhost virtqueue number %d", queue);
+return NULL;
+}
+
+status = g_new0(VirtVhostQueueStatus, 1);
+status->device_name = g_strdup(vdev->name);
+status->kick = hdev->vqs[queue].kick;
+status->call = hdev->vqs[queue].call;
+status->desc = (uint64_t)(unsigned long)hdev->vqs[queue].desc;
+status->avail = (uint64_t)(unsigned long)hdev->vqs[queue].avail;
+status->used = (uint64_t)(unsigned long)hdev->vqs[queue].used;
+status->num = hdev->vqs[queue].num;
+status->desc_phys = hdev->vqs[queue].desc_phys;
+status->desc_size = hdev->vqs[queue].desc_size;
+status->avail_phys = hdev->vqs[queue].avail_phys;
+status->avail_size = hdev->vqs[queue].avail_size;
+status->used_phys = hdev->vqs[queue].used_phys;
+status->used_size = hdev->vqs[queue].used_size;
+
+return status;
+}
+
+VirtQueueStatus *qmp_x_query_virtio_queue_status(const char *path,
+ uint16_t queue,
+ Error **errp)
+{
+VirtIODevice *vdev;
+VirtQueueStatus *status;
+
+vdev = virtio_device_find(path);
+if (vdev == NULL) {
+error_setg(errp, "Path %s is not a VirtIODevice", path);
+return NULL;
+}
+
+if (queue >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev, queue)) {
+error_setg(errp, "Invalid virtqueue number %d", queue);
+return NULL;
+}
+
+status = g_new0(VirtQueueStatus, 1);
+status->device_name = g_strdup(vdev->name);
+status->queue_index = vdev->vq[queue].queue_index;
+status->inuse = vdev->vq[queue].inuse;
+status->vring_num = vdev->vq[queue].vring.num;
+status->vring_num_default = vdev->vq[queue].vring.num_default;
+status->vring_align = vdev->vq[queue].vring.align;
+status->vring_desc = vdev->vq[queue].vring.desc;
+status->vring_avail = vdev->vq[queue].vring.avail;
+status->vring_used = vdev->vq[queue].vring.used;
+status->used_idx = vdev->vq[queue].used_idx;
+status->signalled_used = vdev->vq[queue].signalled_used;
+status->signalled_used_valid = vdev->vq[queue].signalled_used_valid;
+
+if (vdev->vhost_started) {
+VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+struct vhost_dev *hdev = vdc->get_vhost(vdev);
+
+/* check if vq index exists for vhost as well */
+if (queue >= hdev->vq_index && queue < hdev->vq_index + hdev->nvqs) {
+status->has_last_avail_idx = true;
+
+int vhost_vq_index =
+hdev->vhost_ops->vhost_get_vq_index(hdev, queue);
+struct vhost_vring_state state = {
+.index = vhost_vq_index,
+};
+
+
Re: [PATCH v2 0/3] virtio: increase VIRTQUEUE_MAX_SIZE to 32k
On Tue, Nov 09, 2021 at 02:09:59PM +0100, Christian Schoenebeck wrote: > On Dienstag, 9. November 2021 11:56:35 CET Stefan Hajnoczi wrote: > > On Thu, Nov 04, 2021 at 03:41:23PM +0100, Christian Schoenebeck wrote: > > > On Mittwoch, 3. November 2021 12:33:33 CET Stefan Hajnoczi wrote: > > > > On Mon, Nov 01, 2021 at 09:29:26PM +0100, Christian Schoenebeck wrote: > > > > > On Donnerstag, 28. Oktober 2021 11:00:48 CET Stefan Hajnoczi wrote: > > > > > > On Mon, Oct 25, 2021 at 05:03:25PM +0200, Christian Schoenebeck > wrote: > > > > > > > On Montag, 25. Oktober 2021 12:30:41 CEST Stefan Hajnoczi wrote: > > > > > > > > On Thu, Oct 21, 2021 at 05:39:28PM +0200, Christian Schoenebeck > wrote: > > > > > > > > > On Freitag, 8. Oktober 2021 18:08:48 CEST Christian > Schoenebeck wrote: > > > > > > > > > > On Freitag, 8. Oktober 2021 16:24:42 CEST Christian > Schoenebeck wrote: > > > > > > > > > > > On Freitag, 8. Oktober 2021 09:25:33 CEST Greg Kurz wrote: > > > > > > > > > > > > On Thu, 7 Oct 2021 16:42:49 +0100 > > > > > > > > > > > > > > > > > > > > > > > > Stefan Hajnoczi wrote: > > > > > > > > > > > > > On Thu, Oct 07, 2021 at 02:51:55PM +0200, Christian > Schoenebeck wrote: > > > > > > > > > > > > > > On Donnerstag, 7. Oktober 2021 07:23:59 CEST Stefan > Hajnoczi wrote: > > > > > > > > > > > > > > > On Mon, Oct 04, 2021 at 09:38:00PM +0200, > > > > > > > > > > > > > > > Christian > > > > > > > > > > > > > > > Schoenebeck > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > At the moment the maximum transfer size with > > > > > > > > > > > > > > > > virtio > > > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > limited > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > 4M > > > > > > > > > > > > > > > > (1024 * PAGE_SIZE). This series raises this > > > > > > > > > > > > > > > > limit to > > > > > > > > > > > > > > > > its > > > > > > > > > > > > > > > > maximum > > > > > > > > > > > > > > > > theoretical possible transfer size of 128M (32k > > > > > > > > > > > > > > > > pages) > > > > > > > > > > > > > > > > according > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > virtio specs: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://docs.oasis-open.org/virtio/virtio/v1.1/c > > > > > > > > > > > > > > > > s01/ > > > > > > > > > > > > > > > > virt > > > > > > > > > > > > > > > > io-v > > > > > > > > > > > > > > > > 1.1- > > > > > > > > > > > > > > > > cs > > > > > > > > > > > > > > > > 01 > > > > > > > > > > > > > > > > .html# > > > > > > > > > > > > > > > > x1-240006 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Christian, > > > > > > > > > > > > > > > > > > > > > > > > > > > I took a quick look at the code: > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > > > > > Thanks Stefan for sharing virtio expertise and helping > > > > > > > > > > > > Christian > > > > > > > > > > > > ! > > > > > > > > > > > > > > > > > > > > > > > > > > > - The Linux 9p driver restricts descriptor chains > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > 128 > > > > > > > > > > > > > > > elements > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (net/9p/trans_virtio.c:VIRTQUEUE_NUM) > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, that's the limitation that I am about to remove > > > > > > > > > > > > > > (WIP); > > > > > > > > > > > > > > current > > > > > > > > > > > > > > kernel > > > > > > > > > > > > > > patches: > > > > > > > > > > > > > > https://lore.kernel.org/netdev/cover.1632327421.git. > > > > > > > > > > > > > > linu > > > > > > > > > > > > > > x_os > > > > > > > > > > > > > > s@cr > > > > > > > > > > > > > > udeb > > > > > > > > > > > > > > yt > > > > > > > > > > > > > > e. > > > > > > > > > > > > > > com/> > > > > > > > > > > > > > > > > > > > > > > > > > > I haven't read the patches yet but I'm concerned that > > > > > > > > > > > > > today > > > > > > > > > > > > > the > > > > > > > > > > > > > driver > > > > > > > > > > > > > is pretty well-behaved and this new patch series > > > > > > > > > > > > > introduces a > > > > > > > > > > > > > spec > > > > > > > > > > > > > violation. Not fixing existing spec violations is > > > > > > > > > > > > > okay, > > > > > > > > > > > > > but > > > > > > > > > > > > > adding > > > > > > > > > > > > > new > > > > > > > > > > > > > ones is a red flag. I think we need to figure out a > > > > > > > > > > > > > clean > > > > > > > > > > > > > solution. > > > > > > > > > > > > > > > > > > > > > > Nobody has reviewed the kernel patches yet. My main > > > > > > > > > > > concern > > > > > > > > > > > therefore > > > > > > > > > > > actually is that the kernel patches are already too > > > > > > > > > > > complex, > > > > > > > > > > > because > > > > > > > > > > > the > > > > > > > > > > > current situation is that only Dominique is handling 9p > > > > > > > > > > > patches on >
Re: [PULL 0/3] jobs: deprecate drive-backup qmp command
On 11/9/21 6:52 PM, Vladimir Sementsov-Ogievskiy wrote: The following changes since commit 2b22e7540d6ab4efe82d442363e3fc900cea6584: Merge tag 'm68k-for-6.2-pull-request' of git://github.com/vivier/qemu-m68k into staging (2021-11-09 13:16:56 +0100) are available in the Git repository at: https://src.openvz.org/scm/~vsementsov/qemu.git tags/pull-jobs-2021-11-09 for you to fetch changes up to 1084159b31015e003946d199cbfecaec282e0eb2: qapi: deprecate drive-backup (2021-11-09 18:21:19 +0100) qmp: deprecate drive-backup (use blockdev-backup instead) Vladimir Sementsov-Ogievskiy (3): docs/block-replication: use blockdev-backup docs/interop/bitmaps: use blockdev-backup qapi: deprecate drive-backup docs/about/deprecated.rst | 11 ++ docs/block-replication.txt | 4 +- docs/interop/bitmaps.rst | 285 - docs/interop/live-block-operations.rst | 47 +++--- qapi/block-core.json | 5 +- qapi/transaction.json | 6 +- 6 files changed, 268 insertions(+), 90 deletions(-) Applied, thanks. r~
