Indefinite recursion in pci_default_read_config

2009-12-15 Thread Hannes Reinecke
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
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
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: Indefinite recursion in pci_default_read_config

2009-12-15 Thread Avi Kivity

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 = 0x46a5f0assigned_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 = 0x46a5f0assigned_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 = 0x46a050assigned_dev_pci_read_config,
   config_write = 0x469f30assigned_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 = 0x416770pci_default_cap_read_config,
 config_write = 0x46b750assigned_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

--
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: Indefinite recursion in pci_default_read_config

2009-12-15 Thread Michael S. Tsirkin
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 = 0x46a5f0assigned_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 = 0x46a5f0assigned_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 = 0x46a050assigned_dev_pci_read_config,
config_write = 0x469f30assigned_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 = 0x416770pci_default_cap_read_config,
  config_write = 0x46b750assigned_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,
--
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: Indefinite recursion in pci_default_read_config

2009-12-15 Thread Hannes Reinecke
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 = 0x46a5f0assigned_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 = 0x46a5f0assigned_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 = 0x46a050assigned_dev_pci_read_config,
config_write = 0x469f30assigned_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 = 0x416770pci_default_cap_read_config,
  config_write = 0x46b750assigned_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
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
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: Indefinite recursion in pci_default_read_config

2009-12-15 Thread Michael S. Tsirkin
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 = 0x46a5f0assigned_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 = 0x46a5f0assigned_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 = 0x46a050assigned_dev_pci_read_config,
 config_write = 0x469f30assigned_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 = 0x416770pci_default_cap_read_config,
   config_write = 0x46b750assigned_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

Re: Indefinite recursion in pci_default_read_config

2009-12-15 Thread Avi Kivity

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

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