Re: [PATCH 2/2] hw/pci: Assert a bar is not registered multiple times

["Nicholas Piggin" <npiggin@gmail.com>]

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 <npiggin@gmail.com> 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 <npiggin@gmail.com>
>>
> Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
>
>
>> ---
>>  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
>>
>>
>>