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

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

[Alex Williamson]

PCI_MSIX_TABSIZE is 0x07ff

Reported-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 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(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index d8e7cb4..e254203 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -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 +

Re: [PATCH] device-assignment: don't truncate MSIX capabilities table size

[Chris Wright]

* Alex Williamson (alex.williamson@redhat.com) wrote:
> PCI_MSIX_TABSIZE is 0x07ff

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

Re: [PATCH] device-assignment: don't truncate MSIX capabilities table size

[Alex Williamson]

On Tue, 2010-05-25 at 16:09 -0700, Chris Wright wrote:
> * Alex Williamson (alex.williamson@redhat.com) 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

Re: [PATCH] device-assignment: don't truncate MSIX capabilities table size

[Chris Wright]

* Alex Williamson (alex.williamson@redhat.com) wrote:
> On Tue, 2010-05-25 at 16:09 -0700, Chris Wright wrote:
> > * Alex Williamson (alex.williamson@redhat.com) 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 <chrisw@redhat.com>

Re: [PATCH] device-assignment: don't truncate MSIX capabilities table size

[Juan Quintela]

Alex Williamson <alex.williamson@redhat.com> wrote:
> PCI_MSIX_TABSIZE is 0x07ff
>
> Reported-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

Acked-by: Juan Quintela <quintela@redhat.com>

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 :)

Re: [PATCH] device-assignment: don't truncate MSIX capabilities table size

[Avi Kivity]

On 05/26/2010 03:27 AM, Juan Quintela wrote:
> Alex Williamson<alex.williamson@redhat.com>  wrote:
>    
>> PCI_MSIX_TABSIZE is 0x07ff
>>
>> Reported-by: Juan Quintela<quintela@redhat.com>
>> Signed-off-by: Alex Williamson<alex.williamson@redhat.com>
>>      
> Acked-by: Juan Quintela<quintela@redhat.com>
>
> 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.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] device-assignment: don't truncate MSIX capabilities table size

[Juan Quintela]

Avi Kivity <avi@redhat.com> wrote:
> On 05/26/2010 03:27 AM, Juan Quintela wrote:
>> Alex Williamson<alex.williamson@redhat.com>  wrote:
>>    
>>> PCI_MSIX_TABSIZE is 0x07ff
>>>
>>> Reported-by: Juan Quintela<quintela@redhat.com>
>>> Signed-off-by: Alex Williamson<alex.williamson@redhat.com>
>>>      
>> Acked-by: Juan Quintela<quintela@redhat.com>
>>
>> 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.

Re: [PATCH] device-assignment: don't truncate MSIX capabilities table size

[Jes Sorensen]

On 05/26/10 14:48, Avi Kivity wrote:
> On 05/26/2010 03:27 AM, Juan Quintela wrote:
>> 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

Re: [PATCH] device-assignment: don't truncate MSIX capabilities table size

[Avi Kivity]

On 05/26/2010 02:08 AM, Alex Williamson wrote:
> PCI_MSIX_TABSIZE is 0x07ff
>
>    

Applied, thanks.

-- 
error compiling committee.c: too many arguments to function