On 09.07.2025 08:10, Chen, Jiqian wrote: > On 2025/7/8 23:13, Jan Beulich wrote: >> On 04.07.2025 09:08, Jiqian Chen wrote: >>> @@ -193,6 +234,42 @@ static void cf_check mask_write( >>> msi->mask = val; >>> } >>> >>> +static int cf_check cleanup_msi(const struct pci_dev *pdev) >>> +{ >>> + int rc; >>> + unsigned int end; >>> + struct vpci *vpci = pdev->vpci; >>> + const unsigned int msi_pos = pdev->msi_pos; >>> + const unsigned int ctrl = msi_control_reg(msi_pos); >>> + >>> + if ( !msi_pos || !vpci->msi ) >>> + return 0; >>> + >>> + if ( vpci->msi->masking ) >>> + end = msi_pending_bits_reg(msi_pos, vpci->msi->address64); >>> + else >>> + end = msi_mask_bits_reg(msi_pos, vpci->msi->address64) - 2; >> >> What's this "- 2" for? If there's no masking support, you want to cut off >> _at_ the mask register, not 2 bytes ahead of it? Just like you cut off at >> the pending bits register when there is masking support. > "-2" here is to cut the reserved 2 bytes of Message Data if there is no > masking support.
Hmm, init_msi() doesn't intercept Extended Message Data when present. I think that's wrong there, leading to the oddity here. (Imo such use of hard coded numbers would almost always want to be accompanied by a comment.) Jan