IORT: Updates for revision E.d #752
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
IORT revision is now updated to E.d (ARM DEN 0049E.d) and
contains a few additions like,
-Added descriptor in the root complex node for specifying
PASID width supported by the root complex.
-Updated RMR node Flags field.
-Introduced memory access attributes in the RMR node.
Please note that IORT Rev E.c is deprecated and not supported.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
|
The IORT Rev E.d doc can be found here, Thanks, |
rmurphy-arm
reviewed
Mar 22, 2022
|
We would like these defines to be all upper case.
For example:
+#define ACPI_IORT_RMR_ATTR_DEVICE_nGnRnE 0x00
From: Robin Murphy ***@***.***>
Sent: Tuesday, March 22, 2022 12:27 PM
To: acpica/acpica ***@***.***>
Cc: Subscribed ***@***.***>
Subject: Re: [acpica/acpica] IORT: Updates for revision E.d (PR #752)
@rmurphy-arm commented on this pull request.
________________________________
In source/include/actbl2.h<#752 (comment)>:
+#define ACPI_IORT_RMR_ACCESS_PRIVILEGE (1<<1)
+
+/*
+ * Macro to access the Access Attributes in flags field above:
+ * Access Attributes is encoded in bits 9:2
+ */
+#define ACPI_IORT_RMR_ACCESS_ATTRIBUTES(flags) (((flags) >> 2) & 0xFF)
+
+/* Values for above Access Attributes */
+
+#define ACPI_IORT_RMR_ATTR_DEVICE_nGnRnE 0x00
+#define ACPI_IORT_RMR_ATTR_DEVICE_nGnRE 0x01
+#define ACPI_IORT_RMR_ATTR_DEVICE_nGRE 0x02
+#define ACPI_IORT_RMR_ATTR_DEVICE_GRE 0x03
+#define ACPI_IORT_RMR_ATTR_NORMAL_NC 0x04
+#define ACPI_IORT_RMR_ATTR_NORMAL 0x05
Just to minimise any possible ambiguity, could we call it:
⬇️ Suggested change
-#define ACPI_IORT_RMR_ATTR_NORMAL 0x05
+#define ACPI_IORT_RMR_ATTR_NORMAL_IWB_OWB 0x05
—
Reply to this email directly, view it on GitHub<#752 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAHL7LDTB4I75P6RFRIJ7QLVBINIBANCNFSM5O6QITZA>.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.******@***.***>>
|
Co-authored-by: Robin Murphy <robin.murphy@arm.com>
|
This is the way normally device memory is represented in Arm specification
documents and kernel as well.
eg:
https://elixir.bootlin.com/linux/v4.5/source/arch/arm64/include/asm/pgtable.h#L72
So would like to keep that to avoid any confusion, if it is Ok.
Thanks,
Shameer
…On Wed, Mar 23, 2022 at 2:23 PM Robert Moore ***@***.***> wrote:
We would like these defines to be all upper case.
For example:
+#define ACPI_IORT_RMR_ATTR_DEVICE_nGnRnE 0x00
From: Robin Murphy ***@***.***>
Sent: Tuesday, March 22, 2022 12:27 PM
To: acpica/acpica ***@***.***>
Cc: Subscribed ***@***.***>
Subject: Re: [acpica/acpica] IORT: Updates for revision E.d (PR #752)
@rmurphy-arm commented on this pull request.
________________________________
In source/include/actbl2.h<
#752 (comment)>:
> +#define ACPI_IORT_RMR_ACCESS_PRIVILEGE (1<<1)
+
+/*
+ * Macro to access the Access Attributes in flags field above:
+ * Access Attributes is encoded in bits 9:2
+ */
+#define ACPI_IORT_RMR_ACCESS_ATTRIBUTES(flags) (((flags) >> 2) & 0xFF)
+
+/* Values for above Access Attributes */
+
+#define ACPI_IORT_RMR_ATTR_DEVICE_nGnRnE 0x00
+#define ACPI_IORT_RMR_ATTR_DEVICE_nGnRE 0x01
+#define ACPI_IORT_RMR_ATTR_DEVICE_nGRE 0x02
+#define ACPI_IORT_RMR_ATTR_DEVICE_GRE 0x03
+#define ACPI_IORT_RMR_ATTR_NORMAL_NC 0x04
+#define ACPI_IORT_RMR_ATTR_NORMAL 0x05
Just to minimise any possible ambiguity, could we call it:
⬇️ Suggested change
-#define ACPI_IORT_RMR_ATTR_NORMAL 0x05
+#define ACPI_IORT_RMR_ATTR_NORMAL_IWB_OWB 0x05
—
Reply to this email directly, view it on GitHub<
#752 (review)>,
or unsubscribe<
https://github.com/notifications/unsubscribe-auth/AAHL7LDTB4I75P6RFRIJ7QLVBINIBANCNFSM5O6QITZA>.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.******@***.***>>
—
Reply to this email directly, view it on GitHub
<#752 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABECZUQR3HBU56KSRX43FOLVBMSNNANCNFSM5O6QITZA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Are you saying that you want to keep the lower case parts of the change?
I would say that ACPICA capitalizes all #defines, and we would like to keep this convention.
Bob
From: shamiali2008 ***@***.***>
Sent: Thursday, March 24, 2022 2:31 AM
To: acpica/acpica ***@***.***>
Cc: Moore, Robert ***@***.***>; Comment ***@***.***>
Subject: Re: [acpica/acpica] IORT: Updates for revision E.d (PR #752)
This is the way normally device memory is represented in Arm specification
documents and kernel as well.
eg:
https://elixir.bootlin.com/linux/v4.5/source/arch/arm64/include/asm/pgtable.h#L72
So would like to keep that to avoid any confusion, if it is Ok.
Thanks,
Shameer
On Wed, Mar 23, 2022 at 2:23 PM Robert Moore ***@***.***<mailto:***@***.***>> wrote:
We would like these defines to be all upper case.
For example:
+#define ACPI_IORT_RMR_ATTR_DEVICE_nGnRnE 0x00
From: Robin Murphy ***@***.***<mailto:***@***.***>>
Sent: Tuesday, March 22, 2022 12:27 PM
To: acpica/acpica ***@***.***<mailto:***@***.***>>
Cc: Subscribed ***@***.***<mailto:***@***.***>>
Subject: Re: [acpica/acpica] IORT: Updates for revision E.d (PR #752)
@rmurphy-arm commented on this pull request.
________________________________
In source/include/actbl2.h<
#752 (comment)>:
> +#define ACPI_IORT_RMR_ACCESS_PRIVILEGE (1<<1)
+
+/*
+ * Macro to access the Access Attributes in flags field above:
+ * Access Attributes is encoded in bits 9:2
+ */
+#define ACPI_IORT_RMR_ACCESS_ATTRIBUTES(flags) (((flags) >> 2) & 0xFF)
+
+/* Values for above Access Attributes */
+
+#define ACPI_IORT_RMR_ATTR_DEVICE_nGnRnE 0x00
+#define ACPI_IORT_RMR_ATTR_DEVICE_nGnRE 0x01
+#define ACPI_IORT_RMR_ATTR_DEVICE_nGRE 0x02
+#define ACPI_IORT_RMR_ATTR_DEVICE_GRE 0x03
+#define ACPI_IORT_RMR_ATTR_NORMAL_NC 0x04
+#define ACPI_IORT_RMR_ATTR_NORMAL 0x05
Just to minimise any possible ambiguity, could we call it:
⬇️ Suggested change
-#define ACPI_IORT_RMR_ATTR_NORMAL 0x05
+#define ACPI_IORT_RMR_ATTR_NORMAL_IWB_OWB 0x05
—
Reply to this email directly, view it on GitHub<
#752 (review)>,
or unsubscribe<
https://github.com/notifications/unsubscribe-auth/AAHL7LDTB4I75P6RFRIJ7QLVBINIBANCNFSM5O6QITZA>.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.******@***.***<mailto:***@***.******@***.***>>>
—
Reply to this email directly, view it on GitHub
<#752 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABECZUQR3HBU56KSRX43FOLVBMSNNANCNFSM5O6QITZA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***<mailto:***@***.***>>
—
Reply to this email directly, view it on GitHub<#752 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAHL7LEGFAGEZ3U7KBOKO2TVBQY5PANCNFSM5O6QITZA>.
You are receiving this because you commented.Message ID: ***@***.******@***.***>>
|
|
Yes, Ideally would like to keep the lower case. @rmurphy-arm Any thoughts? Can we go with ACPICA convention here? Thanks. |
|
Sure, there's no need to break convention - ACPICA isn't bound by Arm's technical writing guidelines, after all. I'm just keen that whatever ACPICA's readable identifier for value 0x5 is, it explicitly captures that it's Write-Back Cacheable. We don't really expect to need yet more Normal memory types with further varieties of cacheablility, but we've intentionally left room to define additional values later if people do demand them. |
|
Ok. I will close this one and submit another one taking care of all the above comments. Thanks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
IORT revision is now updated to E.d (ARM DEN 0049E.d) and
contains a few additions like,
-Added descriptor in the root complex node for specifying
PASID width supported by the root complex.
-Updated RMR node Flags field.
-Introduced memory access attributes in the RMR node.
Please note that IORT Rev E.c is deprecated and not supported.
Signed-off-by: Shameer Kolothum shameerali.kolothum.thodi@huawei.com