Re: [PATCH v2 8/9] xue: mark DMA buffers as reserved for the device

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Thu, Jul 14, 2022 at 01:51:06PM +0200, Jan Beulich wrote:
> On 06.07.2022 17:32, Marek Marczykowski-Górecki wrote:
> > The important part is to include those buffers in IOMMU page table
> > relevant for the USB controller. Otherwise, DbC will stop working as
> > soon as IOMMU is enabled, regardless of to which domain device assigned
> > (be it xen or dom0).
> > If the device is passed through to dom0 or other domain (see later
> > patches), that domain will effectively have access to those buffers too.
> > It does give such domain yet another way to DoS the system (as is the
> > case when having PCI device assigned already), but also possibly steal
> > the console ring content. Thus, such domain should be a trusted one.
> > In any case, prevent anything else being placed on those pages by adding
> > artificial padding.
> 
> I don't think this device should be allowed to be assigned to any
> DomU. Just like the EHCI driver, at some point in the series you
> will want to call pci_hide_device() (you may already do, and I may
> merely have missed it).

There is the major difference compared to the EHCI driver: the XHCI
hardware interface was designed for debug capability to be driven by
separate driver (see description of patch 9). The device reset is the
only action that needs some coordination and this patch series follows
what Linux does (re-enable DbC part, when it detects the XHCI reset).
Having this capability is especially important when one has only a
single USB controller (many, if not most recent Intel platforms) and has
some important devices on USB (for example system disk, or the only ethernet
controller - I have both of those cases in my test lab...).

Anyway, this patch is necessary even if no domain would use the device.
As Andrew explained in response to the cover letter of the RFC series,
Xen doesn't have IOMMU context for devices used by Xen exclusively. So,
with the current model, it would be pci_ro_device() + dom0's IOMMU
context.

> > Using this API for DbC pages requires raising MAX_USER_RMRR_PAGES.
> 
> I disagree here: This is merely an effect of you re-using the pre-
> existing functionality there with too little customization. I think
> the respective check shouldn't be applied for internal uses.

Ok, I'll move the check.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature