On 07/21/2016 11:54 AM, Cornelia Huck wrote:
On Wed, 20 Jul 2016 18:28:21 +0300
Marcel Apfelbaum <mar...@redhat.com> wrote:
Enable transitional virtio devices by default.
Enable virtio-1.0 for devices plugged into
PCIe ports (Root ports or Downstream ports).
Hi Cornelia,
Thank you for the review.
Add "by default", as this can still be overridden?
Yes, using -device virtio*,disable-modern=x,disable-legacy=y
are respected as before.
Using the virtio-1 mode will remove the limitation
s/Using the virtio-1 mode/Disabling the legacy mode/
?
Well, the way I see it virtio-1 'pure' is not using the IO BAR.
This is why virtio-1 == disable-modern=off && disable-legacy=on IMHO.
If you or Michael see this differently I have nothing against re-wording it.
of the number of devices that can be attached to a machine
by removing the need for the IO BAR.
Signed-off-by: Marcel Apfelbaum <mar...@redhat.com>
(...)
+static inline bool virtio_pci_modern(VirtIOPCIProxy *proxy)
+{
+ return !proxy->disable_modern;
+}
+
+static inline bool virtio_pci_legacy(VirtIOPCIProxy *proxy)
+{
+ return proxy->disable_legacy == ON_OFF_AUTO_OFF;
+}
+
+static inline void virtio_pci_force_virtio_1(VirtIOPCIProxy *proxy)
One thing I still find a bit confusing is that you refer to 'modern'
above, but force to 'virtio_1' here... but that's a minor thing.
I went for 'virtio-1' because of the existing comments (force virtio-1)
and also because 'modern' does not imply "no legacy" - those are independent
flags.
BTW, instead of the 'disable*' properties (which I find hard to follow) I would
go for one
property : "mode" with "legacy"/"transitional"/"virtio-1"/"auto" values.
But is too late for that (at least for 2.7).
+{
+ proxy->disable_modern = false;
+ proxy->disable_legacy = ON_OFF_AUTO_ON;
+}
/*
* virtio-scsi-pci: This extends VirtioPCIProxy.
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 9914e7a..1531399 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -6,6 +6,14 @@
.driver = "virtio-mmio",\
.property = "format_transport_address",\
.value = "off",\
+ },{\
+ .driver = "virtio-pci",\
+ .property = "disable-modern",\
+ .value = "on",\
+ },{\
+ .driver = "virtio-pci",\
+ .property = "disable-legacy",\
+ .value = "off",\
After looking at the code, I think this will work - did you test this
with a compat machine, though?
Yes, I tested it with pc/q35 2.5 and 2.6 machines. The previous
behavior remains the same.
},
#define HW_COMPAT_2_5 \
But generally, looks good and I think this is also an improvement in
readability.
Thanks!
Marcel
