[PATCH v3 0/2] tests/avocado: make sbsa-ref working with >1 core

[PATCH v3 0/2] tests/avocado: make sbsa-ref working with >1 core

[Marcin Juszkiewicz]

Recent changes made sbsa-ref crash when more than 1 cpu core was used.
We handle it in firmware now so one patch updates it to the working
snapshot (TF-A 2.11 + EDK2 snapshot + EDK2-platforms snapshot).

Other change drops "-smp 1" from CI to make sure we test default setup
of sbsa-ref.

Previous firmware worked with 1 cpu by pure luck probably.

To: qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org,
Cc: Peter Maydell <peter.maydell@linaro.org>,
Cc: Leif Lindholm <quic_llindhol@quicinc.com>,
Cc: Radoslaw Biernacki <rad@semihalf.com>,
Cc: Cleber Rosa <crosa@redhat.com>,
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Wainer dos Santos Moschetta <wainersm@redhat.com>,
Cc: Beraldo Leal <bleal@redhat.com>,
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Rebecca Cran <rebecca@bsdio.com>

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
Changes in v3:
- first update firmware, then use all cores (for bisecting)
- changed commit message in 'use all cores' patch

---
Marcin Juszkiewicz (2):
      tests/avocado: update firmware for sbsa-ref
      tests/avocado: use default amount of cores on sbsa-ref

 tests/avocado/machine_aarch64_sbsaref.py | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
---
base-commit: 02d9c38236cf8c9826e5c5be61780c4444cb4ae0
change-id: 20240620-b4-new-firmware-177daccc9d76

Best regards,
-- 
Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>

[PATCH v3 1/2] tests/avocado: update firmware for sbsa-ref

[Marcin Juszkiewicz]

Update firmware to have graphics card memory fix from EDK2 commit
c1d1910be6e04a8b1a73090cf2881fb698947a6e:

    OvmfPkg/QemuVideoDxe: add feature PCD to remap framebuffer W/C

    Some platforms (such as SBSA-QEMU on recent builds of the emulator) only
    tolerate misaligned accesses to normal memory, and raise alignment
    faults on such accesses to device memory, which is the default for PCIe
    MMIO BARs.

    When emulating a PCIe graphics controller, the framebuffer is typically
    exposed via a MMIO BAR, while the disposition of the region is closer to
    memory (no side effects on reads or writes, except for the changing
    picture on the screen; direct random access to any pixel in the image).

    In order to permit the use of such controllers on platforms that only
    tolerate these types of accesses for normal memory, it is necessary to
    remap the memory. Use the DXE services to set the desired capabilities
    and attributes.

    Hide this behavior under a feature PCD so only platforms that really
    need it can enable it. (OVMF on x86 has no need for this)

With this fix enabled we can boot sbsa-ref with more than one cpu core.

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
 tests/avocado/machine_aarch64_sbsaref.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/avocado/machine_aarch64_sbsaref.py b/tests/avocado/machine_aarch64_sbsaref.py
index 6bb82f2a03..e854ec6a1a 100644
--- a/tests/avocado/machine_aarch64_sbsaref.py
+++ b/tests/avocado/machine_aarch64_sbsaref.py
@@ -37,18 +37,18 @@ def fetch_firmware(self):
 
         Used components:
 
-        - Trusted Firmware 2.11.0
-        - Tianocore EDK2 stable202405
-        - Tianocore EDK2-platforms commit 4bbd0ed
+        - Trusted Firmware         v2.11.0
+        - Tianocore EDK2           4d4f569924
+        - Tianocore EDK2-platforms 3f08401
 
         """
 
         # Secure BootRom (TF-A code)
         fs0_xz_url = (
             "https://artifacts.codelinaro.org/artifactory/linaro-419-sbsa-ref/"
-            "20240528-140808/edk2/SBSA_FLASH0.fd.xz"
+            "20240619-148232/edk2/SBSA_FLASH0.fd.xz"
         )
-        fs0_xz_hash = "fa6004900b67172914c908b78557fec4d36a5f784f4c3dd08f49adb75e1892a9"
+        fs0_xz_hash = "0c954842a590988f526984de22e21ae0ab9cb351a0c99a8a58e928f0c7359cf7"
         tar_xz_path = self.fetch_asset(fs0_xz_url, asset_hash=fs0_xz_hash,
                                       algorithm='sha256')
         archive.extract(tar_xz_path, self.workdir)
@@ -57,9 +57,9 @@ def fetch_firmware(self):
         # Non-secure rom (UEFI and EFI variables)
         fs1_xz_url = (
             "https://artifacts.codelinaro.org/artifactory/linaro-419-sbsa-ref/"
-            "20240528-140808/edk2/SBSA_FLASH1.fd.xz"
+            "20240619-148232/edk2/SBSA_FLASH1.fd.xz"
         )
-        fs1_xz_hash = "5f3747d4000bc416d9641e33ff4ac60c3cc8cb74ca51b6e932e58531c62eb6f7"
+        fs1_xz_hash = "c6ec39374c4d79bb9e9cdeeb6db44732d90bb4a334cec92002b3f4b9cac4b5ee"
         tar_xz_path = self.fetch_asset(fs1_xz_url, asset_hash=fs1_xz_hash,
                                       algorithm='sha256')
         archive.extract(tar_xz_path, self.workdir)

-- 
2.45.1

Re: [PATCH v3 1/2] tests/avocado: update firmware for sbsa-ref

[Ard Biesheuvel]

On Thu, 20 Jun 2024 at 12:20, Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> wrote:
>
> Update firmware to have graphics card memory fix from EDK2 commit
> c1d1910be6e04a8b1a73090cf2881fb698947a6e:
>
>     OvmfPkg/QemuVideoDxe: add feature PCD to remap framebuffer W/C
>
>     Some platforms (such as SBSA-QEMU on recent builds of the emulator) only
>     tolerate misaligned accesses to normal memory, and raise alignment
>     faults on such accesses to device memory, which is the default for PCIe
>     MMIO BARs.
>
>     When emulating a PCIe graphics controller, the framebuffer is typically
>     exposed via a MMIO BAR, while the disposition of the region is closer to
>     memory (no side effects on reads or writes, except for the changing
>     picture on the screen; direct random access to any pixel in the image).
>
>     In order to permit the use of such controllers on platforms that only
>     tolerate these types of accesses for normal memory, it is necessary to
>     remap the memory. Use the DXE services to set the desired capabilities
>     and attributes.
>
>     Hide this behavior under a feature PCD so only platforms that really
>     need it can enable it. (OVMF on x86 has no need for this)
>
> With this fix enabled we can boot sbsa-ref with more than one cpu core.
>

This requires an explanation: what does the number of CPU cores have
to do with the memory attributes used for the framebuffer?


> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> ---
>  tests/avocado/machine_aarch64_sbsaref.py | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tests/avocado/machine_aarch64_sbsaref.py b/tests/avocado/machine_aarch64_sbsaref.py
> index 6bb82f2a03..e854ec6a1a 100644
> --- a/tests/avocado/machine_aarch64_sbsaref.py
> +++ b/tests/avocado/machine_aarch64_sbsaref.py
> @@ -37,18 +37,18 @@ def fetch_firmware(self):
>
>          Used components:
>
> -        - Trusted Firmware 2.11.0
> -        - Tianocore EDK2 stable202405
> -        - Tianocore EDK2-platforms commit 4bbd0ed
> +        - Trusted Firmware         v2.11.0
> +        - Tianocore EDK2           4d4f569924
> +        - Tianocore EDK2-platforms 3f08401
>
>          """
>
>          # Secure BootRom (TF-A code)
>          fs0_xz_url = (
>              "https://artifacts.codelinaro.org/artifactory/linaro-419-sbsa-ref/"
> -            "20240528-140808/edk2/SBSA_FLASH0.fd.xz"
> +            "20240619-148232/edk2/SBSA_FLASH0.fd.xz"
>          )
> -        fs0_xz_hash = "fa6004900b67172914c908b78557fec4d36a5f784f4c3dd08f49adb75e1892a9"
> +        fs0_xz_hash = "0c954842a590988f526984de22e21ae0ab9cb351a0c99a8a58e928f0c7359cf7"
>          tar_xz_path = self.fetch_asset(fs0_xz_url, asset_hash=fs0_xz_hash,
>                                        algorithm='sha256')
>          archive.extract(tar_xz_path, self.workdir)
> @@ -57,9 +57,9 @@ def fetch_firmware(self):
>          # Non-secure rom (UEFI and EFI variables)
>          fs1_xz_url = (
>              "https://artifacts.codelinaro.org/artifactory/linaro-419-sbsa-ref/"
> -            "20240528-140808/edk2/SBSA_FLASH1.fd.xz"
> +            "20240619-148232/edk2/SBSA_FLASH1.fd.xz"
>          )
> -        fs1_xz_hash = "5f3747d4000bc416d9641e33ff4ac60c3cc8cb74ca51b6e932e58531c62eb6f7"
> +        fs1_xz_hash = "c6ec39374c4d79bb9e9cdeeb6db44732d90bb4a334cec92002b3f4b9cac4b5ee"
>          tar_xz_path = self.fetch_asset(fs1_xz_url, asset_hash=fs1_xz_hash,
>                                        algorithm='sha256')
>          archive.extract(tar_xz_path, self.workdir)
>
> --
> 2.45.1
>

Re: [PATCH v3 1/2] tests/avocado: update firmware for sbsa-ref

[Marcin Juszkiewicz]

W dniu 30.06.2024 o 16:37, Ard Biesheuvel pisze:
> On Thu, 20 Jun 2024 at 12:20, Marcin Juszkiewicz
> <marcin.juszkiewicz@linaro.org> wrote:
>>
>> Update firmware to have graphics card memory fix from EDK2 commit
>> c1d1910be6e04a8b1a73090cf2881fb698947a6e:
>>
>>      OvmfPkg/QemuVideoDxe: add feature PCD to remap framebuffer W/C
>>
>>      Some platforms (such as SBSA-QEMU on recent builds of the emulator) only
>>      tolerate misaligned accesses to normal memory, and raise alignment
>>      faults on such accesses to device memory, which is the default for PCIe
>>      MMIO BARs.
>>
>>      When emulating a PCIe graphics controller, the framebuffer is typically
>>      exposed via a MMIO BAR, while the disposition of the region is closer to
>>      memory (no side effects on reads or writes, except for the changing
>>      picture on the screen; direct random access to any pixel in the image).
>>
>>      In order to permit the use of such controllers on platforms that only
>>      tolerate these types of accesses for normal memory, it is necessary to
>>      remap the memory. Use the DXE services to set the desired capabilities
>>      and attributes.
>>
>>      Hide this behavior under a feature PCD so only platforms that really
>>      need it can enable it. (OVMF on x86 has no need for this)
>>
>> With this fix enabled we can boot sbsa-ref with more than one cpu core.
>>
> 
> This requires an explanation: what does the number of CPU cores have
> to do with the memory attributes used for the framebuffer?

I have no idea. Older firmware was hanging on several systems but was 
passing in QEMU tests. After closer looking I noticed that Avocado tests 
run with "-smp 1" and pass.

Checked failing system with "-smp 1" and it worked. In meantime you have 
fixed problem in EDK2.

So yes, updating firmware may look like hiding a bug. Which I do not 
know how to track (I can build and test QEMU, but going into its 
internals is something I never done).

Re: [PATCH v3 1/2] tests/avocado: update firmware for sbsa-ref

[Peter Maydell]

On Mon, 1 Jul 2024 at 07:49, Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> wrote:
>
> W dniu 30.06.2024 o 16:37, Ard Biesheuvel pisze:
> > On Thu, 20 Jun 2024 at 12:20, Marcin Juszkiewicz
> > <marcin.juszkiewicz@linaro.org> wrote:
> >>
> >> Update firmware to have graphics card memory fix from EDK2 commit
> >> c1d1910be6e04a8b1a73090cf2881fb698947a6e:
> >>
> >>      OvmfPkg/QemuVideoDxe: add feature PCD to remap framebuffer W/C
> >>
> >>      Some platforms (such as SBSA-QEMU on recent builds of the emulator) only
> >>      tolerate misaligned accesses to normal memory, and raise alignment
> >>      faults on such accesses to device memory, which is the default for PCIe
> >>      MMIO BARs.
> >>
> >>      When emulating a PCIe graphics controller, the framebuffer is typically
> >>      exposed via a MMIO BAR, while the disposition of the region is closer to
> >>      memory (no side effects on reads or writes, except for the changing
> >>      picture on the screen; direct random access to any pixel in the image).
> >>
> >>      In order to permit the use of such controllers on platforms that only
> >>      tolerate these types of accesses for normal memory, it is necessary to
> >>      remap the memory. Use the DXE services to set the desired capabilities
> >>      and attributes.
> >>
> >>      Hide this behavior under a feature PCD so only platforms that really
> >>      need it can enable it. (OVMF on x86 has no need for this)
> >>
> >> With this fix enabled we can boot sbsa-ref with more than one cpu core.
> >>
> >
> > This requires an explanation: what does the number of CPU cores have
> > to do with the memory attributes used for the framebuffer?
>
> I have no idea. Older firmware was hanging on several systems but was
> passing in QEMU tests. After closer looking I noticed that Avocado tests
> run with "-smp 1" and pass.
>
> Checked failing system with "-smp 1" and it worked. In meantime you have
> fixed problem in EDK2.
>
> So yes, updating firmware may look like hiding a bug. Which I do not
> know how to track (I can build and test QEMU, but going into its
> internals is something I never done).

My assumption was that random chance meant that TF-A when
only dealing with one CPU meant that its memory layout etc
was such that it didn't do the unaligned access. I don't think
this is likely to be a QEMU side question.

-- PMM

[PATCH v3 2/2] tests/avocado: use default amount of cores on sbsa-ref

[Marcin Juszkiewicz]

The version of the sbsa-ref EDK2 firmware we used to use in this test
had a bug where it might make an unaligned access to the framebuffer,
which causes a guest crash on newer versions of QEMU where we enforce
the architectural requirement that unaligned accesses to Device memory
should take an exception.

We happened to not notice this because our test was booting with "-smp
1" and through luck this didn't write the boot logo to the framebuffer
at an unaligned address; but trying to boot the same firmware with two
CPUs would result in a guest crash. Now we have updated the firmware
we're using for the test, we can make the test use all the cores on the
board, so we are testing the SMP boot path.

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
 tests/avocado/machine_aarch64_sbsaref.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/avocado/machine_aarch64_sbsaref.py b/tests/avocado/machine_aarch64_sbsaref.py
index e854ec6a1a..e920bbf08c 100644
--- a/tests/avocado/machine_aarch64_sbsaref.py
+++ b/tests/avocado/machine_aarch64_sbsaref.py
@@ -75,8 +75,6 @@ def fetch_firmware(self):
             f"if=pflash,file={fs0_path},format=raw",
             "-drive",
             f"if=pflash,file={fs1_path},format=raw",
-            "-smp",
-            "1",
             "-machine",
             "sbsa-ref",
         )

-- 
2.45.1

Re: [PATCH v3 0/2] tests/avocado: make sbsa-ref working with >1 core

[Philippe Mathieu-Daudé]

On 20/6/24 12:19, Marcin Juszkiewicz wrote:
> Recent changes made sbsa-ref crash when more than 1 cpu core was used.
> We handle it in firmware now so one patch updates it to the working
> snapshot (TF-A 2.11 + EDK2 snapshot + EDK2-platforms snapshot).
> 
> Other change drops "-smp 1" from CI to make sure we test default setup
> of sbsa-ref.
> 
> Previous firmware worked with 1 cpu by pure luck probably.


> Marcin Juszkiewicz (2):
>        tests/avocado: update firmware for sbsa-ref
>        tests/avocado: use default amount of cores on sbsa-ref

Series:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v3 0/2] tests/avocado: make sbsa-ref working with >1 core

[Peter Maydell]

On Thu, 20 Jun 2024 at 11:19, Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> wrote:
>
> Recent changes made sbsa-ref crash when more than 1 cpu core was used.
> We handle it in firmware now so one patch updates it to the working
> snapshot (TF-A 2.11 + EDK2 snapshot + EDK2-platforms snapshot).
>
> Other change drops "-smp 1" from CI to make sure we test default setup
> of sbsa-ref.
>
> Previous firmware worked with 1 cpu by pure luck probably.
>
> To: qemu-devel@nongnu.org
> Cc: qemu-arm@nongnu.org,
> Cc: Peter Maydell <peter.maydell@linaro.org>,
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>,
> Cc: Radoslaw Biernacki <rad@semihalf.com>,
> Cc: Cleber Rosa <crosa@redhat.com>,
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Wainer dos Santos Moschetta <wainersm@redhat.com>,
> Cc: Beraldo Leal <bleal@redhat.com>,
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Rebecca Cran <rebecca@bsdio.com>
>
> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> ---



Applied to target-arm.next, thanks.

-- PMM