Re: [PATCH RFC] vPCI: account for hidden devices in modify_bars()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.09.2021 16:22, Roger Pau Monné wrote:
> On Tue, Sep 14, 2021 at 02:05:01PM +0200, Jan Beulich wrote:
>> On 14.09.2021 13:21, Roger Pau Monné wrote:
>>> On Tue, Sep 14, 2021 at 12:12:12PM +0200, Jan Beulich wrote:
>>>> On 14.09.2021 12:00, Roger Pau Monné wrote:
>>>>> On Mon, Aug 30, 2021 at 03:04:55PM +0200, Jan Beulich wrote:
>>>>>> Hidden devices (e.g. an add-in PCI serial card used for Xen's serial
>>>>>> console) are associated with DomXEN, not Dom0. This means that while
>>>>>> looking for overlapping BARs such devices cannot be found on Dom0's
>>>>>> list of devices; DomXEN's list also needs to be scanned.
>>>>>
>>>>> Thanks for looking into this, I certainly didn't take hidden devices
>>>>> into account for vPCI dom0.
>>>>>
>>>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>>>> ---
>>>>>> RFC: Patch intentionally mis-formatted, as the necessary re-indentation
>>>>>>      would make the diff difficult to read. At this point I'd merely
>>>>>>      like to gather input towards possible better approaches to solve
>>>>>>      the issue (not the least because quite possibly there are further
>>>>>>      places needing changing).
>>>>>
>>>>> I have a couple of questions, AFAICT we currently hide the serial
>>>>> console and/or the VGA adapter if it's in use by Xen.
>>>>>
>>>>> I wonder whether we need to add vPCI handlers for those:
>>>>> setup_one_hwdom_device will attempt to add vPCI handlers to the hidden
>>>>> device because of the temporary override of pdev->domain done in
>>>>> _setup_hwdom_pci_devices, but I think that for hidden devices we
>>>>> shouldn't add vPCI handlers. We should instead block PCI config space
>>>>> access from dom0 to the device so that it doesn't mess with Xen usage.
>>>>
>>>> The answer to this follows (I think) from the one below.
>>>>
>>>>> It's also not clear why does Xen want to have those hidden devices
>>>>> partly controlled by dom0, as it would seem to me that dom0 interfering
>>>>> with hidden devices in use by Xen can only lead to trouble.
>>>>
>>>> Dom0 can't interfere as long as it can only read from the device.
>>>> Restricting accesses to reads is one of the purposes of "hiding"
>>>> (the other is to make it impossible to assign these to a DomU). Not
>>>> allowing Dom0 to read from such devices would lead to wrong PCI
>>>> device discovery - some devices would be missing (which may or may
>>>> not be merely a cosmetic issue). If the missing device is a multi-
>>>> function one at function 0, other devices in the same slot may also
>>>> not be found by Dom0 (depending on whether it looks at non-zero
>>>> function numbers in this case).
>>>
>>> Hm, indeed seems possible that missing function 0 the whole device is
>>> skipped.
>>>
>>> Maybe we need a special vPCI handling for those devices that just
>>> allows reads but not writes, and that doesn't maps the BARs into dom0
>>> p2m?
>>
>> Not sure about mapping. They could be mapped r/o. And they may
>> actually need mapping for multi-function devices, but I guess for
>> such devices to actually function properly then there would be
>> more work required.
> 
> I'm also slightly puzzled as to why ehci-dbgp uses pci_hide_device
> while ns16550 uses pci_ro_device instead.
> 
> Is this because the PCI device used by ehci-dbgp must be shared with
> dom0 for other USB ports to be usable, and hence dom0 will need read
> and write access to the device PCI config space and BARs?
> 
> Note that pci_hide_device is less restrictive than pci_ro_device, as
> it doesn't mark the device as RO.
> 
> That would seem quite risky, as it's likely that dom0 will perform
> some kind of reset of the USB device and thus the console will be
> lost.

There's actually a protocol to prevent exactly that: See PHYSDEVOP_dbgp_op.

Jan