[RFC PATCH v5 0/3] vfio: platform: return device properties for a platform device

[RFC PATCH v5 0/3] vfio: platform: return device properties for a platform device

[Baptiste Reynal]

This RFC's intention is to show what an interface to access device
properties for the VFIO platform driver can look like. These properties
can be retrieved from the device tree node describing the device, or ACPI
properties.

If a device property corresponding to a platform device bound to VFIO PLATFORM
or VFIO AMBA is available, this patch series will allow the user to query
for this information. This can be useful for userspace drivers to automatically
query parameters related to the device.

Specifically for QEMU, reading the "compatible" property of the device tree
node could be of use to find out what device is being assigned to the guest and
handle appropriately a wider range of devices in the future, and to generate an
appropriate device tree for the guest.

Older versions of this series were specifically targeted at device tree
properties. The v3 has been reworked on top of Rafael J. Wysocki's
uniform device properties API for device tree and ACPI devices. This will allow
us to use the API in the future with devices described via ACPI.

The API to get the list of available properties and the device tree full_name
have been removed. These probably don't serve an useful purpose, as the user
of this API need to know anyway what properties are specific to the device he
wants to access with VFIO. If we decide to reintroduce the list of properties
in the future, the generic device properties API in the kernel will have to
be extended accordingly.

A kernel with this series and all the dependencies applied can be pulled from
branch vfio-device-properties-v5 from the repository:
https://git.virtualopensystems.com/dev/linux.git

Changes since v4:
 - Rework code according to Alex Williamson's comments
Changes since v3:
 - Rebased on top on v4.2
 - Rework VFIO_DEVICE_GET_DEV_PROPERTY ioctl
 - Rework code according to Eric Auger's comments
Changes since v2:
 - Reworked on top of Rafael J. Wysocki's uniform device properties API for
   device tree and ACPI
 - Support for u64 array properties
 - Removed API to get list of available properties and device tree full_name
Changes since v1:
 - Updated for VFIO platform patch series v8:
   VFIO AMBA devices now supported in addition to VFIO PLATFORM devices
 - Refactored and cleaned up the code

Antonios Motakis (3):
  vfio: platform: add device properties skeleton and user API
  vfio: platform: access device property as a list of strings
  vfio: platform: return device properties as arrays of unsigned
    integers

 drivers/vfio/platform/Makefile                |   3 +-
 drivers/vfio/platform/properties.c            | 178 ++++++++++++++++++++++++++
 drivers/vfio/platform/vfio_platform_common.c  |  34 +++++
 drivers/vfio/platform/vfio_platform_private.h |   9 ++
 include/uapi/linux/vfio.h                     |  31 +++++
 5 files changed, 254 insertions(+), 1 deletion(-)
 create mode 100644 drivers/vfio/platform/properties.c

-- 
2.6.0

Re: [RFC PATCH v5 0/3] vfio: platform: return device properties for a platform device

[Alex Williamson]

On Wed, 2015-09-30 at 11:07 +0200, Baptiste Reynal wrote:
> This RFC's intention is to show what an interface to access device
> properties for the VFIO platform driver can look like. These properties
> can be retrieved from the device tree node describing the device, or ACPI
> properties.
> 
> If a device property corresponding to a platform device bound to VFIO PLATFORM
> or VFIO AMBA is available, this patch series will allow the user to query
> for this information. This can be useful for userspace drivers to automatically
> query parameters related to the device.
> 
> Specifically for QEMU, reading the "compatible" property of the device tree
> node could be of use to find out what device is being assigned to the guest and
> handle appropriately a wider range of devices in the future, and to generate an
> appropriate device tree for the guest.
> 
> Older versions of this series were specifically targeted at device tree
> properties. The v3 has been reworked on top of Rafael J. Wysocki's
> uniform device properties API for device tree and ACPI devices. This will allow
> us to use the API in the future with devices described via ACPI.
> 
> The API to get the list of available properties and the device tree full_name
> have been removed. These probably don't serve an useful purpose, as the user
> of this API need to know anyway what properties are specific to the device he
> wants to access with VFIO. If we decide to reintroduce the list of properties
> in the future, the generic device properties API in the kernel will have to
> be extended accordingly.

If we wanted to describe device properties via vfio, perhaps this is a
reasonable approach.  However, why would we want to do that?  How are
these device properties unique to vfio?  Are there other users that
might want to learn these properties?  How will they get them?  Perhaps
they would decide to expose them via sysfs for everyone, making this
interface redundant.  Why not start with exposing device properties to
userspace in a way that's not specific to vfio?  Thanks,

Alex

Re: [RFC PATCH v5 0/3] vfio: platform: return device properties for a platform device

[Christoffer Dall]

On Fri, Oct 02, 2015 at 01:28:19PM -0600, Alex Williamson wrote:
> On Wed, 2015-09-30 at 11:07 +0200, Baptiste Reynal wrote:
> > This RFC's intention is to show what an interface to access device
> > properties for the VFIO platform driver can look like. These properties
> > can be retrieved from the device tree node describing the device, or ACPI
> > properties.
> > 
> > If a device property corresponding to a platform device bound to VFIO PLATFORM
> > or VFIO AMBA is available, this patch series will allow the user to query
> > for this information. This can be useful for userspace drivers to automatically
> > query parameters related to the device.
> > 
> > Specifically for QEMU, reading the "compatible" property of the device tree
> > node could be of use to find out what device is being assigned to the guest and
> > handle appropriately a wider range of devices in the future, and to generate an
> > appropriate device tree for the guest.
> > 
> > Older versions of this series were specifically targeted at device tree
> > properties. The v3 has been reworked on top of Rafael J. Wysocki's
> > uniform device properties API for device tree and ACPI devices. This will allow
> > us to use the API in the future with devices described via ACPI.
> > 
> > The API to get the list of available properties and the device tree full_name
> > have been removed. These probably don't serve an useful purpose, as the user
> > of this API need to know anyway what properties are specific to the device he
> > wants to access with VFIO. If we decide to reintroduce the list of properties
> > in the future, the generic device properties API in the kernel will have to
> > be extended accordingly.
> 
> If we wanted to describe device properties via vfio, perhaps this is a
> reasonable approach.  However, why would we want to do that?  How are
> these device properties unique to vfio?  Are there other users that
> might want to learn these properties?  How will they get them?  Perhaps
> they would decide to expose them via sysfs for everyone, making this
> interface redundant.  Why not start with exposing device properties to
> userspace in a way that's not specific to vfio?  Thanks,
> 
We discussed this for the purposes of ARM during SFO15 last week, and
basically arrived at the conclusion that the resonable thing to do is to
rely on sysfs device tree parsing in userspace.  We don't have a great
solution for ACPI yet, but we also don't know of any ACPI-only devices
that want platform device passthrough yet.

-Christoffer

Re: [RFC PATCH v5 0/3] vfio: platform: return device properties for a platform device

[Peter Maydell]

On 2 October 2015 at 21:28, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> We discussed this for the purposes of ARM during SFO15 last week, and
> basically arrived at the conclusion that the resonable thing to do is to
> rely on sysfs device tree parsing in userspace.  We don't have a great
> solution for ACPI yet, but we also don't know of any ACPI-only devices
> that want platform device passthrough yet.

I wasn't hugely happy with that approach though:
 * it's DT specific and just won't work on ACPI platforms; implementing
   features with a "needs DT" dependency seems like it will come back to
   bite us later
 * I don't really want to build in a lot of infrastructure into
   QEMU to either build the DTC compiler into it or else introduce
   a runtime dependency on the dtc binary, if this is just going
   to be a stopgap solution until somebody says "has to work on
   ACPI" and we need to do it some other way

On the other hand I don't exactly have a better approach to suggest
(except "don't do device passthrough for platform devices, insist
on a real bus like PCI"...)

thanks
-- PMM

Re: [RFC PATCH v5 0/3] vfio: platform: return device properties for a platform device

[Christoffer Dall]

On Fri, Oct 02, 2015 at 11:53:07PM +0100, Peter Maydell wrote:
> On 2 October 2015 at 21:28, Christoffer Dall
> <christoffer.dall@linaro.org> wrote:
> > We discussed this for the purposes of ARM during SFO15 last week, and
> > basically arrived at the conclusion that the resonable thing to do is to
> > rely on sysfs device tree parsing in userspace.  We don't have a great
> > solution for ACPI yet, but we also don't know of any ACPI-only devices
> > that want platform device passthrough yet.
> 
> I wasn't hugely happy with that approach though:
>  * it's DT specific and just won't work on ACPI platforms; implementing
>    features with a "needs DT" dependency seems like it will come back to
>    bite us later

I tend to agree with that point of view, but I don't have hugely better
ideas.

>  * I don't really want to build in a lot of infrastructure into
>    QEMU to either build the DTC compiler into it or else introduce
>    a runtime dependency on the dtc binary

what level of complexity are we really talking about here?  Doesn't QEMU
already link against libfdt and doesn't it support exactly what we're
trying to do here?

> , if this is just going
>    to be a stopgap solution until somebody says "has to work on
>    ACPI" and we need to do it some other way
> 
> On the other hand I don't exactly have a better approach to suggest

I also don't think this is the job of VFIO, assuming there is some
better place to do this.  I initially thought exposing device properties
in some canonical format from sysfs independently from whether the
system was booted with ACPI or DT was the right thing to do, but the
counter argument to this was essentially that the guest kernel needs the
same description as the host kernel and therefore we really have to find
a way to pass the HW description bits on to the guest.

> (except "don't do device passthrough for platform devices, insist
> on a real bus like PCI"...)
> 
While I appreciate the simplicity of this solution from our
(maintainers') point of view, I still see the latter point as relatively
moot.  We have hardware with 10G platform ethernet devices that people
want to do device assignment on already, and I think we should try to
find a reasonable set of boundaries for setups that we can support
upstream instead of this becoming a black hole of derivative code bases
to do this sort of thing.

Thanks,
-Christoffer

Re: [RFC PATCH v5 0/3] vfio: platform: return device properties for a platform device

[Baptiste Reynal]

In this patch series we want to wrap an already available kernel
interface to expose a device property to userspace, in order to keep
the code lighter on the userspace. We need those properties in VFIO as
VFIO grants the possibility to develop userspace drivers.

The sysfs doesn't seems to be ready for this kind of usage. We can
only find raw data that require heavy parsing. Here we retrieve
directly usable data and it can be extended later according to new
needs (as it is already done with ACPI).

This interface has been developed for VFIO and is currently bound to
it, though there is no special dependencies with it. We could make it
more generic, but I can only think of VFIO to use it.

On Sat, Oct 3, 2015 at 12:53 AM, Peter Maydell <peter.maydell-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 2 October 2015 at 21:28, Christoffer Dall
> <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> We discussed this for the purposes of ARM during SFO15 last week, and
>> basically arrived at the conclusion that the resonable thing to do is to
>> rely on sysfs device tree parsing in userspace.  We don't have a great
>> solution for ACPI yet, but we also don't know of any ACPI-only devices
>> that want platform device passthrough yet.
>
> I wasn't hugely happy with that approach though:
>  * it's DT specific and just won't work on ACPI platforms; implementing
>    features with a "needs DT" dependency seems like it will come back to
>    bite us later
>  * I don't really want to build in a lot of infrastructure into
>    QEMU to either build the DTC compiler into it or else introduce
>    a runtime dependency on the dtc binary, if this is just going
>    to be a stopgap solution until somebody says "has to work on
>    ACPI" and we need to do it some other way
>
> On the other hand I don't exactly have a better approach to suggest
> (except "don't do device passthrough for platform devices, insist
> on a real bus like PCI"...)
>
> thanks
> -- PMM

Re: [RFC PATCH v5 0/3] vfio: platform: return device properties for a platform device

[Christoffer Dall]

[why are you top-posting?]

On Mon, Oct 05, 2015 at 11:42:38AM +0200, Baptiste Reynal wrote:
> In this patch series we want to wrap an already available kernel
> interface to expose a device property to userspace, 

which 'already available kernel interface' is that exactly?

> in order to keep
> the code lighter on the userspace. We need those properties in VFIO as

I'm not sure I agree with your 'need those properties in VFIO' statement
here, can you elaborate?

> VFIO grants the possibility to develop userspace drivers.
> 
> The sysfs doesn't seems to be ready for this kind of usage. We can
> only find raw data that require heavy parsing. Here we retrieve
> directly usable data and it can be extended later according to new
> needs (as it is already done with ACPI).

Why couldn't you expose this kind of data through sysfs instead of VFIO
and independently of VFIO?  Would that be more wrong/difficult/whatever?

> 
> This interface has been developed for VFIO and is currently bound to
> it, though there is no special dependencies with it. We could make it
> more generic, but I can only think of VFIO to use it.

Thanks,
-Christoffer

Re: [RFC PATCH v5 0/3] vfio: platform: return device properties for a platform device

[Baptiste Reynal]

On Mon, Oct 5, 2015 at 11:53 AM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> [why are you top-posting?]
>
> On Mon, Oct 05, 2015 at 11:42:38AM +0200, Baptiste Reynal wrote:
>> In this patch series we want to wrap an already available kernel
>> interface to expose a device property to userspace,
>
> which 'already available kernel interface' is that exactly?

We use the unified device property interface.

>
>> in order to keep
>> the code lighter on the userspace. We need those properties in VFIO as
>
> I'm not sure I agree with your 'need those properties in VFIO' statement
> here, can you elaborate?
>

For device assignment, we need to transfer those properties to the
guest in order to fully exploit the device (and not simply transfer
generic properties for a given device).

The example I have in mind is the PL330 which can have optionnal
properties, as described in
Documentation/devicetree/bindings/dma/arm-pl330.txt

>> VFIO grants the possibility to develop userspace drivers.
>>
>> The sysfs doesn't seems to be ready for this kind of usage. We can
>> only find raw data that require heavy parsing. Here we retrieve
>> directly usable data and it can be extended later according to new
>> needs (as it is already done with ACPI).
>
> Why couldn't you expose this kind of data through sysfs instead of VFIO
> and independently of VFIO?  Would that be more wrong/difficult/whatever?
>

This has been bound to VFIO as we needed it for VFIO, but I don't see
any reasons not to expose it somewhere else if it may be reused.

>>
>> This interface has been developed for VFIO and is currently bound to
>> it, though there is no special dependencies with it. We could make it
>> more generic, but I can only think of VFIO to use it.
>
> Thanks,
> -Christoffer

Re: [RFC PATCH v5 0/3] vfio: platform: return device properties for a platform device

[Peter Maydell]

On 5 October 2015 at 10:37, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Fri, Oct 02, 2015 at 11:53:07PM +0100, Peter Maydell wrote:
>>  * I don't really want to build in a lot of infrastructure into
>>    QEMU to either build the DTC compiler into it or else introduce
>>    a runtime dependency on the dtc binary
>
> what level of complexity are we really talking about here?  Doesn't QEMU
> already link against libfdt and doesn't it support exactly what we're
> trying to do here?

We link against libfdt, but libfdt is for manipulating and creating
dt binary blobs. As I understand it what sysfs exposes to userspace
is not a dt binary blob (or fragment thereof) but a bit of dt source.
libfdt doesn't do parsing of source into blobs; that is done by the
dtc compiler, which QEMU doesn't currently build and which is set
up to build only a separate command line binary anyway, not to be
a utility library for compiling dt sources.

Do correct me if I'm wrong about the sysfs interface -- if it is
just exposing dt blobs that would be easier to deal with.

> I also don't think this is the job of VFIO, assuming there is some
> better place to do this.  I initially thought exposing device properties
> in some canonical format from sysfs independently from whether the
> system was booted with ACPI or DT was the right thing to do, but the
> counter argument to this was essentially that the guest kernel needs the
> same description as the host kernel and therefore we really have to find
> a way to pass the HW description bits on to the guest.

So we end up with even more code to pass through ACPI table
fragments as well :-(

>> (except "don't do device passthrough for platform devices, insist
>> on a real bus like PCI"...)
>>
> While I appreciate the simplicity of this solution from our
> (maintainers') point of view, I still see the latter point as relatively
> moot.  We have hardware with 10G platform ethernet devices that people
> want to do device assignment on already, and I think we should try to
> find a reasonable set of boundaries for setups that we can support
> upstream instead of this becoming a black hole of derivative code bases
> to do this sort of thing.

Yeah, speaking more seriously I agree we do need to do something
here. It's just all irritatingly awkward...

thanks
-- PMM

Re: [RFC PATCH v5 0/3] vfio: platform: return device properties for a platform device

[Christoffer Dall]

[cc'ing Mark R. and Shannon for their input on FDT and ACPI].

On Mon, Oct 05, 2015 at 11:07:35AM +0100, Peter Maydell wrote:
> On 5 October 2015 at 10:37, Christoffer Dall
> <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Fri, Oct 02, 2015 at 11:53:07PM +0100, Peter Maydell wrote:
> >>  * I don't really want to build in a lot of infrastructure into
> >>    QEMU to either build the DTC compiler into it or else introduce
> >>    a runtime dependency on the dtc binary
> >
> > what level of complexity are we really talking about here?  Doesn't QEMU
> > already link against libfdt and doesn't it support exactly what we're
> > trying to do here?
> 
> We link against libfdt, but libfdt is for manipulating and creating
> dt binary blobs. As I understand it what sysfs exposes to userspace
> is not a dt binary blob (or fragment thereof) but a bit of dt source.
> libfdt doesn't do parsing of source into blobs; that is done by the
> dtc compiler, which QEMU doesn't currently build and which is set
> up to build only a separate command line binary anyway, not to be
> a utility library for compiling dt sources.
> 
> Do correct me if I'm wrong about the sysfs interface -- if it is
> just exposing dt blobs that would be easier to deal with.
> 

I thought there was also /sys/firmware/fdt being the unflattened one
accessible by libfdt, but dtc -I dtb seems to be unhappy parsin this on
my Mustang.

Mark, can you refresh my memory about this?

> > I also don't think this is the job of VFIO, assuming there is some
> > better place to do this.  I initially thought exposing device properties
> > in some canonical format from sysfs independently from whether the
> > system was booted with ACPI or DT was the right thing to do, but the
> > counter argument to this was essentially that the guest kernel needs the
> > same description as the host kernel and therefore we really have to find
> > a way to pass the HW description bits on to the guest.
> 
> So we end up with even more code to pass through ACPI table
> fragments as well :-(
> 

Probaby, yes.  But I think it's even worse, because there's no guarantee
you can get at the original firmware data, because it's all from the
DSDT so invoking the initial 'probe' method of the device entry could
theoretically overwrite itself...  (If I remember the details
correctly).

At least saying ACPI guest on ACPI, and DT guest on DT host, would be a
reasonable limitation for anything like this.

But perhaps this really is a place where you need the transparency of DT
vs. ACPI to do this.

This is all outside my area of expertise, so take whatever I said above
with that disclaim.er

> >> (except "don't do device passthrough for platform devices, insist
> >> on a real bus like PCI"...)
> >>
> > While I appreciate the simplicity of this solution from our
> > (maintainers') point of view, I still see the latter point as relatively
> > moot.  We have hardware with 10G platform ethernet devices that people
> > want to do device assignment on already, and I think we should try to
> > find a reasonable set of boundaries for setups that we can support
> > upstream instead of this becoming a black hole of derivative code bases
> > to do this sort of thing.
> 
> Yeah, speaking more seriously I agree we do need to do something
> here. It's just all irritatingly awkward...
> 

Agreed.

-Christoffer

Re: [RFC PATCH v5 0/3] vfio: platform: return device properties for a platform device

[Mark Rutland]

On Mon, Oct 05, 2015 at 12:32:00PM +0200, Christoffer Dall wrote:
> [cc'ing Mark R. and Shannon for their input on FDT and ACPI].
> 
> On Mon, Oct 05, 2015 at 11:07:35AM +0100, Peter Maydell wrote:
> > On 5 October 2015 at 10:37, Christoffer Dall
> > <christoffer.dall@linaro.org> wrote:
> > > On Fri, Oct 02, 2015 at 11:53:07PM +0100, Peter Maydell wrote:
> > >>  * I don't really want to build in a lot of infrastructure into
> > >>    QEMU to either build the DTC compiler into it or else introduce
> > >>    a runtime dependency on the dtc binary
> > >
> > > what level of complexity are we really talking about here?  Doesn't QEMU
> > > already link against libfdt and doesn't it support exactly what we're
> > > trying to do here?
> > 
> > We link against libfdt, but libfdt is for manipulating and creating
> > dt binary blobs. As I understand it what sysfs exposes to userspace
> > is not a dt binary blob (or fragment thereof) but a bit of dt source.
> > libfdt doesn't do parsing of source into blobs; that is done by the
> > dtc compiler, which QEMU doesn't currently build and which is set
> > up to build only a separate command line binary anyway, not to be
> > a utility library for compiling dt sources.
> > 
> > Do correct me if I'm wrong about the sysfs interface -- if it is
> > just exposing dt blobs that would be easier to deal with.
> > 
> 
> I thought there was also /sys/firmware/fdt being the unflattened one
> accessible by libfdt, but dtc -I dtb seems to be unhappy parsin this on
> my Mustang.
> 
> Mark, can you refresh my memory about this?

/sys/firmware/fdt is the original, unadulterated DTB passed to the
kernel, while /sys/firmware/device-tree is the unflattened form which
may have had overlays (or perhaps other fixups) applied.

For runtime stuff I'd generally expect /sys/firmware/device-tree to be
preferable (e.g. if we want to expose some device added by an overlay).
We added /sys/firmware/fdt for kexec, as memory reservations are in the
DTB header, which doesn't show up in the unflattened tree.

> > > I also don't think this is the job of VFIO, assuming there is some
> > > better place to do this.  I initially thought exposing device properties
> > > in some canonical format from sysfs independently from whether the
> > > system was booted with ACPI or DT was the right thing to do, but the
> > > counter argument to this was essentially that the guest kernel needs the
> > > same description as the host kernel and therefore we really have to find
> > > a way to pass the HW description bits on to the guest.
> > 
> > So we end up with even more code to pass through ACPI table
> > fragments as well :-(
> > 
> 
> Probaby, yes.  But I think it's even worse, because there's no guarantee
> you can get at the original firmware data, because it's all from the
> DSDT so invoking the initial 'probe' method of the device entry could
> theoretically overwrite itself...  (If I remember the details
> correctly).

I don't think you can realistically expose host AML to the guest. Even
if it doesn't overwrite itself, it could perform oneshot configuration
that breaks if executed repeatedly, try to access otehr bits of ACPI
which may or may not be present, call to secure firmware, etc.

Mark.