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 <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: Akihiko Odaki <akihiko.odaki@daynix.com>
> Cc: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Nicholas Piggin <npiggin@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.
> bool enabled;
> } AHCIQState;
>
> diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
> index 36caa7b2999..7d5f73ac40b 100644
> --- a/tests/qtest/ahci-test.c
> +++ b/tests/qtest/ahci-test.c
> @@ -85,6 +85,8 @@ static void verify_state(AHCIQState *ahci, uint64_t hba_old)
> uint64_t hba_base;
> AHCICommandHeader cmd;
>
> + g_assert_cmphex(ahci->hba_bar.addr, ==, hba_old);
> +
> ahci_fingerprint = qpci_config_readl(ahci->dev, PCI_VENDOR_ID);
> g_assert_cmphex(ahci_fingerprint, ==, ahci->fingerprint);
>
> @@ -198,7 +200,9 @@ static void ahci_shutdown(AHCIQState *ahci)
> {
> QOSState *qs = ahci->parent;
>
> - ahci_pci_disable(ahci);
> + if (ahci->pci_enabled) {
> + ahci_pci_disable(ahci);
> + }
> ahci_clean_mem(ahci);
> free_ahci_device(ahci->dev);
> g_free(ahci);
> @@ -1421,6 +1425,7 @@ static void test_reset(void)
> ahci_set(ahci, AHCI_GHC, AHCI_GHC_HR);
> stop_ahci_device(ahci);
> ahci_clean_mem(ahci);
> + start_ahci_device(ahci);
> }
>
> ahci_shutdown(ahci);
> diff --git a/tests/qtest/libqos/ahci.c b/tests/qtest/libqos/ahci.c
> index bd1994a9208..cc4f5b7b534 100644
> --- a/tests/qtest/libqos/ahci.c
> +++ b/tests/qtest/libqos/ahci.c
> @@ -215,17 +215,25 @@ void ahci_pci_disable(AHCIQState *ahci)
> */
> void start_ahci_device(AHCIQState *ahci)
> {
> + g_assert(!ahci->pci_enabled);
> +
> /* Map AHCI's ABAR (BAR5) */
> ahci->hba_bar = qpci_iomap(ahci->dev, 5, &ahci->barsize);
>
> /* turns on pci.cmd.iose, pci.cmd.mse and pci.cmd.bme */
> qpci_device_enable(ahci->dev);
> +
> + ahci->pci_enabled = true;
> }
>
> void stop_ahci_device(AHCIQState *ahci)
> {
> + g_assert(ahci->pci_enabled);
> +
> /* Unmap AHCI's ABAR */
> qpci_iounmap(ahci->dev, ahci->hba_bar);
> +
> + ahci->pci_enabled = false;
> }
>
> /**
> @@ -249,6 +257,7 @@ void ahci_hba_enable(AHCIQState *ahci)
> uint8_t num_cmd_slots;
>
> g_assert(ahci != NULL);
> + g_assert(ahci->pci_enabled);
>
> /* Set GHC.AE to 1 */
> ahci_set(ahci, AHCI_GHC, AHCI_GHC_AE);