7:23 p.m.
An alternative take on David Gibson's patch
https://www.redhat.com/archives/libvir-list/2019-June/msg00061.html
from earlier this month.
Andrea Bolognani (7):
docs: Fix validation of spapr-vio addresses
qemu: Rework qemuDomainDeviceDefValidateAddress()
qemu: Validate spapr-vio addresses
tests: Add pseries-spaprvio-invalid
conf: Format spapr-vio addresses as 32-bit
qemu: Format spapr-vio addresses as 32-bit
docs: Update documentation for spapr-vio addresses
docs/formatdomain.html.in | 6 +--
docs/schemas/domaincommon.rng | 2 +-
src/conf/domain_conf.c | 2 +-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_domain.c | 41 ++++++++++++++++++-
tests/qemuargv2xmldata/nomachine-ppc64.xml | 2 +-
tests/qemuargv2xmldata/pseries-disk.xml | 2 +-
tests/qemuargv2xmldata/pseries-nvram.xml | 2 +-
.../disk-scsi.x86_64-latest.args | 2 +-
.../pseries-spaprvio-invalid.xml | 17 ++++++++
.../pseries-vio-user-assigned.args | 2 +-
tests/qemuxml2argvdata/pseries-vio.args | 4 +-
tests/qemuxml2argvtest.c | 1 +
tests/qemuxml2xmloutdata/disk-scsi.xml | 2 +-
tests/qemuxml2xmloutdata/pseries-nvram.xml | 2 +-
15 files changed, 73 insertions(+), 16 deletions(-)
create mode 100644 tests/qemuxml2argvdata/pseries-spaprvio-invalid.xml
--
2.21.0
7:23 p.m.
New subject: [libvirt] [PATCH 1/7] docs: Fix validation of spapr-vio addresses
According to sPAPR, addresses are 32-bit (8 hex digits) rather
than 64-bit (16 hex digits). Update the schema accordingly.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
docs/schemas/domaincommon.rng | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4bd75e3055..9fd23efb47 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -6386,7 +6386,7 @@
</define>
<define name="spaprvioReg">
<data type="string">
- <param name="pattern">(0x)?[0-9a-fA-F]{1,16}</param>
+ <param name="pattern">(0x)?[0-9a-fA-F]{1,8}</param>
</data>
</define>
<define name='aliasName'>
--
2.21.0
7:23 p.m.
New subject: [libvirt] [PATCH 2/7] qemu: Rework qemuDomainDeviceDefValidateAddress()
Introduce a switch() statement and prepare for validating
more address types than just PCI.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e5c6ef3fda..76963d1c5a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6193,9 +6193,35 @@ qemuDomainDeviceDefValidateAddress(const virDomainDeviceDef *dev,
if (!(info = virDomainDeviceGetInfo((virDomainDeviceDef *)dev)))
return 0;
- if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
+ switch ((virDomainDeviceAddressType) info->type) {
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
return qemuDomainDeviceDefValidateZPCIAddress(info, qemuCaps);
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
+ /* Address validation might happen before we have had a chance to
+ * automatically assign addresses to devices for which the user
+ * didn't specify one themselves */
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:
+ /* No validation for these address types yet */
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virDomainDeviceAddressType, info->type);
+ return -1;
+ }
+
return 0;
}
--
2.21.0
7:23 p.m.
New subject: [libvirt] [PATCH 3/7] qemu: Validate spapr-vio addresses
According to sPAPR, addresses are 32-bit rather than 64-bit.
Update qemuDomainDeviceDefValidateAddress() accordingly.
https://bugzilla.redhat.com/show_bug.cgi?id=1598657
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 76963d1c5a..abc8809b56 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6203,11 +6203,24 @@ qemuDomainDeviceDefValidateAddress(const virDomainDeviceDef *dev,
* didn't specify one themselves */
break;
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO: {
+ virDomainDeviceSpaprVioAddressPtr addr = &(info->addr.spaprvio);
+
+ if (addr->has_reg && addr->reg > 0xffffffff) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("spapr-vio reg='0x%llx' exceeds maximum "
+ "possible value (0xffffffff)"),
+ addr->reg);
+ return -1;
+ }
+
+ break;
+ }
+
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL:
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID:
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
--
2.21.0
7:23 p.m.
New subject: [libvirt] [PATCH 4/7] tests: Add pseries-spaprvio-invalid
This test case shows that we now reject invalid spapr-vio
addresses.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
.../pseries-spaprvio-invalid.xml | 17 +++++++++++++++++
tests/qemuxml2argvtest.c | 1 +
2 files changed, 18 insertions(+)
create mode 100644 tests/qemuxml2argvdata/pseries-spaprvio-invalid.xml
diff --git a/tests/qemuxml2argvdata/pseries-spaprvio-invalid.xml
b/tests/qemuxml2argvdata/pseries-spaprvio-invalid.xml
new file mode 100644
index 0000000000..cd658e314b
--- /dev/null
+++ b/tests/qemuxml2argvdata/pseries-spaprvio-invalid.xml
@@ -0,0 +1,17 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>b35969f7-e7cf-4d90-a9a0-4dd9000f9824</uuid>
+ <memory unit='KiB'>4194304</memory>
+ <vcpu placement='static'>4</vcpu>
+ <os>
+ <type arch='ppc64le' machine='pseries'>hvm</type>
+ </os>
+ <devices>
+ <emulator>/usr/bin/qemu-system-ppc64</emulator>
+ <serial type='pty'>
+ <!-- spapr-vio addresses are 32-bit, so they can go up to 0xffffffff:
+ the value below is too big and should be rejected -->
+ <address type='spapr-vio' reg='0x0000000100000000'/>
+ </serial>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 49220733ae..6a75d663c5 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1898,6 +1898,7 @@ mymain(void)
DO_TEST("pseries-console-virtio",
QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
DO_TEST_PARSE_ERROR("pseries-serial-invalid-machine", NONE);
+ DO_TEST_PARSE_ERROR("pseries-spaprvio-invalid", "ppc64");
DO_TEST("mach-virt-serial-native",
QEMU_CAPS_DEVICE_PL011);
--
2.21.0
7:23 p.m.
New subject: [libvirt] [PATCH 5/7] conf: Format spapr-vio addresses as 32-bit
Using 8 hex digits all the time, regardless of whether the
actual value can fit in fewer, makes it more obvious to the
user what the limits are.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_conf.c | 2 +-
tests/qemuargv2xmldata/nomachine-ppc64.xml | 2 +-
tests/qemuargv2xmldata/pseries-disk.xml | 2 +-
tests/qemuargv2xmldata/pseries-nvram.xml | 2 +-
tests/qemuxml2xmloutdata/disk-scsi.xml | 2 +-
tests/qemuxml2xmloutdata/pseries-nvram.xml | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 97ba8bd53a..54a24b6276 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7135,7 +7135,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
if (info->addr.spaprvio.has_reg)
- virBufferAsprintf(&attrBuf, " reg='0x%llx'",
info->addr.spaprvio.reg);
+ virBufferAsprintf(&attrBuf, " reg='0x%08llx'",
info->addr.spaprvio.reg);
break;
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
diff --git a/tests/qemuargv2xmldata/nomachine-ppc64.xml
b/tests/qemuargv2xmldata/nomachine-ppc64.xml
index 18b0c5c20a..f61006efec 100644
--- a/tests/qemuargv2xmldata/nomachine-ppc64.xml
+++ b/tests/qemuargv2xmldata/nomachine-ppc64.xml
@@ -35,7 +35,7 @@
<target index='0'/>
</controller>
<controller type='scsi' index='0' model='ibmvscsi'>
- <address type='spapr-vio' reg='0x2000'/>
+ <address type='spapr-vio' reg='0x00002000'/>
</controller>
<input type='keyboard' bus='usb'/>
<input type='mouse' bus='usb'/>
diff --git a/tests/qemuargv2xmldata/pseries-disk.xml
b/tests/qemuargv2xmldata/pseries-disk.xml
index 18b0c5c20a..f61006efec 100644
--- a/tests/qemuargv2xmldata/pseries-disk.xml
+++ b/tests/qemuargv2xmldata/pseries-disk.xml
@@ -35,7 +35,7 @@
<target index='0'/>
</controller>
<controller type='scsi' index='0' model='ibmvscsi'>
- <address type='spapr-vio' reg='0x2000'/>
+ <address type='spapr-vio' reg='0x00002000'/>
</controller>
<input type='keyboard' bus='usb'/>
<input type='mouse' bus='usb'/>
diff --git a/tests/qemuargv2xmldata/pseries-nvram.xml
b/tests/qemuargv2xmldata/pseries-nvram.xml
index 500227afc0..78f79f17d7 100644
--- a/tests/qemuargv2xmldata/pseries-nvram.xml
+++ b/tests/qemuargv2xmldata/pseries-nvram.xml
@@ -23,7 +23,7 @@
</controller>
<memballoon model='none'/>
<nvram>
- <address type='spapr-vio' reg='0x4000'/>
+ <address type='spapr-vio' reg='0x00004000'/>
</nvram>
<panic model='pseries'/>
</devices>
diff --git a/tests/qemuxml2xmloutdata/disk-scsi.xml
b/tests/qemuxml2xmloutdata/disk-scsi.xml
index 9cfff4b14e..45fed6423a 100644
--- a/tests/qemuxml2xmloutdata/disk-scsi.xml
+++ b/tests/qemuxml2xmloutdata/disk-scsi.xml
@@ -62,7 +62,7 @@
<address type='pci' domain='0x0000' bus='0x00'
slot='0x05' function='0x0'/>
</controller>
<controller type='scsi' index='3' model='ibmvscsi'>
- <address type='spapr-vio' reg='0x2000'/>
+ <address type='spapr-vio' reg='0x00002000'/>
</controller>
<controller type='pci' index='0' model='pci-root'/>
<input type='mouse' bus='ps2'/>
diff --git a/tests/qemuxml2xmloutdata/pseries-nvram.xml
b/tests/qemuxml2xmloutdata/pseries-nvram.xml
index f89b23b338..fd77c7aa89 100644
--- a/tests/qemuxml2xmloutdata/pseries-nvram.xml
+++ b/tests/qemuxml2xmloutdata/pseries-nvram.xml
@@ -23,7 +23,7 @@
</controller>
<memballoon model='none'/>
<nvram>
- <address type='spapr-vio' reg='0x4000'/>
+ <address type='spapr-vio' reg='0x00004000'/>
</nvram>
<panic model='pseries'/>
</devices>
--
2.21.0
7:23 p.m.
New subject: [libvirt] [PATCH 6/7] qemu: Format spapr-vio addresses as 32-bit
No reason not to be consistent with the user-visible value.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 2 +-
tests/qemuxml2argvdata/disk-scsi.x86_64-latest.args | 2 +-
tests/qemuxml2argvdata/pseries-vio-user-assigned.args | 2 +-
tests/qemuxml2argvdata/pseries-vio.args | 4 ++--
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 59dc134785..9d4d0b8fc2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -395,7 +395,7 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
}
} else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) {
if (info->addr.spaprvio.has_reg)
- virBufferAsprintf(buf, ",reg=0x%llx", info->addr.spaprvio.reg);
+ virBufferAsprintf(buf, ",reg=0x%08llx",
info->addr.spaprvio.reg);
} else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
if (info->addr.ccw.assigned)
virBufferAsprintf(buf, ",devno=%x.%x.%04x",
diff --git a/tests/qemuxml2argvdata/disk-scsi.x86_64-latest.args
b/tests/qemuxml2argvdata/disk-scsi.x86_64-latest.args
index 6c2618ed61..7bf011fd5f 100644
--- a/tests/qemuxml2argvdata/disk-scsi.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-scsi.x86_64-latest.args
@@ -30,7 +30,7 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-device lsi,id=scsi0,bus=pci.0,addr=0x2 \
-device megasas,id=scsi1,bus=pci.0,addr=0x3 \
-device mptsas1068,id=scsi2,bus=pci.0,addr=0x4 \
--device spapr-vscsi,id=scsi3,reg=0x2000 \
+-device spapr-vscsi,id=scsi3,reg=0x00002000 \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
-drive file=/tmp/scsidisk.img,format=raw,if=none,id=drive-scsi0-0-0 \
diff --git a/tests/qemuxml2argvdata/pseries-vio-user-assigned.args
b/tests/qemuxml2argvdata/pseries-vio-user-assigned.args
index 534037607a..a412593c9a 100644
--- a/tests/qemuxml2argvdata/pseries-vio-user-assigned.args
+++ b/tests/qemuxml2argvdata/pseries-vio-user-assigned.args
@@ -23,7 +23,7 @@ server,nowait \
-mon chardev=charmonitor,id=monitor,mode=control \
-rtc base=utc \
-no-shutdown \
--device spapr-vscsi,id=scsi0,reg=0x2000 \
+-device spapr-vscsi,id=scsi0,reg=0x00002000 \
-device spapr-vscsi,id=scsi1,reg=0x30000000 \
-usb \
-drive file=/tmp/scsidisk.img,format=raw,if=none,id=drive-scsi1-0-0-0 \
diff --git a/tests/qemuxml2argvdata/pseries-vio.args
b/tests/qemuxml2argvdata/pseries-vio.args
index 9d4f77b85e..a500e9b4b9 100644
--- a/tests/qemuxml2argvdata/pseries-vio.args
+++ b/tests/qemuxml2argvdata/pseries-vio.args
@@ -23,8 +23,8 @@ server,nowait \
-mon chardev=charmonitor,id=monitor,mode=control \
-rtc base=utc \
-no-shutdown \
--device spapr-vscsi,id=scsi0,reg=0x2000 \
--device spapr-vscsi,id=scsi1,reg=0x3000 \
+-device spapr-vscsi,id=scsi0,reg=0x00002000 \
+-device spapr-vscsi,id=scsi1,reg=0x00003000 \
-usb \
-drive file=/tmp/scsidisk.img,format=raw,if=none,id=drive-scsi1-0-0-0 \
-device scsi-hd,bus=scsi1.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi1-0-0-0,\
--
2.21.0
7:23 p.m.
New subject: [libvirt] [PATCH 7/7] docs: Update documentation for spapr-vio addresses
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
docs/formatdomain.html.in | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 22ddcb71d3..144baa2e85 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4076,9 +4076,9 @@
</dd>
<dt><code>spapr-vio</code></dt>
<dd>On PowerPC pseries guests, devices can be assigned to the
- SPAPR-VIO bus. It has a flat 64-bit address space; by
+ SPAPR-VIO bus. It has a flat 32-bit address space; by
convention, devices are generally assigned at a non-zero
- multiple of 0x1000, but other addresses are valid and
+ multiple of 0x00001000, but other addresses are valid and
permitted by libvirt. Each address has the following
additional attribute: <code>reg</code> (the hex value address
of the starting register). <span class="since">Since
@@ -8291,7 +8291,7 @@ qemu-kvm -net nic,model=? /dev/null
...
<devices>
<nvram>
- <address type='spapr-vio' reg='0x3000'/>
+ <address type='spapr-vio' reg='0x00003000'/>
</nvram>
</devices>
...
--
2.21.0
5:29 p.m.
On 6/14/19 1:23 PM, Andrea Bolognani wrote:
An alternative take on David Gibson's patch
https://www.redhat.com/archives/libvir-list/2019-June/msg00061.html
from earlier this month.
Andrea Bolognani (7):
docs: Fix validation of spapr-vio addresses
qemu: Rework qemuDomainDeviceDefValidateAddress()
qemu: Validate spapr-vio addresses
tests: Add pseries-spaprvio-invalid
conf: Format spapr-vio addresses as 32-bit
qemu: Format spapr-vio addresses as 32-bit
docs: Update documentation for spapr-vio addresses
docs/formatdomain.html.in | 6 +--
docs/schemas/domaincommon.rng | 2 +-
src/conf/domain_conf.c | 2 +-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_domain.c | 41 ++++++++++++++++++-
tests/qemuargv2xmldata/nomachine-ppc64.xml | 2 +-
tests/qemuargv2xmldata/pseries-disk.xml | 2 +-
tests/qemuargv2xmldata/pseries-nvram.xml | 2 +-
.../disk-scsi.x86_64-latest.args | 2 +-
.../pseries-spaprvio-invalid.xml | 17 ++++++++
.../pseries-vio-user-assigned.args | 2 +-
tests/qemuxml2argvdata/pseries-vio.args | 4 +-
tests/qemuxml2argvtest.c | 1 +
tests/qemuxml2xmloutdata/disk-scsi.xml | 2 +-
tests/qemuxml2xmloutdata/pseries-nvram.xml | 2 +-
15 files changed, 73 insertions(+), 16 deletions(-)
create mode 100644 tests/qemuxml2argvdata/pseries-spaprvio-invalid.xml
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
1950
days inactive
1953
days old
8 comments
2 participants
participants (2)
-
Andrea Bolognani -
Michal Privoznik