acpi_tables: Fix VIOT endpoint_id remapping #34
Conversation
tjeznach
requested review from
jiangliu,
liuw,
rbradford and
sboeuf
as code owners
| @@ -188,7 +188,7 @@ impl Aml for PciRange { | |||
| sink.byte(ViotEntryType::PciRange as u8); | |||
| sink.byte(0); // reserved | |||
| sink.word(Self::len() as u16); | |||
| sink.dword(self.first.as_endpoint()); | |||
| sink.dword(self.first.as_bdf() as u32); | |||
There was a problem hiding this comment.
Based on the ACPI spec, and particularly the section about the PCI range node structure here, the Endpoint Start is the first Endpoint ID, and an Endpoint ID is described as follows:
The corresponding endpoint ID is obtained by combining segment and BDF:
Endpoint ID = ((segment - Segment Start) << 16) + BDF - BDF Start + Endpoint Start
I'm a bit confused because what does Endpoint Start stand for in this formula? And if we substract both the Segment Start and BDF Start, does that mean we end up with the endpoint ID being a simple index corresponding to Endpoint Start?
My point is, I think you're patching this in the right direction but we're still missing the BDF substraction. Should we simply always hardcode the Endpoint Start as 0?
There was a problem hiding this comment.
I think 99% of the time Endpoint Start will likely be 0, but the "correct" thing to do is to probably use the corresponding formula from the spec
There was a problem hiding this comment.
For most simple mappings Endpoint Start this will be 0, however for more complex mappings in the IOMMU range assuming the most common case (unity mapping in the segment) Endpoint Start should represent First BDF in the range. This change will provide simple unity mapping for non-contiguous PCI ranges, without adding segment number.
Eg. Mapping example for single IOMMU handle:
0001:00:01.0 - 0000:01.7 --> 0x008
0001:00:06.0 - 0000:06.7 --> 0x030
There was a problem hiding this comment.
Thanks for clarifying @tjeznach
It's a bit of a shame the ACPI spec isn't clear on that aspect, but I believe you've looked into the Linux implementation of the VIOT table, which is the "actual" spec :)
|
@rbradford CI is failing on clippy, is that something you want to fix or should we merge anyway? |
For non-zero segment remappings it's very likely that target endpoint identifiers namespace will start at 0, without requester segment number encoded in the translation request. Signed-off-by: Tomasz Jeznach <tjeznach@rivosinc.com>
tjeznach
force-pushed
the
tjeznach/viot-endpoint
branch
from
42e9318 to
4e45ee4
Compare
|
It's only failing on beta - we can fix this in a separate PR. |
Summary of the PR
For non-zero segment remappings defined in ACPI VIOT data structures it's very likely that target endpoint namespace will start at 0, without requester segment number encoded in the translation request.
This change updates default VIOT endpoint_id generation - starting point of the translated identifiers, to ignore requester segment number.