-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2024-06-06 Review Feedback #1
Conversation
Adjusted alignments and sizes accordingly. Added a Resource ID Type field to allow for different ways to represent a resource as needed. Signed Off by: Vasudevan Srinivasan (vasu@rivosinc.com)
|
||
- 0 - Cache ID | ||
- 1 - Proximity Domain (Memory) | ||
- 2 - ACPI Device |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I am not sure I understand from the spec. Could you elaborate what exactly is the ACPI device here? What type of device is it and is there an example _HID? And what about PCI devices or which have standard enumeration methods?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add a PCI device resource ID type and call out BDF as the identifier to use. Is that acceptable?
As for an ACPI device, if a resource that is managed by a controller is described as an ACPI device, then the Resource ID will capture the ACPI _HID and _UID of such a device.
One potential use case might be if say an ATC is described by an ACPI device (which we don't have any specification today), then we could describe the ATC resource using the ACPI _HID/_UID info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, what will be the Resource Type for IOATC? If it is not Cache, then shouldn't there be new Resource Type itself be defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will need to define a new Resource ID Type and not a new Resource Type.. Resource Type is still Cache for an IOATC, but, the representation of that may or may not go into a PPTT table. We don't know that yet as we haven't defined an IOATC representation yet. Having a Resource ID Type field allows us the flexibility to identify a Cache resource as either a PPTT Cache ID or any other way we may need to represent in future. Makes sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh OK. Thanks!
Signed Off by: Vasu Srinivasan <vasu@rivosinc.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from minor suggestion to change the resource id type to Memory only instead of Memory affinity structure, it looks good to me.
|
||
- 0 - Cache ID | ||
- 1 - Proximity Domain (Memory) | ||
- 2 - ACPI Device |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh OK. Thanks!
Updated Resource Structure with 2 byte length field.
Adjusted alignments and sizes accordingly.
Added a Resource ID Type field to allow for
different ways to represent a resource as needed.
Signed Off by: Vasudevan Srinivasan (vasu@rivosinc.com)