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