Re: [PATCH for-4.21 v2] vpci/msix: improve handling of bogus MSI-X capabilities

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

On 10/8/25 10:59, Roger Pau Monné wrote:
> On Wed, Oct 08, 2025 at 04:39:23PM +0200, Roger Pau Monne wrote:
>> I've had the luck to come across a PCI card that exposes a MSI-X capability
>> where the BIR of the vector and PBA tables points at a BAR that has 0 size.
>>
>> This doesn't play nice with the code in vpci_make_msix_hole(), as it would
>> still use the address of such empty BAR (0) and attempt to carve a hole in
>> the p2m.  This leads to errors like the one below being reported by Xen:
>>
>> d0v0 0000:22:00.0: existing mapping (mfn: 181c4300 type: 0) at 0 clobbers 
>> MSIX MMIO area
>>
>> And the device left unable to enable memory decoding due to the failure
>> reported by vpci_make_msix_hole().
>>
>> Introduce checking in init_msix() to ensure the BARs containing the MSI-X
>> tables are usable.  This requires checking that the BIR points to a
>> non-empty BAR, and the offset and size of the MSI-X tables can fit in the
>> target BAR.
>>
>> This fixes booting PVH dom0 on Supermicro AS -2126HS-TN severs with AMD
>> EPYC 9965 processors.  The broken device is:
>>
>> 22:00.0 SATA controller: Advanced Micro Devices, Inc. [AMD] FCH SATA 
>> Controller [AHCI mode] (rev 93)
>>
>> There are multiple of those integrated controllers in the system, all
>> broken in the same way.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> Released-Acked-By: Oleksii Kurochko<oleksii.kurochko@xxxxxxxxx>
>> ---
>> Cc: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
>> Cc: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
>> ---
>> Changes since v1:
>>  - Introduce a DEVICE BUG prefix.
>>  - Remove extra newline.
>>  - Fix typo in commit message.
>> ---
>>  xen/drivers/vpci/msix.c | 50 ++++++++++++++++++++++++++++++++++++-----
>>  xen/include/xen/lib.h   |  3 +++
>>  2 files changed, 48 insertions(+), 5 deletions(-)
>>
>> diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
>> index 54a5070733aa..4ddcefbcb274 100644
>> --- a/xen/drivers/vpci/msix.c
>> +++ b/xen/drivers/vpci/msix.c
>> @@ -675,6 +675,51 @@ static int cf_check init_msix(struct pci_dev *pdev)
>>      if ( !msix )
>>          return -ENOMEM;
>>  
>> +    msix->tables[VPCI_MSIX_TABLE] =
>> +        pci_conf_read32(pdev->sbdf, msix_table_offset_reg(msix_offset));
>> +    msix->tables[VPCI_MSIX_PBA] =
>> +        pci_conf_read32(pdev->sbdf, msix_pba_offset_reg(msix_offset));
>> +
>> +    /* Check that the provided BAR is valid. */
> 
> I had the following local change that I forgot to update the patch
> with before sending:
> 
> /* Check that the referenced BAR(s) regions are valid. */
> 
> I think this is a better wording.

+1