Re: [PATCH v3 2/6] xen: pci: introduce reference counting for pdev

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.04.2023 12:51, Roger Pau Monné wrote:
> On Mon, Apr 17, 2023 at 12:34:31PM +0200, Jan Beulich wrote:
>> On 17.04.2023 12:17, Roger Pau Monné wrote:
>>> On Fri, Apr 14, 2023 at 01:30:39AM +0000, Volodymyr Babchuk wrote:
>>>> Above I have proposed another view on this. I hope, it will work for
>>>> you. Just to reiterate, idea is to allow "harmless" refcounts to be left
>>>> after returning from pci_remove_device(). By "harmless" I mean that
>>>> owners of those refcounts will not try to access the physical PCI
>>>> device if pci_remove_device() is already finished.
>>>
>>> I'm not strictly a maintainer of this piece code, albeit I have an
>>> opinion.  I will like to also hear Jans opinion, since he is the
>>> maintainer.
>>
>> I'm afraid I can't really appreciate the term "harmless refcounts". Whoever
>> holds a ref is entitled to access the device. As stated before, I see only
>> two ways of getting things consistent: Either pci_remove_device() is
>> invoked upon dropping of the last ref,
> 
> With this approach, what would be the implementation of
> PHYSDEVOP_manage_pci_remove?  Would it just check whether the pdev
> exist and either return 0 or -EBUSY?

If the device doesn't (physically) exist, it would return e.g. -ENODEV.
If it still exists and the pdev also does, it would return e.g. -EBUSY,
yes.

Jan

>> or it checks that it is dropping the
>> last one. The former looks architecturally cleaner to me, but I can accept
>> that moving there might be more of a change, so wouldn't object to going
>> the latter route.
> 
> One of my concerns is what is expected of PHYSDEVOP_manage_pci_remove,
> I don't think it's expected for PHYSDEVOP_manage_pci_remove to return
> 0 while there are users inside the hypervisor still holding a
> reference to the pdev.
> 
> Thanks, Roger.