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

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hello Roger,

Roger Pau Monné <roger.pau@xxxxxxxxxx> writes:

> 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?
>

Okay, I am preparing patches with the behavior you proposed. To test it,
I artificially set refcount to 2 and indeed PHYSDEVOP_manage_pci_remove
returned -EBUSY, which propagated to the linux driver. Problem is that
Linux driver can't do anything with this. It just displayed an error
message and removed device anyways. This is because Linux sends
PHYSDEVOP_manage_pci_remove in device_remove() call path and there is no
way to prevent the device removal. So, admin is not capable to try this
again.

As I workaround, I can create hypercall continuation in case if
pci_remove_device() returns -EBUSY. What is your opinion?


-- 
WBR, Volodymyr