public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <quic_llindhol@quicinc.com>
To: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
Cc: <devel@edk2.groups.io>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Graeme Gregory <graeme@xora.org.uk>,
	Jeremy Linton <jeremy.linton@arm.com>,
	Ling Jia <jialing@phytium.com.cn>,
	Marcin Wojtas <mw@semihalf.com>,
	Peng Xie <xiepeng@phytium.com.cn>,
	Yiqi Shu <shuyiqi@phytium.com.cn>
Subject: Re: [edk2-devel] [PATCH edk2-platforms 1/1] set WritePolicyValid for all cache types
Date: Wed, 15 Nov 2023 13:05:10 +0000	[thread overview]
Message-ID: <ZVTCBiheaB8pwpMe@qc-i7.hemma.eciton.net> (raw)
In-Reply-To: <20231115124608.1712896-1-marcin.juszkiewicz@linaro.org>

On Wed, Nov 15, 2023 at 13:46:08 +0100, Marcin Juszkiewicz wrote:
> acpiview complains:
> 
> ERROR: On Arm based systems, all cache properties must be provided in
> the cache type structure. Missing 'Write Policy Valid' flag.
> 
> ACPI specification says:
> 
> > Set to 1 if the write policy attribute described is valid. A value 
> > of 0 indicates that, where possible, processor architecture specific
> > discovery mechanisms should be used to ascertain the value of this
> > attribute.

Thanks for finding this. This is definitely an error (and I expect
this is a single original error that has then been copied around to
other platforms, so important to fix all of them).

But I can see how that mistake was made. The ACPI specification makes
no reference to what write policy means for instruction caches
(i.e. nothing).

However, the ACPI specification *does* make it crystal clear that "On
Arm-based systems, all cache properties must be provided in the
table.". So indicating that one of the entries is invalid is
... invalid.
So apparently all Arm instruction caches are Write-back. Lol.

> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>

Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>

I'll leave this one for a day or so before merging to allow
reviewers/maintainers to see and respond.

Regards,

Leif

> ---
>  Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h | 2 +-
>  Platform/RaspberryPi/AcpiTables/Pptt.aslc                     | 2 +-
>  Silicon/Marvell/Armada7k8k/AcpiTables/Pptt.aslc               | 2 +-
>  Silicon/Marvell/OcteonTx/AcpiTables/T91/Pptt.aslc             | 2 +-
>  Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Pptt.aslc      | 2 +-
>  Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc              | 2 +-
>  6 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> index 983d17f6fa50..61d8bce8c959 100644
> --- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> +++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> @@ -126,7 +126,7 @@ typedef struct {
>        1,                       /* AssociativityValid */                        \
>        1,                       /* AllocationTypeValid */                       \
>        1,                       /* CacheTypeValid */                            \
> -      0,                       /* WritePolicyValid */                          \
> +      1,                       /* WritePolicyValid */                          \
>        1,                       /* LineSizeValid */                             \
>      },                                                                         \
>      0,                         /* NextLevelOfCache */                          \
> diff --git a/Platform/RaspberryPi/AcpiTables/Pptt.aslc b/Platform/RaspberryPi/AcpiTables/Pptt.aslc
> index a52bc5a31adf..b80f5ff1e057 100644
> --- a/Platform/RaspberryPi/AcpiTables/Pptt.aslc
> +++ b/Platform/RaspberryPi/AcpiTables/Pptt.aslc
> @@ -114,7 +114,7 @@ typedef struct {
>        1,          /* AssociativityValid */                                     \
>        1,          /* AllocationTypeValid */                                    \
>        1,          /* CacheTypeValid */                                         \
> -      0,          /* WritePolicyValid */                                       \
> +      1,          /* WritePolicyValid */                                       \
>        1,          /* LineSizeValid */                                          \
>      },                                                                         \
>      0,            /* NextLevelOfCache */                                       \
> diff --git a/Silicon/Marvell/Armada7k8k/AcpiTables/Pptt.aslc b/Silicon/Marvell/Armada7k8k/AcpiTables/Pptt.aslc
> index e03bfcd6211d..f3a9b90fb564 100644
> --- a/Silicon/Marvell/Armada7k8k/AcpiTables/Pptt.aslc
> +++ b/Silicon/Marvell/Armada7k8k/AcpiTables/Pptt.aslc
> @@ -94,7 +94,7 @@ typedef struct {
>        1,          /* AssociativityValid */                                     \
>        1,          /* AllocationTypeValid */                                    \
>        1,          /* CacheTypeValid */                                         \
> -      0,          /* WritePolicyValid */                                       \
> +      1,          /* WritePolicyValid */                                       \
>        1,          /* LineSizeValid */                                          \
>      },                                                                         \
>      0,            /* NextLevelOfCache */                                       \
> diff --git a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Pptt.aslc b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Pptt.aslc
> index f37c7511134d..3793cbbca0b5 100644
> --- a/Silicon/Marvell/OcteonTx/AcpiTables/T91/Pptt.aslc
> +++ b/Silicon/Marvell/OcteonTx/AcpiTables/T91/Pptt.aslc
> @@ -94,7 +94,7 @@ typedef struct {
>        1,          /* AssociativityValid */                                     \
>        1,          /* AllocationTypeValid */                                    \
>        1,          /* CacheTypeValid */                                         \
> -      0,          /* WritePolicyValid */                                       \
> +      1,          /* WritePolicyValid */                                       \
>        1,          /* LineSizeValid */                                          \
>      },                                                                         \
>      0,            /* NextLevelOfCache */                                       \
> diff --git a/Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Pptt.aslc b/Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Pptt.aslc
> index ae1a21df23b9..04652653563e 100644
> --- a/Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Pptt.aslc
> +++ b/Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Pptt.aslc
> @@ -92,7 +92,7 @@ typedef struct {
>        1,          /* AssociativityValid */                                    \
>        1,          /* AllocationTypeValid */                                   \
>        1,          /* CacheTypeValid */                                        \
> -      0,          /* WritePolicyValid */                                      \
> +      1,          /* WritePolicyValid */                                      \
>        1,          /* LineSizeValid */                                         \
>      },                                                                        \
>      0,            /* NextLevelOfCache */                                      \
> diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc b/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc
> index e351d82b9763..f3e9149769ea 100644
> --- a/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc
> +++ b/Silicon/Socionext/SynQuacer/AcpiTables/Pptt.aslc
> @@ -91,7 +91,7 @@ typedef struct {
>        1,          /* AssociativityValid */                                    \
>        1,          /* AllocationTypeValid */                                   \
>        1,          /* CacheTypeValid */                                        \
> -      0,          /* WritePolicyValid */                                      \
> +      1,          /* WritePolicyValid */                                      \
>        1,          /* LineSizeValid */                                         \
>      },                                                                        \
>      0,            /* NextLevelOfCache */                                      \
> -- 
> 2.41.0
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111273): https://edk2.groups.io/g/devel/message/111273
Mute This Topic: https://groups.io/mt/102603819/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



  reply	other threads:[~2023-11-15 13:05 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-15 12:46 [edk2-devel] [PATCH edk2-platforms 1/1] set WritePolicyValid for all cache types Marcin Juszkiewicz
2023-11-15 13:05 ` Leif Lindholm [this message]
2023-11-22 15:14 ` Leif Lindholm

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZVTCBiheaB8pwpMe@qc-i7.hemma.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox