On Mon May 5, 2025 at 3:25 PM AEST, Akihiko Odaki wrote:
> On 2025/05/02 12:04, Nicholas Piggin wrote:
>> ahci-test has a bunch of tests where the pci bar was not mapped. Avoid
>> unmapping it in these cases, to keep iomaps balanced.
>>
>> Cc: Michael S. Tsirkin <m...@redhat.com>
>> Cc: Marcel Apfelbaum <marcel.apfelb...@gmail.com>
>> Cc: Akihiko Odaki <akihiko.od...@daynix.com>
>> Cc: Fabiano Rosas <faro...@suse.de>
>> Signed-off-by: Nicholas Piggin <npig...@gmail.com>
>> ---
>> tests/qtest/libqos/ahci.h | 1 +
>> tests/qtest/ahci-test.c | 7 ++++++-
>> tests/qtest/libqos/ahci.c | 9 +++++++++
>> 3 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/qtest/libqos/ahci.h b/tests/qtest/libqos/ahci.h
>> index f610bd32a5f..d639692aac4 100644
>> --- a/tests/qtest/libqos/ahci.h
>> +++ b/tests/qtest/libqos/ahci.h
>> @@ -342,6 +342,7 @@ typedef struct AHCIQState {
>> uint32_t cap;
>> uint32_t cap2;
>> AHCIPortQState port[32];
>> + bool pci_enabled;
>
> The following patch also adds a similar variable for virtio and has a
> slightly different semantics; qvirtio_pci_device_disable() is no-op but
> ahci_pci_disable() aborts when no-op.
>
> A bool flag can be added to QPCIBar instead so that we can enforce the
> "no-op if not mapped" semantics everywhere consistently with less code.
Now I think about it, the reason for the patch in the first place is
to ensure tests balance their maps and unmaps.
If we want to just keep allowing iounmap() of not-mapped bar to simplify
error handling and cleanup cases that's fine, I think I can just drop
these patches and remove assert in iounmap.
Thanks,
Nick
