On Sun Jan 19, 2025 at 8:38 PM AEST, Phil Dennis-Jordan wrote: > Looks good to me. There is a risk here that the assertion will fail on > existing code. (Unless you've rigorously audited all callers, which would > be quite the task.) However, I agree that this would constitute a bug in > the calling code, not an issue with this change. Since we've still got a > few months left in the 10.0 release cycle, I say go for it - hopefully such > bugs, if there are any, will be shaken out over the next few weeks.
You're right I didn't do an exhaustive audit or test beyond CI and some browsing. I think it would be quite buggy already if this happens so we should just catch and fix it quickly, but happy to change to a warning first if anybody is concerned. Thanks, Nick > > On Fri, 17 Jan 2025 at 18:29, Nicholas Piggin <[email protected]> wrote: > >> Nothing should be doing this, but it doesn't get caught by >> pci_register_bar(). Add an assertion to prevent misuse. >> >> Signed-off-by: Nicholas Piggin <[email protected]> >> > Reviewed-by: Phil Dennis-Jordan <[email protected]> > > >> --- >> hw/pci/pci.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index 2afa423925c..b067a55c5bc 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -1391,6 +1391,7 @@ void pci_register_bar(PCIDevice *pci_dev, int >> region_num, >> assert(hdr_type != PCI_HEADER_TYPE_BRIDGE || region_num < 2); >> >> r = &pci_dev->io_regions[region_num]; >> + assert(!r->size); >> r->addr = PCI_BAR_UNMAPPED; >> r->size = size; >> r->type = type; >> -- >> 2.45.2 >> >> >>
