From b06fe3e703f833866914c03c3fb0acc02385c824 Mon Sep 17 00:00:00 2001 From: Philippe Mathieu-Daudé Date: Sat, 24 Oct 2020 22:38:59 +0200 Subject: hw/pci: Extract pci_bus_change_irq_level() from pci_change_irq_level() Extract pci_bus_change_irq_level() from pci_change_irq_level() to make it clearer it operates on the bus. Reported-by: Mark Cave-Ayland Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20201024203900.3619498-2-f4bug@amsat.org> Reviewed-by: Mark Cave-Ayland Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci/pci.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'hw/pci/pci.c') diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 100c9381c2..081ddcadd1 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -248,6 +248,12 @@ static inline void pci_set_irq_state(PCIDevice *d, int irq_num, int level) d->irq_state |= level << irq_num; } +static void pci_bus_change_irq_level(PCIBus *bus, int irq_num, int change) +{ + bus->irq_count[irq_num] += change; + bus->set_irq(bus->irq_opaque, irq_num, bus->irq_count[irq_num] != 0); +} + static void pci_change_irq_level(PCIDevice *pci_dev, int irq_num, int change) { PCIBus *bus; @@ -258,8 +264,7 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int irq_num, int change) break; pci_dev = bus->parent_dev; } - bus->irq_count[irq_num] += change; - bus->set_irq(bus->irq_opaque, irq_num, bus->irq_count[irq_num] != 0); + pci_bus_change_irq_level(bus, irq_num, change); } int pci_bus_get_irq_level(PCIBus *bus, int irq_num) -- cgit v1.2.3-55-g7522 From 459ca8bfa41b42b9d80739929f09f792207f15f3 Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Sat, 24 Oct 2020 22:39:00 +0200 Subject: pci: Assert irqnum is between 0 and bus->nirqs in pci_bus_change_irq_level These assertions similar to those in the adjacent pci_bus_get_irq_level() function ensure that irqnum lies within the valid PCI bus IRQ range. Signed-off-by: Mark Cave-Ayland Message-Id: <20201011082022.3016-1-mark.cave-ayland@ilande.co.uk> Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20201024203900.3619498-3-f4bug@amsat.org> Reviewed-by: Mark Cave-Ayland Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci/pci.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'hw/pci/pci.c') diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 081ddcadd1..dc4019865b 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -250,6 +250,8 @@ static inline void pci_set_irq_state(PCIDevice *d, int irq_num, int level) static void pci_bus_change_irq_level(PCIBus *bus, int irq_num, int change) { + assert(irq_num >= 0); + assert(irq_num < bus->nirq); bus->irq_count[irq_num] += change; bus->set_irq(bus->irq_opaque, irq_num, bus->irq_count[irq_num] != 0); } -- cgit v1.2.3-55-g7522 From 2c729dc8ceaab88f213c7724de0fa181ffc7f078 Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Thu, 15 Oct 2020 11:14:10 -0700 Subject: pci: Change error_report to assert(3) Asserts are used for developer bugs. As registering a bar of the wrong size is not something that should be possible for a user to achieve, this is a developer bug. While here, use the more obvious helper function. Signed-off-by: Ben Widawsky Message-Id: <20201015181411.89104-1-ben.widawsky@intel.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Philippe Mathieu-Daudé --- hw/pci/pci.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'hw/pci/pci.c') diff --git a/hw/pci/pci.c b/hw/pci/pci.c index dc4019865b..e5b7c9a42b 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -1151,11 +1151,7 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, assert(region_num >= 0); assert(region_num < PCI_NUM_REGIONS); - if (size & (size-1)) { - error_report("ERROR: PCI region size must be pow2 " - "type=0x%x, size=0x%"FMT_PCIBUS"", type, size); - exit(1); - } + assert(is_power_of_2(size)); r = &pci_dev->io_regions[region_num]; r->addr = PCI_BAR_UNMAPPED; -- cgit v1.2.3-55-g7522 From 6a5b19ca63b1795011f53244f2fd9a2cf8189b72 Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Thu, 15 Oct 2020 11:14:11 -0700 Subject: pci: Disallow improper BAR registration for type 1 Prevent future developers working on root complexes, root ports, or bridges that also wish to implement a BAR for those, from shooting themselves in the foot. PCI type 1 headers only support 2 base address registers. It is incorrect and difficult to figure out what is wrong with the device when this mistake is made. With this, it is immediate and obvious what has gone wrong. Signed-off-by: Ben Widawsky Message-Id: <20201015181411.89104-2-ben.widawsky@intel.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci/pci.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'hw/pci/pci.c') diff --git a/hw/pci/pci.c b/hw/pci/pci.c index e5b7c9a42b..0131d9d02c 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -1148,11 +1148,17 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, uint32_t addr; /* offset in pci config space */ uint64_t wmask; pcibus_t size = memory_region_size(memory); + uint8_t hdr_type; assert(region_num >= 0); assert(region_num < PCI_NUM_REGIONS); assert(is_power_of_2(size)); + /* A PCI bridge device (with Type 1 header) may only have at most 2 BARs */ + hdr_type = + pci_dev->config[PCI_HEADER_TYPE] & ~PCI_HEADER_TYPE_MULTI_FUNCTION; + assert(hdr_type != PCI_HEADER_TYPE_BRIDGE || region_num < 2); + r = &pci_dev->io_regions[region_num]; r->addr = PCI_BAR_UNMAPPED; r->size = size; -- cgit v1.2.3-55-g7522