Re: [PATCH 3/5] pci-assign: Fix dword read at PCI_COMMAND
On Thu, 2011-04-28 at 10:59 +0200, Jan Kiszka wrote:
If we emulate the command register, we must only read its content from
the shadow config space. For dword read of both PCI_COMMAND and
PCI_STATUS, at least the latter must be read from the device.
For simplicity reasons and as the code path is not considered
performance critical for the affected SRIOV devices, the fix performes
device access to the command word unconditionally, even if emulation is
enabled and only that word is read.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
hw/device-assignment.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index f37f108..ee81434 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -482,14 +482,11 @@ static uint32_t assigned_dev_pci_read_config(PCIDevice
*d, uint32_t address,
/*
* Catch access to
* - vendor device ID
- * - command register (if emulation needed)
* - base address registers
* - ROM base address capability pointer
* - interrupt line pin
*/
if (ranges_overlap(address, len, PCI_VENDOR_ID, 4) ||
-(pci_dev-need_emulate_cmd
- ranges_overlap(address, len, PCI_COMMAND, 2)) ||
ranges_overlap(address, len, PCI_BASE_ADDRESS_0, 24) ||
ranges_overlap(address, len, PCI_ROM_ADDRESS, 8) ||
ranges_overlap(address, len, PCI_INTERRUPT_LINE, 2)) {
@@ -521,6 +518,11 @@ do_log:
DEBUG((%x.%x): address=%04x val=0x%08x len=%d\n,
(d-devfn 3) 0x1F, (d-devfn 0x7), address, val, len);
+if (pci_dev-need_emulate_cmd) {
+val = merge_bits(val, pci_default_read_config(d, PCI_COMMAND, 2),
+ address, len, PCI_COMMAND, 0x);
Shouldn't this be pci_default_read_config(d, address, len)? I think
what you have works since PCI_COMMAND is at the start of a dword, but it
violates the merge_bits() assumption that val and mval are from the same
address with the same length. Thanks,
Alex
+}
+
if (!pci_dev-cap.available) {
/* kill the special capabilities */
if (address == PCI_COMMAND len == 4) {
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] pci-assign: Fix dword read at PCI_COMMAND
On 2011-04-28 16:51, Alex Williamson wrote:
On Thu, 2011-04-28 at 10:59 +0200, Jan Kiszka wrote:
If we emulate the command register, we must only read its content from
the shadow config space. For dword read of both PCI_COMMAND and
PCI_STATUS, at least the latter must be read from the device.
For simplicity reasons and as the code path is not considered
performance critical for the affected SRIOV devices, the fix performes
device access to the command word unconditionally, even if emulation is
enabled and only that word is read.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
hw/device-assignment.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index f37f108..ee81434 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -482,14 +482,11 @@ static uint32_t assigned_dev_pci_read_config(PCIDevice
*d, uint32_t address,
/*
* Catch access to
* - vendor device ID
- * - command register (if emulation needed)
* - base address registers
* - ROM base address capability pointer
* - interrupt line pin
*/
if (ranges_overlap(address, len, PCI_VENDOR_ID, 4) ||
-(pci_dev-need_emulate_cmd
- ranges_overlap(address, len, PCI_COMMAND, 2)) ||
ranges_overlap(address, len, PCI_BASE_ADDRESS_0, 24) ||
ranges_overlap(address, len, PCI_ROM_ADDRESS, 8) ||
ranges_overlap(address, len, PCI_INTERRUPT_LINE, 2)) {
@@ -521,6 +518,11 @@ do_log:
DEBUG((%x.%x): address=%04x val=0x%08x len=%d\n,
(d-devfn 3) 0x1F, (d-devfn 0x7), address, val, len);
+if (pci_dev-need_emulate_cmd) {
+val = merge_bits(val, pci_default_read_config(d, PCI_COMMAND, 2),
+ address, len, PCI_COMMAND, 0x);
Shouldn't this be pci_default_read_config(d, address, len)? I think
what you have works since PCI_COMMAND is at the start of a dword, but it
violates the merge_bits() assumption that val and mval are from the same
address with the same length. Thanks,
This is actually a real issue, reading a byte from PCI_COMMAND+1 is broken.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
