Skip to contentSkip to conversation

device-assignment: don't truncate MSIX capabilities table size

PATCH Applied Alex Williamson 5 participants 9 messages May 25, 2010 – May 27, 2010 KVM

lore
Share & tools
lore

Alex Williamson

author
#0 lore raw

PATCH 1/1 device-assignment: don't truncate MSIX capabilities table size

PCI_MSIX_TABSIZE is 0x07ff

Reported-by Juan Quintela <[email protected]>
Signed-off-by Alex Williamson <[email protected]>

---

Applies on top of:
[PATCH qemu-kvm 1/2] device-assignment: use stdint types [PATCH qemu-kvm 2/2] device-assignment: Don't use libpci

 hw/device-assignment.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
hw/device-assignment.c +3 -3
index d8e7cb4..e254203 100644
@@ -1073,7 +1073,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
     else
         pos = pci_dev->cap.start;
 
-    entries_max_nr = pci_dev->config[pos + 2];
+    entries_max_nr = *(uint16_t *)(pci_dev->config + pos + 2);
     entries_max_nr &= PCI_MSIX_TABSIZE;
     entries_max_nr += 1;
 

@@ -1255,8 +1255,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
         entry_nr = assigned_dev_pci_read_word(pci_dev, pos + 2) &
                                                              PCI_MSIX_TABSIZE;
         pci_dev->config[pci_dev->cap.start + pci_dev->cap.length] = 0x11;
-        pci_dev->config[pci_dev->cap.start +
-                        pci_dev->cap.length + 2] = entry_nr;
+        *(uint16_t *)(pci_dev->config + pci_dev->cap.start +
+                      pci_dev->cap.length + 2) = entry_nr;
         msix_table_entry = assigned_dev_pci_read_long(pci_dev,
                                                       pos + PCI_MSIX_TABLE);
         *(uint32_t *)(pci_dev->config + pci_dev->cap.start +
#1 lore raw
Quoted 1 line · * Alex Williamson ([email protected]) wrote:

PCI_MSIX_TABSIZE is 0x07ff

while not an issue now, i think that only works for little endian

author
#2 lore raw
Quoted 4 lines · Chris Wright
Quoted 1 line · * Alex Williamson ([email protected]) wrote:

PCI_MSIX_TABSIZE is 0x07ff

while not an issue now, i think that only works for little endian

Yep. This is consistent with rest of the device assignment code though, so I suspect there are numerous issues if it needed to support big endian.

Alex

Acked-by
#3 lore raw
Quoted 9 lines · * Alex Williamson ([email protected]) wrote:
Quoted 4 lines · Chris Wright
Quoted 1 line · * Alex Williamson ([email protected]) wrote:

PCI_MSIX_TABSIZE is 0x07ff

while not an issue now, i think that only works for little endian

Yep. This is consistent with rest of the device assignment code though, so I suspect there are numerous issues if it needed to support big endian.

Suppose it's worth fixing it all up, can be in follow-up though.

For this one...

Acked-by Chris Wright <[email protected]>
#5 lore raw
Quoted 4 lines · Juan Quintela
Quoted 2 lines · Alex Williamson

PCI_MSIX_TABSIZE is 0x07ff

Quoted 9 lines
Quoted 3 lines
Reported-by Juan Quintela<[email protected]>
Signed-off-by Alex Williamson<[email protected]>
Acked-by Juan Quintela<[email protected]>

BTW, I also noticed the lack of pci_set_long() and friend functions, but arrived to the same conclusion that you: all the device assignment assumes that the world is x86_64 :)

IIRC it used to work on ia64 as well.

Signature · 1 line
error compiling committee.c: too many arguments to function
#6 lore raw
Quoted 8 lines · Avi Kivity
Quoted 7 lines · Juan Quintela
Quoted 5 lines · Alex Williamson

PCI_MSIX_TABSIZE is 0x07ff

Reported-by Juan Quintela<[email protected]>
Signed-off-by Alex Williamson<[email protected]>
Quoted 8 lines
Quoted 6 lines
Acked-by Juan Quintela<[email protected]>

BTW, I also noticed the lack of pci_set_long() and friend functions, but arrived to the same conclusion that you: all the device assignment assumes that the world is x86_64 :)

IIRC it used to work on ia64 as well.

But ia64 is little endian :)

static inline void
pci_set_word(uint8_t *config, uint16_t val)

{
    cpu_to_le16wu((uint16_t *)config, val);
}

on little endian, this is always an assignment :)

Problem is if we have big endian somewhere in the middle.

Later, Juan.

#7 lore raw
In reply to Avi Kivity
Quoted 8 lines · Avi Kivity
Quoted 4 lines · Juan Quintela

BTW, I also noticed the lack of pci_set_long() and friend functions, but arrived to the same conclusion that you: all the device assignment assumes that the world is x86_64 :)

IIRC it used to work on ia64 as well.

Same endianess, unless you run HPUX on the CPU.

Jes

Applied
#8 lore raw
In reply to Alex Williamson
Quoted 3 lines · Alex Williamson

PCI_MSIX_TABSIZE is 0x07ff

Applied, thanks.

Signature · 1 line
error compiling committee.c: too many arguments to function