On Fri, Apr 07 2023 at 22:06, David Laight wrote:
> From: Thomas Gleixner
>> Sent: 07 April 2023 20:56
>> So the devices gets:
>>
>> 1) write to control register with MASKBIT set
>> 2) write to LOWER_ADDRESS
>> 3) write to UPPER_ADDRESS
>> 4) write to ENTRY_DATA
>> 5) write to control register with MASKBIT cleared
>>
>> #1 disables the vector and the device is not allowed to use the msg data
>> from the table entry until the mask bit is cleared again.
>>
>> If the device gets that wrong then that's a bug in the device and not a
>> kernel problem.
>
> Maybe, but the kernel isn't making it easy for a device
> state-engine that has to do four separate reads of an
> internal 32-bit memory area.
> Adding a short delay between #4 and #5 is likely to avoid
> some very hard to debug issues if the hardware reads the
> values 'mask last', if it reads them 'mask first' you need
> a short delay between #1 and #2.
Whatever order this reads does not matter. The point is:
"Mask Bit - When this bit is Set, the Function is prohibited from
sending a message using this MSI-X Table entry."
So you cannot make this read order dependent at all.
> Anything fpga based is likely to be using a 32bit memory
> block for the MSI-X data (possibly even 16bit).
It's trivial enough to latch the message on unmask into a shadow
register set and let the state engine work from there.
And no, we are not adding random delays to that code just because.
Thanks,
tglx
