All of lore.kernel.org
 help / color / mirror / Atom feed
* Indefinite recursion in pci_default_read_config
@ 2009-12-15 10:57 Hannes Reinecke
  2009-12-15 10:59 ` Avi Kivity
  0 siblings, 1 reply; 6+ messages in thread
From: Hannes Reinecke @ 2009-12-15 10:57 UTC (permalink / raw)
  To: kvm

Hi all,

I just triggered a nasty indefinite recursion in pci_default_read_config:

uint32_t pci_default_read_config(PCIDevice *d,
                                 uint32_t address, int len)
{
    uint32_t val = 0;
    assert(len == 1 || len == 2 || len == 4);

    if (pci_access_cap_config(d, address, len)) {
        return d->cap.config_read(d, address, len);
    }

    len = MIN(len, pci_config_size(d) - address);
    memcpy(&val, d->config + address, len);
    return le32_to_cpu(val);
}

And d->cap.config_read is pointing to pci_default_read_config:

(gdb) print *d
$3 = {qdev = {id = 0xc99b10 "01:10.0", state = DEV_STATE_INITIALIZED, 
    opts = 0xc99ad0, hotplugged = 0, info = 0x837e60, parent_bus = 0xc71710, 
    num_gpio_out = 0, gpio_out = 0x0, num_gpio_in = 0, gpio_in = 0x0, 
    child_bus = {lh_first = 0x0}, num_child_bus = 0, sibling = {
      le_next = 0xc99c30, le_prev = 0xc71730}}, 
  config = 0xca3010 "\206\200\312\020\003", 
  cmask = 0xca3120 "\377\377\377\377", wmask = 0xca3230 "", 
  used = 0xca3340 "", bus = 0xc71710, devfn = 32, 
  name = "pci-assign", '\000' <repeats 53 times>, io_regions = {{
      addr = 4060102656, size = 16384, filtered_size = 16384, type = 0 '\000', 
      map_func = 0x46a5f0 <assigned_dev_iomem_map>}, {addr = 0, size = 0, 
      filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0, 
      filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 4060119040, 
      size = 16384, filtered_size = 16384, type = 0 '\000', 
      map_func = 0x46a5f0 <assigned_dev_iomem_map>}, {addr = 0, size = 0, 
      filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0, 
      filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0, 
      filtered_size = 0, type = 0 '\000', map_func = 0}}, 
  config_read = 0x46a050 <assigned_dev_pci_read_config>, 
  config_write = 0x469f30 <assigned_dev_pci_write_config>, irq = 0xca3450, 
  irq_state = 0 '\000', cap_present = 0, msix_cap = 0 '\000', 
  msix_entries_nr = 0, msix_table_page = 0x0, msix_mmio_index = 0, 
  msix_entry_used = 0x0, msix_bar_size = 0, version_id = 2, 
  msix_page_size = 0, msix_irq_entries = 0x0, cap = {supported = 1, 
    start = 64, length = 16, 
    config_read = 0x416770 <pci_default_cap_read_config>, 
    config_write = 0x46b750 <assigned_device_pci_cap_write_config>}}

Not good ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Indefinite recursion in pci_default_read_config
  2009-12-15 10:57 Indefinite recursion in pci_default_read_config Hannes Reinecke
@ 2009-12-15 10:59 ` Avi Kivity
  2009-12-15 11:11   ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2009-12-15 10:59 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: kvm, Michael S. Tsirkin

On 12/15/2009 12:57 PM, Hannes Reinecke wrote:
> Hi all,
>
> I just triggered a nasty indefinite recursion in pci_default_read_config:
>
> uint32_t pci_default_read_config(PCIDevice *d,
>                                   uint32_t address, int len)
> {
>      uint32_t val = 0;
>      assert(len == 1 || len == 2 || len == 4);
>
>      if (pci_access_cap_config(d, address, len)) {
>          return d->cap.config_read(d, address, len);
>      }
>
>      len = MIN(len, pci_config_size(d) - address);
>      memcpy(&val, d->config + address, len);
>      return le32_to_cpu(val);
> }
>
> And d->cap.config_read is pointing to pci_default_read_config:
>
> (gdb) print *d
> $3 = {qdev = {id = 0xc99b10 "01:10.0", state = DEV_STATE_INITIALIZED,
>      opts = 0xc99ad0, hotplugged = 0, info = 0x837e60, parent_bus = 0xc71710,
>      num_gpio_out = 0, gpio_out = 0x0, num_gpio_in = 0, gpio_in = 0x0,
>      child_bus = {lh_first = 0x0}, num_child_bus = 0, sibling = {
>        le_next = 0xc99c30, le_prev = 0xc71730}},
>    config = 0xca3010 "\206\200\312\020\003",
>    cmask = 0xca3120 "\377\377\377\377", wmask = 0xca3230 "",
>    used = 0xca3340 "", bus = 0xc71710, devfn = 32,
>    name = "pci-assign", '\000'<repeats 53 times>, io_regions = {{
>        addr = 4060102656, size = 16384, filtered_size = 16384, type = 0 '\000',
>        map_func = 0x46a5f0<assigned_dev_iomem_map>}, {addr = 0, size = 0,
>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0,
>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 4060119040,
>        size = 16384, filtered_size = 16384, type = 0 '\000',
>        map_func = 0x46a5f0<assigned_dev_iomem_map>}, {addr = 0, size = 0,
>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0,
>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0,
>        filtered_size = 0, type = 0 '\000', map_func = 0}},
>    config_read = 0x46a050<assigned_dev_pci_read_config>,
>    config_write = 0x469f30<assigned_dev_pci_write_config>, irq = 0xca3450,
>    irq_state = 0 '\000', cap_present = 0, msix_cap = 0 '\000',
>    msix_entries_nr = 0, msix_table_page = 0x0, msix_mmio_index = 0,
>    msix_entry_used = 0x0, msix_bar_size = 0, version_id = 2,
>    msix_page_size = 0, msix_irq_entries = 0x0, cap = {supported = 1,
>      start = 64, length = 16,
>      config_read = 0x416770<pci_default_cap_read_config>,
>      config_write = 0x46b750<assigned_device_pci_cap_write_config>}}
>    

Michael?  This is likely a bad merge on my part.  Can you help?

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Indefinite recursion in pci_default_read_config
  2009-12-15 10:59 ` Avi Kivity
@ 2009-12-15 11:11   ` Michael S. Tsirkin
  2009-12-15 11:26     ` Hannes Reinecke
  0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2009-12-15 11:11 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Hannes Reinecke, kvm

On Tue, Dec 15, 2009 at 12:59:41PM +0200, Avi Kivity wrote:
> On 12/15/2009 12:57 PM, Hannes Reinecke wrote:
>> Hi all,
>>
>> I just triggered a nasty indefinite recursion in pci_default_read_config:
>>
>> uint32_t pci_default_read_config(PCIDevice *d,
>>                                   uint32_t address, int len)
>> {
>>      uint32_t val = 0;
>>      assert(len == 1 || len == 2 || len == 4);
>>
>>      if (pci_access_cap_config(d, address, len)) {
>>          return d->cap.config_read(d, address, len);
>>      }
>>
>>      len = MIN(len, pci_config_size(d) - address);
>>      memcpy(&val, d->config + address, len);
>>      return le32_to_cpu(val);
>> }
>>
>> And d->cap.config_read is pointing to pci_default_read_config:
>>
>> (gdb) print *d
>> $3 = {qdev = {id = 0xc99b10 "01:10.0", state = DEV_STATE_INITIALIZED,
>>      opts = 0xc99ad0, hotplugged = 0, info = 0x837e60, parent_bus = 0xc71710,
>>      num_gpio_out = 0, gpio_out = 0x0, num_gpio_in = 0, gpio_in = 0x0,
>>      child_bus = {lh_first = 0x0}, num_child_bus = 0, sibling = {
>>        le_next = 0xc99c30, le_prev = 0xc71730}},
>>    config = 0xca3010 "\206\200\312\020\003",
>>    cmask = 0xca3120 "\377\377\377\377", wmask = 0xca3230 "",
>>    used = 0xca3340 "", bus = 0xc71710, devfn = 32,
>>    name = "pci-assign", '\000'<repeats 53 times>, io_regions = {{
>>        addr = 4060102656, size = 16384, filtered_size = 16384, type = 0 '\000',
>>        map_func = 0x46a5f0<assigned_dev_iomem_map>}, {addr = 0, size = 0,
>>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0,
>>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 4060119040,
>>        size = 16384, filtered_size = 16384, type = 0 '\000',
>>        map_func = 0x46a5f0<assigned_dev_iomem_map>}, {addr = 0, size = 0,
>>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0,
>>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0,
>>        filtered_size = 0, type = 0 '\000', map_func = 0}},
>>    config_read = 0x46a050<assigned_dev_pci_read_config>,
>>    config_write = 0x469f30<assigned_dev_pci_write_config>, irq = 0xca3450,
>>    irq_state = 0 '\000', cap_present = 0, msix_cap = 0 '\000',
>>    msix_entries_nr = 0, msix_table_page = 0x0, msix_mmio_index = 0,
>>    msix_entry_used = 0x0, msix_bar_size = 0, version_id = 2,
>>    msix_page_size = 0, msix_irq_entries = 0x0, cap = {supported = 1,
>>      start = 64, length = 16,
>>      config_read = 0x416770<pci_default_cap_read_config>,
>>      config_write = 0x46b750<assigned_device_pci_cap_write_config>}}
>>    
>
> Michael?  This is likely a bad merge on my part.  Can you help?
>
> -- 
> error compiling committee.c: too many arguments to function


Um, yes. I think the following is the right way to do this.
As a side note, we really should work to remove all these
hacks and make assignment use capability support
in upstream qemu.

--

diff --git a/hw/pci.c b/hw/pci.c
index 110a5fc..a74d3d4 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1016,19 +1016,26 @@ static void pci_update_irq_disabled(PCIDevice *d, int was_irq_disabled)
     }
 }
 
+uint32_t pci_read_config(PCIDevice *d,
+                         uint32_t address, int len)
+{
+    uint32_t val = 0;
+
+    len = MIN(len, pci_config_size(d) - address);
+    memcpy(&val, d->config + address, len);
+    return le32_to_cpu(val);
+}
+
 uint32_t pci_default_read_config(PCIDevice *d,
                                  uint32_t address, int len)
 {
-    uint32_t val = 0;
     assert(len == 1 || len == 2 || len == 4);
 
     if (pci_access_cap_config(d, address, len)) {
         return d->cap.config_read(d, address, len);
     }
 
-    len = MIN(len, pci_config_size(d) - address);
-    memcpy(&val, d->config + address, len);
-    return le32_to_cpu(val);
+    return pci_read_config(d, address, len);
 }
 
 static void pci_write_config(PCIDevice *pci_dev,
@@ -1052,7 +1059,7 @@ int pci_access_cap_config(PCIDevice *pci_dev, uint32_t address, int len)
 uint32_t pci_default_cap_read_config(PCIDevice *pci_dev,
                                      uint32_t address, int len)
 {
-    return pci_default_read_config(pci_dev, address, len);
+    return pci_read_config(pci_dev, address, len);
 }
 
 void pci_default_cap_write_config(PCIDevice *pci_dev,

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: Indefinite recursion in pci_default_read_config
  2009-12-15 11:11   ` Michael S. Tsirkin
@ 2009-12-15 11:26     ` Hannes Reinecke
  2009-12-15 11:35       ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Hannes Reinecke @ 2009-12-15 11:26 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Avi Kivity, kvm

Michael S. Tsirkin wrote:
> On Tue, Dec 15, 2009 at 12:59:41PM +0200, Avi Kivity wrote:
>> On 12/15/2009 12:57 PM, Hannes Reinecke wrote:
>>> Hi all,
>>>
>>> I just triggered a nasty indefinite recursion in pci_default_read_config:
>>>
>>> uint32_t pci_default_read_config(PCIDevice *d,
>>>                                   uint32_t address, int len)
>>> {
>>>      uint32_t val = 0;
>>>      assert(len == 1 || len == 2 || len == 4);
>>>
>>>      if (pci_access_cap_config(d, address, len)) {
>>>          return d->cap.config_read(d, address, len);
>>>      }
>>>
>>>      len = MIN(len, pci_config_size(d) - address);
>>>      memcpy(&val, d->config + address, len);
>>>      return le32_to_cpu(val);
>>> }
>>>
>>> And d->cap.config_read is pointing to pci_default_read_config:
>>>
>>> (gdb) print *d
>>> $3 = {qdev = {id = 0xc99b10 "01:10.0", state = DEV_STATE_INITIALIZED,
>>>      opts = 0xc99ad0, hotplugged = 0, info = 0x837e60, parent_bus = 0xc71710,
>>>      num_gpio_out = 0, gpio_out = 0x0, num_gpio_in = 0, gpio_in = 0x0,
>>>      child_bus = {lh_first = 0x0}, num_child_bus = 0, sibling = {
>>>        le_next = 0xc99c30, le_prev = 0xc71730}},
>>>    config = 0xca3010 "\206\200\312\020\003",
>>>    cmask = 0xca3120 "\377\377\377\377", wmask = 0xca3230 "",
>>>    used = 0xca3340 "", bus = 0xc71710, devfn = 32,
>>>    name = "pci-assign", '\000'<repeats 53 times>, io_regions = {{
>>>        addr = 4060102656, size = 16384, filtered_size = 16384, type = 0 '\000',
>>>        map_func = 0x46a5f0<assigned_dev_iomem_map>}, {addr = 0, size = 0,
>>>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0,
>>>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 4060119040,
>>>        size = 16384, filtered_size = 16384, type = 0 '\000',
>>>        map_func = 0x46a5f0<assigned_dev_iomem_map>}, {addr = 0, size = 0,
>>>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0,
>>>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0,
>>>        filtered_size = 0, type = 0 '\000', map_func = 0}},
>>>    config_read = 0x46a050<assigned_dev_pci_read_config>,
>>>    config_write = 0x469f30<assigned_dev_pci_write_config>, irq = 0xca3450,
>>>    irq_state = 0 '\000', cap_present = 0, msix_cap = 0 '\000',
>>>    msix_entries_nr = 0, msix_table_page = 0x0, msix_mmio_index = 0,
>>>    msix_entry_used = 0x0, msix_bar_size = 0, version_id = 2,
>>>    msix_page_size = 0, msix_irq_entries = 0x0, cap = {supported = 1,
>>>      start = 64, length = 16,
>>>      config_read = 0x416770<pci_default_cap_read_config>,
>>>      config_write = 0x46b750<assigned_device_pci_cap_write_config>}}
>>>    
>> Michael?  This is likely a bad merge on my part.  Can you help?
>>
>> -- 
>> error compiling committee.c: too many arguments to function
> 
> 
> Um, yes. I think the following is the right way to do this.
> As a side note, we really should work to remove all these
> hacks and make assignment use capability support
> in upstream qemu.
> 
> --
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 110a5fc..a74d3d4 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1016,19 +1016,26 @@ static void pci_update_irq_disabled(PCIDevice *d, int was_irq_disabled)
>      }
>  }
>  
> +uint32_t pci_read_config(PCIDevice *d,
> +                         uint32_t address, int len)
> +{
> +    uint32_t val = 0;
> +
> +    len = MIN(len, pci_config_size(d) - address);
> +    memcpy(&val, d->config + address, len);
> +    return le32_to_cpu(val);
> +}
> +
>  uint32_t pci_default_read_config(PCIDevice *d,
>                                   uint32_t address, int len)
>  {
> -    uint32_t val = 0;
>      assert(len == 1 || len == 2 || len == 4);
>  
>      if (pci_access_cap_config(d, address, len)) {
>          return d->cap.config_read(d, address, len);
>      }
>  
> -    len = MIN(len, pci_config_size(d) - address);
> -    memcpy(&val, d->config + address, len);
> -    return le32_to_cpu(val);
> +    return pci_read_config(d, address, len);
>  }
>  
>  static void pci_write_config(PCIDevice *pci_dev,
> @@ -1052,7 +1059,7 @@ int pci_access_cap_config(PCIDevice *pci_dev, uint32_t address, int len)
>  uint32_t pci_default_cap_read_config(PCIDevice *pci_dev,
>                                       uint32_t address, int len)
>  {
> -    return pci_default_read_config(pci_dev, address, len);
> +    return pci_read_config(pci_dev, address, len);
>  }
>  
>  void pci_default_cap_write_config(PCIDevice *pci_dev,

Ok, works. Except for a missing prototype in hw/pci.h :-)

Cheers,

Hannes

-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Indefinite recursion in pci_default_read_config
  2009-12-15 11:26     ` Hannes Reinecke
@ 2009-12-15 11:35       ` Michael S. Tsirkin
  2009-12-15 12:21         ` Avi Kivity
  0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2009-12-15 11:35 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Avi Kivity, kvm

On Tue, Dec 15, 2009 at 12:26:15PM +0100, Hannes Reinecke wrote:
> Michael S. Tsirkin wrote:
> > On Tue, Dec 15, 2009 at 12:59:41PM +0200, Avi Kivity wrote:
> >> On 12/15/2009 12:57 PM, Hannes Reinecke wrote:
> >>> Hi all,
> >>>
> >>> I just triggered a nasty indefinite recursion in pci_default_read_config:
> >>>
> >>> uint32_t pci_default_read_config(PCIDevice *d,
> >>>                                   uint32_t address, int len)
> >>> {
> >>>      uint32_t val = 0;
> >>>      assert(len == 1 || len == 2 || len == 4);
> >>>
> >>>      if (pci_access_cap_config(d, address, len)) {
> >>>          return d->cap.config_read(d, address, len);
> >>>      }
> >>>
> >>>      len = MIN(len, pci_config_size(d) - address);
> >>>      memcpy(&val, d->config + address, len);
> >>>      return le32_to_cpu(val);
> >>> }
> >>>
> >>> And d->cap.config_read is pointing to pci_default_read_config:
> >>>
> >>> (gdb) print *d
> >>> $3 = {qdev = {id = 0xc99b10 "01:10.0", state = DEV_STATE_INITIALIZED,
> >>>      opts = 0xc99ad0, hotplugged = 0, info = 0x837e60, parent_bus = 0xc71710,
> >>>      num_gpio_out = 0, gpio_out = 0x0, num_gpio_in = 0, gpio_in = 0x0,
> >>>      child_bus = {lh_first = 0x0}, num_child_bus = 0, sibling = {
> >>>        le_next = 0xc99c30, le_prev = 0xc71730}},
> >>>    config = 0xca3010 "\206\200\312\020\003",
> >>>    cmask = 0xca3120 "\377\377\377\377", wmask = 0xca3230 "",
> >>>    used = 0xca3340 "", bus = 0xc71710, devfn = 32,
> >>>    name = "pci-assign", '\000'<repeats 53 times>, io_regions = {{
> >>>        addr = 4060102656, size = 16384, filtered_size = 16384, type = 0 '\000',
> >>>        map_func = 0x46a5f0<assigned_dev_iomem_map>}, {addr = 0, size = 0,
> >>>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0,
> >>>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 4060119040,
> >>>        size = 16384, filtered_size = 16384, type = 0 '\000',
> >>>        map_func = 0x46a5f0<assigned_dev_iomem_map>}, {addr = 0, size = 0,
> >>>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0,
> >>>        filtered_size = 0, type = 0 '\000', map_func = 0}, {addr = 0, size = 0,
> >>>        filtered_size = 0, type = 0 '\000', map_func = 0}},
> >>>    config_read = 0x46a050<assigned_dev_pci_read_config>,
> >>>    config_write = 0x469f30<assigned_dev_pci_write_config>, irq = 0xca3450,
> >>>    irq_state = 0 '\000', cap_present = 0, msix_cap = 0 '\000',
> >>>    msix_entries_nr = 0, msix_table_page = 0x0, msix_mmio_index = 0,
> >>>    msix_entry_used = 0x0, msix_bar_size = 0, version_id = 2,
> >>>    msix_page_size = 0, msix_irq_entries = 0x0, cap = {supported = 1,
> >>>      start = 64, length = 16,
> >>>      config_read = 0x416770<pci_default_cap_read_config>,
> >>>      config_write = 0x46b750<assigned_device_pci_cap_write_config>}}
> >>>    
> >> Michael?  This is likely a bad merge on my part.  Can you help?
> >>
> >> -- 
> >> error compiling committee.c: too many arguments to function
> > 
> > 
> > Um, yes. I think the following is the right way to do this.
> > As a side note, we really should work to remove all these
> > hacks and make assignment use capability support
> > in upstream qemu.
> > 
> > --
> > 
> > diff --git a/hw/pci.c b/hw/pci.c
> > index 110a5fc..a74d3d4 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -1016,19 +1016,26 @@ static void pci_update_irq_disabled(PCIDevice *d, int was_irq_disabled)
> >      }
> >  }
> >  
> > +uint32_t pci_read_config(PCIDevice *d,
> > +                         uint32_t address, int len)
> > +{
> > +    uint32_t val = 0;
> > +
> > +    len = MIN(len, pci_config_size(d) - address);
> > +    memcpy(&val, d->config + address, len);
> > +    return le32_to_cpu(val);
> > +}
> > +
> >  uint32_t pci_default_read_config(PCIDevice *d,
> >                                   uint32_t address, int len)
> >  {
> > -    uint32_t val = 0;
> >      assert(len == 1 || len == 2 || len == 4);
> >  
> >      if (pci_access_cap_config(d, address, len)) {
> >          return d->cap.config_read(d, address, len);
> >      }
> >  
> > -    len = MIN(len, pci_config_size(d) - address);
> > -    memcpy(&val, d->config + address, len);
> > -    return le32_to_cpu(val);
> > +    return pci_read_config(d, address, len);
> >  }
> >  
> >  static void pci_write_config(PCIDevice *pci_dev,
> > @@ -1052,7 +1059,7 @@ int pci_access_cap_config(PCIDevice *pci_dev, uint32_t address, int len)
> >  uint32_t pci_default_cap_read_config(PCIDevice *pci_dev,
> >                                       uint32_t address, int len)
> >  {
> > -    return pci_default_read_config(pci_dev, address, len);
> > +    return pci_read_config(pci_dev, address, len);
> >  }
> >  
> >  void pci_default_cap_write_config(PCIDevice *pci_dev,
> 
> Ok, works. Except for a missing prototype in hw/pci.h :-)

Should just be static in fact. Here's a better one:


diff --git a/hw/pci.c b/hw/pci.c
index 110a5fc..a74d3d4 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1016,19 +1016,26 @@ static void pci_update_irq_disabled(PCIDevice *d, int was_irq_disabled)
     }
 }
 
+static uint32_t pci_read_config(PCIDevice *d,
+                                uint32_t address, int len)
+{
+    uint32_t val = 0;
+
+    len = MIN(len, pci_config_size(d) - address);
+    memcpy(&val, d->config + address, len);
+    return le32_to_cpu(val);
+}
+
 uint32_t pci_default_read_config(PCIDevice *d,
                                  uint32_t address, int len)
 {
-    uint32_t val = 0;
     assert(len == 1 || len == 2 || len == 4);
 
     if (pci_access_cap_config(d, address, len)) {
         return d->cap.config_read(d, address, len);
     }
 
-    len = MIN(len, pci_config_size(d) - address);
-    memcpy(&val, d->config + address, len);
-    return le32_to_cpu(val);
+    return pci_read_config(d, address, len);
 }
 
 static void pci_write_config(PCIDevice *pci_dev,
@@ -1052,7 +1059,7 @@ int pci_access_cap_config(PCIDevice *pci_dev, uint32_t address, int len)
 uint32_t pci_default_cap_read_config(PCIDevice *pci_dev,
                                      uint32_t address, int len)
 {
-    return pci_default_read_config(pci_dev, address, len);
+    return pci_read_config(pci_dev, address, len);
 }
 
 void pci_default_cap_write_config(PCIDevice *pci_dev,

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: Indefinite recursion in pci_default_read_config
  2009-12-15 11:35       ` Michael S. Tsirkin
@ 2009-12-15 12:21         ` Avi Kivity
  0 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2009-12-15 12:21 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Hannes Reinecke, kvm

On 12/15/2009 01:35 PM, Michael S. Tsirkin wrote:
>
> Should just be static in fact. Here's a better one:
>
>    

Changelog and signoff please.

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-12-15 12:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-15 10:57 Indefinite recursion in pci_default_read_config Hannes Reinecke
2009-12-15 10:59 ` Avi Kivity
2009-12-15 11:11   ` Michael S. Tsirkin
2009-12-15 11:26     ` Hannes Reinecke
2009-12-15 11:35       ` Michael S. Tsirkin
2009-12-15 12:21         ` Avi Kivity

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.