On 18.11.2022 03:35, Marek Marczykowski-Górecki wrote: > Linux enables MSI-X before disabling INTx, but keeps MSI-X masked until > the table is filled. Then it disables INTx just before clearing MASKALL > bit. Currently this approach is rejected by xen-pciback. > According to the PCIe spec, device cannot use INTx when MSI/MSI-X is > enabled.
Similarly the spec doesn't allow using MSI and MSI-X at the same time. Before your change xen_pcibk_get_interrupt_type() is consistent for all three forms of interrupt delivery; imo it also wants to be consistent after your change. This effectively would mean setting only one bit at a time (or using an enum right away), but then the question is what order you do the checks in. IOW I think the change to the function is wrong. Furthermore it looks to me as if you're making msi_msix_flags_write() inconsistent with command_write() - you'd now want to also permit clearing "INTx disable" when MSI or MSI-X are enabled. Which, I think, would simply mean allowing the domain unconditional control of the bit (as long as allow_interrupt_control is set of course). Especially with these further changes I'm afraid at least for now I view this as moving in the wrong direction. My view might change in particular if the description made more clear what was wrong with the original change (476878e4b2be ["xen-pciback: optionally allow interrupt enable flag writes"]), or perhaps the discussion having led to the form which was committed in the end. Jan
