[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <YV036H5iGd7FKg+E@kroah.com>
Date: Wed, 6 Oct 2021 07:45:12 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Dan Williams <dan.j.williams@...el.com>
Cc: Mika Westerberg <mika.westerberg@...ux.intel.com>,
Alan Stern <stern@...land.harvard.edu>,
"Kuppuswamy, Sathyanarayanan"
<sathyanarayanan.kuppuswamy@...ux.intel.com>,
"Michael S. Tsirkin" <mst@...hat.com>,
Borislav Petkov <bp@...en8.de>, X86 ML <x86@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
Andreas Noever <andreas.noever@...il.com>,
Michael Jamet <michael.jamet@...el.com>,
Yehezkel Bernat <YehezkelShB@...il.com>,
"Rafael J . Wysocki" <rafael@...nel.org>,
Jonathan Corbet <corbet@....net>,
Jason Wang <jasowang@...hat.com>,
Andi Kleen <ak@...ux.intel.com>,
Kuppuswamy Sathyanarayanan <knsathya@...nel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux PCI <linux-pci@...r.kernel.org>,
USB list <linux-usb@...r.kernel.org>,
virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH v2 4/6] virtio: Initialize authorized attribute for
confidential guest
On Tue, Oct 05, 2021 at 03:33:29PM -0700, Dan Williams wrote:
> On Sun, Oct 3, 2021 at 10:16 PM Mika Westerberg
> <mika.westerberg@...ux.intel.com> wrote:
> >
> > Hi,
> >
> > On Fri, Oct 01, 2021 at 12:57:18PM -0700, Dan Williams wrote:
> > > > > Ah, so are you saying that it would be sufficient for USB if the
> > > > > generic authorized implementation did something like:
> > > > >
> > > > > dev->authorized = 1;
> > > > > device_attach(dev);
> > > > >
> > > > > ...for the authorize case, and:
> > > > >
> > > > > dev->authorize = 0;
> > > > > device_release_driver(dev);
> > > > >
> > > > > ...for the deauthorize case?
> > > >
> > > > Yes, I think so. But I haven't tried making this change to test and
> > > > see what really happens.
> > >
> > > Sounds like a useful path for this effort to explore. Especially as
> > > Greg seems to want the proposed "has_probe_authorization" flag in the
> > > bus_type to disappear and make this all generic. It just seems that
> > > Thunderbolt would need deeper surgery to move what it does in the
> > > authorization toggle path into the probe and remove paths.
> > >
> > > Mika, do you see a path for Thunderbolt to align its authorization
> > > paths behind bus ->probe() ->remove() events similar to what USB might
> > > be able to support for a generic authorization path?
> >
> > In Thunderbolt "authorization" actually means whether there is a PCIe
> > tunnel to the device or not. There is no driver bind/unbind happening
> > when authorization toggles (well on Thunderbolt bus, there can be on PCI
> > bus after the tunnel is established) so I'm not entirely sure how we
> > could use the bus ->probe() or ->remove for that to be honest.
>
> Greg, per your comment:
>
> "... which was to move the way that busses are allowed to authorize
> the devices they wish to control into a generic way instead of being
> bus-specific logic."
>
> We have USB and TB that have already diverged on the ABI here. The USB
> behavior is more in line with the "probe authorization" concept, while
> TB is about tunnel establishment and not cleanly tied to probe
> authorization. So while I see a path to a common authorization
> implementation for USB and other buses (per the insight from Alan), TB
> needs to retain the ability to record the authorization state as an
> enum rather than a bool, and emit a uevent on authorization status
> change.
>
> So how about something like the following that moves the attribute
> into the core, but still calls back to TB and USB to perform their
> legacy authorization work. This new authorized attribute only shows up
> when devices default to not authorized, i.e. when userspace owns the
> allow list past critical-boot built-in drivers, or if the bus (USB /
> TB) implements ->authorize().
At quick glance, this looks better, but it would be good to see someone
test it :)
thanks,
greg k-h
Powered by blists - more mailing lists