Xen Security Advisory 131 (CVE-2015-4106) - Unmediated PCI register access in qemu
Xen.org security team <[email protected]>
| Newsgroups | gmane.comp.emulators.xen.announce |
|---|---|
| Message-ID | <E1YzmoF-0001rG-Vs__41312.9559056417$1433254411$gmane$org@xenbits.xen.org> |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Xen Security Advisory CVE-2015-4106 / XSA-131
version 3
Unmediated PCI register access in qemu
UPDATES IN VERSION 3
====================
Public release.
CVE assigned.
ISSUE DESCRIPTION
=================
Qemu allows guests to not only read, but also write all parts of the
PCI config space (but not extended config space) of passed through PCI
devices not explicitly dealt with for (partial) emulation purposes.
IMPACT
======
Since the effect depends on the specific purpose of the the config
space field, it's not possbile to give a general statement about the
exact impact on the host or other guests. Privilege escalation, host
crash (Denial of Service), and leaked information all cannot be
excluded.
VULNERABLE SYSTEMS
==================
Xen versions 3.3 and onwards are vulnerable due to supporting PCI
pass-through.
Only x86 systems are vulnerable. ARM systems are not vulnerable.
Only HVM guests with their device model run in Dom0 can take advantage
of this vulnerability.
Only HVM guests which have been granted access to physical PCI devices
(`PCI passthrough') can take advantage of this vulnerability.
MITIGATION
==========
This issue can be avoided by not assigning PCI devices to untrusted HVM
guests.
This issue can also be avoided by only using PV guests.
It can also be avoided by configuring HVM guests with their device
model run in a separate (stub) domain. (When using xl, this can be
requested with "device_model_stubdomain_override=1" in the domain
configuration file.)
CREDITS
=======
This issue was discovered by Jan Beulich of SUSE.
RESOLUTION
==========
Applying the appropriate attached patch resolves this issue.
xsa131-qemuu-$n.patch qemu-upstream-unstable, Xen 4.5.x, Xen 4.4.x
xsa131-qemuu-4.4-1.patch Xen 4.4.x replacement for xsa131-qemuu-1.patch
xsa131-qemuu-4.3-$n.patch Xen 4.3.x
xsa131-qemut-$n.patch qemu-xen-unstable, Xen 4.5.x, Xen 4.4.x, Xen 4.3.x, Xen 4.2.x
xsa131-qemut-4.2-1.patch Xen 4.2.x replacement for xsa131-qemut-1.patch
$ sha256sum xsa131*.patch
2ff4aa092247ff0911d837adc5f4de1ffa8ed32a39eaea9b0bfc4a40b7921b06 xsa131-qemut-1.patch
dafa524374d890e517d4e2600a594064b55af645172422b9e81a64b5f4a64575 xsa131-qemut-2.patch
b37d3e22ce4410bf0db87217c60a543f0143a23ab0652f1746bd5fe17dbadd70 xsa131-qemut-3.patch
b5f0882717129142f11297a62b2ed826da94ce5ed42f6b2ea60f9101b652aed9 xsa131-qemut-4.2-1.patch
3bfc58b6288bafb4c2039265be32c6bd9e048b63a4cae279ead3ec1154af9abe xsa131-qemut-4.patch
60c44b63d2c7bd7e12631db7fd05622d782e1a5ccd7dfa17a1671b36b5ff7bee xsa131-qemut-5.patch
8f2a9c4333155fac670ad3a932703051ce8a47f4f6d3a067458e5ab49da7e93a xsa131-qemut-6.patch
ed4facfa80b2ab7ecfc9b232878d3f4d54ad93214c75f4b4af71c8f07a1d04c4 xsa131-qemut-7.patch
d400d03ae792699fec9a54bbb6b08c2f5523427ef8af85b0c5ede497ba87f61c xsa131-qemut-8.patch
7a7f294303a8bcf9a316e3e6b8a0511dac3e92dbf7e373b21c94b97835c03f2f xsa131-qemuu-1.patch
dc72bd4993fdcea3dc98d18f314da3ac1c7e73e0b99dac325b0e59d0229f67e5 xsa131-qemuu-2.patch
61524a47fd29406ba9a2983ea9cb59e45a56d716d65d78689177d9c8e95f76e6 xsa131-qemuu-3.patch
21493c5db68115d97a6aecf1159ee05023b59545627d7f03d7fdaa238bb3bd27 xsa131-qemuu-4.3-1.patch
5828647db6f090ce6c7ea20f90331008f2a0bba18b3a3a371f2ba9054871a7cb xsa131-qemuu-4.3-2.patch
eab05df32e8a7c729cc52affd28b109a8f75cabb8fd4027934059d303b2232fa xsa131-qemuu-4.3-3.patch
8dc95a2a8a45d851476b938e4cab2e65d87b8dc28c721949824ce900552ba489 xsa131-qemuu-4.3-4.patch
7a358fba18ae9c0dde1134564151a97c8e6d6f5982ac74c450f81d2ed8e9d540 xsa131-qemuu-4.3-5.patch
fcb77a8d2adde1daf03f8faeb6e92788b2727f5b11563b6f770c74251b0964a4 xsa131-qemuu-4.3-6.patch
79933b2744e7b69c4eb23f3974d242e2592cb4553be115a4aec1c6e30e7564cf xsa131-qemuu-4.3-7.patch
bb4021a36a9f36dc0082cfd42869adc737ec4afea92ac1100f0971118174b58c xsa131-qemuu-4.3-8.patch
f70516fa38a3d2e0cf906c41e3b7dfd7cf998c9189b232dac20633c7b0d1ab8b xsa131-qemuu-4.4-1.patch
041c82a341755bcbab18f834a0fccf9c031674d956958092cbfa5e64f05b6318 xsa131-qemuu-4.patch
91aeb9c0d3e9a251faf12840e0519a342cfb7e35af3fea429bedb452182fae47 xsa131-qemuu-5.patch
60482fe37fd405032b92de85ed5d333c210c85662b1645016dce2f0053aa6ec0 xsa131-qemuu-6.patch
05fc2e614620449e52a056ce6e5f4033970ade22fde623e3b789fc57b3e4143e xsa131-qemuu-7.patch
358849d7c0dff29bf96f49e56d00c4d7bd4c8d0c71c122a7b3655e10f45cb53b xsa131-qemuu-8.patch
$
DEPLOYMENT DURING EMBARGO
=========================
Deployment of patches or migitations is NOT permitted (except on
systems used and administered only by organisations which are members
of the Xen Project Security Issues Predisclosure List). Specifically,
deployent on public cloud systems is NOT permitted.
This is because the altered PCI config space access behavior is visible
to guests.
Deployment is permitted only AFTER the embargo ends.
(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable. This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)
For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
iQEcBAEBAgAGBQJVbbdZAAoJEIP+FMlX6CvZ1yEIAKWoq6O8Nk8zewvKojXnmt0J
irQ4p9uXBDN682d9Vloq+y86PSt5NLs83ZfAHWSkWPkkgyDXy4tmnte9LGMLmVI+
Z7nZs4dsH2bixFMJfqjKWE//py37TIVmI4M37xOgkNV8HTQJ0ZHWgYur5ilNJu9x
HJ1duL3//+zkelA+zUQQSNMPvc2OUCSRGW5UVDwn95xJDAgURWe2d6c6bg8yG7T6
ufwO0x1CWTRaVsbLRSCST3NEVl7bxmYR5RBxlBaUIpgzT53aK3XHoiAezjTdK1Ul
TiZ3Hb0XVtFbNEz2cCWQBEdQPKYhJjxpUBdRi9zlsiFwHa+lG+CA3i1IcqXIXQo=
=tNVc
-----END PGP SIGNATURE-----
_______________________________________________
Xen-announce mailing list
[email protected]
http://lists.xen.org/xen-announce
xsa131-qemut-1.patch
(application/octet-stream, 2.2 KB)
xen/MSI: don't open-code pass-through of enable bit modifications Without this the actual XSA-131 fix would cause the enable bit to not get set anymore (due to the write back getting suppressed there based on the OR of emu_mask, ro_mask, and res_mask). Note that the fiddling with the enable bit shouldn't really be done by qemu, but making this work right (via libxc and the hypervisor) will require more extensive changes, which can be postponed until after the security issue got addressed. This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> Acked-by: Stefano Stabellini <[email protected]> --- a/hw/pass-through.c +++ b/hw/pass-through.c @@ -648,7 +648,7 @@ static struct pt_reg_info_tbl pt_emu_reg .size = 2, .init_val = 0x0000, .ro_mask = 0xFF8E, - .emu_mask = 0x017F, + .emu_mask = 0x017E, .init = pt_msgctrl_reg_init, .u.w.read = pt_word_reg_read, .u.w.write = pt_msgctrl_reg_write, @@ -3901,6 +3901,9 @@ static int pt_msgctrl_reg_write(struct p /* modify emulate register */ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask; + /* also emulate MSI_ENABLE bit for MSI-INTx translation */ + if (ptdev->msi_trans_en) + writable_mask |= PCI_MSI_FLAGS_ENABLE & valid_mask; cfg_entry->data = PT_MERGE_VALUE(*value, cfg_entry->data, writable_mask); /* update the msi_info too */ ptdev->msi->flags |= cfg_entry->data & @@ -3909,6 +3912,9 @@ static int pt_msgctrl_reg_write(struct p /* create value for writing to I/O device register */ val = *value; throughable_mask = ~reg->emu_mask & valid_mask; + /* don't pass through MSI_ENABLE bit for MSI-INTx translation */ + if (ptdev->msi_trans_en) + throughable_mask &= ~PCI_MSI_FLAGS_ENABLE; *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask); /* update MSI */ @@ -3952,12 +3958,6 @@ static int pt_msgctrl_reg_write(struct p } } - /* pass through MSI_ENABLE bit when no MSI-INTx translation */ - if (!ptdev->msi_trans_en) { - *value &= ~PCI_MSI_FLAGS_ENABLE; - *value |= val & PCI_MSI_FLAGS_ENABLE; - } - return 0; }
xsa131-qemut-2.patch
(application/octet-stream, 5 KB)
xen/pt: consolidate PM capability emu_mask
There's no point in xen_pt_pmcsr_reg_{read,write}() each ORing
PCI_PM_CTRL_STATE_MASK and PCI_PM_CTRL_NO_SOFT_RESET into a local
emu_mask variable - we can have the same effect by setting the field
descriptor's emu_mask member suitably right away. Note that
xen_pt_pmcsr_reg_write() is being retained in order to allow later
patches to be less intrusive.
This is a preparatory patch for XSA-131.
Signed-off-by: Jan Beulich <[email protected]>
Acked-by: Stefano Stabellini <[email protected]>
Acked-by: Ian Campbell <[email protected]>
--- a/hw/pass-through.c
+++ b/hw/pass-through.c
@@ -179,9 +179,6 @@ static int pt_long_reg_read(struct pt_de
static int pt_bar_reg_read(struct pt_dev *ptdev,
struct pt_reg_tbl *cfg_entry,
uint32_t *value, uint32_t valid_mask);
-static int pt_pmcsr_reg_read(struct pt_dev *ptdev,
- struct pt_reg_tbl *cfg_entry,
- uint16_t *value, uint16_t valid_mask);
static int pt_byte_reg_write(struct pt_dev *ptdev,
struct pt_reg_tbl *cfg_entry,
uint8_t *value, uint8_t dev_value, uint8_t valid_mask);
@@ -494,7 +491,7 @@ static struct pt_reg_info_tbl pt_emu_reg
.u.w.write = pt_word_reg_write,
.u.w.restore = NULL,
},
- /* PCI Power Management Control/Status reg */
+ /* PCI Power Management Control/Status reg (->power_mgmt on) */
{
.offset = PCI_PM_CTRL,
.size = 2,
@@ -502,7 +499,19 @@ static struct pt_reg_info_tbl pt_emu_reg
.ro_mask = 0xE1FC,
.emu_mask = 0x8100,
.init = pt_pmcsr_reg_init,
- .u.w.read = pt_pmcsr_reg_read,
+ .u.w.read = pt_word_reg_read,
+ .u.w.write = pt_pmcsr_reg_write,
+ .u.w.restore = pt_pmcsr_reg_restore,
+ },
+ /* PCI Power Management Control/Status reg (->power_mgmt off) */
+ {
+ .offset = PCI_PM_CTRL,
+ .size = 2,
+ .init_val = 0x0008,
+ .ro_mask = 0xE1FC,
+ .emu_mask = 0x810B,
+ .init = pt_pmcsr_reg_init,
+ .u.w.read = pt_word_reg_read,
.u.w.write = pt_pmcsr_reg_write,
.u.w.restore = pt_pmcsr_reg_restore,
},
@@ -2919,6 +2928,7 @@ static uint32_t pt_pmc_reg_init(struct p
return reg->init_val;
}
+/* this function will be called twice (for ->power_mgmt on and off cases) */
/* initialize PCI Power Management Control/Status register */
static uint32_t pt_pmcsr_reg_init(struct pt_dev *ptdev,
struct pt_reg_info_tbl *reg, uint32_t real_offset)
@@ -2926,8 +2936,23 @@ static uint32_t pt_pmcsr_reg_init(struct
PCIDevice *d = &ptdev->dev;
uint16_t cap_ver = 0;
- if (!ptdev->power_mgmt)
- return reg->init_val;
+ switch (reg->emu_mask & (PCI_PM_CTRL_STATE_MASK |
+ PCI_PM_CTRL_NO_SOFT_RESET))
+ {
+ case 0:
+ if (!ptdev->power_mgmt)
+ return PT_INVALID_REG;
+ break;
+ case PCI_PM_CTRL_STATE_MASK | PCI_PM_CTRL_NO_SOFT_RESET:
+ if (!ptdev->power_mgmt)
+ return reg->init_val;
+ return PT_INVALID_REG;
+ default:
+ /* exit I/O emulator */
+ PT_LOG("Internal error: Invalid PMCSR emulation mask %04x."
+ " I/O emulator exit.\n", reg->emu_mask);
+ exit(1);
+ }
/* check PCI Power Management support version */
cap_ver = ptdev->pm_state->pmc_field & PCI_PM_CAP_VER_MASK;
@@ -3417,24 +3442,6 @@ static int pt_bar_reg_read(struct pt_dev
}
-/* read Power Management Control/Status register */
-static int pt_pmcsr_reg_read(struct pt_dev *ptdev,
- struct pt_reg_tbl *cfg_entry,
- uint16_t *value, uint16_t valid_mask)
-{
- struct pt_reg_info_tbl *reg = cfg_entry->reg;
- uint16_t valid_emu_mask = reg->emu_mask;
-
- if (!ptdev->power_mgmt)
- valid_emu_mask |= PCI_PM_CTRL_STATE_MASK | PCI_PM_CTRL_NO_SOFT_RESET;
-
- valid_emu_mask = valid_emu_mask & valid_mask ;
- *value = PT_MERGE_VALUE(*value, cfg_entry->data, ~valid_emu_mask);
-
- return 0;
-}
-
-
/* write byte size emulate register */
static int pt_byte_reg_write(struct pt_dev *ptdev,
struct pt_reg_tbl *cfg_entry,
@@ -3768,21 +3775,17 @@ static int pt_pmcsr_reg_write(struct pt_
{
struct pt_reg_info_tbl *reg = cfg_entry->reg;
PCIDevice *d = &ptdev->dev;
- uint16_t emu_mask = reg->emu_mask;
uint16_t writable_mask = 0;
uint16_t throughable_mask = 0;
struct pt_pm_info *pm_state = ptdev->pm_state;
uint16_t read_val = 0;
- if (!ptdev->power_mgmt)
- emu_mask |= PCI_PM_CTRL_STATE_MASK | PCI_PM_CTRL_NO_SOFT_RESET;
-
/* modify emulate register */
- writable_mask = emu_mask & ~reg->ro_mask & valid_mask;
+ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask;
cfg_entry->data = PT_MERGE_VALUE(*value, cfg_entry->data, writable_mask);
/* create value for writing to I/O device register */
- throughable_mask = ~emu_mask & valid_mask;
+ throughable_mask = ~reg->emu_mask & valid_mask;
*value = PT_MERGE_VALUE(*value, dev_value, throughable_mask);
if (!ptdev->power_mgmt)
xsa131-qemut-3.patch
(application/octet-stream, 795 B)
xen/pt: correctly handle PM status bit xen_pt_pmcsr_reg_write() needs an adjustment to deal with the RW1C nature of the not passed through bit 15 (PCI_PM_CTRL_PME_STATUS). This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> Reviewed-by: Stefano Stabellini <[email protected]> --- a/hw/pass-through.c +++ b/hw/pass-through.c @@ -3786,7 +3786,8 @@ static int pt_pmcsr_reg_write(struct pt_ /* create value for writing to I/O device register */ throughable_mask = ~reg->emu_mask & valid_mask; - *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask); + *value = PT_MERGE_VALUE(*value, dev_value & ~PCI_PM_CTRL_PME_STATUS, + throughable_mask); if (!ptdev->power_mgmt) return 0;
xsa131-qemut-4.2-1.patch
(application/octet-stream, 2.2 KB)
xen/MSI: don't open-code pass-through of enable bit modifications Without this the actual XSA-131 fix would cause the enable bit to not get set anymore (due to the write back getting suppressed there based on the OR of emu_mask, ro_mask, and res_mask). Note that the fiddling with the enable bit shouldn't really be done by qemu, but making this work right (via libxc and the hypervisor) will require more extensive changes, which can be postponed until after the security issue got addressed. This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> Acked-by: Stefano Stabellini <[email protected]> --- a/hw/pass-through.c +++ b/hw/pass-through.c @@ -648,7 +648,7 @@ static struct pt_reg_info_tbl pt_emu_reg .size = 2, .init_val = 0x0000, .ro_mask = 0xFF8E, - .emu_mask = 0x017F, + .emu_mask = 0x017E, .init = pt_msgctrl_reg_init, .u.w.read = pt_word_reg_read, .u.w.write = pt_msgctrl_reg_write, @@ -3899,6 +3899,9 @@ static int pt_msgctrl_reg_write(struct p /* modify emulate register */ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask; + /* also emulate MSI_ENABLE bit for MSI-INTx translation */ + if (ptdev->msi_trans_en) + writable_mask |= PCI_MSI_FLAGS_ENABLE & valid_mask; cfg_entry->data = PT_MERGE_VALUE(*value, cfg_entry->data, writable_mask); /* update the msi_info too */ ptdev->msi->flags |= cfg_entry->data & @@ -3907,6 +3910,9 @@ static int pt_msgctrl_reg_write(struct p /* create value for writing to I/O device register */ val = *value; throughable_mask = ~reg->emu_mask & valid_mask; + /* don't pass through MSI_ENABLE bit for MSI-INTx translation */ + if (ptdev->msi_trans_en) + throughable_mask &= ~PCI_MSI_FLAGS_ENABLE; *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask); /* update MSI */ @@ -3946,12 +3952,6 @@ static int pt_msgctrl_reg_write(struct p else ptdev->msi->flags &= ~PCI_MSI_FLAGS_ENABLE; - /* pass through MSI_ENABLE bit when no MSI-INTx translation */ - if (!ptdev->msi_trans_en) { - *value &= ~PCI_MSI_FLAGS_ENABLE; - *value |= val & PCI_MSI_FLAGS_ENABLE; - } - return 0; }
xsa131-qemut-4.patch
(application/octet-stream, 10.3 KB)
xen/pt: split out calculation of throughable mask in PCI config space handling This is just to avoid having to adjust that calculation later in multiple places. Note that including ->ro_mask in get_throughable_mask()'s calculation is only an apparent (i.e. benign) behavioral change: For r/o fields it doesn't matter > whether they get passed through - either the same flag is also set in emu_mask (then there's no change at all) or the field is r/o in hardware (and hence a write won't change it anyway). This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> Acked-by: Stefano Stabellini <[email protected]> Reviewed-by: Anthony PERARD <[email protected]> --- a/hw/pass-through.c +++ b/hw/pass-through.c @@ -3442,6 +3442,15 @@ static int pt_bar_reg_read(struct pt_dev } +static uint32_t get_throughable_mask(const struct pt_dev *ptdev, + const struct pt_reg_info_tbl *reg, + uint32_t valid_mask) +{ + uint32_t throughable_mask = ~(reg->emu_mask | reg->ro_mask); + + return throughable_mask & valid_mask; +} + /* write byte size emulate register */ static int pt_byte_reg_write(struct pt_dev *ptdev, struct pt_reg_tbl *cfg_entry, @@ -3449,14 +3458,13 @@ static int pt_byte_reg_write(struct pt_d { struct pt_reg_info_tbl *reg = cfg_entry->reg; uint8_t writable_mask = 0; - uint8_t throughable_mask = 0; + uint8_t throughable_mask = get_throughable_mask(ptdev, reg, valid_mask); /* modify emulate register */ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask; cfg_entry->data = PT_MERGE_VALUE(*value, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask); return 0; @@ -3469,14 +3477,13 @@ static int pt_word_reg_write(struct pt_d { struct pt_reg_info_tbl *reg = cfg_entry->reg; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; + uint16_t throughable_mask = get_throughable_mask(ptdev, reg, valid_mask); /* modify emulate register */ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask; cfg_entry->data = PT_MERGE_VALUE(*value, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask); return 0; @@ -3489,14 +3496,13 @@ static int pt_long_reg_write(struct pt_d { struct pt_reg_info_tbl *reg = cfg_entry->reg; uint32_t writable_mask = 0; - uint32_t throughable_mask = 0; + uint32_t throughable_mask = get_throughable_mask(ptdev, reg, valid_mask); /* modify emulate register */ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask; cfg_entry->data = PT_MERGE_VALUE(*value, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask); return 0; @@ -3509,7 +3515,7 @@ static int pt_cmd_reg_write(struct pt_de { struct pt_reg_info_tbl *reg = cfg_entry->reg; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; + uint16_t throughable_mask = get_throughable_mask(ptdev, reg, valid_mask); uint16_t wr_value = *value; /* modify emulate register */ @@ -3517,8 +3523,6 @@ static int pt_cmd_reg_write(struct pt_de cfg_entry->data = PT_MERGE_VALUE(*value, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; - if (*value & PCI_COMMAND_DISABLE_INTx) { if (ptdev->msi_trans_en) @@ -3564,7 +3568,6 @@ static int pt_bar_reg_write(struct pt_de PCIDevice *d = (PCIDevice *)&ptdev->dev; PCIIORegion *r; uint32_t writable_mask = 0; - uint32_t throughable_mask = 0; uint32_t bar_emu_mask = 0; uint32_t bar_ro_mask = 0; uint32_t new_addr, last_addr; @@ -3691,8 +3694,7 @@ static int pt_bar_reg_write(struct pt_de exit: /* create value for writing to I/O device register */ - throughable_mask = ~bar_emu_mask & valid_mask; - *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask); + *value = PT_MERGE_VALUE(*value, dev_value, 0); /* After BAR reg update, we need to remap BAR*/ reg_grp_entry = pt_find_reg_grp(ptdev, PCI_COMMAND); @@ -3719,9 +3721,8 @@ static int pt_exp_rom_bar_reg_write(stru PCIDevice *d = (PCIDevice *)&ptdev->dev; PCIIORegion *r; uint32_t writable_mask = 0; - uint32_t throughable_mask = 0; + uint32_t throughable_mask = get_throughable_mask(ptdev, reg, valid_mask); uint32_t r_size = 0; - uint32_t bar_emu_mask = 0; uint32_t bar_ro_mask = 0; r = &d->io_regions[PCI_ROM_SLOT]; @@ -3731,7 +3732,6 @@ static int pt_exp_rom_bar_reg_write(stru PT_GET_EMUL_SIZE(base->bar_flag, r_size); /* set emulate mask and read-only mask */ - bar_emu_mask = reg->emu_mask; bar_ro_mask = (reg->ro_mask | (r_size - 1)) & ~PCI_ROM_ADDRESS_ENABLE; /* modify emulate register */ @@ -3751,7 +3751,6 @@ static int pt_exp_rom_bar_reg_write(stru r->addr = cfg_entry->data; /* create value for writing to I/O device register */ - throughable_mask = ~bar_emu_mask & valid_mask; *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask); /* After BAR reg update, we need to remap BAR*/ @@ -3776,7 +3775,7 @@ static int pt_pmcsr_reg_write(struct pt_ struct pt_reg_info_tbl *reg = cfg_entry->reg; PCIDevice *d = &ptdev->dev; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; + uint16_t throughable_mask = get_throughable_mask(ptdev, reg, valid_mask); struct pt_pm_info *pm_state = ptdev->pm_state; uint16_t read_val = 0; @@ -3785,7 +3784,6 @@ static int pt_pmcsr_reg_write(struct pt_ cfg_entry->data = PT_MERGE_VALUE(*value, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *value = PT_MERGE_VALUE(*value, dev_value & ~PCI_PM_CTRL_PME_STATUS, throughable_mask); @@ -3894,7 +3892,7 @@ static int pt_msgctrl_reg_write(struct p { struct pt_reg_info_tbl *reg = cfg_entry->reg; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; + uint16_t throughable_mask = get_throughable_mask(ptdev, reg, valid_mask); uint16_t old_ctrl = cfg_entry->data; PCIDevice *pd = (PCIDevice *)ptdev; uint16_t val; @@ -3906,8 +3904,10 @@ static int pt_msgctrl_reg_write(struct p /* modify emulate register */ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask; /* also emulate MSI_ENABLE bit for MSI-INTx translation */ - if (ptdev->msi_trans_en) + if (ptdev->msi_trans_en) { writable_mask |= PCI_MSI_FLAGS_ENABLE & valid_mask; + throughable_mask &= ~PCI_MSI_FLAGS_ENABLE; + } cfg_entry->data = PT_MERGE_VALUE(*value, cfg_entry->data, writable_mask); /* update the msi_info too */ ptdev->msi->flags |= cfg_entry->data & @@ -3915,10 +3915,6 @@ static int pt_msgctrl_reg_write(struct p /* create value for writing to I/O device register */ val = *value; - throughable_mask = ~reg->emu_mask & valid_mask; - /* don't pass through MSI_ENABLE bit for MSI-INTx translation */ - if (ptdev->msi_trans_en) - throughable_mask &= ~PCI_MSI_FLAGS_ENABLE; *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask); /* update MSI */ @@ -3972,7 +3968,6 @@ static int pt_msgaddr32_reg_write(struct { struct pt_reg_info_tbl *reg = cfg_entry->reg; uint32_t writable_mask = 0; - uint32_t throughable_mask = 0; uint32_t old_addr = cfg_entry->data; /* modify emulate register */ @@ -3982,8 +3977,7 @@ static int pt_msgaddr32_reg_write(struct ptdev->msi->addr_lo = cfg_entry->data; /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; - *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask); + *value = PT_MERGE_VALUE(*value, dev_value, 0); /* update MSI */ if (cfg_entry->data != old_addr) @@ -4002,7 +3996,6 @@ static int pt_msgaddr64_reg_write(struct { struct pt_reg_info_tbl *reg = cfg_entry->reg; uint32_t writable_mask = 0; - uint32_t throughable_mask = 0; uint32_t old_addr = cfg_entry->data; /* check whether the type is 64 bit or not */ @@ -4020,8 +4013,7 @@ static int pt_msgaddr64_reg_write(struct ptdev->msi->addr_hi = cfg_entry->data; /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; - *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask); + *value = PT_MERGE_VALUE(*value, dev_value, 0); /* update MSI */ if (cfg_entry->data != old_addr) @@ -4041,7 +4033,6 @@ static int pt_msgdata_reg_write(struct p { struct pt_reg_info_tbl *reg = cfg_entry->reg; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; uint16_t old_data = cfg_entry->data; uint32_t flags = ptdev->msi->flags; uint32_t offset = reg->offset; @@ -4062,8 +4053,7 @@ static int pt_msgdata_reg_write(struct p ptdev->msi->data = cfg_entry->data; /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; - *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask); + *value = PT_MERGE_VALUE(*value, dev_value, 0); /* update MSI */ if (cfg_entry->data != old_data) @@ -4082,7 +4072,7 @@ static int pt_msixctrl_reg_write(struct { struct pt_reg_info_tbl *reg = cfg_entry->reg; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; + uint16_t throughable_mask = get_throughable_mask(ptdev, reg, valid_mask); uint16_t old_ctrl = cfg_entry->data; /* modify emulate register */ @@ -4090,7 +4080,6 @@ static int pt_msixctrl_reg_write(struct cfg_entry->data = PT_MERGE_VALUE(*value, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask); /* update MSI-X */
xsa131-qemut-5.patch
(application/octet-stream, 779 B)
xen/pt: mark all PCIe capability bits read-only xen_pt_emu_reg_pcie[]'s PCI_EXP_DEVCAP needs to cover all bits as read- only to avoid unintended write-back (just a precaution, the field ought to be read-only in hardware). This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> Reviewed-by: Stefano Stabellini <[email protected]> --- a/hw/pass-through.c +++ b/hw/pass-through.c @@ -577,7 +577,7 @@ static struct pt_reg_info_tbl pt_emu_reg .offset = PCI_EXP_DEVCAP, .size = 4, .init_val = 0x00000000, - .ro_mask = 0x1FFCFFFF, + .ro_mask = 0xFFFFFFFF, .emu_mask = 0x10000000, .init = pt_common_reg_init, .u.dw.read = pt_long_reg_read,
xsa131-qemut-6.patch
(application/octet-stream, 3 KB)
xen/pt: mark reserved bits in PCI config space fields The adjustments are solely to make the subsequent patches work right (and hence make the patch set consistent), namely if permissive mode (introduced by the last patch) gets used (as both reserved registers and reserved fields must be similarly protected from guest access in default mode, but the guest should be allowed access to them in permissive mode). This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> --- a/hw/pass-through.c +++ b/hw/pass-through.c @@ -283,7 +283,7 @@ static struct pt_reg_info_tbl pt_emu_reg .offset = PCI_COMMAND, .size = 2, .init_val = 0x0000, - .ro_mask = 0xF880, + .res_mask = 0xF880, .emu_mask = 0x0743, .init = pt_common_reg_init, .u.w.read = pt_word_reg_read, @@ -310,7 +310,8 @@ static struct pt_reg_info_tbl pt_emu_reg .offset = PCI_STATUS, .size = 2, .init_val = 0x0000, - .ro_mask = 0x06FF, + .res_mask = 0x0007, + .ro_mask = 0x06F8, .emu_mask = 0x0010, .init = pt_status_reg_init, .u.w.read = pt_word_reg_read, @@ -496,7 +497,8 @@ static struct pt_reg_info_tbl pt_emu_reg .offset = PCI_PM_CTRL, .size = 2, .init_val = 0x0008, - .ro_mask = 0xE1FC, + .res_mask = 0x00F0, + .ro_mask = 0xE10C, .emu_mask = 0x8100, .init = pt_pmcsr_reg_init, .u.w.read = pt_word_reg_read, @@ -508,7 +510,8 @@ static struct pt_reg_info_tbl pt_emu_reg .offset = PCI_PM_CTRL, .size = 2, .init_val = 0x0008, - .ro_mask = 0xE1FC, + .res_mask = 0x00F0, + .ro_mask = 0xE10C, .emu_mask = 0x810B, .init = pt_pmcsr_reg_init, .u.w.read = pt_word_reg_read, @@ -656,7 +659,8 @@ static struct pt_reg_info_tbl pt_emu_reg .offset = PCI_MSI_FLAGS, // 2 .size = 2, .init_val = 0x0000, - .ro_mask = 0xFF8E, + .res_mask = 0xFE00, + .ro_mask = 0x018E, .emu_mask = 0x017E, .init = pt_msgctrl_reg_init, .u.w.read = pt_word_reg_read, @@ -779,7 +783,8 @@ static struct pt_reg_info_tbl pt_emu_reg .offset = PCI_MSI_FLAGS, // 2 .size = 2, .init_val = 0x0000, - .ro_mask = 0x3FFF, + .res_mask = 0x3800, + .ro_mask = 0x07FF, .emu_mask = 0x0000, .init = pt_msixctrl_reg_init, .u.w.read = pt_word_reg_read, --- a/hw/pass-through.h +++ b/hw/pass-through.h @@ -376,6 +376,8 @@ struct pt_reg_info_tbl { uint32_t size; /* reg initial value */ uint32_t init_val; + /* reg reserved field mask (ON:reserved, OFF:defined) */ + uint32_t res_mask; /* reg read only field mask (ON:RO/ROS, OFF:other) */ uint32_t ro_mask; /* reg emulate field mask (ON:emu, OFF:passthrough) */
xsa131-qemut-7.patch
(application/octet-stream, 2.4 KB)
xen/pt: add a few PCI config space field descriptions Since the next patch will turn all not explicitly described fields read-only by default, those fields that have guest writable bits need to be given explicit descriptors. This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> --- a/hw/pass-through.c +++ b/hw/pass-through.c @@ -538,6 +538,16 @@ static struct pt_reg_info_tbl pt_emu_reg .u.b.restore = NULL, }, { + .offset = PCI_VPD_ADDR, + .size = 2, + .ro_mask = 0x0003, + .emu_mask = 0x0003, + .init = pt_common_reg_init, + .u.w.read = pt_word_reg_read, + .u.w.write = pt_word_reg_write, + .u.w.restore = pt_word_reg_restore, + }, + { .size = 0, }, }; @@ -599,6 +609,17 @@ static struct pt_reg_info_tbl pt_emu_reg .u.w.write = pt_word_reg_write, .u.w.restore = pt_word_reg_restore, }, + /* Device Status reg */ + { + .offset = PCI_EXP_DEVSTA, + .size = 2, + .res_mask = 0xFFC0, + .ro_mask = 0x0030, + .init = pt_common_reg_init, + .u.w.read = pt_word_reg_read, + .u.w.write = pt_word_reg_write, + .u.w.restore = pt_word_reg_restore, + }, /* Link Control reg */ { .offset = PCI_EXP_LNKCTL, @@ -611,6 +632,16 @@ static struct pt_reg_info_tbl pt_emu_reg .u.w.write = pt_word_reg_write, .u.w.restore = pt_word_reg_restore, }, + /* Link Status reg */ + { + .offset = PCI_EXP_LNKSTA, + .size = 2, + .ro_mask = 0x3FFF, + .init = pt_common_reg_init, + .u.w.read = pt_word_reg_read, + .u.w.write = pt_word_reg_write, + .u.w.restore = pt_word_reg_restore, + }, /* Device Control 2 reg */ { .offset = 0x28, --- a/hw/pass-through.h +++ b/hw/pass-through.h @@ -105,6 +105,14 @@ #define PCI_EXP_TYPE_ROOT_EC 0xa #endif +#ifndef PCI_VPD_ADDR +/* Vital Product Data */ +#define PCI_VPD_ADDR 2 /* Address to access (15 bits!) */ +#define PCI_VPD_ADDR_MASK 0x7fff /* Address mask */ +#define PCI_VPD_ADDR_F 0x8000 /* Write 0, 1 indicates completion */ +#define PCI_VPD_DATA 4 /* 32-bits of data returned here */ +#endif + #ifndef PCI_ERR_UNCOR_MASK /* Uncorrectable Error Mask */ #define PCI_ERR_UNCOR_MASK 8
xsa131-qemut-8.patch
(application/octet-stream, 5.2 KB)
xen/pt: unknown PCI config space fields should be read-only ... by default. Add a per-device "permissive" mode similar to pciback's to allow restoring previous behavior (and hence break security again, i.e. should be used only for trusted guests). This is part of XSA-131. Signed-off-by: Jan Beulich <[email protected]> Acked-by: Stefano Stabellini <[email protected]> Reviewed-by: Anthony PERARD <[email protected]>) --- a/hw/pass-through.c +++ b/hw/pass-through.c @@ -1613,10 +1613,10 @@ static void pt_pci_write_config(PCIDevic uint32_t find_addr = address; uint32_t real_offset = 0; uint32_t valid_mask = 0xFFFFFFFF; - uint32_t read_val = 0, wb_mask; + uint32_t read_val = 0, wb_mask, wp_mask; uint8_t *ptr_val = NULL; int emul_len = 0; - int index = 0; + int index = 0, wp_flag = 0; int ret = 0; #ifdef PT_DEBUG_PCI_CONFIG_ACCESS @@ -1695,7 +1695,14 @@ static void pt_pci_write_config(PCIDevic /* pass directly to libpci for passthrough type register group */ if (reg_grp_entry == NULL) + { + if (!assigned_device->permissive) + { + wb_mask = 0; + wp_flag = 1; + } goto out; + } /* adjust the read and write value to appropriate CFC-CFF window */ read_val <<= ((address & 3) << 3); @@ -1714,11 +1721,12 @@ static void pt_pci_write_config(PCIDevic valid_mask = (0xFFFFFFFF >> ((4 - emul_len) << 3)); valid_mask <<= ((find_addr - real_offset) << 3); ptr_val = ((uint8_t *)&val + (real_offset & 3)); - if (reg->emu_mask == (0xFFFFFFFF >> ((4 - reg->size) << 3))) { - wb_mask &= ~((reg->emu_mask - >> ((find_addr - real_offset) << 3)) + wp_mask = reg->emu_mask | reg->ro_mask; + if (!assigned_device->permissive) + wp_mask |= reg->res_mask; + if (wp_mask == (0xFFFFFFFF >> ((4 - reg->size) << 3))) + wb_mask &= ~((wp_mask >> ((find_addr - real_offset) << 3)) << ((len - emul_len) << 3)); - } /* do emulation depend on register size */ switch (reg->size) { @@ -1767,6 +1775,16 @@ static void pt_pci_write_config(PCIDevic /* nothing to do with passthrough type register, * continue to find next byte */ + if (!assigned_device->permissive) + { + wb_mask &= ~(0xff << ((len - emul_len) << 3)); + /* Unused BARs will make it here, but we don't want to issue + * warnings for writes to them (bogus writes get dealt with + * above). + */ + if (index < 0) + wp_flag = 1; + } emul_len--; find_addr++; } @@ -1776,6 +1794,15 @@ static void pt_pci_write_config(PCIDevic val >>= ((address & 3) << 3); out: + if (wp_flag && !assigned_device->permissive_warned) + { + assigned_device->permissive_warned = 1; + PT_LOG("Write-back to unknown field 0x%02x (partially) inhibited (0x%0*x)\n", + addr, len * 2, wb_mask); + PT_LOG("If device %02x:%02x.%o doesn't work, try enabling permissive\n", + pci_bus_num(d->bus), PCI_SLOT(d->devfn), PCI_FUNC(d->devfn)); + PT_LOG("mode (unsafe) and if it helps report the problem to xen-devel\n"); + } for (index = 0; wb_mask; index += len) { /* unknown regs are passed through */ while (!(wb_mask & 0xff)) { @@ -3484,6 +3511,9 @@ static uint32_t get_throughable_mask(con { uint32_t throughable_mask = ~(reg->emu_mask | reg->ro_mask); + if (!ptdev->permissive) + throughable_mask &= ~reg->res_mask; + return throughable_mask & valid_mask; } @@ -4322,7 +4352,7 @@ static struct pt_dev * register_real_dev uint8_t e_device, e_intx; uint16_t cmd = 0; char *key, *val; - int msi_translate, power_mgmt; + int msi_translate, power_mgmt, permissive = 0; PT_LOG("Assigning real physical device %02x:%02x.%x ...\n", r_bus, r_dev, r_func); @@ -4366,6 +4396,8 @@ static struct pt_dev * register_real_dev else PT_LOG("Error: unrecognized value for msitranslate=\n"); } + else if (strcmp(key, "permissive") == 0) + permissive = 1; else if (strcmp(key, "power_mgmt") == 0) { if (strcmp(val, "0") == 0) @@ -4403,6 +4435,7 @@ static struct pt_dev * register_real_dev assigned_device->msi_trans_cap = msi_translate; assigned_device->power_mgmt = power_mgmt; assigned_device->is_virtfn = pt_dev_is_virtfn(pci_dev); + assigned_device->permissive = permissive; pt_iomul_init(assigned_device, r_bus, r_dev, r_func); /* Initialize virtualized PCI configuration (Extended 256 Bytes) */ --- a/hw/pass-through.h +++ b/hw/pass-through.h @@ -242,6 +242,8 @@ struct pt_dev { unsigned power_mgmt:1; struct pt_pm_info *pm_state; /* PM virtualization */ unsigned is_virtfn:1; + unsigned permissive:1; + unsigned permissive_warned:1; /* io port multiplexing */ #define PCI_IOMUL_INVALID_FD (-1)
xsa131-qemuu-1.patch
(application/octet-stream, 2.1 KB)
xen/MSI: don't open-code pass-through of enable bit modifications Without this the actual XSA-131 fix would cause the enable bit to not get set anymore (due to the write back getting suppressed there based on the OR of emu_mask, ro_mask, and res_mask). Note that the fiddling with the enable bit shouldn't really be done by qemu, but making this work right (via libxc and the hypervisor) will require more extensive changes, which can be postponed until after the security issue got addressed. This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> Acked-by: Stefano Stabellini <[email protected]> --- a/hw/xen/xen_pt_config_init.c +++ b/hw/xen/xen_pt_config_init.c @@ -1059,7 +1059,6 @@ static int xen_pt_msgctrl_reg_write(XenP XenPTMSI *msi = s->msi; uint16_t writable_mask = 0; uint16_t throughable_mask = 0; - uint16_t raw_val; /* Currently no support for multi-vector */ if (*val & PCI_MSI_FLAGS_QSIZE) { @@ -1072,12 +1071,11 @@ static int xen_pt_msgctrl_reg_write(XenP msi->flags |= cfg_entry->data & ~PCI_MSI_FLAGS_ENABLE; /* create value for writing to I/O device register */ - raw_val = *val; throughable_mask = ~reg->emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); /* update MSI */ - if (raw_val & PCI_MSI_FLAGS_ENABLE) { + if (*val & PCI_MSI_FLAGS_ENABLE) { /* setup MSI pirq for the first time */ if (!msi->initialized) { /* Init physical one */ @@ -1105,10 +1103,6 @@ static int xen_pt_msgctrl_reg_write(XenP xen_pt_msi_disable(s); } - /* pass through MSI_ENABLE bit */ - *val &= ~PCI_MSI_FLAGS_ENABLE; - *val |= raw_val & PCI_MSI_FLAGS_ENABLE; - return 0; } @@ -1311,7 +1305,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] .size = 2, .init_val = 0x0000, .ro_mask = 0xFF8E, - .emu_mask = 0x017F, + .emu_mask = 0x017E, .init = xen_pt_msgctrl_reg_init, .u.w.read = xen_pt_word_reg_read, .u.w.write = xen_pt_msgctrl_reg_write,
xsa131-qemuu-2.patch
(application/octet-stream, 2.6 KB)
xen/pt: consolidate PM capability emu_mask
There's no point in xen_pt_pmcsr_reg_{read,write}() each ORing
PCI_PM_CTRL_STATE_MASK and PCI_PM_CTRL_NO_SOFT_RESET into a local
emu_mask variable - we can have the same effect by setting the field
descriptor's emu_mask member suitably right away. Note that
xen_pt_pmcsr_reg_write() is being retained in order to allow later
patches to be less intrusive.
This is a preparatory patch for XSA-131.
Signed-off-by: Jan Beulich <[email protected]>
Acked-by: Stefano Stabellini <[email protected]>
Acked-by: Ian Campbell <[email protected]>
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -935,38 +935,21 @@ static XenPTRegInfo xen_pt_emu_reg_pcie[
* Power Management Capability
*/
-/* read Power Management Control/Status register */
-static int xen_pt_pmcsr_reg_read(XenPCIPassthroughState *s, XenPTReg *cfg_entry,
- uint16_t *value, uint16_t valid_mask)
-{
- XenPTRegInfo *reg = cfg_entry->reg;
- uint16_t valid_emu_mask = reg->emu_mask;
-
- valid_emu_mask |= PCI_PM_CTRL_STATE_MASK | PCI_PM_CTRL_NO_SOFT_RESET;
-
- valid_emu_mask = valid_emu_mask & valid_mask;
- *value = XEN_PT_MERGE_VALUE(*value, cfg_entry->data, ~valid_emu_mask);
-
- return 0;
-}
/* write Power Management Control/Status register */
static int xen_pt_pmcsr_reg_write(XenPCIPassthroughState *s,
XenPTReg *cfg_entry, uint16_t *val,
uint16_t dev_value, uint16_t valid_mask)
{
XenPTRegInfo *reg = cfg_entry->reg;
- uint16_t emu_mask = reg->emu_mask;
uint16_t writable_mask = 0;
uint16_t throughable_mask = 0;
- emu_mask |= PCI_PM_CTRL_STATE_MASK | PCI_PM_CTRL_NO_SOFT_RESET;
-
/* modify emulate register */
- writable_mask = emu_mask & ~reg->ro_mask & valid_mask;
+ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask;
cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask);
/* create value for writing to I/O device register */
- throughable_mask = ~emu_mask & valid_mask;
+ throughable_mask = ~reg->emu_mask & valid_mask;
*val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
return 0;
@@ -1002,9 +985,9 @@ static XenPTRegInfo xen_pt_emu_reg_pm[]
.size = 2,
.init_val = 0x0008,
.ro_mask = 0xE1FC,
- .emu_mask = 0x8100,
+ .emu_mask = 0x810B,
.init = xen_pt_common_reg_init,
- .u.w.read = xen_pt_pmcsr_reg_read,
+ .u.w.read = xen_pt_word_reg_read,
.u.w.write = xen_pt_pmcsr_reg_write,
},
{
xsa131-qemuu-3.patch
(application/octet-stream, 785 B)
xen/pt: correctly handle PM status bit xen_pt_pmcsr_reg_write() needs an adjustment to deal with the RW1C nature of the not passed through bit 15 (PCI_PM_CTRL_PME_STATUS). This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> Reviewed-by: Stefano Stabellini <[email protected]> --- a/hw/xen/xen_pt_config_init.c +++ b/hw/xen/xen_pt_config_init.c @@ -950,7 +950,8 @@ static int xen_pt_pmcsr_reg_write(XenPCI /* create value for writing to I/O device register */ throughable_mask = ~reg->emu_mask & valid_mask; - *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); + *val = XEN_PT_MERGE_VALUE(*val, dev_value & ~PCI_PM_CTRL_PME_STATUS, + throughable_mask); return 0; }
xsa131-qemuu-4.3-1.patch
(application/octet-stream, 2.1 KB)
xen/MSI: don't open-code pass-through of enable bit modifications Without this the actual XSA-131 fix would cause the enable bit to not get set anymore (due to the write back getting suppressed there based on the OR of emu_mask, ro_mask, and res_mask). Note that the fiddling with the enable bit shouldn't really be done by qemu, but making this work right (via libxc and the hypervisor) will require more extensive changes, which can be postponed until after the security issue got addressed. This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> Acked-by: Stefano Stabellini <[email protected]> --- a/hw/xen_pt_config_init.c +++ b/hw/xen_pt_config_init.c @@ -1055,7 +1055,6 @@ static int xen_pt_msgctrl_reg_write(XenP XenPTMSI *msi = s->msi; uint16_t writable_mask = 0; uint16_t throughable_mask = 0; - uint16_t raw_val; /* Currently no support for multi-vector */ if (*val & PCI_MSI_FLAGS_QSIZE) { @@ -1068,12 +1067,11 @@ static int xen_pt_msgctrl_reg_write(XenP msi->flags |= cfg_entry->data & ~PCI_MSI_FLAGS_ENABLE; /* create value for writing to I/O device register */ - raw_val = *val; throughable_mask = ~reg->emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); /* update MSI */ - if (raw_val & PCI_MSI_FLAGS_ENABLE) { + if (*val & PCI_MSI_FLAGS_ENABLE) { /* setup MSI pirq for the first time */ if (!msi->initialized) { /* Init physical one */ @@ -1101,10 +1099,6 @@ static int xen_pt_msgctrl_reg_write(XenP msi->flags &= ~PCI_MSI_FLAGS_ENABLE; } - /* pass through MSI_ENABLE bit */ - *val &= ~PCI_MSI_FLAGS_ENABLE; - *val |= raw_val & PCI_MSI_FLAGS_ENABLE; - return 0; } @@ -1303,7 +1297,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] .size = 2, .init_val = 0x0000, .ro_mask = 0xFF8E, - .emu_mask = 0x017F, + .emu_mask = 0x017E, .init = xen_pt_msgctrl_reg_init, .u.w.read = xen_pt_word_reg_read, .u.w.write = xen_pt_msgctrl_reg_write,
xsa131-qemuu-4.3-2.patch
(application/octet-stream, 2.6 KB)
xen/pt: consolidate PM capability emu_mask
There's no point in xen_pt_pmcsr_reg_{read,write}() each ORing
PCI_PM_CTRL_STATE_MASK and PCI_PM_CTRL_NO_SOFT_RESET into a local
emu_mask variable - we can have the same effect by setting the field
descriptor's emu_mask member suitably right away. Note that
xen_pt_pmcsr_reg_write() is being retained in order to allow later
patches to be less intrusive.
This is a preparatory patch for XSA-131.
Signed-off-by: Jan Beulich <[email protected]>
Acked-by: Stefano Stabellini <[email protected]>
Acked-by: Ian Campbell <[email protected]>
--- a/hw/xen_pt_config_init.c
+++ b/hw/xen_pt_config_init.c
@@ -935,38 +935,21 @@ static XenPTRegInfo xen_pt_emu_reg_pcie[
* Power Management Capability
*/
-/* read Power Management Control/Status register */
-static int xen_pt_pmcsr_reg_read(XenPCIPassthroughState *s, XenPTReg *cfg_entry,
- uint16_t *value, uint16_t valid_mask)
-{
- XenPTRegInfo *reg = cfg_entry->reg;
- uint16_t valid_emu_mask = reg->emu_mask;
-
- valid_emu_mask |= PCI_PM_CTRL_STATE_MASK | PCI_PM_CTRL_NO_SOFT_RESET;
-
- valid_emu_mask = valid_emu_mask & valid_mask;
- *value = XEN_PT_MERGE_VALUE(*value, cfg_entry->data, ~valid_emu_mask);
-
- return 0;
-}
/* write Power Management Control/Status register */
static int xen_pt_pmcsr_reg_write(XenPCIPassthroughState *s,
XenPTReg *cfg_entry, uint16_t *val,
uint16_t dev_value, uint16_t valid_mask)
{
XenPTRegInfo *reg = cfg_entry->reg;
- uint16_t emu_mask = reg->emu_mask;
uint16_t writable_mask = 0;
uint16_t throughable_mask = 0;
- emu_mask |= PCI_PM_CTRL_STATE_MASK | PCI_PM_CTRL_NO_SOFT_RESET;
-
/* modify emulate register */
- writable_mask = emu_mask & ~reg->ro_mask & valid_mask;
+ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask;
cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask);
/* create value for writing to I/O device register */
- throughable_mask = ~emu_mask & valid_mask;
+ throughable_mask = ~reg->emu_mask & valid_mask;
*val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
return 0;
@@ -1002,9 +985,9 @@ static XenPTRegInfo xen_pt_emu_reg_pm[]
.size = 2,
.init_val = 0x0008,
.ro_mask = 0xE1FC,
- .emu_mask = 0x8100,
+ .emu_mask = 0x810B,
.init = xen_pt_common_reg_init,
- .u.w.read = xen_pt_pmcsr_reg_read,
+ .u.w.read = xen_pt_word_reg_read,
.u.w.write = xen_pt_pmcsr_reg_write,
},
{
xsa131-qemuu-4.3-3.patch
(application/octet-stream, 777 B)
xen/pt: correctly handle PM status bit xen_pt_pmcsr_reg_write() needs an adjustment to deal with the RW1C nature of the not passed through bit 15 (PCI_PM_CTRL_PME_STATUS). This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> Reviewed-by: Stefano Stabellini <[email protected]> --- a/hw/xen_pt_config_init.c +++ b/hw/xen_pt_config_init.c @@ -950,7 +950,8 @@ static int xen_pt_pmcsr_reg_write(XenPCI /* create value for writing to I/O device register */ throughable_mask = ~reg->emu_mask & valid_mask; - *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); + *val = XEN_PT_MERGE_VALUE(*val, dev_value & ~PCI_PM_CTRL_PME_STATUS, + throughable_mask); return 0; }
xsa131-qemuu-4.3-4.patch
(application/octet-stream, 9.3 KB)
xen/pt: split out calculation of throughable mask in PCI config space handling This is just to avoid having to adjust that calculation later in multiple places. Note that including ->ro_mask in get_throughable_mask()'s calculation is only an apparent (i.e. benign) behavioral change: For r/o fields it doesn't matter > whether they get passed through - either the same flag is also set in emu_mask (then there's no change at all) or the field is r/o in hardware (and hence a write won't change it anyway). This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> Acked-by: Stefano Stabellini <[email protected]> Reviewed-by: Anthony PERARD <[email protected]> --- a/hw/xen_pt_config_init.c +++ b/hw/xen_pt_config_init.c @@ -95,6 +95,14 @@ XenPTReg *xen_pt_find_reg(XenPTRegGroup return NULL; } +static uint32_t get_throughable_mask(const XenPCIPassthroughState *s, + const XenPTRegInfo *reg, + uint32_t valid_mask) +{ + uint32_t throughable_mask = ~(reg->emu_mask | reg->ro_mask); + + return throughable_mask & valid_mask; +} /**************** * general register functions @@ -157,14 +165,13 @@ static int xen_pt_byte_reg_write(XenPCIP { XenPTRegInfo *reg = cfg_entry->reg; uint8_t writable_mask = 0; - uint8_t throughable_mask = 0; + uint8_t throughable_mask = get_throughable_mask(s, reg, valid_mask); /* modify emulate register */ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask; cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); return 0; @@ -175,14 +182,13 @@ static int xen_pt_word_reg_write(XenPCIP { XenPTRegInfo *reg = cfg_entry->reg; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; + uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask); /* modify emulate register */ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask; cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); return 0; @@ -193,14 +199,13 @@ static int xen_pt_long_reg_write(XenPCIP { XenPTRegInfo *reg = cfg_entry->reg; uint32_t writable_mask = 0; - uint32_t throughable_mask = 0; + uint32_t throughable_mask = get_throughable_mask(s, reg, valid_mask); /* modify emulate register */ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask; cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); return 0; @@ -292,15 +297,13 @@ static int xen_pt_cmd_reg_write(XenPCIPa { XenPTRegInfo *reg = cfg_entry->reg; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; + uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask); /* modify emulate register */ writable_mask = ~reg->ro_mask & valid_mask; cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; - if (*val & PCI_COMMAND_INTX_DISABLE) { throughable_mask |= PCI_COMMAND_INTX_DISABLE; } else { @@ -456,7 +459,6 @@ static int xen_pt_bar_reg_write(XenPCIPa PCIDevice *d = &s->dev; const PCIIORegion *r; uint32_t writable_mask = 0; - uint32_t throughable_mask = 0; uint32_t bar_emu_mask = 0; uint32_t bar_ro_mask = 0; uint32_t r_size = 0; @@ -513,8 +515,7 @@ static int xen_pt_bar_reg_write(XenPCIPa } /* create value for writing to I/O device register */ - throughable_mask = ~bar_emu_mask & valid_mask; - *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); + *val = XEN_PT_MERGE_VALUE(*val, dev_value, 0); return 0; } @@ -528,9 +529,8 @@ static int xen_pt_exp_rom_bar_reg_write( XenPTRegion *base = NULL; PCIDevice *d = (PCIDevice *)&s->dev; uint32_t writable_mask = 0; - uint32_t throughable_mask = 0; + uint32_t throughable_mask = get_throughable_mask(s, reg, valid_mask); pcibus_t r_size = 0; - uint32_t bar_emu_mask = 0; uint32_t bar_ro_mask = 0; r_size = d->io_regions[PCI_ROM_SLOT].size; @@ -539,7 +539,6 @@ static int xen_pt_exp_rom_bar_reg_write( r_size = xen_pt_get_emul_size(base->bar_flag, r_size); /* set emulate mask and read-only mask */ - bar_emu_mask = reg->emu_mask; bar_ro_mask = (reg->ro_mask | (r_size - 1)) & ~PCI_ROM_ADDRESS_ENABLE; /* modify emulate register */ @@ -547,7 +546,6 @@ static int xen_pt_exp_rom_bar_reg_write( cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~bar_emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); return 0; @@ -942,14 +940,13 @@ static int xen_pt_pmcsr_reg_write(XenPCI { XenPTRegInfo *reg = cfg_entry->reg; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; + uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask); /* modify emulate register */ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask; cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value & ~PCI_PM_CTRL_PME_STATUS, throughable_mask); @@ -1038,7 +1035,7 @@ static int xen_pt_msgctrl_reg_write(XenP XenPTRegInfo *reg = cfg_entry->reg; XenPTMSI *msi = s->msi; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; + uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask); /* Currently no support for multi-vector */ if (*val & PCI_MSI_FLAGS_QSIZE) { @@ -1051,7 +1048,6 @@ static int xen_pt_msgctrl_reg_write(XenP msi->flags |= cfg_entry->data & ~PCI_MSI_FLAGS_ENABLE; /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); /* update MSI */ @@ -1163,7 +1159,6 @@ static int xen_pt_msgaddr32_reg_write(Xe { XenPTRegInfo *reg = cfg_entry->reg; uint32_t writable_mask = 0; - uint32_t throughable_mask = 0; uint32_t old_addr = cfg_entry->data; /* modify emulate register */ @@ -1172,8 +1167,7 @@ static int xen_pt_msgaddr32_reg_write(Xe s->msi->addr_lo = cfg_entry->data; /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; - *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); + *val = XEN_PT_MERGE_VALUE(*val, dev_value, 0); /* update MSI */ if (cfg_entry->data != old_addr) { @@ -1191,7 +1185,6 @@ static int xen_pt_msgaddr64_reg_write(Xe { XenPTRegInfo *reg = cfg_entry->reg; uint32_t writable_mask = 0; - uint32_t throughable_mask = 0; uint32_t old_addr = cfg_entry->data; /* check whether the type is 64 bit or not */ @@ -1208,8 +1201,7 @@ static int xen_pt_msgaddr64_reg_write(Xe s->msi->addr_hi = cfg_entry->data; /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; - *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); + *val = XEN_PT_MERGE_VALUE(*val, dev_value, 0); /* update MSI */ if (cfg_entry->data != old_addr) { @@ -1231,7 +1223,6 @@ static int xen_pt_msgdata_reg_write(XenP XenPTRegInfo *reg = cfg_entry->reg; XenPTMSI *msi = s->msi; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; uint16_t old_data = cfg_entry->data; uint32_t offset = reg->offset; @@ -1249,8 +1240,7 @@ static int xen_pt_msgdata_reg_write(XenP msi->data = cfg_entry->data; /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; - *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); + *val = XEN_PT_MERGE_VALUE(*val, dev_value, 0); /* update MSI */ if (cfg_entry->data != old_data) { @@ -1412,7 +1402,7 @@ static int xen_pt_msixctrl_reg_write(Xen { XenPTRegInfo *reg = cfg_entry->reg; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; + uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask); int debug_msix_enabled_old; /* modify emulate register */ @@ -1420,7 +1410,6 @@ static int xen_pt_msixctrl_reg_write(Xen cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); /* update MSI-X */
xsa131-qemuu-4.3-5.patch
(application/octet-stream, 799 B)
xen/pt: mark all PCIe capability bits read-only xen_pt_emu_reg_pcie[]'s PCI_EXP_DEVCAP needs to cover all bits as read- only to avoid unintended write-back (just a precaution, the field ought to be read-only in hardware). This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> Reviewed-by: Stefano Stabellini <[email protected]> --- a/hw/xen_pt_config_init.c +++ b/hw/xen_pt_config_init.c @@ -873,7 +873,7 @@ static XenPTRegInfo xen_pt_emu_reg_pcie[ .offset = PCI_EXP_DEVCAP, .size = 4, .init_val = 0x00000000, - .ro_mask = 0x1FFCFFFF, + .ro_mask = 0xFFFFFFFF, .emu_mask = 0x10000000, .init = xen_pt_common_reg_init, .u.dw.read = xen_pt_long_reg_read,
xsa131-qemuu-4.3-6.patch
(application/octet-stream, 2.7 KB)
xen/pt: mark reserved bits in PCI config space fields The adjustments are solely to make the subsequent patches work right (and hence make the patch set consistent), namely if permissive mode (introduced by the last patch) gets used (as both reserved registers and reserved fields must be similarly protected from guest access in default mode, but the guest should be allowed access to them in permissive mode). This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> --- a/hw/xen_pt.h +++ b/hw/xen_pt.h @@ -101,6 +101,8 @@ struct XenPTRegInfo { uint32_t offset; uint32_t size; uint32_t init_val; + /* reg reserved field mask (ON:reserved, OFF:defined) */ + uint32_t res_mask; /* reg read only field mask (ON:RO/ROS, OFF:other) */ uint32_t ro_mask; /* reg emulate field mask (ON:emu, OFF:passthrough) */ --- a/hw/xen_pt_config_init.c +++ b/hw/xen_pt_config_init.c @@ -580,7 +580,7 @@ static XenPTRegInfo xen_pt_emu_reg_heade .offset = PCI_COMMAND, .size = 2, .init_val = 0x0000, - .ro_mask = 0xF880, + .res_mask = 0xF880, .emu_mask = 0x0743, .init = xen_pt_common_reg_init, .u.w.read = xen_pt_word_reg_read, @@ -605,7 +605,8 @@ static XenPTRegInfo xen_pt_emu_reg_heade .offset = PCI_STATUS, .size = 2, .init_val = 0x0000, - .ro_mask = 0x06FF, + .res_mask = 0x0007, + .ro_mask = 0x06F8, .emu_mask = 0x0010, .init = xen_pt_status_reg_init, .u.w.read = xen_pt_word_reg_read, @@ -982,7 +983,8 @@ static XenPTRegInfo xen_pt_emu_reg_pm[] .offset = PCI_PM_CTRL, .size = 2, .init_val = 0x0008, - .ro_mask = 0xE1FC, + .res_mask = 0x00F0, + .ro_mask = 0xE10C, .emu_mask = 0x810B, .init = xen_pt_common_reg_init, .u.w.read = xen_pt_word_reg_read, @@ -1270,7 +1272,8 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] .offset = PCI_MSI_FLAGS, .size = 2, .init_val = 0x0000, - .ro_mask = 0xFF8E, + .res_mask = 0xFE00, + .ro_mask = 0x018E, .emu_mask = 0x017E, .init = xen_pt_msgctrl_reg_init, .u.w.read = xen_pt_word_reg_read, @@ -1446,7 +1449,8 @@ static XenPTRegInfo xen_pt_emu_reg_msix[ .offset = PCI_MSI_FLAGS, .size = 2, .init_val = 0x0000, - .ro_mask = 0x3FFF, + .res_mask = 0x3800, + .ro_mask = 0x07FF, .emu_mask = 0x0000, .init = xen_pt_msixctrl_reg_init, .u.w.read = xen_pt_word_reg_read,
xsa131-qemuu-4.3-7.patch
(application/octet-stream, 2.2 KB)
xen/pt: add a few PCI config space field descriptions Since the next patch will turn all not explicitly described fields read-only by default, those fields that have guest writable bits need to be given explicit descriptors. This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> --- Notes: - blindly allowing all VPD reads may still be a problem (out of bounds addresses aren't allowed, but the spec doesn't say what the effect would be) ==> also an issue in pciback? - Vendor Specific cap regs aren't in the table (will become r/o by default with this change) - many PCIe cap regs aren't in the table (will again become r/o) - same for PM cap regs at offsets 6 and 7 --- a/hw/xen_pt_config_init.c +++ b/hw/xen_pt_config_init.c @@ -756,6 +756,15 @@ static XenPTRegInfo xen_pt_emu_reg_vpd[] .u.b.write = xen_pt_byte_reg_write, }, { + .offset = PCI_VPD_ADDR, + .size = 2, + .ro_mask = 0x0003, + .emu_mask = 0x0003, + .init = xen_pt_common_reg_init, + .u.w.read = xen_pt_word_reg_read, + .u.w.write = xen_pt_word_reg_write, + }, + { .size = 0, }, }; @@ -891,6 +900,16 @@ static XenPTRegInfo xen_pt_emu_reg_pcie[ .u.w.read = xen_pt_word_reg_read, .u.w.write = xen_pt_word_reg_write, }, + /* Device Status reg */ + { + .offset = PCI_EXP_DEVSTA, + .size = 2, + .res_mask = 0xFFC0, + .ro_mask = 0x0030, + .init = xen_pt_common_reg_init, + .u.w.read = xen_pt_word_reg_read, + .u.w.write = xen_pt_word_reg_write, + }, /* Link Control reg */ { .offset = PCI_EXP_LNKCTL, @@ -902,6 +921,15 @@ static XenPTRegInfo xen_pt_emu_reg_pcie[ .u.w.read = xen_pt_word_reg_read, .u.w.write = xen_pt_word_reg_write, }, + /* Link Status reg */ + { + .offset = PCI_EXP_LNKSTA, + .size = 2, + .ro_mask = 0x3FFF, + .init = xen_pt_common_reg_init, + .u.w.read = xen_pt_word_reg_read, + .u.w.write = xen_pt_word_reg_write, + }, /* Device Control 2 reg */ { .offset = 0x28,
xsa131-qemuu-4.3-8.patch
(application/octet-stream, 4.5 KB)
xen/pt: unknown PCI config space fields should be read-only ... by default. Add a per-device "permissive" mode similar to pciback's to allow restoring previous behavior (and hence break security again, i.e. should be used only for trusted guests). This is part of XSA-131. Signed-off-by: Jan Beulich <[email protected]> Acked-by: Stefano Stabellini <[email protected]> Reviewed-by: Anthony PERARD <[email protected]>) --- Notes: - What purpose does xen_pt_header_type_reg_init() serve (with .emu_mask being zero)? - In the qemu-trad case no equivalent logic to that setting/using direct_pci_{msi_translate,power_mgmt} is being added, as that logic seems broken (setting globals from device 0 xenstore settings). --- a/hw/xen_pt.c +++ b/hw/xen_pt.c @@ -239,6 +239,7 @@ static void xen_pt_pci_write_config(PCID XenPTReg *reg_entry = NULL; uint32_t find_addr = addr; XenPTRegInfo *reg = NULL; + bool wp_flag = false; if (xen_pt_pci_config_access_check(d, addr, len)) { return; @@ -278,6 +279,10 @@ static void xen_pt_pci_write_config(PCID /* pass directly to the real device for passthrough type register group */ if (reg_grp_entry == NULL) { + if (!s->permissive) { + wb_mask = 0; + wp_flag = true; + } goto out; } @@ -298,12 +303,15 @@ static void xen_pt_pci_write_config(PCID uint32_t real_offset = reg_grp_entry->base_offset + reg->offset; uint32_t valid_mask = 0xFFFFFFFF >> ((4 - emul_len) << 3); uint8_t *ptr_val = NULL; + uint32_t wp_mask = reg->emu_mask | reg->ro_mask; valid_mask <<= (find_addr - real_offset) << 3; ptr_val = (uint8_t *)&val + (real_offset & 3); - if (reg->emu_mask == (0xFFFFFFFF >> ((4 - reg->size) << 3))) { - wb_mask &= ~((reg->emu_mask - >> ((find_addr - real_offset) << 3)) + if (!s->permissive) { + wp_mask |= reg->res_mask; + } + if (wp_mask == (0xFFFFFFFF >> ((4 - reg->size) << 3))) { + wb_mask &= ~((wp_mask >> ((find_addr - real_offset) << 3)) << ((len - emul_len) << 3)); } @@ -347,6 +355,16 @@ static void xen_pt_pci_write_config(PCID } else { /* nothing to do with passthrough type register, * continue to find next byte */ + if (!s->permissive) { + wb_mask &= ~(0xff << ((len - emul_len) << 3)); + /* Unused BARs will make it here, but we don't want to issue + * warnings for writes to them (bogus writes get dealt with + * above). + */ + if (index < 0) { + wp_flag = true; + } + } emul_len--; find_addr++; } @@ -358,6 +376,13 @@ static void xen_pt_pci_write_config(PCID memory_region_transaction_commit(); out: + if (wp_flag && !s->permissive_warned) { + s->permissive_warned = true; + xen_pt_log(d, "Write-back to unknown field 0x%02x (partially) inhibited (0x%0*x)\n", + addr, len * 2, wb_mask); + xen_pt_log(d, "If the device doesn't work, try enabling permissive mode\n"); + xen_pt_log(d, "(unsafe) and if it helps report the problem to xen-devel\n"); + } for (index = 0; wb_mask; index += len) { /* unknown regs are passed through */ while (!(wb_mask & 0xff)) { @@ -838,6 +863,7 @@ static void xen_pt_unregister_device(PCI static Property xen_pci_passthrough_properties[] = { DEFINE_PROP_PCI_HOST_DEVADDR("hostaddr", XenPCIPassthroughState, hostaddr), + DEFINE_PROP_UINT8("permissive", XenPCIPassthroughState, permissive, 0), DEFINE_PROP_END_OF_LIST(), }; --- a/hw/xen_pt.h +++ b/hw/xen_pt.h @@ -197,6 +197,8 @@ struct XenPCIPassthroughState { PCIHostDeviceAddress hostaddr; bool is_virtfn; + uint8_t permissive; + bool permissive_warned; XenHostPCIDevice real_device; XenPTRegion bases[PCI_NUM_REGIONS]; /* Access regions */ QLIST_HEAD(, XenPTRegGroup) reg_grps; --- a/hw/xen_pt_config_init.c +++ b/hw/xen_pt_config_init.c @@ -101,6 +101,10 @@ static uint32_t get_throughable_mask(con { uint32_t throughable_mask = ~(reg->emu_mask | reg->ro_mask); + if (!s->permissive) { + throughable_mask &= ~reg->res_mask; + } + return throughable_mask & valid_mask; }
xsa131-qemuu-4.4-1.patch
(application/octet-stream, 2.1 KB)
xen/MSI: don't open-code pass-through of enable bit modifications Without this the actual XSA-131 fix would cause the enable bit to not get set anymore (due to the write back getting suppressed there based on the OR of emu_mask, ro_mask, and res_mask). Note that the fiddling with the enable bit shouldn't really be done by qemu, but making this work right (via libxc and the hypervisor) will require more extensive changes, which can be postponed until after the security issue got addressed. This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> Acked-by: Stefano Stabellini <[email protected]> --- a/hw/xen/xen_pt_config_init.c +++ b/hw/xen/xen_pt_config_init.c @@ -1055,7 +1055,6 @@ static int xen_pt_msgctrl_reg_write(XenP XenPTMSI *msi = s->msi; uint16_t writable_mask = 0; uint16_t throughable_mask = 0; - uint16_t raw_val; /* Currently no support for multi-vector */ if (*val & PCI_MSI_FLAGS_QSIZE) { @@ -1068,12 +1067,11 @@ static int xen_pt_msgctrl_reg_write(XenP msi->flags |= cfg_entry->data & ~PCI_MSI_FLAGS_ENABLE; /* create value for writing to I/O device register */ - raw_val = *val; throughable_mask = ~reg->emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); /* update MSI */ - if (raw_val & PCI_MSI_FLAGS_ENABLE) { + if (*val & PCI_MSI_FLAGS_ENABLE) { /* setup MSI pirq for the first time */ if (!msi->initialized) { /* Init physical one */ @@ -1101,10 +1099,6 @@ static int xen_pt_msgctrl_reg_write(XenP msi->flags &= ~PCI_MSI_FLAGS_ENABLE; } - /* pass through MSI_ENABLE bit */ - *val &= ~PCI_MSI_FLAGS_ENABLE; - *val |= raw_val & PCI_MSI_FLAGS_ENABLE; - return 0; } @@ -1303,7 +1297,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] .size = 2, .init_val = 0x0000, .ro_mask = 0xFF8E, - .emu_mask = 0x017F, + .emu_mask = 0x017E, .init = xen_pt_msgctrl_reg_init, .u.w.read = xen_pt_word_reg_read, .u.w.write = xen_pt_msgctrl_reg_write,
xsa131-qemuu-4.patch
(application/octet-stream, 9.3 KB)
xen/pt: split out calculation of throughable mask in PCI config space handling This is just to avoid having to adjust that calculation later in multiple places. Note that including ->ro_mask in get_throughable_mask()'s calculation is only an apparent (i.e. benign) behavioral change: For r/o fields it doesn't matter > whether they get passed through - either the same flag is also set in emu_mask (then there's no change at all) or the field is r/o in hardware (and hence a write won't change it anyway). This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> Acked-by: Stefano Stabellini <[email protected]> Reviewed-by: Anthony PERARD <[email protected]> --- a/hw/xen/xen_pt_config_init.c +++ b/hw/xen/xen_pt_config_init.c @@ -95,6 +95,14 @@ XenPTReg *xen_pt_find_reg(XenPTRegGroup return NULL; } +static uint32_t get_throughable_mask(const XenPCIPassthroughState *s, + const XenPTRegInfo *reg, + uint32_t valid_mask) +{ + uint32_t throughable_mask = ~(reg->emu_mask | reg->ro_mask); + + return throughable_mask & valid_mask; +} /**************** * general register functions @@ -157,14 +165,13 @@ static int xen_pt_byte_reg_write(XenPCIP { XenPTRegInfo *reg = cfg_entry->reg; uint8_t writable_mask = 0; - uint8_t throughable_mask = 0; + uint8_t throughable_mask = get_throughable_mask(s, reg, valid_mask); /* modify emulate register */ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask; cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); return 0; @@ -175,14 +182,13 @@ static int xen_pt_word_reg_write(XenPCIP { XenPTRegInfo *reg = cfg_entry->reg; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; + uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask); /* modify emulate register */ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask; cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); return 0; @@ -193,14 +199,13 @@ static int xen_pt_long_reg_write(XenPCIP { XenPTRegInfo *reg = cfg_entry->reg; uint32_t writable_mask = 0; - uint32_t throughable_mask = 0; + uint32_t throughable_mask = get_throughable_mask(s, reg, valid_mask); /* modify emulate register */ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask; cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); return 0; @@ -292,15 +297,13 @@ static int xen_pt_cmd_reg_write(XenPCIPa { XenPTRegInfo *reg = cfg_entry->reg; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; + uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask); /* modify emulate register */ writable_mask = ~reg->ro_mask & valid_mask; cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; - if (*val & PCI_COMMAND_INTX_DISABLE) { throughable_mask |= PCI_COMMAND_INTX_DISABLE; } else { @@ -456,7 +459,6 @@ static int xen_pt_bar_reg_write(XenPCIPa PCIDevice *d = &s->dev; const PCIIORegion *r; uint32_t writable_mask = 0; - uint32_t throughable_mask = 0; uint32_t bar_emu_mask = 0; uint32_t bar_ro_mask = 0; uint32_t r_size = 0; @@ -513,8 +515,7 @@ static int xen_pt_bar_reg_write(XenPCIPa } /* create value for writing to I/O device register */ - throughable_mask = ~bar_emu_mask & valid_mask; - *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); + *val = XEN_PT_MERGE_VALUE(*val, dev_value, 0); return 0; } @@ -528,9 +529,8 @@ static int xen_pt_exp_rom_bar_reg_write( XenPTRegion *base = NULL; PCIDevice *d = (PCIDevice *)&s->dev; uint32_t writable_mask = 0; - uint32_t throughable_mask = 0; + uint32_t throughable_mask = get_throughable_mask(s, reg, valid_mask); pcibus_t r_size = 0; - uint32_t bar_emu_mask = 0; uint32_t bar_ro_mask = 0; r_size = d->io_regions[PCI_ROM_SLOT].size; @@ -539,7 +539,6 @@ static int xen_pt_exp_rom_bar_reg_write( r_size = xen_pt_get_emul_size(base->bar_flag, r_size); /* set emulate mask and read-only mask */ - bar_emu_mask = reg->emu_mask; bar_ro_mask = (reg->ro_mask | (r_size - 1)) & ~PCI_ROM_ADDRESS_ENABLE; /* modify emulate register */ @@ -547,7 +546,6 @@ static int xen_pt_exp_rom_bar_reg_write( cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~bar_emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); return 0; @@ -942,14 +940,13 @@ static int xen_pt_pmcsr_reg_write(XenPCI { XenPTRegInfo *reg = cfg_entry->reg; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; + uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask); /* modify emulate register */ writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask; cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value & ~PCI_PM_CTRL_PME_STATUS, throughable_mask); @@ -1042,7 +1039,7 @@ static int xen_pt_msgctrl_reg_write(XenP XenPTRegInfo *reg = cfg_entry->reg; XenPTMSI *msi = s->msi; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; + uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask); /* Currently no support for multi-vector */ if (*val & PCI_MSI_FLAGS_QSIZE) { @@ -1055,7 +1052,6 @@ static int xen_pt_msgctrl_reg_write(XenP msi->flags |= cfg_entry->data & ~PCI_MSI_FLAGS_ENABLE; /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); /* update MSI */ @@ -1171,7 +1167,6 @@ static int xen_pt_msgaddr32_reg_write(Xe { XenPTRegInfo *reg = cfg_entry->reg; uint32_t writable_mask = 0; - uint32_t throughable_mask = 0; uint32_t old_addr = cfg_entry->data; /* modify emulate register */ @@ -1180,8 +1175,7 @@ static int xen_pt_msgaddr32_reg_write(Xe s->msi->addr_lo = cfg_entry->data; /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; - *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); + *val = XEN_PT_MERGE_VALUE(*val, dev_value, 0); /* update MSI */ if (cfg_entry->data != old_addr) { @@ -1199,7 +1193,6 @@ static int xen_pt_msgaddr64_reg_write(Xe { XenPTRegInfo *reg = cfg_entry->reg; uint32_t writable_mask = 0; - uint32_t throughable_mask = 0; uint32_t old_addr = cfg_entry->data; /* check whether the type is 64 bit or not */ @@ -1216,8 +1209,7 @@ static int xen_pt_msgaddr64_reg_write(Xe s->msi->addr_hi = cfg_entry->data; /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; - *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); + *val = XEN_PT_MERGE_VALUE(*val, dev_value, 0); /* update MSI */ if (cfg_entry->data != old_addr) { @@ -1239,7 +1231,6 @@ static int xen_pt_msgdata_reg_write(XenP XenPTRegInfo *reg = cfg_entry->reg; XenPTMSI *msi = s->msi; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; uint16_t old_data = cfg_entry->data; uint32_t offset = reg->offset; @@ -1257,8 +1248,7 @@ static int xen_pt_msgdata_reg_write(XenP msi->data = cfg_entry->data; /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; - *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); + *val = XEN_PT_MERGE_VALUE(*val, dev_value, 0); /* update MSI */ if (cfg_entry->data != old_data) { @@ -1420,7 +1410,7 @@ static int xen_pt_msixctrl_reg_write(Xen { XenPTRegInfo *reg = cfg_entry->reg; uint16_t writable_mask = 0; - uint16_t throughable_mask = 0; + uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask); int debug_msix_enabled_old; /* modify emulate register */ @@ -1428,7 +1418,6 @@ static int xen_pt_msixctrl_reg_write(Xen cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask); /* create value for writing to I/O device register */ - throughable_mask = ~reg->emu_mask & valid_mask; *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask); /* update MSI-X */
xsa131-qemuu-5.patch
(application/octet-stream, 807 B)
xen/pt: mark all PCIe capability bits read-only xen_pt_emu_reg_pcie[]'s PCI_EXP_DEVCAP needs to cover all bits as read- only to avoid unintended write-back (just a precaution, the field ought to be read-only in hardware). This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> Reviewed-by: Stefano Stabellini <[email protected]> --- a/hw/xen/xen_pt_config_init.c +++ b/hw/xen/xen_pt_config_init.c @@ -873,7 +873,7 @@ static XenPTRegInfo xen_pt_emu_reg_pcie[ .offset = PCI_EXP_DEVCAP, .size = 4, .init_val = 0x00000000, - .ro_mask = 0x1FFCFFFF, + .ro_mask = 0xFFFFFFFF, .emu_mask = 0x10000000, .init = xen_pt_common_reg_init, .u.dw.read = xen_pt_long_reg_read,
xsa131-qemuu-6.patch
(application/octet-stream, 2.7 KB)
xen/pt: mark reserved bits in PCI config space fields The adjustments are solely to make the subsequent patches work right (and hence make the patch set consistent), namely if permissive mode (introduced by the last patch) gets used (as both reserved registers and reserved fields must be similarly protected from guest access in default mode, but the guest should be allowed access to them in permissive mode). This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> --- a/hw/xen/xen_pt.h +++ b/hw/xen/xen_pt.h @@ -101,6 +101,8 @@ struct XenPTRegInfo { uint32_t offset; uint32_t size; uint32_t init_val; + /* reg reserved field mask (ON:reserved, OFF:defined) */ + uint32_t res_mask; /* reg read only field mask (ON:RO/ROS, OFF:other) */ uint32_t ro_mask; /* reg emulate field mask (ON:emu, OFF:passthrough) */ --- a/hw/xen/xen_pt_config_init.c +++ b/hw/xen/xen_pt_config_init.c @@ -580,7 +580,7 @@ static XenPTRegInfo xen_pt_emu_reg_heade .offset = PCI_COMMAND, .size = 2, .init_val = 0x0000, - .ro_mask = 0xF880, + .res_mask = 0xF880, .emu_mask = 0x0743, .init = xen_pt_common_reg_init, .u.w.read = xen_pt_word_reg_read, @@ -605,7 +605,8 @@ static XenPTRegInfo xen_pt_emu_reg_heade .offset = PCI_STATUS, .size = 2, .init_val = 0x0000, - .ro_mask = 0x06FF, + .res_mask = 0x0007, + .ro_mask = 0x06F8, .emu_mask = 0x0010, .init = xen_pt_status_reg_init, .u.w.read = xen_pt_word_reg_read, @@ -982,7 +983,8 @@ static XenPTRegInfo xen_pt_emu_reg_pm[] .offset = PCI_PM_CTRL, .size = 2, .init_val = 0x0008, - .ro_mask = 0xE1FC, + .res_mask = 0x00F0, + .ro_mask = 0xE10C, .emu_mask = 0x810B, .init = xen_pt_common_reg_init, .u.w.read = xen_pt_word_reg_read, @@ -1278,7 +1280,8 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] .offset = PCI_MSI_FLAGS, .size = 2, .init_val = 0x0000, - .ro_mask = 0xFF8E, + .res_mask = 0xFE00, + .ro_mask = 0x018E, .emu_mask = 0x017E, .init = xen_pt_msgctrl_reg_init, .u.w.read = xen_pt_word_reg_read, @@ -1456,7 +1459,8 @@ static XenPTRegInfo xen_pt_emu_reg_msix[ .offset = PCI_MSI_FLAGS, .size = 2, .init_val = 0x0000, - .ro_mask = 0x3FFF, + .res_mask = 0x3800, + .ro_mask = 0x07FF, .emu_mask = 0x0000, .init = xen_pt_msixctrl_reg_init, .u.w.read = xen_pt_word_reg_read,
xsa131-qemuu-7.patch
(application/octet-stream, 2.2 KB)
xen/pt: add a few PCI config space field descriptions Since the next patch will turn all not explicitly described fields read-only by default, those fields that have guest writable bits need to be given explicit descriptors. This is a preparatory patch for XSA-131. Signed-off-by: Jan Beulich <[email protected]> --- Notes: - blindly allowing all VPD reads may still be a problem (out of bounds addresses aren't allowed, but the spec doesn't say what the effect would be) ==> also an issue in pciback? - Vendor Specific cap regs aren't in the table (will become r/o by default with this change) - many PCIe cap regs aren't in the table (will again become r/o) - same for PM cap regs at offsets 6 and 7 --- a/hw/xen/xen_pt_config_init.c +++ b/hw/xen/xen_pt_config_init.c @@ -756,6 +756,15 @@ static XenPTRegInfo xen_pt_emu_reg_vpd[] .u.b.write = xen_pt_byte_reg_write, }, { + .offset = PCI_VPD_ADDR, + .size = 2, + .ro_mask = 0x0003, + .emu_mask = 0x0003, + .init = xen_pt_common_reg_init, + .u.w.read = xen_pt_word_reg_read, + .u.w.write = xen_pt_word_reg_write, + }, + { .size = 0, }, }; @@ -891,6 +900,16 @@ static XenPTRegInfo xen_pt_emu_reg_pcie[ .u.w.read = xen_pt_word_reg_read, .u.w.write = xen_pt_word_reg_write, }, + /* Device Status reg */ + { + .offset = PCI_EXP_DEVSTA, + .size = 2, + .res_mask = 0xFFC0, + .ro_mask = 0x0030, + .init = xen_pt_common_reg_init, + .u.w.read = xen_pt_word_reg_read, + .u.w.write = xen_pt_word_reg_write, + }, /* Link Control reg */ { .offset = PCI_EXP_LNKCTL, @@ -902,6 +921,15 @@ static XenPTRegInfo xen_pt_emu_reg_pcie[ .u.w.read = xen_pt_word_reg_read, .u.w.write = xen_pt_word_reg_write, }, + /* Link Status reg */ + { + .offset = PCI_EXP_LNKSTA, + .size = 2, + .ro_mask = 0x3FFF, + .init = xen_pt_common_reg_init, + .u.w.read = xen_pt_word_reg_read, + .u.w.write = xen_pt_word_reg_write, + }, /* Device Control 2 reg */ { .offset = 0x28,
xsa131-qemuu-8.patch
(application/octet-stream, 4.5 KB)
xen/pt: unknown PCI config space fields should be read-only ... by default. Add a per-device "permissive" mode similar to pciback's to allow restoring previous behavior (and hence break security again, i.e. should be used only for trusted guests). This is part of XSA-131. Signed-off-by: Jan Beulich <[email protected]> Acked-by: Stefano Stabellini <[email protected]> Reviewed-by: Anthony PERARD <[email protected]>) --- Notes: - What purpose does xen_pt_header_type_reg_init() serve (with .emu_mask being zero)? - In the qemu-trad case no equivalent logic to that setting/using direct_pci_{msi_translate,power_mgmt} is being added, as that logic seems broken (setting globals from device 0 xenstore settings). --- a/hw/xen/xen_pt.c +++ b/hw/xen/xen_pt.c @@ -239,6 +239,7 @@ static void xen_pt_pci_write_config(PCID XenPTReg *reg_entry = NULL; uint32_t find_addr = addr; XenPTRegInfo *reg = NULL; + bool wp_flag = false; if (xen_pt_pci_config_access_check(d, addr, len)) { return; @@ -280,6 +281,10 @@ static void xen_pt_pci_write_config(PCID /* pass directly to the real device for passthrough type register group */ if (reg_grp_entry == NULL) { + if (!s->permissive) { + wb_mask = 0; + wp_flag = true; + } goto out; } @@ -300,12 +305,15 @@ static void xen_pt_pci_write_config(PCID uint32_t real_offset = reg_grp_entry->base_offset + reg->offset; uint32_t valid_mask = 0xFFFFFFFF >> ((4 - emul_len) << 3); uint8_t *ptr_val = NULL; + uint32_t wp_mask = reg->emu_mask | reg->ro_mask; valid_mask <<= (find_addr - real_offset) << 3; ptr_val = (uint8_t *)&val + (real_offset & 3); - if (reg->emu_mask == (0xFFFFFFFF >> ((4 - reg->size) << 3))) { - wb_mask &= ~((reg->emu_mask - >> ((find_addr - real_offset) << 3)) + if (!s->permissive) { + wp_mask |= reg->res_mask; + } + if (wp_mask == (0xFFFFFFFF >> ((4 - reg->size) << 3))) { + wb_mask &= ~((wp_mask >> ((find_addr - real_offset) << 3)) << ((len - emul_len) << 3)); } @@ -349,6 +357,16 @@ static void xen_pt_pci_write_config(PCID } else { /* nothing to do with passthrough type register, * continue to find next byte */ + if (!s->permissive) { + wb_mask &= ~(0xff << ((len - emul_len) << 3)); + /* Unused BARs will make it here, but we don't want to issue + * warnings for writes to them (bogus writes get dealt with + * above). + */ + if (index < 0) { + wp_flag = true; + } + } emul_len--; find_addr++; } @@ -360,6 +378,13 @@ static void xen_pt_pci_write_config(PCID memory_region_transaction_commit(); out: + if (wp_flag && !s->permissive_warned) { + s->permissive_warned = true; + xen_pt_log(d, "Write-back to unknown field 0x%02x (partially) inhibited (0x%0*x)\n", + addr, len * 2, wb_mask); + xen_pt_log(d, "If the device doesn't work, try enabling permissive mode\n"); + xen_pt_log(d, "(unsafe) and if it helps report the problem to xen-devel\n"); + } for (index = 0; wb_mask; index += len) { /* unknown regs are passed through */ while (!(wb_mask & 0xff)) { @@ -821,6 +846,7 @@ static void xen_pt_unregister_device(PCI static Property xen_pci_passthrough_properties[] = { DEFINE_PROP_PCI_HOST_DEVADDR("hostaddr", XenPCIPassthroughState, hostaddr), + DEFINE_PROP_BOOL("permissive", XenPCIPassthroughState, permissive, false), DEFINE_PROP_END_OF_LIST(), }; --- a/hw/xen/xen_pt.h +++ b/hw/xen/xen_pt.h @@ -197,6 +197,8 @@ struct XenPCIPassthroughState { PCIHostDeviceAddress hostaddr; bool is_virtfn; + bool permissive; + bool permissive_warned; XenHostPCIDevice real_device; XenPTRegion bases[PCI_NUM_REGIONS]; /* Access regions */ QLIST_HEAD(, XenPTRegGroup) reg_grps; --- a/hw/xen/xen_pt_config_init.c +++ b/hw/xen/xen_pt_config_init.c @@ -101,6 +101,10 @@ static uint32_t get_throughable_mask(con { uint32_t throughable_mask = ~(reg->emu_mask | reg->ro_mask); + if (!s->permissive) { + throughable_mask &= ~reg->res_mask; + } + return throughable_mask & valid_mask; }