Hi Alex,

On 23/06/2026 00:13, Alex Williamson wrote:
> On Fri, 19 Jun 2026 16:13:17 +0100
> Matt Evans <[email protected]> wrote:
> 
>> Hi Jason,
>>
>> On 19/06/2026 14:31, Jason Gunthorpe wrote:
>>> On Thu, Jun 18, 2026 at 05:02:58PM +0100, Matt Evans wrote:
>>>   
>>>> My understanding is that the sequences above wake a device that happens
>>>> to have previously been put into D3, and AFAICT it could only have got
>>>> there because of a previous vfio_pci_set_power_state().  Seems its only
>>>> caller is from the emulation of PCI_PM_CTRL using
>>>> vfio_lock_and_set_power_state(), and this zaps/revokes BAR access before
>>>> a transition to D3.  Similarly, an attempt to access a BAR via an
>>>> ioctl/through vfio_pci_core_do_io_rw() fails the D3 check in
>>>> __vfio_pci_memory_enabled(), and besides will try to take the memory_lock. 
>>>>  
>>>
>>> I thought the general design was the bars were made inaccessible
>>> before going to a low power state, and remain inaccessible while it is
>>> in low power?
>>>
>>> So the order of D0 doesn't matter. If it is not in D0 then there is no
>>> mappings and zap/revoke is a NOP.
>>>
>>> If is it in D0 then it doesn't matter because D0 is a nop.  
>> Yes, that's what I'm getting at. :)  If it's in D3 then BARs are
>> inaccessible, so as long as we go into D0 before the DMABUF move, the
>> order of the zap relative to the "go to D0" doesn't matter.
> 
> I believe this is correct as well, but importantly we cannot assume
> that a stray read or write just returns -1 or gets dropped.  This is
> exactly why we have such hard protections against the user accessing
> the device while it's disabled.  Not all platforms, even within
> architectures that might otherwise be considered lenient of such
> accesses, consider this benign and might escalate to system level
> faults.

We are in enthusiastic agreement here.

> Let's be careful not to frame this as "the access doesn't matter
> anyway", the answer is instead that non-D0 devices already lack any
> mappings to access the device.  Thanks,

I agree that is not the right thing to say, for exactly that reason.
(For avoidance of any doubt, I didn't say that :) )

Thanks for confirming the behaviour.  I hope Praan and Kevin are
satisfied that this patch doesn't cause the issues they first worried
about (the changed order of the zap relative to the D0 transition
doesn't have a detrimental effect because of the existing inaccessibility).

Alex, I'll post v4 soon, but if you have any comments in the pipeline
please shout and I'll hold off awhile.


Thanks,


Matt

Reply via email to