Re: [PATCH v2 3/5] tests/qtest: ahci-test: Check only implemented ports in verify_state

[Peter Maydell <peter.maydell@linaro.org>]

On Thu, 27 Nov 2025 at 00:12, Navid Emamdoost <navidem@google.com> wrote:
>
> The verify_state helper function in ahci-test.c incorrectly
> assumed that all 32 potential AHCI ports were implemented. During post-
> migration checks, it would loop through all 32 ports, attempting to
> read registers for non-existent ones.
> This resulted in an out-of-bounds access on the main HBA BAR. This
> latent bug was exposed by the recent introduction of strict bounds
> checking in the libqos PCI accessors, which now correctly triggers a
> fatal assertion.
> Fix this by modifying the loop in verify_state to first read the
> AHCI_PI (Ports Implemented) register and then only check the state
> for ports that the device reports as present.
>
> Signed-off-by: Navid Emamdoost <navidem@google.com>
> ---
>  tests/qtest/ahci-test.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
> index e8aabfc13f..06c5bd08d8 100644
> --- a/tests/qtest/ahci-test.c
> +++ b/tests/qtest/ahci-test.c
> @@ -81,6 +81,7 @@ static void string_bswap16(uint16_t *s, size_t bytes)
>  static void verify_state(AHCIQState *ahci, uint64_t hba_old)
>  {
>      int i, j;
> +    uint32_t ports_impl;
>      uint32_t ahci_fingerprint;
>      uint64_t hba_base;
>      AHCICommandHeader cmd;
> @@ -99,7 +100,14 @@ static void verify_state(AHCIQState *ahci, uint64_t hba_old)
>      g_assert_cmphex(ahci_rreg(ahci, AHCI_CAP), ==, ahci->cap);
>      g_assert_cmphex(ahci_rreg(ahci, AHCI_CAP2), ==, ahci->cap2);
>
> +    ports_impl = ahci_rreg(ahci, AHCI_PI);
> +
>      for (i = 0; i < 32; i++) {
> +
> +        if (!(ports_impl & (1 << i))) {

We should use "1U << i" here, because coverity will probably
complain about shifting into the sign bit of a signed integer
otherwise.

> +            continue; /* Skip unimplemented ports */
> +        }
> +
>          g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_FB), ==,
>                          ahci->port[i].fb);
>          g_assert_cmphex(ahci_px_rreg(ahci, i, AHCI_PX_CLB), ==,

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM