On 4.8.2025 9.37, Aneesh Kumar K.V wrote:
Arto Merilainen <amerilainen@xxxxxxxxxx> writes:
On 28.7.2025 16.52, Aneesh Kumar K.V (Arm) wrote:
+ for (int i = 0; i < interface_report->mmio_range_count; i++, mmio_range++) {
+
+ /*FIXME!! units in 4K size*/
+ range_id = FIELD_GET(TSM_INTF_REPORT_MMIO_RANGE_ID, mmio_range->range_attributes);
+
+ /* no secure interrupts */
+ if (msix_tbl_bar != -1 && range_id == msix_tbl_bar) {
+ pr_info("Skipping misx table\n");
+ continue;
+ }
+
+ if (msix_pba_bar != -1 && range_id == msix_pba_bar) {
+ pr_info("Skipping misx pba\n");
+ continue;
+ }
+
MSI-X and PBA can be placed to a BAR that has other registers as well.
While the PCIe specification recommends BAR-level isolation for MSI-X
structures, it is not mandated. It is enough to have sufficient
isolation within the BAR. Therefore, skipping the MSI-X and PBA BARs
altogether may leave registers unintentionally mapped via unprotected
IPA when they should have been mapped via protected IPA.
Instead of skipping the whole BAR, would it make sense to determine
where the MSI-X related regions reside, and skip validation only from
these regions?
Yes, that was added because at one point the FVP model was including the
MSI-X table and PBA regions in the interface report.
The issue I am raising is a different one. The MSI-X table and PBA may
reside in the middle of a BAR range, and the BAR range may contain
registers which should be accessible only via protected IPA. In this
case I would expect the pages before and after the MSI-X table (and PBA)
to be validated while the pages related to MSI-X structures would be
left unprotected.
Given that the MSI-X table or PBA regions shouldn't be present in the
interface report, the code needs to first find out where the MSI-X
structures reside, and use this information to determine which
"sub-ranges" should be skipped over during validation.
If I understand correctly, we shouldn't expect to see those regions in
the report unless secure interrupts are supported. The BAR-based
skipping was added as a workaround to handle the FVP issue.
Correct. Assuming that RMM doesn't lock the MSI-X table, I'd expect
these regions to be omitted in the interface report.
I believe we can drop that workaround now—if those regions are
incorrectly present, the below validation logic should catch and
reject them appropriately. Does that sound reasonable?
/* No secure interrupts, we should not find this set, ignore for now. */
if (FIELD_GET(TSM_INTF_REPORT_MMIO_MSIX_TABLE, mmio_range->range_attributes) ||
FIELD_GET(TSM_INTF_REPORT_MMIO_PBA, mmio_range->range_attributes)) {
pci_info(pdev, "Skipping MSIX (%ld/%ld) range [%d] #%d %d pages, %llx..%llx\n",
FIELD_GET(TSM_INTF_REPORT_MMIO_MSIX_TABLE, mmio_range->range_attributes),
FIELD_GET(TSM_INTF_REPORT_MMIO_PBA, mmio_range->range_attributes),
i, range_id, mmio_range->num_pages, r->start, r->end);
continue;
}
First, I think this is skipping over the whole range => if there are
registers outside of the MSI-X structures (but within the same BAR),
they won't be validated.
Second, if the MSI-X regions are present in the interface report,
wouldn't it - in the common case - mean that the device expects the
ranges to be accessed with T=1? If this happens unexpectedly, it sounds
that MSI-X wouldn't be usable. I wonder if the code should simply return
an error instead if informing the user via pci_info()...
- R2
