Re: [virt-tools-list] [virt-manager PATCH 0/9] Add RISC-V support
On 4/4/19 6:49 AM, Andrea Bolognani wrote: > With this series, virt-manager gains support for RISC-V guests > on par with other architectures, including graphics and input > devices. > > Andrea Bolognani (9): > tests: Add RISC-V support > tests: Add riscv64-headless and riscv64-graphics > os: Add RISC-V support > guest: Recommend virt machine for RISC-V > guest: RISC-V virt guests have VirtIO support > guest: RISC-V virt guests support virtio-rng > video: RISC-V virt guests support virtio-gpu > guest: Enable USB for RISC-V virt guests > guest: Enable USB input devices for RISC-V virt guests > Reviewed-by: Cole Robinson And pushed Thanks, Cole ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-manager PATCH 0/9] Add RISC-V support
With this series, virt-manager gains support for RISC-V guests on par with other architectures, including graphics and input devices. Andrea Bolognani (9): tests: Add RISC-V support tests: Add riscv64-headless and riscv64-graphics os: Add RISC-V support guest: Recommend virt machine for RISC-V guest: RISC-V virt guests have VirtIO support guest: RISC-V virt guests support virtio-rng video: RISC-V virt guests support virtio-gpu guest: Enable USB for RISC-V virt guests guest: Enable USB input devices for RISC-V virt guests .../capabilities-xml/qemu-riscv64-domcaps.xml | 91 ++ tests/capabilities-xml/qemu-riscv64.xml | 113 ++ .../compare/virt-install-riscv64-graphics.xml | 45 +++ .../compare/virt-install-riscv64-headless.xml | 39 ++ tests/clitest.py | 3 + tests/utils.py| 2 + virtinst/devices/video.py | 2 +- virtinst/domain/os.py | 5 + virtinst/guest.py | 18 ++- virtinst/support.py | 1 + 10 files changed, 317 insertions(+), 2 deletions(-) create mode 100644 tests/capabilities-xml/qemu-riscv64-domcaps.xml create mode 100644 tests/capabilities-xml/qemu-riscv64.xml create mode 100644 tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml create mode 100644 tests/cli-test-xml/compare/virt-install-riscv64-headless.xml -- 2.20.1 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-manager PATCH 4/9] guest: Recommend virt machine for RISC-V
The default machine at the QEMU level is spike_v1.10, but most people will really want to use the virt machine type instead. Signed-off-by: Andrea Bolognani --- tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml | 2 +- tests/cli-test-xml/compare/virt-install-riscv64-headless.xml | 2 +- virtinst/guest.py| 4 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml b/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml index 8a9f7a73..ff7324a9 100644 --- a/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml +++ b/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml @@ -10,7 +10,7 @@ 65536 1 -hvm +hvm diff --git a/tests/cli-test-xml/compare/virt-install-riscv64-headless.xml b/tests/cli-test-xml/compare/virt-install-riscv64-headless.xml index 663081c9..ef0ffb88 100644 --- a/tests/cli-test-xml/compare/virt-install-riscv64-headless.xml +++ b/tests/cli-test-xml/compare/virt-install-riscv64-headless.xml @@ -10,7 +10,7 @@ 65536 1 -hvm +hvm diff --git a/virtinst/guest.py b/virtinst/guest.py index 0538ccfd..254aaa58 100644 --- a/virtinst/guest.py +++ b/virtinst/guest.py @@ -129,6 +129,10 @@ class Guest(XMLBuilder): if "s390-ccw-virtio" in capsinfo.machines: return "s390-ccw-virtio" +if capsinfo.arch in ["riscv64", "riscv32"]: +if "virt" in capsinfo.machines: +return "virt" + if capsinfo.conn.is_qemu() or capsinfo.conn.is_test(): return _qemu_machine() return None -- 2.20.1 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-manager PATCH 8/9] guest: Enable USB for RISC-V virt guests
Unlike other features we have enabled earlier, this one requires
version checks because RISC-V guests have only started using PCI
by default very recently, and we can't have USB without PCI.
More specifically, we need QEMU commit d6c1bd4a2237 (included
in 4.0.0) and libvirt commit 7c48fb08e0cd (included in 5.3.0).
Signed-off-by: Andrea Bolognani
---
.../compare/virt-install-riscv64-graphics.xml | 1 +
.../compare/virt-install-riscv64-headless.xml | 1 +
tests/clitest.py | 4 ++--
virtinst/guest.py | 8
virtinst/support.py | 1 +
5 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml
b/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml
index 211c062e..2f5f0e7a 100644
--- a/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml
+++ b/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml
@@ -21,6 +21,7 @@
+
diff --git a/tests/cli-test-xml/compare/virt-install-riscv64-headless.xml
b/tests/cli-test-xml/compare/virt-install-riscv64-headless.xml
index ba0a48b0..875a4214 100644
--- a/tests/cli-test-xml/compare/virt-install-riscv64-headless.xml
+++ b/tests/cli-test-xml/compare/virt-install-riscv64-headless.xml
@@ -21,6 +21,7 @@
+
diff --git a/tests/clitest.py b/tests/clitest.py
index a7fc12a6..693b6a6e 100644
--- a/tests/clitest.py
+++ b/tests/clitest.py
@@ -811,7 +811,7 @@ c.add_compare("--connect %(URI-KVM-AARCH64)s --disk size=1
--os-variant fedora22
c = vinst.add_category("kvm-headless", "--os-variant fedora29 --import --disk
%(EXISTIMG1)s --network default --graphics none")
c.add_compare("--connect %(URI-KVM-AARCH64)s --arch aarch64",
"aarch64-headless")
c.add_compare("--connect %(URI-KVM-PPC64LE)s --arch ppc64le", "ppc64-headless")
-c.add_compare("--connect %(URI-QEMU-RISCV64)s --arch riscv64",
"riscv64-headless")
+c.add_compare("--connect %(URI-QEMU-RISCV64)s --arch riscv64",
"riscv64-headless", check_version="5.3.0")
c.add_compare("--connect %(URI-KVM-S390X)s --arch s390x", "s390x-headless")
c.add_compare("--connect %(URI-KVM)s --arch x86_64", "x86_64-headless")
@@ -820,7 +820,7 @@ c.add_compare("--connect %(URI-KVM)s --arch x86_64",
"x86_64-headless")
c = vinst.add_category("kvm-graphics", "--os-variant fedora29 --import --disk
%(EXISTIMG1)s --network default --graphics vnc")
c.add_compare("--connect %(URI-KVM-AARCH64)s --arch aarch64",
"aarch64-graphics")
c.add_compare("--connect %(URI-KVM-PPC64LE)s --arch ppc64le", "ppc64-graphics")
-c.add_compare("--connect %(URI-QEMU-RISCV64)s --arch riscv64",
"riscv64-graphics")
+c.add_compare("--connect %(URI-QEMU-RISCV64)s --arch riscv64",
"riscv64-graphics", check_version="5.3.0")
c.add_compare("--connect %(URI-KVM-S390X)s --arch s390x", "s390x-graphics")
c.add_compare("--connect %(URI-KVM)s --arch x86_64", "x86_64-graphics")
diff --git a/virtinst/guest.py b/virtinst/guest.py
index 7249686e..9d38859c 100644
--- a/virtinst/guest.py
+++ b/virtinst/guest.py
@@ -784,6 +784,14 @@ class Guest(XMLBuilder):
self.conn.check_support(
self.conn.SUPPORT_CONN_MACHVIRT_PCI_DEFAULT)):
usb3 = True
+elif self.os.is_riscv_virt():
+# For RISC-V we can assume the guest OS supports USB3, but we
+# have to make sure libvirt and QEMU are new enough to be using
+# PCI by default
+if (qemu_usb3 and
+self.conn.check_support(
+self.conn.SUPPORT_CONN_RISCV_VIRT_PCI_DEFAULT)):
+usb3 = True
elif self.os.is_pseries():
# For pseries, we always assume OS supports usb3
if qemu_usb3:
diff --git a/virtinst/support.py b/virtinst/support.py
index 7ace3ef6..015a01b7 100644
--- a/virtinst/support.py
+++ b/virtinst/support.py
@@ -265,6 +265,7 @@ SUPPORT_CONN_MACHVIRT_PCI_DEFAULT = _make(version="3.0.0")
SUPPORT_CONN_QEMU_XHCI = _make(version="3.3.0", hv_version={"qemu": "2.9.0"})
SUPPORT_CONN_VNC_NONE_AUTH = _make(hv_version={"qemu": "2.9.0"})
SUPPORT_CONN_DEVICE_BOOT_ORDER = _make(hv_version={"qemu": 0, "test": 0})
+SUPPORT_CONN_RISCV_VIRT_PCI_DEFAULT = _make(version="5.3.0",
hv_version={"qemu": "4.0.0"})
# We choose qemu 2.11.0 as the first version to target for q35 default.
# That's not really based on anything except reasonably modern at the
--
2.20.1
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-manager PATCH 9/9] guest: Enable USB input devices for RISC-V virt guests
If USB support is available, we can use USB input devices too. Signed-off-by: Andrea Bolognani --- tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml | 2 ++ virtinst/guest.py| 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml b/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml index 2f5f0e7a..f2f1b842 100644 --- a/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml +++ b/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml @@ -32,6 +32,8 @@ + + diff --git a/virtinst/guest.py b/virtinst/guest.py index 9d38859c..d79b91eb 100644 --- a/virtinst/guest.py +++ b/virtinst/guest.py @@ -719,7 +719,9 @@ class Guest(XMLBuilder): usb_keyboard = False if self.os.is_x86() and not self.os.is_xenpv(): usb_tablet = self.osinfo.supports_usbtablet() -if self.os.is_arm_machvirt() or self.os.is_pseries(): +if (self.os.is_arm_machvirt() or +self.os.is_riscv_virt() or +self.os.is_pseries()): usb_tablet = True usb_keyboard = True -- 2.20.1 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-manager PATCH 1/9] tests: Add RISC-V support
RISC-V doesn't support KVM yet, so we are forced to use TCG
on x86 until that's working.
Signed-off-by: Andrea Bolognani
---
.../capabilities-xml/qemu-riscv64-domcaps.xml | 91 ++
tests/capabilities-xml/qemu-riscv64.xml | 113 ++
tests/clitest.py | 1 +
tests/utils.py| 2 +
4 files changed, 207 insertions(+)
create mode 100644 tests/capabilities-xml/qemu-riscv64-domcaps.xml
create mode 100644 tests/capabilities-xml/qemu-riscv64.xml
diff --git a/tests/capabilities-xml/qemu-riscv64-domcaps.xml
b/tests/capabilities-xml/qemu-riscv64-domcaps.xml
new file mode 100644
index ..6b1a7498
--- /dev/null
+++ b/tests/capabilities-xml/qemu-riscv64-domcaps.xml
@@ -0,0 +1,91 @@
+
+
+
+
+ /usr/bin/qemu-system-riscv64
+ qemu
+ spike_v1.10
+ riscv64
+
+
+
+
+ /usr/share/OVMF/OVMF_CODE.fd
+ /usr/share/OVMF/OVMF_CODE.secboot.fd
+
+rom
+pflash
+
+
+yes
+no
+
+
+
+
+
+
+
+
+
+
+
+disk
+cdrom
+floppy
+lun
+
+
+fdc
+scsi
+virtio
+usb
+sata
+
+
+virtio
+virtio-transitional
+virtio-non-transitional
+
+
+
+
+sdl
+vnc
+spice
+
+
+
+
+vga
+cirrus
+vmvga
+virtio
+
+
+
+
+subsystem
+
+
+default
+mandatory
+requisite
+optional
+
+
+usb
+pci
+scsi
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/capabilities-xml/qemu-riscv64.xml
b/tests/capabilities-xml/qemu-riscv64.xml
new file mode 100644
index ..c8b69f15
--- /dev/null
+++ b/tests/capabilities-xml/qemu-riscv64.xml
@@ -0,0 +1,113 @@
+
+
+
+
+
+2b763ba8-82a1-44dd-a197-9132a8d520a2
+
+ x86_64
+ Skylake-Client-IBRS
+ Intel
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+tcp
+rdma
+
+
+
+
+
+ 16298176
+ 4074544
+ 0
+ 0
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ selinux
+ 0
+ system_u:system_r:svirt_t:s0
+ system_u:system_r:svirt_tcg_t:s0
+
+
+ dac
+ 0
+ +0:+0
+ +0:+0
+
+
+
+
+hvm
+
+ 64
+ /usr/bin/qemu-system-riscv64
+ spike_v1.10
+ virt
+ sifive_u
+ sifive_e
+ spike_v1.9.1
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/clitest.py b/tests/clitest.py
index d3bd6044..41f10cd6 100644
--- a/tests/clitest.py
+++ b/tests/clitest.py
@@ -65,6 +65,7 @@ test_files = {
'URI-KVM-AARCH64': utils.URIs.kvm_aarch64,
'URI-KVM-PPC64LE': utils.URIs.kvm_ppc64le,
'URI-KVM-S390X': utils.URIs.kvm_s390x,
+'URI-QEMU-RISCV64': utils.URIs.qemu_riscv64,
'NEWIMG1': "/dev/default-pool/new1.img",
'NEWIMG2': "/dev/default-pool/new2.img",
diff --git a/tests/utils.py b/tests/utils.py
index 14b9d75c..f8d5b2ad 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -74,6 +74,7 @@ class _URIs(object):
_uri_kvm = _uri_qemu + _domcaps("kvm-x86_64-domcaps.xml")
_uri_kvm_q35 = _uri_qemu + _domcaps("kvm-x86_64-domcaps-q35.xml")
_uri_kvm_aarch64 = _uri_qemu + _domcaps("kvm-aarch64-domcaps.xml")
+_uri_qemu_riscv64 = _uri_qemu + _domcaps("qemu-riscv64-domcaps.xml")
self.kvm = _uri_kvm + _caps("kvm-x86_64.xml")
self.kvm_remote = _uri_kvm + _caps("kvm-x86_64.xml") + ",remote"
@@ -88,6 +89,7 @@ class _URIs(object):
self.kvm_ppc64le = _uri_kvm + _caps("kvm-ppc64le.xml")
self.kvm_s390x = _uri_kvm + _caps("kvm-s390x.xml")
self.kvm_s390x_KVMIBM = _uri_kvm + _caps("kvm-s390x-KVMIBM.xml")
+self.qemu_riscv64 = _uri_qemu_riscv64 + _caps("qemu-riscv64.xml")
--
2.20.1
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-manager PATCH 5/9] guest: RISC-V virt guests have VirtIO support
Signed-off-by: Andrea Bolognani --- .../cli-test-xml/compare/virt-install-riscv64-graphics.xml | 7 ++- .../cli-test-xml/compare/virt-install-riscv64-headless.xml | 7 ++- virtinst/guest.py | 1 + 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml b/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml index ff7324a9..803d857d 100644 --- a/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml +++ b/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml @@ -19,13 +19,18 @@ - + + + + + + diff --git a/tests/cli-test-xml/compare/virt-install-riscv64-headless.xml b/tests/cli-test-xml/compare/virt-install-riscv64-headless.xml index ef0ffb88..be29d059 100644 --- a/tests/cli-test-xml/compare/virt-install-riscv64-headless.xml +++ b/tests/cli-test-xml/compare/virt-install-riscv64-headless.xml @@ -19,12 +19,17 @@ - + + + + + + diff --git a/virtinst/guest.py b/virtinst/guest.py index 254aaa58..1f4e6a37 100644 --- a/virtinst/guest.py +++ b/virtinst/guest.py @@ -401,6 +401,7 @@ class Guest(XMLBuilder): # These _only_ support virtio so don't check the OS if (self.os.is_arm_machvirt() or +self.os.is_riscv_virt() or self.os.is_s390x() or self.os.is_pseries()): return True -- 2.20.1 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-manager PATCH 3/9] os: Add RISC-V support
Signed-off-by: Andrea Bolognani
---
virtinst/domain/os.py | 5 +
1 file changed, 5 insertions(+)
diff --git a/virtinst/domain/os.py b/virtinst/domain/os.py
index 32343ab6..eae7f71f 100644
--- a/virtinst/domain/os.py
+++ b/virtinst/domain/os.py
@@ -61,6 +61,11 @@ class DomainOs(XMLBuilder):
def is_s390x(self):
return self.arch == "s390x"
+def is_riscv(self):
+return self.arch == "riscv64" or self.arch == "riscv32"
+def is_riscv_virt(self):
+return self.is_riscv() and str(self.machine).startswith("virt")
+
XML_NAME = "os"
_XML_PROP_ORDER = ["arch", "os_type", "loader", "loader_ro", "loader_type",
"nvram", "nvram_template", "kernel", "initrd",
--
2.20.1
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-manager PATCH 7/9] video: RISC-V virt guests support virtio-gpu
QXL, on the other hand, is still x86-only for some reason. Signed-off-by: Andrea Bolognani --- tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml | 2 +- virtinst/devices/video.py| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml b/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml index 5186cc82..211c062e 100644 --- a/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml +++ b/tests/cli-test-xml/compare/virt-install-riscv64-graphics.xml @@ -33,7 +33,7 @@ - + /dev/urandom diff --git a/virtinst/devices/video.py b/virtinst/devices/video.py index 785d4127..91708935 100644 --- a/virtinst/devices/video.py +++ b/virtinst/devices/video.py @@ -43,7 +43,7 @@ class DeviceVideo(Device): def default_model(guest): if guest.os.is_pseries(): return "vga" -if guest.os.is_arm_machvirt(): +if guest.os.is_arm_machvirt() or guest.os.is_riscv_virt(): return "virtio" if guest.conn.is_qemu() and guest.os.is_s390x(): return "virtio" -- 2.20.1 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 5/5] virt-manager: add new checkbox to control CPU security features
On Thu, Apr 04, 2019 at 10:18:23AM +0100, Daniel P. Berrangé wrote: > On Wed, Apr 03, 2019 at 03:52:51PM +0200, Pavel Hrdina wrote: > > By default we copy CPU security features to the guest if specific CPU > > model is selected. However, this may break migration and will affect > > performance of the guest. This adds an option to disable this default > > behavior. > > > > The checkbox is clickable only on x86 and only on host where we can > > detect any CPU security features, otherwise a tooltip is set to notify > > users that there is nothing to copy. > > > > Signed-off-by: Pavel Hrdina > > --- > > ui/details.ui | 25 + > > virtManager/details.py | 21 + > > virtManager/domain.py | 5 +++-- > > virtinst/domain/cpu.py | 30 ++ > > 4 files changed, 79 insertions(+), 2 deletions(-) > > > > diff --git a/ui/details.ui b/ui/details.ui > > index 8b3f939e..2ae1c104 100644 > > --- a/ui/details.ui > > +++ b/ui/details.ui > > @@ -2122,6 +2122,31 @@ > > > name="top_attach">1 > > > > > > + > > + > id="cpu-secure-label"> > > + > name="visible">True > > + > name="can_focus">False > > + > translatable="yes">Copy host security features: > > I'd probably change the label to > > "Enable known CPU security flaw mitigations" "known" might be misleading since not all known mitigations might be available on the host, how about "available" instead? Otherwise looks better the suggested label. Thanks, Pavel signature.asc Description: PGP signature ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 4/5] domcapabilities: add caching of CPU security features
On Thu, Apr 04, 2019 at 10:17:10AM +0100, Daniel P. Berrangé wrote: > On Wed, Apr 03, 2019 at 03:52:50PM +0200, Pavel Hrdina wrote: > > We will call this function multiple times so it makes sense to cache the > > result so we don't have to call libvirt APIs every time we will check > > what security features are available on the host. > > > > Signed-off-by: Pavel Hrdina > > --- > > virtinst/domcapabilities.py | 11 --- > > 1 file changed, 8 insertions(+), 3 deletions(-) > > Arguably you can cache the entire capabilities API call for virt-install > since it won't change in the lifetime if a short virt-install > invokation and even if it does change you likely want virt-install to > have a consistent view of capabilities throughout its execution. We do the entire capabilities caching as well, the host capabilities are cached per connection and domain capabilities are cached per guest and refreshed if something changes like QEMU binary, machine type, etc. This adds caching of the baseline API call. > For virt-manager though being more dynamic is possibly more desirable > given its long life. > > Reviewed-by: Daniel P. Berrangé > > > Regards, > Daniel > -- > |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o-https://fstop138.berrange.com :| > |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| > > ___ > virt-tools-list mailing list > virt-tools-list@redhat.com > https://www.redhat.com/mailman/listinfo/virt-tools-list signature.asc Description: PGP signature ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 3/5] cli: introduce CPU secure parameter
On Thu, Apr 04, 2019 at 10:14:21AM +0100, Daniel P. Berrangé wrote: > On Wed, Apr 03, 2019 at 03:52:49PM +0200, Pavel Hrdina wrote: > > This will allow users to override the default behavior of virt-install > > which copies CPU security features available on the host to the guest > > XML if specific CPU model is configured. > > > > Signed-off-by: Pavel Hrdina > > --- > > man/virt-install.pod | 8 +- > > .../compare/virt-install-cpu-disable-sec.xml | 93 +++ > > tests/clitest.py | 1 + > > virtinst/cli.py | 1 + > > virtinst/domain/cpu.py| 7 +- > > 5 files changed, 108 insertions(+), 2 deletions(-) > > create mode 100644 > > tests/cli-test-xml/compare/virt-install-cpu-disable-sec.xml > > > > diff --git a/man/virt-install.pod b/man/virt-install.pod > > index 8407e795..18d44808 100644 > > --- a/man/virt-install.pod > > +++ b/man/virt-install.pod > > @@ -216,7 +216,13 @@ required value is MODEL, which is a valid CPU model as > > known to libvirt. > > > > Libvirt's feature policy values force, require, optional, disable, or > > forbid, > > or with the shorthand '+feature' and '-feature', which equal > > 'force=feature' > > -and 'disable=feature' respectively > > +and 'disable=feature' respectively. > > + > > +If exact CPU model is specified virt-install will automatically copy CPU > > +security features available on the host to mitigate recent CPU CVEs. > > I'd tweak it slightly to > > s/security features/features/ > > s/CPU CVEs/CPU speculative execution side channel security vulnerabilities./ > > > +This however will have some impact on performance and will break migration > > +to hosts without security patches. In order to turn off this default > > behavior > > +there is a B parameter. Possible values are I and I. > > At the end, add > > , with I as the default. It is highly recommended to leave this > enabled and ensure all virtualization hosts have fully up to date > microcode, kernel & virtualization software installed. Thanks, I'll tweak it before pushing. Pavel signature.asc Description: PGP signature ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 1/5] domcapabilities: remove recommended CPU features from security features
On Thu, Apr 04, 2019 at 10:10:44AM +0100, Daniel P. Berrangé wrote: > On Wed, Apr 03, 2019 at 03:52:47PM +0200, Pavel Hrdina wrote: > > These features are only recommended to be enabled since they improve > > performance of the VMs if security features are enabled. > > > > Signed-off-by: Pavel Hrdina > > --- > > tests/cli-test-xml/compare/virt-install-qemu-plain.xml | 2 -- > > .../compare/virt-install-singleton-config-2.xml | 4 > > virtinst/domcapabilities.py | 6 +- > > 3 files changed, 1 insertion(+), 11 deletions(-) > > > diff --git a/virtinst/domcapabilities.py b/virtinst/domcapabilities.py > > index d1b0f4ed..72844512 100644 > > --- a/virtinst/domcapabilities.py > > +++ b/virtinst/domcapabilities.py > > @@ -274,14 +274,10 @@ class DomainCapabilities(XMLBuilder): > > > > def get_cpu_security_features(self): > > sec_features = [ > > -'pcid', > > 'spec-ctrl', > > 'ssbd', > > -'pdpe1gb', > > 'ibpb', > > -'virt-ssbd', > > -'amd-ssbd', > > -'amd-no-ssb'] > > +'virt-ssbd'] > > This all makes sense - rationale for each removed one is: > > pcid is a very useful perf feature, but missing in some silicon > so not portable. > > pdpe1gb lets the guest use 1 GB pages which is good for perf > but again not all silicon can do it > > amd-ssbd is a security feature which fixes the same SSBD flaws as the > virt-ssbd feature does. virt-ssbd is usable across all CPU models > affected by SSBD, while amd-ssbd is only available in very new silicon. > So virt-ssbd is the bette rchoice. > > amd-no-ssb just indicates that the CPU is not affected by SSBD, so not > critical to expose. I expect a future named CPU model will include that > where appropriate. > > Reviewed-by: Daniel P. Berrangé Thanks, I'll add the rationale into the commit message. Pavel signature.asc Description: PGP signature ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 5/5] virt-manager: add new checkbox to control CPU security features
On Wed, Apr 03, 2019 at 03:52:51PM +0200, Pavel Hrdina wrote: > By default we copy CPU security features to the guest if specific CPU > model is selected. However, this may break migration and will affect > performance of the guest. This adds an option to disable this default > behavior. > > The checkbox is clickable only on x86 and only on host where we can > detect any CPU security features, otherwise a tooltip is set to notify > users that there is nothing to copy. > > Signed-off-by: Pavel Hrdina > --- > ui/details.ui | 25 + > virtManager/details.py | 21 + > virtManager/domain.py | 5 +++-- > virtinst/domain/cpu.py | 30 ++ > 4 files changed, 79 insertions(+), 2 deletions(-) > > diff --git a/ui/details.ui b/ui/details.ui > index 8b3f939e..2ae1c104 100644 > --- a/ui/details.ui > +++ b/ui/details.ui > @@ -2122,6 +2122,31 @@ > name="top_attach">1 > > > + > + id="cpu-secure-label"> > + name="visible">True > + name="can_focus">False > + translatable="yes">Copy host security features: I'd probably change the label to "Enable known CPU security flaw mitigations" Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 4/5] domcapabilities: add caching of CPU security features
On Wed, Apr 03, 2019 at 03:52:50PM +0200, Pavel Hrdina wrote: > We will call this function multiple times so it makes sense to cache the > result so we don't have to call libvirt APIs every time we will check > what security features are available on the host. > > Signed-off-by: Pavel Hrdina > --- > virtinst/domcapabilities.py | 11 --- > 1 file changed, 8 insertions(+), 3 deletions(-) Arguably you can cache the entire capabilities API call for virt-install since it won't change in the lifetime if a short virt-install invokation and even if it does change you likely want virt-install to have a consistent view of capabilities throughout its execution. For virt-manager though being more dynamic is possibly more desirable given its long life. Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 3/5] cli: introduce CPU secure parameter
On Thu, Apr 04, 2019 at 10:14:21AM +0100, Daniel P. Berrangé wrote: > On Wed, Apr 03, 2019 at 03:52:49PM +0200, Pavel Hrdina wrote: > > This will allow users to override the default behavior of virt-install > > which copies CPU security features available on the host to the guest > > XML if specific CPU model is configured. > > > > Signed-off-by: Pavel Hrdina > > --- > > man/virt-install.pod | 8 +- > > .../compare/virt-install-cpu-disable-sec.xml | 93 +++ > > tests/clitest.py | 1 + > > virtinst/cli.py | 1 + > > virtinst/domain/cpu.py| 7 +- > > 5 files changed, 108 insertions(+), 2 deletions(-) > > create mode 100644 > > tests/cli-test-xml/compare/virt-install-cpu-disable-sec.xml > > > > diff --git a/man/virt-install.pod b/man/virt-install.pod > > index 8407e795..18d44808 100644 > > --- a/man/virt-install.pod > > +++ b/man/virt-install.pod > > @@ -216,7 +216,13 @@ required value is MODEL, which is a valid CPU model as > > known to libvirt. > > > > Libvirt's feature policy values force, require, optional, disable, or > > forbid, > > or with the shorthand '+feature' and '-feature', which equal > > 'force=feature' > > -and 'disable=feature' respectively > > +and 'disable=feature' respectively. > > + > > +If exact CPU model is specified virt-install will automatically copy CPU > > +security features available on the host to mitigate recent CPU CVEs. > > I'd tweak it slightly to > > s/security features/features/ > > s/CPU CVEs/CPU speculative execution side channel security vulnerabilities./ > > > +This however will have some impact on performance and will break migration > > +to hosts without security patches. In order to turn off this default > > behavior > > +there is a B parameter. Possible values are I and I. > > At the end, add > > , with I as the default. It is highly recommended to leave this > enabled and ensure all virtualization hosts have fully up to date > microcode, kernel & virtualization software installed. With those changes applied Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 3/5] cli: introduce CPU secure parameter
On Wed, Apr 03, 2019 at 03:52:49PM +0200, Pavel Hrdina wrote: > This will allow users to override the default behavior of virt-install > which copies CPU security features available on the host to the guest > XML if specific CPU model is configured. > > Signed-off-by: Pavel Hrdina > --- > man/virt-install.pod | 8 +- > .../compare/virt-install-cpu-disable-sec.xml | 93 +++ > tests/clitest.py | 1 + > virtinst/cli.py | 1 + > virtinst/domain/cpu.py| 7 +- > 5 files changed, 108 insertions(+), 2 deletions(-) > create mode 100644 > tests/cli-test-xml/compare/virt-install-cpu-disable-sec.xml > > diff --git a/man/virt-install.pod b/man/virt-install.pod > index 8407e795..18d44808 100644 > --- a/man/virt-install.pod > +++ b/man/virt-install.pod > @@ -216,7 +216,13 @@ required value is MODEL, which is a valid CPU model as > known to libvirt. > > Libvirt's feature policy values force, require, optional, disable, or forbid, > or with the shorthand '+feature' and '-feature', which equal 'force=feature' > -and 'disable=feature' respectively > +and 'disable=feature' respectively. > + > +If exact CPU model is specified virt-install will automatically copy CPU > +security features available on the host to mitigate recent CPU CVEs. I'd tweak it slightly to s/security features/features/ s/CPU CVEs/CPU speculative execution side channel security vulnerabilities./ > +This however will have some impact on performance and will break migration > +to hosts without security patches. In order to turn off this default > behavior > +there is a B parameter. Possible values are I and I. At the end, add , with I as the default. It is highly recommended to leave this enabled and ensure all virtualization hosts have fully up to date microcode, kernel & virtualization software installed. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 1/5] domcapabilities: remove recommended CPU features from security features
On Wed, Apr 03, 2019 at 03:52:47PM +0200, Pavel Hrdina wrote: > These features are only recommended to be enabled since they improve > performance of the VMs if security features are enabled. > > Signed-off-by: Pavel Hrdina > --- > tests/cli-test-xml/compare/virt-install-qemu-plain.xml | 2 -- > .../compare/virt-install-singleton-config-2.xml | 4 > virtinst/domcapabilities.py | 6 +- > 3 files changed, 1 insertion(+), 11 deletions(-) > diff --git a/virtinst/domcapabilities.py b/virtinst/domcapabilities.py > index d1b0f4ed..72844512 100644 > --- a/virtinst/domcapabilities.py > +++ b/virtinst/domcapabilities.py > @@ -274,14 +274,10 @@ class DomainCapabilities(XMLBuilder): > > def get_cpu_security_features(self): > sec_features = [ > -'pcid', > 'spec-ctrl', > 'ssbd', > -'pdpe1gb', > 'ibpb', > -'virt-ssbd', > -'amd-ssbd', > -'amd-no-ssb'] > +'virt-ssbd'] This all makes sense - rationale for each removed one is: pcid is a very useful perf feature, but missing in some silicon so not portable. pdpe1gb lets the guest use 1 GB pages which is good for perf but again not all silicon can do it amd-ssbd is a security feature which fixes the same SSBD flaws as the virt-ssbd feature does. virt-ssbd is usable across all CPU models affected by SSBD, while amd-ssbd is only available in very new silicon. So virt-ssbd is the bette rchoice. amd-no-ssb just indicates that the CPU is not affected by SSBD, so not critical to expose. I expect a future named CPU model will include that where appropriate. Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 2/5] domcapabilities: fix typo in function name
On Wed, Apr 03, 2019 at 03:52:48PM +0200, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina > --- > virtinst/domcapabilities.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 0/5] CPU security features improvements
On Thu, Apr 04, 2019 at 10:00:21AM +0100, Peter Crowther wrote: > On Thu, 4 Apr 2019 at 09:15, Daniel P. Berrangé wrote: > > > On Wed, Apr 03, 2019 at 07:49:48PM -0400, Cole Robinson wrote: > > I > > think it is reasonable to assume that if the user has upgraded the > > microcode on some hosts they will have done it on all hosts. > > > Don't rely on it - certainly in the cases of the smaller organisations I > work with / audit, patching can be politely described as "haphazard". Even > in the large public-sector organisations I work with, there's almost never > money for test rigs that have the same hardware as the live hosts. It's > not uncommon for a live host to be the "guinea pig" receiving new microcode > before others, then returned to live service. > > The real world is not tidy :-(. Sure, that's why this series proposes a command line option to let the user disable the security features if they are in a messy situations. > > If they > > have not upgraded on all hosts, I still think it is sensible to > > apply the security mitigations on the hosts which have been upgraded > > unless the user explicitly says to use the insecure mode. > > > > Agree. > > We should do the right thing out of the box to enable the > > security mitigations. The fact that virt-intsall doesn't do this for > > named CPU models is arguably worthy of filing a CVE against virt-install > > itself. > > > > Agree. > > > > > Regards, > > Daniel > > > > - Peter Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 0/5] CPU security features improvements
On Thu, 4 Apr 2019 at 09:15, Daniel P. Berrangé wrote: > On Wed, Apr 03, 2019 at 07:49:48PM -0400, Cole Robinson wrote: > I > think it is reasonable to assume that if the user has upgraded the > microcode on some hosts they will have done it on all hosts. Don't rely on it - certainly in the cases of the smaller organisations I work with / audit, patching can be politely described as "haphazard". Even in the large public-sector organisations I work with, there's almost never money for test rigs that have the same hardware as the live hosts. It's not uncommon for a live host to be the "guinea pig" receiving new microcode before others, then returned to live service. The real world is not tidy :-(. If they > have not upgraded on all hosts, I still think it is sensible to > apply the security mitigations on the hosts which have been upgraded > unless the user explicitly says to use the insecure mode. > Agree. We should do the right thing out of the box to enable the > security mitigations. The fact that virt-intsall doesn't do this for > named CPU models is arguably worthy of filing a CVE against virt-install > itself. > Agree. > > Regards, > Daniel > - Peter ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 0/5] CPU security features improvements
On Wed, Apr 03, 2019 at 07:49:48PM -0400, Cole Robinson wrote: > ccing danpb > > On 4/3/19 9:52 AM, Pavel Hrdina wrote: > > Pavel Hrdina (5): > > domcapabilities: remove recommended CPU features from security > > features > > domcapabilities: fix typo in function name > > cli: introduce CPU secure parameter > > domcapabilities: add caching of CPU security features > > virt-manager: add new checkbox to control CPU security features > > > > So the bug spawning the original work is: > > https://bugzilla.redhat.com/show_bug.cgi?id=1582667 > > Some CPU model names lack security critical CPU flags. Example > Opteron_G5 lacks ibpb. Even if the user is running on an Opteron_G5 host > CPU which has ibpb, if they select Opteron_G5 the guest doesn't see it. > > In that case, the existing patches in git will check to see if the host > supports ibpb, and if so, manually specify that in the > config. So now use of --cpu Opteron_G5 is secure by default. > > But the problem is this creates a migration compat issue: selecting > --cpu Opteron_G5 means different things for different hosts. So these > patches basically make the 'secure' additions the default > behavior, but gives users the option to turn it off on the cli and > virt-manager UI. You are correct about migration compat in the general case, but I think it is reasonable to assume that if the user has upgraded the microcode on some hosts they will have done it on all hosts. If they have not upgraded on all hosts, I still think it is sensible to apply the security mitigations on the hosts which have been upgraded unless the user explicitly says to use the insecure mode. > Taking a step back, I don't really get what the usecase is for users to > be selecting manual cpu model names with virt-manager or virt-install. I > understand the migration compat thing, if you need to generate a > baseline CPU that works for migrating a machine across multiple hosts, > but in practice that seems like pretty advanced usage for people using > virt-manager or even virt-install, and if they are using virt-install > they have all the tools at their disposal to generate a full + secure > config if they want. I think using a explicit CPU model is quite a reasonable thing todo. It isn't solely live migration it helps. Even save/restore where you restore the image on 2nd machine wants a portable CPU model. It may not be the out of the box common case, but I don't think we should write it off as "advanced" usage and require people to take extra steps to figure out which security flags they need to set themselves. > Anyways, since the current code requires the user to explicitly opt in > to choosing a manual CPU model name, what if instead we just warn them? For these hardware security flaws I don't think it is acceptable to merely warn the user. We should do the right thing out of the box to enable the security mitigations. The fact that virt-intsall doesn't do this for named CPU models is arguably worthy of filing a CVE against virt-install itself. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 0/5] CPU security features improvements
On Wed, Apr 03, 2019 at 07:49:48PM -0400, Cole Robinson wrote: > ccing danpb > > On 4/3/19 9:52 AM, Pavel Hrdina wrote: > > Pavel Hrdina (5): > > domcapabilities: remove recommended CPU features from security > > features > > domcapabilities: fix typo in function name > > cli: introduce CPU secure parameter > > domcapabilities: add caching of CPU security features > > virt-manager: add new checkbox to control CPU security features > > > > So the bug spawning the original work is: > > https://bugzilla.redhat.com/show_bug.cgi?id=1582667 > > Some CPU model names lack security critical CPU flags. Example > Opteron_G5 lacks ibpb. Even if the user is running on an Opteron_G5 host > CPU which has ibpb, if they select Opteron_G5 the guest doesn't see it. > > In that case, the existing patches in git will check to see if the host > supports ibpb, and if so, manually specify that in the > config. So now use of --cpu Opteron_G5 is secure by default. > > But the problem is this creates a migration compat issue: selecting > --cpu Opteron_G5 means different things for different hosts. So these > patches basically make the 'secure' additions the default > behavior, but gives users the option to turn it off on the cli and > virt-manager UI. > > Taking a step back, I don't really get what the usecase is for users to > be selecting manual cpu model names with virt-manager or virt-install. I > understand the migration compat thing, if you need to generate a > baseline CPU that works for migrating a machine across multiple hosts, > but in practice that seems like pretty advanced usage for people using > virt-manager or even virt-install, and if they are using virt-install > they have all the tools at their disposal to generate a full + secure > config if they want. > > That bug was filed in may of last year. When it was filed, virt-manager > was still using the CPU model from 'virsh capabilities' as the default. > With spectre stuff that is very bad for sure, because once we set > Opteron_G5 it's there forever in the VM config, even after the host is > updated to possibly expose more CPU flags or whatever, the VM always > sees the old feature set. > > However now we are defaulting to host-model for new enough qemu+libvirt, > including in RHEL7 it seems. The default VM CPU config is as safe as the > host CPU allows. So maybe the original motivation for the bug isn't > really as valid any more because we use a better default. > > So I kind of suspect that the original urgency around the bug was due to > virt-manager potentially defaulting to the insecure model=Opteron_G5 > > Anyways, since the current code requires the user to explicitly opt in > to choosing a manual CPU model name, what if instead we just warn them? > A warning label that appears like: > > "The following host security features are not supported by the selected > CPU: ipbp, ... > > Use 'Copy host CPU' for maximum security'" > > And a virt-install printed warning to that effect too. Maybe we keep the > --cpu secure=on CLI option but we keep it off by default. Basically if > people are going to deviate from the default and specify a manual cpu > name I don't think we need to bend over backwards to protect them in > that case, just tell them 'dont do that' or if they have valid poweruser > usecases then they have all the tools at their disposal to make it work, > at least on the cli. I agree with all of the reasoning you wrote except the RHEL7 part, we cannot default to host-model, in order to do that you need to have libvirt >= 3.2.0 and QEMU >= 2.9.0, or without a version check in domain capabilities you can check if fallback='forbid': ... ... Skylake-Client-IBRS ... ... ... From upstream POV all of these CPU security patches will probably help only few users which will for whatever reason select specific CPU model without where the warning would be good enough, but on RHEL7 the default is still and always will be host-model-only so RHEL7 is by default insecure. Pavel signature.asc Description: PGP signature ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
