Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1239023 > unrolled thread

[PATCH v3 0/4] SysFS driver for QEMU fw_cfg device

Started by"Gabriel L. Somlo" <somlo@cmu.edu>
First post2015-10-04 01:50 +0200
Last post2015-10-06 09:20 +0200
Articles 13 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device "Gabriel L. Somlo" <somlo@cmu.edu> - 2015-10-04 01:50 +0200
    Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device Mark Rutland <mark.rutland@arm.com> - 2015-10-05 12:10 +0200
      Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device Paolo Bonzini <pbonzini@redhat.com> - 2015-10-05 13:50 +0200
        Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device Mark Rutland <mark.rutland@arm.com> - 2015-10-05 14:30 +0200
          Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device "Gabriel L. Somlo" <somlo@cmu.edu> - 2015-10-05 14:50 +0200
            Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device Mark Rutland <mark.rutland@arm.com> - 2015-10-05 15:00 +0200
              Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device "Gabriel L. Somlo" <somlo@cmu.edu> - 2015-10-05 15:30 +0200
      Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device "Gabriel L. Somlo" <somlo@cmu.edu> - 2015-10-05 14:50 +0200
        Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device Peter Maydell <peter.maydell@linaro.org> - 2015-10-05 15:00 +0200
          Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device "Gabriel L. Somlo" <somlo@cmu.edu> - 2015-10-05 15:20 +0200
          Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device Paolo Bonzini <pbonzini@redhat.com> - 2015-10-05 15:20 +0200
        Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device Mark Rutland <mark.rutland@arm.com> - 2015-10-05 15:10 +0200
          Re: [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device Laszlo Ersek <lersek@redhat.com> - 2015-10-06 09:20 +0200

#1239023 — [PATCH v3 0/4] SysFS driver for QEMU fw_cfg device

From"Gabriel L. Somlo" <somlo@cmu.edu>
Date2015-10-04 01:50 +0200
Subject[PATCH v3 0/4] SysFS driver for QEMU fw_cfg device
Message-ID<qfEIV-8ui-3@gated-at.bofh.it>
From: "Gabriel Somlo" <somlo@cmu.edu>

Allow access to QEMU firmware blobs, passed into the guest VM via
the fw_cfg device, through SysFS entries. Blob meta-data (e.g. name,
size, and fw_cfg key), as well as the raw binary blob data may be
accessed.

The SysFS access location is /sys/firmware/qemu_fw_cfg/... and was
selected based on overall similarity to the type of information
exposed under /sys/firmware/dmi/entries/...


NEW (since v2): Using ACPI to detect the presence and details of the
fw_cfg virtual hardware device.

    Device Tree has been suggested by Ard as a comment on v2 of this
    patch, but after some deliberation I decided to go with ACPI,
    since it's supported on both x86 and some (uefi-enabled) versions
    of aarch64. I really don't see how I'd reasonably use *both* DT (on
    ARM) *and* ACPI (on x86), and after all I'm mostly concerned with
    x86, but originally wanted to maximize portability (which is where
    the register probing in earlier versions came from).

    A patch set generating an ACPI device node for qemu's fw_cfg is
    currently under review on the qemu-devel list:

    http://lists.nongnu.org/archive/html/qemu-devel/2015-09/msg06946.html
    (sorry, gmane appears down at the moment...)

In consequence:

	- Patch 1/4 is mostly the same as in v2;
	- Patch 2/4 switches device initialization from register
	  probing to using ACPI; this is a separate patch only to
	  illustrate the transition from probing to ACPI, and I'm
	  assuming it will end up squashed on top of patch 1/4 in
	  the final version.

	- Patches 3/4 and 4/4 add a "human-readable" directory
	  hierarchy built from tokenizing fw_cfg blob names into
	  '/'-separated components, with symlinks to each 'by_key'
	  blob folder (same as in earlier versions). At Greg's
	  suggestion I tried to build this folder hierarchy and
	  leaf symlinks using udev rules, but so far I haven't been
	  successful in figuring that out. If udev turns out to 
	  be applicable after all, these two patches can be dropped
	  from this series.

In other words, patches 1 and 2 give us the following "by_key" listing
of blobs contained in the qemu fw_cfg device (example pulled from a PC
qemu guest running Fedora 22), with the value of each "name" attribute
shown on the right:

$ tree /sys/firmware/qemu_fw_cfg/
/sys/firmware/qemu_fw_cfg/
|-- by_key
|   |-- 32
|   |   |-- key
|   |   |-- name			("etc/boot-fail-wait")
|   |   |-- raw
|   |   `-- size
|   |-- 33
|   |   |-- key
|   |   |-- name			("etc/smbios/smbios-tables")
|   |   |-- raw
|   |   `-- size
|   |-- 34
|   |   |-- key
|   |   |-- name			("etc/smbios/smbios-anchor")
|   |   |-- raw
|   |   `-- size
|   |-- 35
|   |   |-- key
|   |   |-- name			("etc/e820")
|   |   |-- raw
|   |   `-- size
|   |-- 36
|   |   |-- key
|   |   |-- name			("genroms/kvmvapic.bin")
|   |   |-- raw
|   |   `-- size
|   |-- 37
|   |   |-- key
|   |   |-- name			("etc/system-states")
|   |   |-- raw
|   |   `-- size
|   |-- 38
|   |   |-- key
|   |   |-- name			("etc/acpi/tables")
|   |   |-- raw
|   |   `-- size
|   |-- 39
|   |   |-- key
|   |   |-- name			("etc/table-loader")
|   |   |-- raw
|   |   `-- size
|   |-- 40
|   |   |-- key
|   |   |-- name			("etc/tpm/log")
|   |   |-- raw
|   |   `-- size
|   |-- 41
|   |   |-- key
|   |   |-- name			("etc/acpi/rsdp")
|   |   |-- raw
|   |   `-- size
|   `-- 42
|       |-- key
|       |-- name			("bootorder")
|       |-- raw
|       `-- size
|
...

Additionally, patches 3 and 4 (mostly 4) give us the following
"user friendly" directory hierarchy as a complement to the above,
based on tokenizing each blob name into symlink-tipped (sub)directories:

...
|-- by_name
|   |-- bootorder -> ../by_key/42
|   |-- etc
|   |   |-- acpi
|   |   |   |-- rsdp -> ../../../by_key/41
|   |   |   `-- tables -> ../../../by_key/38
|   |   |-- boot-fail-wait -> ../../by_key/32
|   |   |-- e820 -> ../../by_key/35
|   |   |-- smbios
|   |   |   |-- smbios-anchor -> ../../../by_key/34
|   |   |   `-- smbios-tables -> ../../../by_key/33
|   |   |-- system-states -> ../../by_key/37
|   |   |-- table-loader -> ../../by_key/39
|   |   `-- tpm
|   |       `-- log -> ../../../by_key/40
|   `-- genroms
|       `-- kvmvapic.bin -> ../../by_key/36
`-- rev

The trick is to figure out how to replace patches 3 and 4 with a
udev rule that would read the contents of each "name" attribute,
and build the "by_name" hierarchy and symlinks in userspace.

I tried:

$ udevadm info -a -p /sys/firmware/qemu_fw_cfg/by_key/33

  looking at device '/firmware/qemu_fw_cfg/by_key/33':
    KERNEL=="33"
    SUBSYSTEM==""
    DRIVER==""
    ATTR{key}=="33"
    ATTR{name}=="etc/smbios/smbios-tables"
    ATTR{size}=="388"

  looking at parent device '/firmware/qemu_fw_cfg/by_key':
    KERNELS=="by_key"
    SUBSYSTEMS==""
    DRIVERS==""

  looking at parent device '/firmware/qemu_fw_cfg':
    KERNELS=="qemu_fw_cfg"
    SUBSYSTEMS==""
    DRIVERS==""
    ATTRS{rev}=="1"

Then I tried creating a file, /usr/lib/udev/rules.d/99-qemu-fw-cfg.rules
containing the following line:

KERNELS=="qemu_fw_cfg", ATTRS{rev}=="1", SYMLINK="%p/%s{name}"

but NOTHING happens when I insert/remove qemu_fw_cfg.ko.  I also tried:

KERNELS=="qemu_fw_cfg", ATTRS{rev}=="1", PROGRAM="/foo %k"

where "/foo" basically did "echo $* > /tmp/bar", but no /tmp/bar file ever
showed up as a consequence of inserting/removing the qemu_fw_cfg.ko module.

At this point, I need help figuring out whether udev is really what would
get the second, user-friendly, "by_name" /sysfs directory tree created,
and how I'd go about that...

Thanks much,
  --Gabriel

Gabriel Somlo (4):
  firmware: introduce sysfs driver for QEMU's fw_cfg device
  firmware: use acpi to detect QEMU fw_cfg device for sysfs fw_cfg
    driver
  kobject: export kset_find_obj() for module use
  firmware: create directory hierarchy for sysfs fw_cfg entries

 .../ABI/testing/sysfs-firmware-qemu_fw_cfg         | 213 ++++++++
 drivers/firmware/Kconfig                           |  10 +
 drivers/firmware/Makefile                          |   1 +
 drivers/firmware/qemu_fw_cfg.c                     | 575 +++++++++++++++++++++
 lib/kobject.c                                      |   1 +
 5 files changed, 800 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
 create mode 100644 drivers/firmware/qemu_fw_cfg.c

-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1239404

FromMark Rutland <mark.rutland@arm.com>
Date2015-10-05 12:10 +0200
Message-ID<qgaSu-4bV-11@gated-at.bofh.it>
In reply to#1239023
On Sat, Oct 03, 2015 at 07:28:05PM -0400, Gabriel L. Somlo wrote:
> From: "Gabriel Somlo" <somlo@cmu.edu>
> 
> Allow access to QEMU firmware blobs, passed into the guest VM via
> the fw_cfg device, through SysFS entries. Blob meta-data (e.g. name,
> size, and fw_cfg key), as well as the raw binary blob data may be
> accessed.
> 
> The SysFS access location is /sys/firmware/qemu_fw_cfg/... and was
> selected based on overall similarity to the type of information
> exposed under /sys/firmware/dmi/entries/...

What is the intended use of these?

Some of the keys in the example look like they'd come from other sources
(e.g. the *-tables entries), while others look like kernel/bootloader
configuration options (e.g. etc/boot-fail-wait, bootorder) -- I'm
concerned about redundancy here.

> NEW (since v2): Using ACPI to detect the presence and details of the
> fw_cfg virtual hardware device.
> 
>     Device Tree has been suggested by Ard as a comment on v2 of this
>     patch, but after some deliberation I decided to go with ACPI,
>     since it's supported on both x86 and some (uefi-enabled) versions
>     of aarch64. I really don't see how I'd reasonably use *both* DT (on
>     ARM) *and* ACPI (on x86), and after all I'm mostly concerned with
>     x86, but originally wanted to maximize portability (which is where
>     the register probing in earlier versions came from).

There are defintitely going to be arm64 VMs that don't use ACPI, so we
may need DT support depending on what the intended use is.

I'm not sure I follow what the difficulty with supporting DT in addition
to ACPI is? It looks like all you need is a compatible string and a reg
entry.

Thanks,
Mark.

>     A patch set generating an ACPI device node for qemu's fw_cfg is
>     currently under review on the qemu-devel list:
> 
>     http://lists.nongnu.org/archive/html/qemu-devel/2015-09/msg06946.html
>     (sorry, gmane appears down at the moment...)
> 
> In consequence:
> 
> 	- Patch 1/4 is mostly the same as in v2;
> 	- Patch 2/4 switches device initialization from register
> 	  probing to using ACPI; this is a separate patch only to
> 	  illustrate the transition from probing to ACPI, and I'm
> 	  assuming it will end up squashed on top of patch 1/4 in
> 	  the final version.
> 
> 	- Patches 3/4 and 4/4 add a "human-readable" directory
> 	  hierarchy built from tokenizing fw_cfg blob names into
> 	  '/'-separated components, with symlinks to each 'by_key'
> 	  blob folder (same as in earlier versions). At Greg's
> 	  suggestion I tried to build this folder hierarchy and
> 	  leaf symlinks using udev rules, but so far I haven't been
> 	  successful in figuring that out. If udev turns out to 
> 	  be applicable after all, these two patches can be dropped
> 	  from this series.
> 
> In other words, patches 1 and 2 give us the following "by_key" listing
> of blobs contained in the qemu fw_cfg device (example pulled from a PC
> qemu guest running Fedora 22), with the value of each "name" attribute
> shown on the right:
> 
> $ tree /sys/firmware/qemu_fw_cfg/
> /sys/firmware/qemu_fw_cfg/
> |-- by_key
> |   |-- 32
> |   |   |-- key
> |   |   |-- name			("etc/boot-fail-wait")
> |   |   |-- raw
> |   |   `-- size
> |   |-- 33
> |   |   |-- key
> |   |   |-- name			("etc/smbios/smbios-tables")
> |   |   |-- raw
> |   |   `-- size
> |   |-- 34
> |   |   |-- key
> |   |   |-- name			("etc/smbios/smbios-anchor")
> |   |   |-- raw
> |   |   `-- size
> |   |-- 35
> |   |   |-- key
> |   |   |-- name			("etc/e820")
> |   |   |-- raw
> |   |   `-- size
> |   |-- 36
> |   |   |-- key
> |   |   |-- name			("genroms/kvmvapic.bin")
> |   |   |-- raw
> |   |   `-- size
> |   |-- 37
> |   |   |-- key
> |   |   |-- name			("etc/system-states")
> |   |   |-- raw
> |   |   `-- size
> |   |-- 38
> |   |   |-- key
> |   |   |-- name			("etc/acpi/tables")
> |   |   |-- raw
> |   |   `-- size
> |   |-- 39
> |   |   |-- key
> |   |   |-- name			("etc/table-loader")
> |   |   |-- raw
> |   |   `-- size
> |   |-- 40
> |   |   |-- key
> |   |   |-- name			("etc/tpm/log")
> |   |   |-- raw
> |   |   `-- size
> |   |-- 41
> |   |   |-- key
> |   |   |-- name			("etc/acpi/rsdp")
> |   |   |-- raw
> |   |   `-- size
> |   `-- 42
> |       |-- key
> |       |-- name			("bootorder")
> |       |-- raw
> |       `-- size
> |
> ...
> 
> Additionally, patches 3 and 4 (mostly 4) give us the following
> "user friendly" directory hierarchy as a complement to the above,
> based on tokenizing each blob name into symlink-tipped (sub)directories:
> 
> ...
> |-- by_name
> |   |-- bootorder -> ../by_key/42
> |   |-- etc
> |   |   |-- acpi
> |   |   |   |-- rsdp -> ../../../by_key/41
> |   |   |   `-- tables -> ../../../by_key/38
> |   |   |-- boot-fail-wait -> ../../by_key/32
> |   |   |-- e820 -> ../../by_key/35
> |   |   |-- smbios
> |   |   |   |-- smbios-anchor -> ../../../by_key/34
> |   |   |   `-- smbios-tables -> ../../../by_key/33
> |   |   |-- system-states -> ../../by_key/37
> |   |   |-- table-loader -> ../../by_key/39
> |   |   `-- tpm
> |   |       `-- log -> ../../../by_key/40
> |   `-- genroms
> |       `-- kvmvapic.bin -> ../../by_key/36
> `-- rev
> 
> The trick is to figure out how to replace patches 3 and 4 with a
> udev rule that would read the contents of each "name" attribute,
> and build the "by_name" hierarchy and symlinks in userspace.
> 
> I tried:
> 
> $ udevadm info -a -p /sys/firmware/qemu_fw_cfg/by_key/33
> 
>   looking at device '/firmware/qemu_fw_cfg/by_key/33':
>     KERNEL=="33"
>     SUBSYSTEM==""
>     DRIVER==""
>     ATTR{key}=="33"
>     ATTR{name}=="etc/smbios/smbios-tables"
>     ATTR{size}=="388"
> 
>   looking at parent device '/firmware/qemu_fw_cfg/by_key':
>     KERNELS=="by_key"
>     SUBSYSTEMS==""
>     DRIVERS==""
> 
>   looking at parent device '/firmware/qemu_fw_cfg':
>     KERNELS=="qemu_fw_cfg"
>     SUBSYSTEMS==""
>     DRIVERS==""
>     ATTRS{rev}=="1"
> 
> Then I tried creating a file, /usr/lib/udev/rules.d/99-qemu-fw-cfg.rules
> containing the following line:
> 
> KERNELS=="qemu_fw_cfg", ATTRS{rev}=="1", SYMLINK="%p/%s{name}"
> 
> but NOTHING happens when I insert/remove qemu_fw_cfg.ko.  I also tried:
> 
> KERNELS=="qemu_fw_cfg", ATTRS{rev}=="1", PROGRAM="/foo %k"
> 
> where "/foo" basically did "echo $* > /tmp/bar", but no /tmp/bar file ever
> showed up as a consequence of inserting/removing the qemu_fw_cfg.ko module.
> 
> At this point, I need help figuring out whether udev is really what would
> get the second, user-friendly, "by_name" /sysfs directory tree created,
> and how I'd go about that...
> 
> Thanks much,
>   --Gabriel
> 
> Gabriel Somlo (4):
>   firmware: introduce sysfs driver for QEMU's fw_cfg device
>   firmware: use acpi to detect QEMU fw_cfg device for sysfs fw_cfg
>     driver
>   kobject: export kset_find_obj() for module use
>   firmware: create directory hierarchy for sysfs fw_cfg entries
> 
>  .../ABI/testing/sysfs-firmware-qemu_fw_cfg         | 213 ++++++++
>  drivers/firmware/Kconfig                           |  10 +
>  drivers/firmware/Makefile                          |   1 +
>  drivers/firmware/qemu_fw_cfg.c                     | 575 +++++++++++++++++++++
>  lib/kobject.c                                      |   1 +
>  5 files changed, 800 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
>  create mode 100644 drivers/firmware/qemu_fw_cfg.c
> 
> -- 
> 2.4.3
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1239482

FromPaolo Bonzini <pbonzini@redhat.com>
Date2015-10-05 13:50 +0200
Message-ID<qgcrg-6gj-7@gated-at.bofh.it>
In reply to#1239404

On 05/10/2015 12:00, Mark Rutland wrote:
> Some of the keys in the example look like they'd come from other sources
> (e.g. the *-tables entries), while others look like kernel/bootloader
> configuration options (e.g. etc/boot-fail-wait, bootorder) -- I'm
> concerned about redundancy here.

The redundancy is because the firmware and the bootloader actually
_consume_ these fw_cfg strings to produce the others (the ACPI tables,
the kernel configuration options).

On the other hand, hiding some strings just because they ought to have
been consumed already makes little sense.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1239505

FromMark Rutland <mark.rutland@arm.com>
Date2015-10-05 14:30 +0200
Message-ID<qgd3Y-7eX-17@gated-at.bofh.it>
In reply to#1239482
On Mon, Oct 05, 2015 at 01:48:52PM +0200, Paolo Bonzini wrote:
> 
> 
> On 05/10/2015 12:00, Mark Rutland wrote:
> > Some of the keys in the example look like they'd come from other sources
> > (e.g. the *-tables entries), while others look like kernel/bootloader
> > configuration options (e.g. etc/boot-fail-wait, bootorder) -- I'm
> > concerned about redundancy here.
> 
> The redundancy is because the firmware and the bootloader actually
> _consume_ these fw_cfg strings to produce the others (the ACPI tables,
> the kernel configuration options).
> 
> On the other hand, hiding some strings just because they ought to have
> been consumed already makes little sense.

Sure. However, I'm concerned that providing redundant interfaces for
those could lead to people grabbing information from here (because it's
convenient) rather than the existing canonical locations, which means we
get more software that works on fewer systems for no good reason.

What I couldn't figure out was what _additional_ information this
provided; it looked like a mixed bag of details we could already get
from disparate sources. If that's all it does, then it seems to me like
it doesn't add any benefit and potentially makes things worse.

So what do we get from this interface that we cannot get elsewhere, and
why is this the best way of exposing it?

Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1239510

From"Gabriel L. Somlo" <somlo@cmu.edu>
Date2015-10-05 14:50 +0200
Message-ID<qgdnj-7Bp-1@gated-at.bofh.it>
In reply to#1239505
On Mon, Oct 05, 2015 at 01:23:33PM +0100, Mark Rutland wrote:
> On Mon, Oct 05, 2015 at 01:48:52PM +0200, Paolo Bonzini wrote:
> > 
> > 
> > On 05/10/2015 12:00, Mark Rutland wrote:
> > > Some of the keys in the example look like they'd come from other sources
> > > (e.g. the *-tables entries), while others look like kernel/bootloader
> > > configuration options (e.g. etc/boot-fail-wait, bootorder) -- I'm
> > > concerned about redundancy here.
> > 
> > The redundancy is because the firmware and the bootloader actually
> > _consume_ these fw_cfg strings to produce the others (the ACPI tables,
> > the kernel configuration options).
> > 
> > On the other hand, hiding some strings just because they ought to have
> > been consumed already makes little sense.
> 
> Sure. However, I'm concerned that providing redundant interfaces for
> those could lead to people grabbing information from here (because it's
> convenient) rather than the existing canonical locations, which means we
> get more software that works on fewer systems for no good reason.
> 
> What I couldn't figure out was what _additional_ information this
> provided; it looked like a mixed bag of details we could already get
> from disparate sources. If that's all it does, then it seems to me like
> it doesn't add any benefit and potentially makes things worse.
> 
> So what do we get from this interface that we cannot get elsewhere, and
> why is this the best way of exposing it?

Starting with qemu 2.4, it is possible to insert arbitrary named
blobs into fw_cfg from the qemu command line. *Those* entries
might be interesting to userspace, which is why it might be handy
to access to fw_cfg blobs in general.

Thanks,
--Gabriel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1239524

FromMark Rutland <mark.rutland@arm.com>
Date2015-10-05 15:00 +0200
Message-ID<qgdx0-7MJ-13@gated-at.bofh.it>
In reply to#1239510
On Mon, Oct 05, 2015 at 08:43:46AM -0400, Gabriel L. Somlo wrote:
> On Mon, Oct 05, 2015 at 01:23:33PM +0100, Mark Rutland wrote:
> > On Mon, Oct 05, 2015 at 01:48:52PM +0200, Paolo Bonzini wrote:
> > > 
> > > 
> > > On 05/10/2015 12:00, Mark Rutland wrote:
> > > > Some of the keys in the example look like they'd come from other sources
> > > > (e.g. the *-tables entries), while others look like kernel/bootloader
> > > > configuration options (e.g. etc/boot-fail-wait, bootorder) -- I'm
> > > > concerned about redundancy here.
> > > 
> > > The redundancy is because the firmware and the bootloader actually
> > > _consume_ these fw_cfg strings to produce the others (the ACPI tables,
> > > the kernel configuration options).
> > > 
> > > On the other hand, hiding some strings just because they ought to have
> > > been consumed already makes little sense.
> > 
> > Sure. However, I'm concerned that providing redundant interfaces for
> > those could lead to people grabbing information from here (because it's
> > convenient) rather than the existing canonical locations, which means we
> > get more software that works on fewer systems for no good reason.
> > 
> > What I couldn't figure out was what _additional_ information this
> > provided; it looked like a mixed bag of details we could already get
> > from disparate sources. If that's all it does, then it seems to me like
> > it doesn't add any benefit and potentially makes things worse.
> > 
> > So what do we get from this interface that we cannot get elsewhere, and
> > why is this the best way of exposing it?
> 
> Starting with qemu 2.4, it is possible to insert arbitrary named
> blobs into fw_cfg from the qemu command line. *Those* entries
> might be interesting to userspace, which is why it might be handy
> to access to fw_cfg blobs in general.

So this is a mechanism to pass arbitrary key:value pairs to a guest
userspace? What would those be used for, and why would this be the
correct location for that?

How do we avoid clashes between user-selected names and those we need to
pass actual FW data?

Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1239558

From"Gabriel L. Somlo" <somlo@cmu.edu>
Date2015-10-05 15:30 +0200
Message-ID<qge02-85-13@gated-at.bofh.it>
In reply to#1239524
On Mon, Oct 05, 2015 at 01:56:47PM +0100, Mark Rutland wrote:
> On Mon, Oct 05, 2015 at 08:43:46AM -0400, Gabriel L. Somlo wrote:
> > On Mon, Oct 05, 2015 at 01:23:33PM +0100, Mark Rutland wrote:
> > > On Mon, Oct 05, 2015 at 01:48:52PM +0200, Paolo Bonzini wrote:
> > > > 
> > > > 
> > > > On 05/10/2015 12:00, Mark Rutland wrote:
> > > > > Some of the keys in the example look like they'd come from other sources
> > > > > (e.g. the *-tables entries), while others look like kernel/bootloader
> > > > > configuration options (e.g. etc/boot-fail-wait, bootorder) -- I'm
> > > > > concerned about redundancy here.
> > > > 
> > > > The redundancy is because the firmware and the bootloader actually
> > > > _consume_ these fw_cfg strings to produce the others (the ACPI tables,
> > > > the kernel configuration options).
> > > > 
> > > > On the other hand, hiding some strings just because they ought to have
> > > > been consumed already makes little sense.
> > > 
> > > Sure. However, I'm concerned that providing redundant interfaces for
> > > those could lead to people grabbing information from here (because it's
> > > convenient) rather than the existing canonical locations, which means we
> > > get more software that works on fewer systems for no good reason.
> > > 
> > > What I couldn't figure out was what _additional_ information this
> > > provided; it looked like a mixed bag of details we could already get
> > > from disparate sources. If that's all it does, then it seems to me like
> > > it doesn't add any benefit and potentially makes things worse.
> > > 
> > > So what do we get from this interface that we cannot get elsewhere, and
> > > why is this the best way of exposing it?
> > 
> > Starting with qemu 2.4, it is possible to insert arbitrary named
> > blobs into fw_cfg from the qemu command line. *Those* entries
> > might be interesting to userspace, which is why it might be handy
> > to access to fw_cfg blobs in general.
> 
> So this is a mechanism to pass arbitrary key:value pairs to a guest
> userspace? What would those be used for, and why would this be the
> correct location for that?

Yes to arbitrary host->guest arbitrary key:value pairs.
fw_cfg because it's asynchronous (host supplies the data at guest
start time, and no longer has to worry about whether and when guests
may or may not start some sort of agent in order to be able to accept
connections, etc); also because it's guest-os agnostic (no
piggy-backing on e.g. kernel command line). Drivers to make data
available to guest userspace can be written for any guest OS.

> How do we avoid clashes between user-selected names and those we need to
> pass actual FW data?

Internally supplied blobs (by QEMU) meant for the firmware are, by
convention, prefixed with "/etc/...". Command-line blobs are expected
to use "opt/...". QEMU issues a warning if a name is used on the
command line that doesn't begin with 'opt/'.

Thanks,
--Gabriel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1239511

From"Gabriel L. Somlo" <somlo@cmu.edu>
Date2015-10-05 14:50 +0200
Message-ID<qgdnj-7Bp-3@gated-at.bofh.it>
In reply to#1239404
On Mon, Oct 05, 2015 at 11:00:36AM +0100, Mark Rutland wrote:
> On Sat, Oct 03, 2015 at 07:28:05PM -0400, Gabriel L. Somlo wrote:
> > From: "Gabriel Somlo" <somlo@cmu.edu>
> > 
> > Allow access to QEMU firmware blobs, passed into the guest VM via
> > the fw_cfg device, through SysFS entries. Blob meta-data (e.g. name,
> > size, and fw_cfg key), as well as the raw binary blob data may be
> > accessed.
> > 
> > The SysFS access location is /sys/firmware/qemu_fw_cfg/... and was
> > selected based on overall similarity to the type of information
> > exposed under /sys/firmware/dmi/entries/...
> 
> What is the intended use of these?
> 
> Some of the keys in the example look like they'd come from other sources
> (e.g. the *-tables entries), while others look like kernel/bootloader
> configuration options (e.g. etc/boot-fail-wait, bootorder) -- I'm
> concerned about redundancy here.

Paolo already answered that (more eloquently than I would have) so I'll
leave it at that, for now...

> 
> > NEW (since v2): Using ACPI to detect the presence and details of the
> > fw_cfg virtual hardware device.
> > 
> >     Device Tree has been suggested by Ard as a comment on v2 of this
> >     patch, but after some deliberation I decided to go with ACPI,
> >     since it's supported on both x86 and some (uefi-enabled) versions
> >     of aarch64. I really don't see how I'd reasonably use *both* DT (on
> >     ARM) *and* ACPI (on x86), and after all I'm mostly concerned with
> >     x86, but originally wanted to maximize portability (which is where
> >     the register probing in earlier versions came from).
> 
> There are defintitely going to be arm64 VMs that don't use ACPI, so we
> may need DT support depending on what the intended use is.
> 
> I'm not sure I follow what the difficulty with supporting DT in addition
> to ACPI is? It looks like all you need is a compatible string and a reg
> entry.

Bearing in mind that I have almost no experience with arm:

I started out by probing all possible port-io and mmio locations where
fw_cfg registers might have been found, from a "classic" module_init
method.

Arm has DT, which as far as I understand will answer the following two
questions: 1. Do I have fw_cfg ? 2. If yes, what address range does it use ?
So that I could continue using a classic module_init, but won't need
to probe for the device.

PC (my primary architecture, the one I actually care about) does not
have DT. If I want to share the same code, I can't probe, so if I try
DT and don't find fw_cfg there (or somehow DT is no-op-ed out because
I'm on a PC guest), I could somehow look it up in ACPI the same way
(i.e., use ACPI as sort of a stand-in for DT).

But all ACPI-enabled drivers I could find use dedicated macros (i.e.
no more classic module_init() and module_exit(), but rather
module_acpi_driver() with .add and .remove methods on an acpi_driver
object, etc.) Not sure how I'd glue DT back into something like that.

In addition, Michael's comment earlier in the thread suggests that
even my current acpi version isn't sufficiently "orthodox" w.r.t.
ACPI, and I should be providing the hardware access routine as
an ACPI/AML routine, to avoid race conditions with the rest of ACPI,
and for encapsulation. I.e. it's even rude to use the fw_cfg node's
ACPI _CRS method (the part where I'd be treating it like a DT stand-in
only to query fw_cfg's hardware specifics).

So far, all the information I've been able to pull together points
away from a dual DT + ACPI all-in-one solution for fw_cfg. If you know
of an example where that's done in an acceptable way, please let
me know so I can use it for inspiration...

Thanks much,
--Gabriel

> 
> >     A patch set generating an ACPI device node for qemu's fw_cfg is
> >     currently under review on the qemu-devel list:
> > 
> >     http://lists.nongnu.org/archive/html/qemu-devel/2015-09/msg06946.html
> >     (sorry, gmane appears down at the moment...)
> > 
> > In consequence:
> > 
> > 	- Patch 1/4 is mostly the same as in v2;
> > 	- Patch 2/4 switches device initialization from register
> > 	  probing to using ACPI; this is a separate patch only to
> > 	  illustrate the transition from probing to ACPI, and I'm
> > 	  assuming it will end up squashed on top of patch 1/4 in
> > 	  the final version.
> > 
> > 	- Patches 3/4 and 4/4 add a "human-readable" directory
> > 	  hierarchy built from tokenizing fw_cfg blob names into
> > 	  '/'-separated components, with symlinks to each 'by_key'
> > 	  blob folder (same as in earlier versions). At Greg's
> > 	  suggestion I tried to build this folder hierarchy and
> > 	  leaf symlinks using udev rules, but so far I haven't been
> > 	  successful in figuring that out. If udev turns out to 
> > 	  be applicable after all, these two patches can be dropped
> > 	  from this series.
> > 
> > In other words, patches 1 and 2 give us the following "by_key" listing
> > of blobs contained in the qemu fw_cfg device (example pulled from a PC
> > qemu guest running Fedora 22), with the value of each "name" attribute
> > shown on the right:
> > 
> > $ tree /sys/firmware/qemu_fw_cfg/
> > /sys/firmware/qemu_fw_cfg/
> > |-- by_key
> > |   |-- 32
> > |   |   |-- key
> > |   |   |-- name			("etc/boot-fail-wait")
> > |   |   |-- raw
> > |   |   `-- size
> > |   |-- 33
> > |   |   |-- key
> > |   |   |-- name			("etc/smbios/smbios-tables")
> > |   |   |-- raw
> > |   |   `-- size
> > |   |-- 34
> > |   |   |-- key
> > |   |   |-- name			("etc/smbios/smbios-anchor")
> > |   |   |-- raw
> > |   |   `-- size
> > |   |-- 35
> > |   |   |-- key
> > |   |   |-- name			("etc/e820")
> > |   |   |-- raw
> > |   |   `-- size
> > |   |-- 36
> > |   |   |-- key
> > |   |   |-- name			("genroms/kvmvapic.bin")
> > |   |   |-- raw
> > |   |   `-- size
> > |   |-- 37
> > |   |   |-- key
> > |   |   |-- name			("etc/system-states")
> > |   |   |-- raw
> > |   |   `-- size
> > |   |-- 38
> > |   |   |-- key
> > |   |   |-- name			("etc/acpi/tables")
> > |   |   |-- raw
> > |   |   `-- size
> > |   |-- 39
> > |   |   |-- key
> > |   |   |-- name			("etc/table-loader")
> > |   |   |-- raw
> > |   |   `-- size
> > |   |-- 40
> > |   |   |-- key
> > |   |   |-- name			("etc/tpm/log")
> > |   |   |-- raw
> > |   |   `-- size
> > |   |-- 41
> > |   |   |-- key
> > |   |   |-- name			("etc/acpi/rsdp")
> > |   |   |-- raw
> > |   |   `-- size
> > |   `-- 42
> > |       |-- key
> > |       |-- name			("bootorder")
> > |       |-- raw
> > |       `-- size
> > |
> > ...
> > 
> > Additionally, patches 3 and 4 (mostly 4) give us the following
> > "user friendly" directory hierarchy as a complement to the above,
> > based on tokenizing each blob name into symlink-tipped (sub)directories:
> > 
> > ...
> > |-- by_name
> > |   |-- bootorder -> ../by_key/42
> > |   |-- etc
> > |   |   |-- acpi
> > |   |   |   |-- rsdp -> ../../../by_key/41
> > |   |   |   `-- tables -> ../../../by_key/38
> > |   |   |-- boot-fail-wait -> ../../by_key/32
> > |   |   |-- e820 -> ../../by_key/35
> > |   |   |-- smbios
> > |   |   |   |-- smbios-anchor -> ../../../by_key/34
> > |   |   |   `-- smbios-tables -> ../../../by_key/33
> > |   |   |-- system-states -> ../../by_key/37
> > |   |   |-- table-loader -> ../../by_key/39
> > |   |   `-- tpm
> > |   |       `-- log -> ../../../by_key/40
> > |   `-- genroms
> > |       `-- kvmvapic.bin -> ../../by_key/36
> > `-- rev
> > 
> > The trick is to figure out how to replace patches 3 and 4 with a
> > udev rule that would read the contents of each "name" attribute,
> > and build the "by_name" hierarchy and symlinks in userspace.
> > 
> > I tried:
> > 
> > $ udevadm info -a -p /sys/firmware/qemu_fw_cfg/by_key/33
> > 
> >   looking at device '/firmware/qemu_fw_cfg/by_key/33':
> >     KERNEL=="33"
> >     SUBSYSTEM==""
> >     DRIVER==""
> >     ATTR{key}=="33"
> >     ATTR{name}=="etc/smbios/smbios-tables"
> >     ATTR{size}=="388"
> > 
> >   looking at parent device '/firmware/qemu_fw_cfg/by_key':
> >     KERNELS=="by_key"
> >     SUBSYSTEMS==""
> >     DRIVERS==""
> > 
> >   looking at parent device '/firmware/qemu_fw_cfg':
> >     KERNELS=="qemu_fw_cfg"
> >     SUBSYSTEMS==""
> >     DRIVERS==""
> >     ATTRS{rev}=="1"
> > 
> > Then I tried creating a file, /usr/lib/udev/rules.d/99-qemu-fw-cfg.rules
> > containing the following line:
> > 
> > KERNELS=="qemu_fw_cfg", ATTRS{rev}=="1", SYMLINK="%p/%s{name}"
> > 
> > but NOTHING happens when I insert/remove qemu_fw_cfg.ko.  I also tried:
> > 
> > KERNELS=="qemu_fw_cfg", ATTRS{rev}=="1", PROGRAM="/foo %k"
> > 
> > where "/foo" basically did "echo $* > /tmp/bar", but no /tmp/bar file ever
> > showed up as a consequence of inserting/removing the qemu_fw_cfg.ko module.
> > 
> > At this point, I need help figuring out whether udev is really what would
> > get the second, user-friendly, "by_name" /sysfs directory tree created,
> > and how I'd go about that...
> > 
> > Thanks much,
> >   --Gabriel
> > 
> > Gabriel Somlo (4):
> >   firmware: introduce sysfs driver for QEMU's fw_cfg device
> >   firmware: use acpi to detect QEMU fw_cfg device for sysfs fw_cfg
> >     driver
> >   kobject: export kset_find_obj() for module use
> >   firmware: create directory hierarchy for sysfs fw_cfg entries
> > 
> >  .../ABI/testing/sysfs-firmware-qemu_fw_cfg         | 213 ++++++++
> >  drivers/firmware/Kconfig                           |  10 +
> >  drivers/firmware/Makefile                          |   1 +
> >  drivers/firmware/qemu_fw_cfg.c                     | 575 +++++++++++++++++++++
> >  lib/kobject.c                                      |   1 +
> >  5 files changed, 800 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
> >  create mode 100644 drivers/firmware/qemu_fw_cfg.c
> > 
> > -- 
> > 2.4.3
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1239520

FromPeter Maydell <peter.maydell@linaro.org>
Date2015-10-05 15:00 +0200
Message-ID<qgdwZ-7MJ-9@gated-at.bofh.it>
In reply to#1239511
On 5 October 2015 at 13:40, Gabriel L. Somlo <somlo@cmu.edu> wrote:
> In addition, Michael's comment earlier in the thread suggests that
> even my current acpi version isn't sufficiently "orthodox" w.r.t.
> ACPI, and I should be providing the hardware access routine as
> an ACPI/AML routine, to avoid race conditions with the rest of ACPI,
> and for encapsulation. I.e. it's even rude to use the fw_cfg node's
> ACPI _CRS method (the part where I'd be treating it like a DT stand-in
> only to query fw_cfg's hardware specifics).

If you want to try to support "firmware might also be reading
fw_cfg at the same time as the kernel" this is a (painful)
problem regardless of how the kernel figures out whether a
fw_cfg device is present. I had assumed that one of the design
assumptions of this series was that firmware would only
read the fw_cfg before booting the guest kernel and never touch
it afterwards. If it might touch it later then letting the
guest kernel also mess with fw_cfg seems like a really bad idea.

thanks
-- PMM
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1239547

From"Gabriel L. Somlo" <somlo@cmu.edu>
Date2015-10-05 15:20 +0200
Message-ID<qgdQm-8oB-17@gated-at.bofh.it>
In reply to#1239520
On Mon, Oct 05, 2015 at 01:50:47PM +0100, Peter Maydell wrote:
> On 5 October 2015 at 13:40, Gabriel L. Somlo <somlo@cmu.edu> wrote:
> > In addition, Michael's comment earlier in the thread suggests that
> > even my current acpi version isn't sufficiently "orthodox" w.r.t.
> > ACPI, and I should be providing the hardware access routine as
> > an ACPI/AML routine, to avoid race conditions with the rest of ACPI,
> > and for encapsulation. I.e. it's even rude to use the fw_cfg node's
> > ACPI _CRS method (the part where I'd be treating it like a DT stand-in
> > only to query fw_cfg's hardware specifics).
> 
> If you want to try to support "firmware might also be reading
> fw_cfg at the same time as the kernel" this is a (painful)
> problem regardless of how the kernel figures out whether a
> fw_cfg device is present. I had assumed that one of the design
> assumptions of this series was that firmware would only
> read the fw_cfg before booting the guest kernel and never touch
> it afterwards. If it might touch it later then letting the
> guest kernel also mess with fw_cfg seems like a really bad idea.

I don't know of any case where firmware and kernel might race each
other to access fw_cfg.

The issue AFAICT is whether it's safe (future-proof) to rely on
parsing _CRS for the fw_cfg i/o access information, or whether
such logic could be rendered obsolete by potential future updates
to fw_cfg's _CRS. If I "outsource" the fw_cfg_dump_blob_by_key()
functionality entirely to an ACPI method, my kernel driver won't
have to worry about keeping up with said future updates.

On the down-side, that means the kernel driver will be ACPI or
nothing (but I'm OK with that, at my curent level of understanding :)

Thanks,
--Gabriel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1239553

FromPaolo Bonzini <pbonzini@redhat.com>
Date2015-10-05 15:20 +0200
Message-ID<qgdQn-8oB-33@gated-at.bofh.it>
In reply to#1239520

On 05/10/2015 14:50, Peter Maydell wrote:
> If you want to try to support "firmware might also be reading
> fw_cfg at the same time as the kernel" this is a (painful)
> problem regardless of how the kernel figures out whether a
> fw_cfg device is present. I had assumed that one of the design
> assumptions of this series was that firmware would only
> read the fw_cfg before booting the guest kernel and never touch
> it afterwards. If it might touch it later then letting the
> guest kernel also mess with fw_cfg seems like a really bad idea.

The idea of tinkering with fw_cfg from the AML code (DSDT/SSDT) has been
proposed many times, and always dropped.  One of the reasons was that
the OS could have a driver for fw_cfg.

So I think that we can define the QEMU0002 id as owned by the OSPM,
similar to the various standard ACPI ids that are usually found in the
x86 world (e.g. PNP0B00 is a mc146818 RTC, PNP0303 is an 8042 keyboard
controller, PNP0501 is a 16550 or similar UART, and so on).  This
basically sanctions _CRS as the way to pass information from the
firmware to the OSPM, also similarly to those standard PNP ids.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1239528

FromMark Rutland <mark.rutland@arm.com>
Date2015-10-05 15:10 +0200
Message-ID<qgdGF-8dn-11@gated-at.bofh.it>
In reply to#1239511
> > I'm not sure I follow what the difficulty with supporting DT in addition
> > to ACPI is? It looks like all you need is a compatible string and a reg
> > entry.
> 
> Bearing in mind that I have almost no experience with arm:
> 
> I started out by probing all possible port-io and mmio locations where
> fw_cfg registers might have been found, from a "classic" module_init
> method.
> 
> Arm has DT, which as far as I understand will answer the following two
> questions: 1. Do I have fw_cfg ? 2. If yes, what address range does it use ?
> So that I could continue using a classic module_init, but won't need
> to probe for the device.
> 
> PC (my primary architecture, the one I actually care about) does not
> have DT. If I want to share the same code, I can't probe, so if I try
> DT and don't find fw_cfg there (or somehow DT is no-op-ed out because
> I'm on a PC guest), I could somehow look it up in ACPI the same way
> (i.e., use ACPI as sort of a stand-in for DT).

I'd imagine that it's simple to have something in your probe path like:

if (pdev->dev.of_node)
	parse_dt(pdev);
else
	parse_acpi(pdev);

> But all ACPI-enabled drivers I could find use dedicated macros (i.e.
> no more classic module_init() and module_exit(), but rather
> module_acpi_driver() with .add and .remove methods on an acpi_driver
> object, etc.) Not sure how I'd glue DT back into something like that.

You don't have to use those macros, and can simply use the classic
module_{init,exit} functions, calling the requisite acpi driver
registration functions at module {init,exit} time.

> In addition, Michael's comment earlier in the thread suggests that
> even my current acpi version isn't sufficiently "orthodox" w.r.t.
> ACPI, and I should be providing the hardware access routine as
> an ACPI/AML routine, to avoid race conditions with the rest of ACPI,
> and for encapsulation. I.e. it's even rude to use the fw_cfg node's
> ACPI _CRS method (the part where I'd be treating it like a DT stand-in
> only to query fw_cfg's hardware specifics).

As Peter stated, this sounds very much like it rules out sharing the
interface with FW generally (and is certainly scary).

> So far, all the information I've been able to pull together points
> away from a dual DT + ACPI all-in-one solution for fw_cfg. If you know
> of an example where that's done in an acceptable way, please let
> me know so I can use it for inspiration...

I'm not immediately aware, but I would imagine you could search for
files that had both an of_match_table and a acpi_bus_register_driver
call.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1240192

FromLaszlo Ersek <lersek@redhat.com>
Date2015-10-06 09:20 +0200
Message-ID<qguHx-7x1-39@gated-at.bofh.it>
In reply to#1239528
On 10/05/15 15:05, Mark Rutland wrote:
>>> I'm not sure I follow what the difficulty with supporting DT in addition
>>> to ACPI is? It looks like all you need is a compatible string and a reg
>>> entry.
>>
>> Bearing in mind that I have almost no experience with arm:
>>
>> I started out by probing all possible port-io and mmio locations where
>> fw_cfg registers might have been found, from a "classic" module_init
>> method.
>>
>> Arm has DT, which as far as I understand will answer the following two
>> questions: 1. Do I have fw_cfg ? 2. If yes, what address range does it use ?
>> So that I could continue using a classic module_init, but won't need
>> to probe for the device.
>>
>> PC (my primary architecture, the one I actually care about) does not
>> have DT. If I want to share the same code, I can't probe, so if I try
>> DT and don't find fw_cfg there (or somehow DT is no-op-ed out because
>> I'm on a PC guest), I could somehow look it up in ACPI the same way
>> (i.e., use ACPI as sort of a stand-in for DT).
> 
> I'd imagine that it's simple to have something in your probe path like:
> 
> if (pdev->dev.of_node)
> 	parse_dt(pdev);
> else
> 	parse_acpi(pdev);
> 
>> But all ACPI-enabled drivers I could find use dedicated macros (i.e.
>> no more classic module_init() and module_exit(), but rather
>> module_acpi_driver() with .add and .remove methods on an acpi_driver
>> object, etc.) Not sure how I'd glue DT back into something like that.
> 
> You don't have to use those macros, and can simply use the classic
> module_{init,exit} functions, calling the requisite acpi driver
> registration functions at module {init,exit} time.
> 
>> In addition, Michael's comment earlier in the thread suggests that
>> even my current acpi version isn't sufficiently "orthodox" w.r.t.
>> ACPI, and I should be providing the hardware access routine as
>> an ACPI/AML routine, to avoid race conditions with the rest of ACPI,
>> and for encapsulation. I.e. it's even rude to use the fw_cfg node's
>> ACPI _CRS method (the part where I'd be treating it like a DT stand-in
>> only to query fw_cfg's hardware specifics).
> 
> As Peter stated, this sounds very much like it rules out sharing the
> interface with FW generally (and is certainly scary).
> 
>> So far, all the information I've been able to pull together points
>> away from a dual DT + ACPI all-in-one solution for fw_cfg. If you know
>> of an example where that's done in an acceptable way, please let
>> me know so I can use it for inspiration...
> 
> I'm not immediately aware, but I would imagine you could search for
> files that had both an of_match_table and a acpi_bus_register_driver
> call.

One file that I think is an example for this (and I have looked at
before) is: "drivers/virtio/virtio_mmio.c".

Virtio-mmio is supposed to be enumerable in both ACPI and DT virtual
machines. For the QEMU side, grep QEMU for "LNRO0005" vs. "virtio,mmio".

Thanks
Laszlo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web