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.
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.
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;
}
-aneesh
