[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Tue, 14 Sep 2021 16:22:07 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=XicimkWAG4CtDkeE7vKpUgbHBafeFsFVkiFg74ln4wU=; b=R6BeZxWKraIWYi1AU4xHP3VdT2wNGJjEdgRg1ChQiMc1YC679FCedAcGY1EXW6LhUEOeVNqhNmhexnboUhTTy5ZXrN9dXvx/iRcAoTOIDHIrW1z8Zt3W4fCGWSaD7ks0MC/Y3gL7MsWljbnAhfYIoVd3UJXVXM37mgVpzHPkcpiIkNREI0NgfsFxXo77NQ1lQuee+S+JxvdkZI+iLpmd3fCXH/SfRgl4cVxQYaiAa9hE1JM4IEObol8ISfGTmwMkf37TO0d2czPPJBmNiGSYe1XXcu1HOzJUOKdZamCbEwgPaifAbuHB7C1dGQxYE4Un0489fTKd23yxz3SNh00hzw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=aN9SdOWQBI7himeFWWuql3zozSG5OlxnxX3b1CzAhM6cZqKIxGEfw4JFAhAU8im8GPEnT54VpX0VnOe8An8LCwOiJpvm8ZggVp/Lnw5eOwUW2Fy5f+k9PkqESPSARP2R9Py3JWue60uj5puNfhRyk5zBKMFq1jQ2mTwiw17N1rTBKpsIYDGlOhl5PVwiQiGEKkf37JuOj20YmDzsG53fUZ/hkRDCNG1xXhLBMRAv0PiW0AwXANY2elQ+4BKOQ+P0fYXtUYZLrrmpxowZtdrtzbvsiIagViNWNqcE3oRf/oS1xjQTNxCL5DNR1bGxMZtW9GY8ngVx+RU68aoeaKEQVg==
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 14 Sep 2021 14:22:25 +0000
  • Ironport-data: A9a23:QfTsGKLznZ0uVsKpFE+R+5IlxSXFcZb7ZxGr2PjKsXjdYENSg2EFn zEeWWqPP//eYWf1ctAjaYvn9RkC78PSzt42TlFlqX01Q3x08seUXt7xwmUcns+xwm8vaGo9s q3yv/GZdJhcokcxIn5BC5C5xZVG/fjgqoHUVaiUZ0ideSc+EH140UM6x7Zj6mJVqYPR7z2l6 IuaT/L3YDdJ6xYsWo7Dw/vewP/HlK2aVAIw5jTSV9gS1LPtvyB94KYkDbOwNxPFrrx8RYZWc QphIIaRpQs19z91Yj+sfy2SnkciGtY+NiDW4pZatjTLbrGvaUXe345iXMfwZ3u7hB20vO507 Il0uaaxEw0HAbaQwu0QFAlhRnQW0a1uoNcrIFC6uM2XiUbHb2Ht07NlC0Re0Y8wo7gtRzsUr LpBdW5LPkvra+GemdpXTsF2gcsuNo/zNZ43sXB81zDJS/0hRPgvRo2Xv4AAhmlh3aiiG977V ok6OSgycS6QSDpEAUowN40Rhcen0yyXnzpw9wvO+PtfD3Lo5Bx81v3hPcTYfvSORN5JhQCIq 2Te5WP7DxoGctuFxlKt8Hihm+vOliPTQ58JGfuz8fsCvbGI7jVNUltMDwL9+KTnzB7lMz5CF 6AK0hcNk60WqHWndNinVTOHuyLetxNbS+MFRoXW9zqxJrroDxexXzZfF2MQN4N47afaVhRxi QTYxIqB6ShH9eTPECPDrO/8QSaaZHBNRVLucxPoWufsDzPLm4g1khuHZdJqCqfdYjbdSGyon mziQMTTgdwuYS83O0eTpguvb9GE/MGhousJCuP/BDzNAuRRPtLNWmBQwQKHhcus1a7AJrV7g JThpyR4xLpfZX1qvHfWKNjh4Znzv6rVWNEiqQc3QvHNCAhBC1b8JNsNsVmS1W9CM9oeeC+BX aMgkVoKv/du0I+RRfYvOeqZUp1ypYC5TIiNfq2EP7JmP8kqHCfarX4GWKJl9z20+KTaufpkY snznAfFJStyNJmLOxLtG75GgON0n3tnrY4RLLiipymaPXOlTCf9YZ8OMUeUb/B/66WBoQ7P9 M1YOdfMwBJaONASqAGOmWLKBVxVf3U9G77srMlbKryKLgZ8QTlzAP7N27IxPYdimv0NxOvP+ 3i8XG5eyUb+2iKbeVnbNCg7ZeO9R4t7oFI6ITcoYQSi1U88bNv996wYbZY2I+UqrbQx0f5uQ vAZUMycGfATGC/f8jEQYMCl/oxvfRimnyyUOC+hbGRtdpJsXVWRqNTlYhHu5G8FCS/u7Zkyp Lip1wX6R5sfRls9UJaKOaz3l17o5CoTguN/WUfMM+J/QkS0/dg4MTH1g982P9oIdUfJyAyF2 lvEGhwfv+TM/dM4qYGbmaCeoo61OOJiBU4GTXLD5LO7OCSGrGquxYhMDLSBcTzHDT6m/ayjY aNezu3mMe1Bl1FP6tIuH7FuxKM4xt3uu74FkVg0QCSVNwymWuF6P32L/chTrakclLZWtDy/V l+L5tQHa66CP9noEQJJKQcoBghZOSr4RtUGASwJHXjH
  • Ironport-hdrordr: A9a23:d64gLak9pdlzOlSGaDxaC4J/H/TpDfO2imdD5ihNYBxZY6Wkfp +V88jzhCWZtN9OYhwdcLC7WZVpQRvnhPlICK0qTM2ftW7dyRaVxeBZnPDfKljbdREWmdQtt5 uIH5IObeEYSGIK8foSgzPIYurIouP3iZxA7N22pxwGLXAIGtFdBkVCe36m+yVNNXd77PECZf yhD6R81l6dkSN9VLXFOpBJZZmIm/T70LbdJTIWDR8u7weDyRuu9b7BChCdmjMTSSlGz7sO+X XM11WR3NTuj9iLjjvnk0PD5ZVfn9XsjvNFGcy3k8AQbhHhkByhaohNU6CL+Bo1vOaswlA3l8 SkmWZsA+1Dr1fqOk2lqxrk3AftlB4o9n/Z0FedxUDupMToLQhKQvZptMZ8SF/0+kAgtNZz3O ZgxGSCradaChvGgWDU+8XIfwsCrDv0nVMS1cooy1BPW4oXb7Fc6aYF+llOLZsGFCXmrKg6De hVCt3G7vo+SyLVU5nghBgt/DWQZAVwIv/fKXJy//B9kgIm00yR9nFohPD2xRw7hdYAo5ot3Z WzDk0nrsAIciYsV9MOOA42e7rBNoX8e2O+DIusGyWTKEgmAQOEl3el2sR/2AmVEKZ4uKfa3q 6xFm9liQ==
  • Ironport-sdr: rdj6xcF5TXdN6VUWrTJUw5+NrOS/HQVoAywAn0CynvTvLVRH3RAHeijlfGvVCaWYANbntBMpib b9oTXHu5kDjyhs5VCNu9RyI1kckZElh8eP2hlDM4NZRBG4sxIy5UB0PZVIEcEn3qDUY5CyYdeK Y8l9cBkQkdlX3FNxTbcs0eOzan8eExRLeeByU5AyDAMxwzwolYpJJM8pfdduIah/O3EN5gDvIG 6VfbVLo/UxJUpQty3VzOmNrV1HLnJv+xMjm+4LFpQ5fJlZBTv4jmcUXaUD8RsHLbkxxEhisczP UAA+AuMVSBU4hCxHnlj0APTI
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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

> > Or could the BARs potentially be shared between multiple
> > functions on the same device?
> I don't think that's allowed (or would even work) - if anything
> (like in general) small BARs may share a page, but without overlaps.

BARs sharing a page should already be handled correctly by vPCI.

> > This also makes me wonder why they have to be added to the IOMMU, is
> > that related to device groups and the fact that Xen is not clever
> > enough to figure whether devices belong to the same IOMMU group and
> > thus must be assigned together?
> I'm actually not sure whether they need to be added to the IOMMU.
> With multi-function devices in mind, back at the time I though it
> would make more sense that way. But I may have been wrong, and
> hence we may want to revisit this. Problem with such changes is
> that they may cause rare issues on very specific systems, which we
> may have no way noticing even during a full release cycle.

I guess it depends on whether the device is to be used by dom0, see
my question above about ehci-dbgp for example.

Thanks, Roger.



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.