Re: [PATCH v4] vpci/msix: handle accesses adjacent to the MSI-X table

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.03.2023 10:14, Roger Pau Monné wrote:
> On Wed, Mar 29, 2023 at 09:55:27AM +0200, Jan Beulich wrote:
>> On 24.03.2023 13:17, Roger Pau Monne wrote:
>>> The handling of the MSI-X table accesses by Xen requires that any
>>> pages part of the MSI-X related tables are not mapped into the domain
>>> physmap.  As a result, any device registers in the same pages as the
>>> start or the end of the MSIX or PBA tables is not currently
>>> accessible, as the accesses are just dropped.
>>>
>>> Note the spec forbids such placing of registers, as the MSIX and PBA
>>> tables must be 4K isolated from any other registers:
>>>
>>> "If a Base Address register that maps address space for the MSI-X
>>> Table or MSI-X PBA also maps other usable address space that is not
>>> associated with MSI-X structures, locations (e.g., for CSRs) used in
>>> the other address space must not share any naturally aligned 4-KB
>>> address range with one where either MSI-X structure resides."
>>>
>>> Yet the 'Intel Wi-Fi 6 AX201' device on one of my boxes has registers
>>> in the same page as the MSIX tables, and thus won't work on a PVH dom0
>>> without this fix.
>>>
>>> In order to cope with the behavior passthrough any accesses that fall
>>> on the same page as the MSIX tables (but don't fall in between) to the
>>> underlying hardware.  Such forwarding also takes care of the PBA
>>> accesses, so it allows to remove the code doing this handling in
>>> msix_{read,write}.  Note that as a result accesses to the PBA array
>>> are no longer limited to 4 and 8 byte sizes, there's no access size
>>> restriction for PBA accesses documented in the specification.
>>
>> I should have (re)checked the spec. There is a restriction, common to table
>> and PBA: "For all accesses to MSI-X Table and MSI-X PBA fields, software
>> must use aligned full DWORD or aligned full QWORD transactions; otherwise,
>> the result is undefined."
> 
> Thanks for noticing, I really wasn't able to spot it when I checked.
> I guess because of the intermixed "Implementation Note" sections in
> the spec.
> 
> Will amend and resend v5 then.

The patch was committed already, so it'll need to be a incremental new one
if we want to enforce to constraint again. (For just a description change
it's too late now.)

Jan