[Qemu-devel] Re: 2.6.21 kernel on emulated/real Malta board
On Sat, 16 Jun 2007, Aurelien Jarno wrote:
> Since I switch to 2.6.21 kernel on my emulated Malta board (QEMU), I
> have noticed something strange. The kernel starts to boot up to the
> timer calibration, and then it restart the boot again. A small example:
>
> ...
>
> Primary data cache 0kB, 4-way, linesize 0 bytes.
> Synthesized TLB refill handler (36 instructions).
> Synthesized TLB load handler fastpath (48 instructions).
> Synthesized TLB store handler fastpath (48 instructions).
> Synthesized TLB modify handler fastpath (47 instructions).
> Enable cache parity protection for MIPS 20KC/25KF CPUs.
> PID hash table entries: 512 (order: 9, 4096 bytes)
> CPU frequency 100.00 MHz
> Using 100.003 MHz high precision timer.
> Linux version 2.6.21.1 ([EMAIL PROTECTED]) (gcc version 4.1.1 ()) #1 Tue
> May 15 12:22:07 CEST 2007
>
> LINUX started...
> CPU revision is: 000182a0
> FPU revision is: 000f8200
>
> ...
>
> I have traced the problem down to the CONFIG_EARLY_PRINTK option.
> Disabling it in the .config file (be aware that running make *config
> will reenable this function), removes the problem.
I guess it's just the printk buffer that's being output again to the new
console, when the console subsystem switches from early console to real
console.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [EMAIL PROTECTED]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH v5 2/5] gpiolib: Add support for GPIO line table lookup
Currently GPIOs can only be referred to by GPIO controller and offset in
GPIO lookup tables.
Add support for looking them up by line name.
Rename gpiod_lookup.chip_label to gpiod_lookup.key, to make it clear
that this field can have two meanings, and update the kerneldoc and
GPIO_LOOKUP*() macros.
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Ulrich Hecht
Reviewed-by: Eugeniu Rosca
Tested-by: Eugeniu Rosca
---
v5:
- Add Reviewed-by, Tested-by,
v4:
- Add Reviewed-by,
- Rename gpiod_lookup.chip_label.
- Use U16_MAX instead of (u16)-1,
v3:
- New.
---
drivers/gpio/gpiolib.c | 22 +-
include/linux/gpio/machine.h | 15 ---
2 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 200c2d2be4b78043..24c02167f9e5472f 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4453,7 +4453,7 @@ static struct gpio_desc *gpiod_find(struct device *dev,
const char *con_id,
if (!table)
return desc;
- for (p = &table->table[0]; p->chip_label; p++) {
+ for (p = &table->table[0]; p->key; p++) {
struct gpio_chip *chip;
/* idx must always match exactly */
@@ -4464,18 +4464,30 @@ static struct gpio_desc *gpiod_find(struct device *dev,
const char *con_id,
if (p->con_id && (!con_id || strcmp(p->con_id, con_id)))
continue;
- chip = find_chip_by_name(p->chip_label);
+ if (p->chip_hwnum == U16_MAX) {
+ desc = gpio_name_to_desc(p->key);
+ if (desc) {
+ *flags = p->flags;
+ return desc;
+ }
+
+ dev_warn(dev, "cannot find GPIO line %s, deferring\n",
+p->key);
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+
+ chip = find_chip_by_name(p->key);
if (!chip) {
/*
* As the lookup table indicates a chip with
-* p->chip_label should exist, assume it may
+* p->key should exist, assume it may
* still appear later and let the interested
* consumer be probed again or let the Deferred
* Probe infrastructure handle the error.
*/
dev_warn(dev, "cannot find GPIO chip %s, deferring\n",
-p->chip_label);
+p->key);
return ERR_PTR(-EPROBE_DEFER);
}
@@ -4506,7 +4518,7 @@ static int platform_gpio_count(struct device *dev, const
char *con_id)
if (!table)
return -ENOENT;
- for (p = &table->table[0]; p->chip_label; p++) {
+ for (p = &table->table[0]; p->key; p++) {
if ((con_id && p->con_id && !strcmp(con_id, p->con_id)) ||
(!con_id && !p->con_id))
count++;
diff --git a/include/linux/gpio/machine.h b/include/linux/gpio/machine.h
index 1ebe5be05d5f81fa..84c66fbf54fd5811 100644
--- a/include/linux/gpio/machine.h
+++ b/include/linux/gpio/machine.h
@@ -20,8 +20,9 @@ enum gpio_lookup_flags {
/**
* struct gpiod_lookup - lookup table
- * @chip_label: name of the chip the GPIO belongs to
- * @chip_hwnum: hardware number (i.e. relative to the chip) of the GPIO
+ * @key: either the name of the chip the GPIO belongs to, or the GPIO line name
+ * @chip_hwnum: hardware number (i.e. relative to the chip) of the GPIO, or
+ * U16_MAX to indicate that @key is a GPIO line name
* @con_id: name of the GPIO from the device's point of view
* @idx: index of the GPIO in case several GPIOs share the same name
* @flags: bitmask of gpio_lookup_flags GPIO_* values
@@ -30,7 +31,7 @@ enum gpio_lookup_flags {
* functions using platform data.
*/
struct gpiod_lookup {
- const char *chip_label;
+ const char *key;
u16 chip_hwnum;
const char *con_id;
unsigned int idx;
@@ -63,17 +64,17 @@ struct gpiod_hog {
/*
* Simple definition of a single GPIO under a con_id
*/
-#define GPIO_LOOKUP(_chip_label, _chip_hwnum, _con_id, _flags) \
- GPIO_LOOKUP_IDX(_chip_label, _chip_hwnum, _con_id, 0, _flags)
+#define GPIO_LOOKUP(_key, _chip_hwnum, _con_id, _flags) \
+ GPIO_LOOKUP_IDX(_key, _chip_hwnum, _con_id, 0, _flags)
/*
* Use this macro if you need to have several GPIOs under the same con_id.
* Each GPIO needs to use a different index and can be accessed using
* gpiod_get_index()
*/
[PATCH v5 1/5] gpiolib: Add support for gpiochipN-based table lookup
Currently GPIO controllers can only be referred to by label in GPIO
lookup tables.
Add support for looking them up by "gpiochipN" name, with "N" the
corresponding GPIO device's ID number.
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Ulrich Hecht
Reviewed-by: Eugeniu Rosca
Tested-by: Eugeniu Rosca
---
v5:
- Add Reviewed-by, Tested-by,
v4:
- Add Reviewed-by,
- Drop support for legacy sysfs interface based name matching,
- Replace complex custom matching by a simple additional check in the
existing gpiochip_match_name() function,
- Add kerneldoc() for find_chip_by_name(), documenting matching order.
v3:
- New.
---
drivers/gpio/gpiolib.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 4d0106ceeba7bb24..200c2d2be4b78043 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1738,9 +1738,18 @@ static int gpiochip_match_name(struct gpio_chip *chip,
void *data)
{
const char *name = data;
- return !strcmp(chip->label, name);
+ return !strcmp(chip->label, name) ||
+ !strcmp(dev_name(&chip->gpiodev->dev), name);
}
+/**
+ * find_chip_by_name() - Find a specific gpio_chip by name
+ * @name: Name to match
+ *
+ * Return a reference to a gpio_chip that matches the passed name.
+ * This function first tries matching on the gpio_chip's label, followed by
+ * matching on dev_name() of the corresponding gpio_device.
+ */
static struct gpio_chip *find_chip_by_name(const char *name)
{
return gpiochip_find((void *)name, gpiochip_match_name);
--
2.17.1
[PATCH v5 3/5] gpio: Add GPIO Aggregator
GPIO controllers are exported to userspace using /dev/gpiochip* character devices. Access control to these devices is provided by standard UNIX file system permissions, on an all-or-nothing basis: either a GPIO controller is accessible for a user, or it is not. Currently no mechanism exists to control access to individual GPIOs. Hence add a GPIO driver to aggregate existing GPIOs, and expose them as a new gpiochip. This supports the following use cases: - Aggregating GPIOs using Sysfs This is useful for implementing access control, and assigning a set of GPIOs to a specific user or virtual machine. - Generic GPIO Driver This is useful for industrial control, where it can provide userspace access to a simple GPIO-operated device described in DT, cfr. e.g. spidev for SPI-operated devices. Signed-off-by: Geert Uytterhoeven Reviewed-by: Eugeniu Rosca Tested-by: Eugeniu Rosca --- v5: - Add Reviewed-by, Tested-by, v4: - Remove unused assignment to n in isrange(), - Check correct pointer after aggr->lookups->dev_id allocation, - Preinitialize flags to 0 in gpio_fwd_[gs]et_multiple() to avoid may-be-used-uninitialized warning, - Drop controversial GPIO repeater, - Update for gpiod_lookup.chip_label rename, - Use %pe to format error pointers, - Use U16_MAX instead of (u16)-1, - Correct comment indentation, - Use skip_spaces() helper, - Rename a and b to first_index resp. last_index, - Add comment to tmp[] use, - Improve Kconfig help text, - Include for gpiod_[gs]et_*(), - Drop unneeded valid_mask handling, - Add comment about sleeping and .set_config() support, v3: - Absorb GPIO forwarder, - Integrate GPIO Repeater and Generic GPIO driver functionality, - Use the aggregator parameters to create a GPIO lookup table instead of an array of GPIO descriptors, which allows to simplify the code: 1. This removes the need for calling gpio_name_to_desc(), gpiochip_find(), gpiochip_get_desc(), and gpiod_request(), 2. This allows the platform device to always use devm_gpiod_get_index(), regardless of the origin of the GPIOs, - Move parameter parsing from platform device probe to sysfs attribute store, removing the need for platform data passing, - Use more devm_*() functions to simplify cleanup, - Add pr_fmt(), - General refactoring, v2: - Add missing initialization of i in gpio_virt_agg_probe(), - Update for removed .need_valid_mask field and changed .init_valid_mask() signature, - Drop "virtual", rename to gpio-aggregator, - Drop bogus FIXME related to gpiod_set_transitory() expectations, - Use new GPIO Forwarder Helper, - Lift limit on the maximum number of GPIOs, - Improve parsing: - add support for specifying GPIOs by line name, - add support for specifying GPIO chips by ID, - add support for GPIO offset ranges, - names and offset specifiers must be separated by whitespace, - GPIO offsets must separated by spaces, - Use str_has_prefix() and kstrtouint(). --- drivers/gpio/Kconfig | 12 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-aggregator.c | 574 + 3 files changed, 587 insertions(+) create mode 100644 drivers/gpio/gpio-aggregator.c diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index b8013cf90064d505..b701984fdc930aa6 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -1534,6 +1534,18 @@ config GPIO_VIPERBOARD endmenu +config GPIO_AGGREGATOR + tristate "GPIO Aggregator" + help + Say yes here to enable the GPIO Aggregator, which provides a way to + aggregate existing GPIO lines into a new virtual GPIO chip. + This can serve the following purposes: + - Assign permissions for a collection of GPIO lines to a user, + - Export a collection of GPIO lines to a virtual machine, + - Provide a generic driver for a GPIO-operated device in an + industrial control context, to be operated from userspace using + the GPIO chardev interface. + config GPIO_MOCKUP tristate "GPIO Testing Driver" select IRQ_SIM diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 0b571264ddbcdb49..2a7d85a0004a6f41 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -25,6 +25,7 @@ obj-$(CONFIG_GPIO_74XX_MMIO) += gpio-74xx-mmio.o obj-$(CONFIG_GPIO_ADNP)+= gpio-adnp.o obj-$(CONFIG_GPIO_ADP5520) += gpio-adp5520.o obj-$(CONFIG_GPIO_ADP5588) += gpio-adp5588.o +obj-$(CONFIG_GPIO_AGGREGATOR) += gpio-aggregator.o obj-$(CONFIG_GPIO_ALTERA_A10SR)+= gpio-altera-a10sr.o obj-$(CONFIG_GPIO_ALTERA) += gpio-altera.o obj-$(CONFIG_GPIO_AMD8111) += gpio-amd8111.o diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-agg
[PATCH v5 5/5] MAINTAINERS: Add GPIO Aggregator section
Add a maintainership section for the GPIO Aggregator, covering documentation and driver source code. Signed-off-by: Geert Uytterhoeven Reviewed-by: Eugeniu Rosca Tested-by: Eugeniu Rosca --- v5: - Add Reviewed-by, Tested-by, v4: - Drop controversial GPIO repeater, v3: - New. --- MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 634376400709d6e8..d39f550ab1555a87 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7128,6 +7128,13 @@ F: Documentation/firmware-guide/acpi/gpio-properties.rst F: drivers/gpio/gpiolib-acpi.c F: drivers/gpio/gpiolib-acpi.h +GPIO AGGREGATOR +M: Geert Uytterhoeven +L: linux-g...@vger.kernel.org +S: Maintained +F: Documentation/admin-guide/gpio/gpio-aggregator.rst +F: drivers/gpio/gpio-aggregator.c + GPIO IR Transmitter M: Sean Young L: linux-me...@vger.kernel.org -- 2.17.1
[PATCH v5 4/5] docs: gpio: Add GPIO Aggregator documentation
Document the GPIO Aggregator, and the two typical use-cases.
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Ulrich Hecht
Reviewed-by: Eugeniu Rosca
Tested-by: Eugeniu Rosca
---
v5:
- Add Reviewed-by, Tested-by,
- Fix inconsistent indentation.
v4:
- Add Reviewed-by,
- Drop controversial GPIO repeater,
- Clarify industrial control use case,
- Fix typo s/communicated/communicate/,
- Replace abstract frobnicator example by concrete door example with
gpio-line-names,
v3:
- New.
---
.../admin-guide/gpio/gpio-aggregator.rst | 102 ++
Documentation/admin-guide/gpio/index.rst | 1 +
2 files changed, 103 insertions(+)
create mode 100644 Documentation/admin-guide/gpio/gpio-aggregator.rst
diff --git a/Documentation/admin-guide/gpio/gpio-aggregator.rst
b/Documentation/admin-guide/gpio/gpio-aggregator.rst
new file mode 100644
index ..114f72be33c2571e
--- /dev/null
+++ b/Documentation/admin-guide/gpio/gpio-aggregator.rst
@@ -0,0 +1,102 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+GPIO Aggregator
+===
+
+The GPIO Aggregator allows to aggregate GPIOs, and expose them as a new
+gpio_chip. This supports the following use cases.
+
+
+Aggregating GPIOs using Sysfs
+-
+
+GPIO controllers are exported to userspace using /dev/gpiochip* character
+devices. Access control to these devices is provided by standard UNIX file
+system permissions, on an all-or-nothing basis: either a GPIO controller is
+accessible for a user, or it is not.
+
+The GPIO Aggregator allows access control for individual GPIOs, by aggregating
+them into a new gpio_chip, which can be assigned to a group or user using
+standard UNIX file ownership and permissions. Furthermore, this simplifies and
+hardens exporting GPIOs to a virtual machine, as the VM can just grab the full
+GPIO controller, and no longer needs to care about which GPIOs to grab and
+which not, reducing the attack surface.
+
+Aggregated GPIO controllers are instantiated and destroyed by writing to
+write-only attribute files in sysfs.
+
+/sys/bus/platform/drivers/gpio-aggregator/
+
+ "new_device" ...
+ Userspace may ask the kernel to instantiate an aggregated GPIO
+ controller by writing a string describing the GPIOs to
+ aggregate to the "new_device" file, using the format
+
+ .. code-block:: none
+
+ [] [ ] ...
+
+ Where:
+
+ "" ...
+ is a GPIO line name,
+
+ "" ...
+ is a GPIO chip label or name, and
+
+ "" ...
+ is a comma-separated list of GPIO offsets and/or
+ GPIO offset ranges denoted by dashes.
+
+ Example: Instantiate a new GPIO aggregator by aggregating GPIO
+ 19 of "e6052000.gpio" and GPIOs 20-21 of "gpiochip2" into a new
+ gpio_chip:
+
+ .. code-block:: bash
+
+ echo 'e6052000.gpio 19 gpiochip2 20-21' > new_device
+
+ "delete_device" ...
+ Userspace may ask the kernel to destroy an aggregated GPIO
+ controller after use by writing its device name to the
+ "delete_device" file.
+
+ Example: Destroy the previously-created aggregated GPIO
+ controller "gpio-aggregator.0":
+
+ .. code-block:: bash
+
+ echo gpio-aggregator.0 > delete_device
+
+
+Generic GPIO Driver
+---
+
+The GPIO Aggregator can also be used as a generic driver for a simple
+GPIO-operated device described in DT, without a dedicated in-kernel driver.
+This is useful in industrial control, and is not unlike e.g. spidev, which
+allows to communicate with an SPI device from userspace.
+
+Binding a device to the GPIO Aggregator is performed either by modifying the
+gpio-aggregator driver, or by writing to the "driver_override" file in Sysfs.
+
+Example: If "door" is a GPIO-operated device described in DT, using its own
+compatible value::
+
+ door {
+ compatible = "myvendor,mydoor";
+
+ gpios = <&gpio2 19 GPIO_ACTIVE_HIGH>,
+ <&gpio2 20 GPIO_ACTIVE_LOW>;
+ gpio-line-names = "open", "lock";
+ };
+
+it can be bound to the GPIO Aggregator by either:
+
+1. Adding its compatible value to ``gpio_aggregator_dt_ids[]``,
+2. Binding manually using "driver_override":
+
+.. code-block:: bash
+
+echo gpio-aggregator > /sys/bus/platform/devices/door/driver_override
+echo door > /sys/bus/platform/drivers/gpio-aggregator/bind
diff --git a/Documentation/admin-guide/gpio/index.rst
b/D
[PATCH v5 0/5] gpio: Add GPIO Aggregator
6-18620-3-git-send-email-harish_kand...@mentor.com/)
[2] "[PATCH v4 0/5] gpio: Add GPIO Aggregator"
(https://lore.kernel.org/r/20200115181523.23556-1-geert+rene...@glider.be)
[3] "[PATCH v3 0/7] gpio: Add GPIO Aggregator/Repeater"
(https://lore.kernel.org/r/20191127084253.16356-1-geert+rene...@glider.be/)
[4] "[PATCH/RFC v2 0/5] gpio: Add GPIO Aggregator Driver"
(https://lore.kernel.org/r/20190911143858.13024-1-geert+rene...@glider.be/)
[5] "[PATCH RFC] gpio: Add Virtual Aggregator GPIO Driver"
(https://lore.kernel.org/r/20190705160536.12047-1-geert+rene...@glider.be/)
[6] "[PATCH QEMU POC] Add a GPIO backend"
(https://lore.kernel.org/r/20181003152521.23144-1-geert+rene...@glider.be/)
[7] "Getting To Blinky: Virt Edition / Making device pass-through
work on embedded ARM"
(https://fosdem.org/2019/schedule/event/vai_getting_to_blinky/)
Geert Uytterhoeven (5):
gpiolib: Add support for gpiochipN-based table lookup
gpiolib: Add support for GPIO line table lookup
gpio: Add GPIO Aggregator
docs: gpio: Add GPIO Aggregator documentation
MAINTAINERS: Add GPIO Aggregator section
.../admin-guide/gpio/gpio-aggregator.rst | 102
Documentation/admin-guide/gpio/index.rst | 1 +
MAINTAINERS | 7 +
drivers/gpio/Kconfig | 12 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-aggregator.c| 574 ++
drivers/gpio/gpiolib.c| 33 +-
include/linux/gpio/machine.h | 15 +-
8 files changed, 732 insertions(+), 13 deletions(-)
create mode 100644 Documentation/admin-guide/gpio/gpio-aggregator.rst
create mode 100644 drivers/gpio/gpio-aggregator.c
--
2.17.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v5 4/5] docs: gpio: Add GPIO Aggregator documentation
Hi Randy,
On Tue, Feb 18, 2020 at 7:30 PM Randy Dunlap wrote:
> On 2/18/20 7:18 AM, Geert Uytterhoeven wrote:
> > Document the GPIO Aggregator, and the two typical use-cases.
> >
> > Signed-off-by: Geert Uytterhoeven
> > --- /dev/null
> > +++ b/Documentation/admin-guide/gpio/gpio-aggregator.rst
> > @@ -0,0 +1,102 @@
> > +.. SPDX-License-Identifier: GPL-2.0-only
> > +
> > +GPIO Aggregator
> > +===
> > +
> > +The GPIO Aggregator allows to aggregate GPIOs, and expose them as a new
>
> "allows" really wants an object following the verb [although the kernel
> sources
> and docs have many cases of it not having an object]. Something like
>
>allows {you, one, someone, users, a user} to aggregate
Thanks for the hint!
> > + Example: Instantiate a new GPIO aggregator by aggregating GPIO
> > + 19 of "e6052000.gpio" and GPIOs 20-21 of "gpiochip2" into a
> > new
> > + gpio_chip:
> > +
> > + .. code-block:: bash
> > +
> > + echo 'e6052000.gpio 19 gpiochip2 20-21' > new_device
> > +
>
> Does the above command tell the user that the new device is named
> "gpio-aggregator.0", as used below?
Yes, it will be printed through the kernel log, cfr. the sample session in
the cover letter.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v5 2/5] gpiolib: Add support for GPIO line table lookup
On Tue, Feb 18, 2020 at 4:18 PM Geert Uytterhoeven
wrote:
> Currently GPIOs can only be referred to by GPIO controller and offset in
> GPIO lookup tables.
>
> Add support for looking them up by line name.
> Rename gpiod_lookup.chip_label to gpiod_lookup.key, to make it clear
> that this field can have two meanings, and update the kerneldoc and
> GPIO_LOOKUP*() macros.
>
> Signed-off-by: Geert Uytterhoeven
> Reviewed-by: Ulrich Hecht
> Reviewed-by: Eugeniu Rosca
> Tested-by: Eugeniu Rosca
> --- a/include/linux/gpio/machine.h
> +++ b/include/linux/gpio/machine.h
> @@ -20,8 +20,9 @@ enum gpio_lookup_flags {
>
> /**
> * struct gpiod_lookup - lookup table
> - * @chip_label: name of the chip the GPIO belongs to
> - * @chip_hwnum: hardware number (i.e. relative to the chip) of the GPIO
> + * @key: either the name of the chip the GPIO belongs to, or the GPIO line
> name
> + * @chip_hwnum: hardware number (i.e. relative to the chip) of the GPIO, or
> + * U16_MAX to indicate that @key is a GPIO line name
> * @con_id: name of the GPIO from the device's point of view
> * @idx: index of the GPIO in case several GPIOs share the same name
> * @flags: bitmask of gpio_lookup_flags GPIO_* values
> @@ -30,7 +31,7 @@ enum gpio_lookup_flags {
> * functions using platform data.
> */
> struct gpiod_lookup {
> - const char *chip_label;
> + const char *key;
> u16 chip_hwnum;
> const char *con_id;
> unsigned int idx;
This needs an update in the documentation:
--- a/Documentation/driver-api/gpio/board.rst
+++ b/Documentation/driver-api/gpio/board.rst
@@ -113,13 +113,15 @@ files that desire to do so need to include the
following header::
GPIOs are mapped by the means of tables of lookups, containing instances of the
gpiod_lookup structure. Two macros are defined to help declaring such
mappings::
- GPIO_LOOKUP(chip_label, chip_hwnum, con_id, flags)
- GPIO_LOOKUP_IDX(chip_label, chip_hwnum, con_id, idx, flags)
+ GPIO_LOOKUP(key, chip_hwnum, con_id, flags)
+ GPIO_LOOKUP_IDX(key, chip_hwnum, con_id, idx, flags)
where
- - chip_label is the label of the gpiod_chip instance providing the GPIO
- - chip_hwnum is the hardware number of the GPIO within the chip
+ - key is either the label of the gpiod_chip instance providing the GPIO, or
+the GPIO line name
+ - chip_hwnum is the hardware number of the GPIO within the chip, or U16_MAX
+to indicate that key is a GPIO line name
- con_id is the name of the GPIO function from the device point of view. It
can be NULL, in which case it will match any function.
- idx is the index of the GPIO within the function.
Furthermore, a few drivers populate the gpiod_lookup members directly,
instead of using the convenience macros:
arch/arm/mach-integrator/impd1.c
drivers/i2c/busses/i2c-i801.c
drivers/mfd/sm501.c
Either they have to be updated s/chip_label/key/, or start using the macros,
e.g.
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1444,9 +1444,9 @@ static int i801_add_mux(struct i801_priv *priv)
return -ENOMEM;
lookup->dev_id = "i2c-mux-gpio";
for (i = 0; i < mux_config->n_gpios; i++) {
- lookup->table[i].chip_label = mux_config->gpio_chip;
- lookup->table[i].chip_hwnum = mux_config->gpios[i];
- lookup->table[i].con_id = "mux";
+ lookup->table[i] = (struct gpiod_lookup)
+ GPIO_LOOKUP(mux_config->gpio_chip,
+ mux_config->gpios[i], "mux", 0);
}
gpiod_add_lookup_table(lookup);
priv->lookup = lookup;
Do you have any preference?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3 4/7] dt-bindings: gpio: Add gpio-repeater bindings
Hi Rob,
On Thu, Dec 5, 2019 at 10:06 PM Rob Herring wrote:
> On Wed, Nov 27, 2019 at 09:42:50AM +0100, Geert Uytterhoeven wrote:
> > Add Device Tree bindings for a GPIO repeater, with optional translation
> > of physical signal properties. This is useful for describing explicitly
> > the presence of e.g. an inverter on a GPIO line, and was inspired by the
> > non-YAML gpio-inverter bindings by Harish Jenny K N
> > [1].
> >
> > Note that this is different from a GPIO Nexus Node[2], which cannot do
> > physical signal property translation.
>
> It can't? Why not? The point of the passthru mask is to not do
> translation of flags, but without it you are always doing translation of
> cells.
Thanks for pushing me deeper into nexuses!
You're right, you can map from one type to another.
However, you cannot handle the "double inversion" of an ACTIVE_LOW
signal with a physical inverter added:
nexus: led-nexus {
#gpio-cells = <2>;
gpio-map = <0 0 &gpio2 19 GPIO_ACTIVE_LOW>, // inverted
<1 0 &gpio2 20 GPIO_ACTIVE_HIGH>,// noninverted
<2 0 &gpio2 21 GPIO_ACTIVE_LOW>; // inverted
gpio-map-mask = <3 0>;
// default gpio-map-pass-thru = <0 0>;
};
leds {
compatible = "gpio-leds";
led6-inverted {
gpios = <&nexus 0 GPIO_ACTIVE_HIGH>;
};
led7-noninverted {
gpios = <&nexus 1 GPIO_ACTIVE_HIGH>;
};
led8-double-inverted { // FAILS: still inverted
gpios = <&nexus 2 GPIO_ACTIVE_LOW>;
};
};
It "works" if the last entry in gpio-map is changed to GPIO_ACTIVE_HIGH.
Still, the consumer would see the final translated polarity, and not the
actual one it needs to program the consumer for.
> > While an inverter can be described implicitly by exchanging the
> > GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags, this has its limitations.
> > Each GPIO line has only a single GPIO_ACTIVE_* flag, but applies to both
> > th provider and consumer sides:
> > 1. The GPIO provider (controller) looks at the flags to know the
> > polarity, so it can translate between logical (active/not active)
> > and physical (high/low) signal levels.
> > 2. While the signal polarity is usually fixed on the GPIO consumer
> > side (e.g. an LED is tied to either the supply voltage or GND),
> > it may be configurable on some devices, and both sides need to
> > agree. Hence the GPIO_ACTIVE_* flag as seen by the consumer must
> > match the actual polarity.
> > There exists a similar issue with interrupt flags, where both the
> > interrupt controller and the device generating the interrupt need
> > to agree, which breaks in the presence of a physical inverter not
> > described in DT (see e.g. [3]).
>
> Adding an inverted flag as I've suggested would also solve this issue.
As per your suggestion in "Re: [PATCH V4 2/2] gpio: inverter: document
the inverter bindings"?
https://lore.kernel.org/linux-devicetree/cal_jsqlp___2o-nau+2ppqy0qmjx6+an3hbyz-ob9+qfvwg...@mail.gmail.com/
Oh, now I understand. I was misguided by Harish' interpretation
https://lore.kernel.org/linux-devicetree/dde73334-a26d-b53f-6b97-4101c1cdc...@mentor.com/
which assumed an "inverted" property, e.g.
inverted = /bits/ 8 <0 0 0 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0>;
But you actually meant a new GPIO_INVERTED flag, to be ORed into the 2nd
cell of a GPIO specifier? I.e. add to include/dt-bindings/gpio/gpio.h"
/* Bit 6 expresses the presence of a physical inverter */
#define GPIO_INVERTED 64
We need to be very careful in defining to which side the GPIO_ACTIVE_*
applies to (consumer?), and which side the GPIO_INVERTED flag (provider?).
Still, this doesn't help if e.g. a FET is used instead of a push-pull
inverter, as the former needs translation of other flags (which the
nexus can do, the caveats above still applies, though).
Same for adding IRQ_TYPE_INVERTED.
Related issue: how to handle physical inverters on SPI chip select lines,
if the SPI slave can be configured for both polarities?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3 2/7] gpiolib: Add support for gpiochipN-based table lookup
Hi Linus,
On Thu, Dec 12, 2019 at 2:20 PM Linus Walleij wrote:
> On Wed, Nov 27, 2019 at 9:43 AM Geert Uytterhoeven
> wrote:
> > Currently GPIO controllers can only be referred to by label in GPIO
> > lookup tables.
> >
> > Add support for looking them up by "gpiochipN" name, with "N" either the
> > corresponding GPIO device's ID number, or the GPIO controller's first
> > GPIO number.
> >
> > Signed-off-by: Geert Uytterhoeven
>
> What the commit message is missing is a rationale, why is this needed?
Right. To be added: so they can be looked up in the GPIO lookup table
using either the chip's label, or the "gpiochipN" name.
> > If this is rejected, the GPIO Aggregator documentation must be updated.
> >
> > The second variant is currently used by the legacy sysfs interface only,
> > so perhaps the chip->base check should be dropped?
>
> Anything improving the sysfs is actively discouraged by me.
> If it is just about staying compatible it is another thing.
OK, so N must be the corresponding GPIO device's ID number.
> > +static int gpiochip_match_id(struct gpio_chip *chip, void *data)
> > +{
> > + int id = (uintptr_t)data;
> > +
> > + return id == chip->base || id == chip->gpiodev->id;
> > +}
> > static struct gpio_chip *find_chip_by_name(const char *name)
> > {
> > - return gpiochip_find((void *)name, gpiochip_match_name);
> > + struct gpio_chip *chip;
> > + int id;
> > +
> > + chip = gpiochip_find((void *)name, gpiochip_match_name);
> > + if (chip)
> > + return chip;
> > +
> > + if (!str_has_prefix(name, GPIOCHIP_NAME))
> > + return NULL;
> > +
> > + if (kstrtoint(name + strlen(GPIOCHIP_NAME), 10, &id))
> > + return NULL;
> > +
> > + return gpiochip_find((void *)(uintptr_t)id, gpiochip_match_id);
>
> Isn't it easier to just augment the existing match function to
> check like this:
>
> static int gpiochip_match_name(struct gpio_chip *chip, void *data)
> {
> const char *name = data;
>
> if (!strcmp(chip->label, name))
>return 0;
return true;
> return !strcmp(dev_name(&chip->gpiodev->dev), name);
> }
Oh, didn't think of using dev_name() on the gpiodev.
Yes, with the chip->base check removed, the code can be simplified.
Or just
return !strcmp(chip->label, name) ||
!strcmp(dev_name(&chip->gpiodev->dev), name);
> We should I guess also add some kerneldoc to say we first
> match on the label and second on dev_name().
OK.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3 6/7] docs: gpio: Add GPIO Aggregator/Repeater documentation
Hi Linus,
On Thu, Dec 12, 2019 at 3:42 PM Linus Walleij wrote:
> On Wed, Nov 27, 2019 at 9:43 AM Geert Uytterhoeven
> wrote:
> > +The GPIO Aggregator allows access control for individual GPIOs, by
> > aggregating
> > +them into a new gpio_chip, which can be assigned to a group or user using
> > +standard UNIX file ownership and permissions. Furthermore, this
> > simplifies and
> > +hardens exporting GPIOs to a virtual machine, as the VM can just grab the
> > full
> > +GPIO controller, and no longer needs to care about which GPIOs to grab and
> > +which not, reducing the attack surface.
> > +
> > +Aggregated GPIO controllers are instantiated and destroyed by writing to
> > +write-only attribute files in sysfs.
>
> I suppose virtual machines will have a lengthy config file where
> they specify which GPIO lines to pick and use for their GPIO
> aggregator, and that will all be fine, the VM starts and the aggregator
> is there and we can start executing.
>
> I would perhaps point out a weakness as with all sysfs and with the current
> gpio sysfs: if a process creates an aggregator device, and then that
> process crashes, what happens when you try to restart the process and
> run e.g. your VM again?
>
> Time for a hard reboot? Or should we add some design guidelines for
> these machines so that they can cleanly tear down aggregators
> previously created by the crashed VM?
No, the VM does not create the aggregator.
The idea is for the user to create one or more aggregators, set up
permissions on /dev/gpiochipX, and launch the VM, passing the aggregated
/dev/gpiochipX as parameters.
If the VM crashes, just launch it again.
Destroying the aggregators is a manual and independent process, after
the VM has exited.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3 5/7] gpio: Add GPIO Aggregator/Repeater driver
Hi Linus,
On Thu, Dec 12, 2019 at 3:34 PM Linus Walleij wrote:
> On Wed, Nov 27, 2019 at 9:43 AM Geert Uytterhoeven
> wrote:
> > GPIO controllers are exported to userspace using /dev/gpiochip*
> > character devices. Access control to these devices is provided by
> > standard UNIX file system permissions, on an all-or-nothing basis:
> > either a GPIO controller is accessible for a user, or it is not.
> > Currently no mechanism exists to control access to individual GPIOs.
> >
> > Hence add a GPIO driver to aggregate existing GPIOs, and expose them as
> > a new gpiochip.
> >
> > This supports the following use cases:
> > 1. Aggregating GPIOs using Sysfs
> > This is useful for implementing access control, and assigning a set
> > of GPIOs to a specific user or virtual machine.
> >
> > 2. GPIO Repeater in Device Tree
> > This supports modelling e.g. GPIO inverters in DT.
> >
> > 3. Generic GPIO Driver
> > This provides userspace access to a simple GPIO-operated device
> > described in DT, cfr. e.g. spidev for SPI-operated devices.
> >
> > Signed-off-by: Geert Uytterhoeven
>
> Overall I like how this is developing!
>
> > +config GPIO_AGGREGATOR
> > + tristate "GPIO Aggregator/Repeater"
> > + help
> > + Say yes here to enable the GPIO Aggregator and repeater, which
> > + provides a way to aggregate and/or repeat existing GPIOs into a
> > new
> > + GPIO device.
>
> Should it say a "new virtual GPIO chip"?
OK.
> > + This can serve the following purposes:
> > + 1. Assign a collection of GPIOs to a user, or export them to a
> > + virtual machine,
>
> This is ambiguous. What is a "user"? A process calling from
> userspace? A device tree node?
A user is an entity with a UID, typically listed in /etc/passwd.
This is similar to letting some, not all, people on the machine access
the CD-ROM drive.
> I would write "assign a collection of GPIO lines from any lines on
> existing physical GPIO chips to form a new virtual GPIO chip"
>
> That should be to the point, right?
Yes, that's WHAT it does. The WHY is the granular access control.
> > + 2. Support GPIOs that are connected to a physical inverter,
>
> s/to/through/g
OK.
> > + 3. Provide a generic driver for a GPIO-operated device, to be
> > + controlled from userspace using the GPIO chardev interface.
>
> I don't understand this, it needs to be elaborated. What is meant
> by a "GPIO-operated device" in this context? Example?
E.g. a motor. Or a door opener.
door-opener {
compatible = "mydoor,opener";
gpios = <&gpio2 19 GPIO_ACTIVE_HIGH>;
};
You don't need a full-featured kernel driver for that, so just bind the
gpio-aggregator to the door-opener, and control it through libgpiod.
> I consistently use the term "GPIO line" as opposed to "GPIO"
> or "GPIO number" etc that are abigous, so please rephrase using
> "GPIO lines" rather than just "GPIOs" above.
OK.
> > +#include "gpiolib.h"
>
> Whenever this is included in a driver I want it to come with a comment
> explicitly stating exactly why and which internal symbols the driver
> needs to access. Ideally all drivers should just need ...
"gpiolib.h" is needed to access gpio_desc.gdev->chip in
gpio_fwd_set_config(). And for gpio_chip_hwgpio() (see below).
But indeed, I should add #include , for e.g. the
various gpiod_[gs]et_*() functions.
> > +static int aggr_add_gpio(struct gpio_aggregator *aggr, const char *label,
> > +int hwnum, unsigned int *n)
>
> u16 hwnum for the hardware number but if it is always -1/U16_MAX
> then why pass the parameter at all.
>
> Is "label" the right name of this parameter if that is going to actually
> be line_name then use that.
It's not always -1.
This function can be called either with a gpiochip label/name and an
offset, or a line-name and -1.
> > +{
> > + struct gpiod_lookup_table *lookups;
> > +
> > + lookups = krealloc(aggr->lookups, struct_size(lookups, table, *n +
> > 2),
> > + GFP_KERNEL);
> > + if (!lookups)
> > + return -ENOMEM;
> > +
> > + lookups->table[*n].chip_label = label;
>
> This is pending the discussion on whether to just use "key" for this
> name.
Which would require touching all users (board file
[PATCH v3 3/7] gpiolib: Add support for GPIO line table lookup
Currently GPIOs can only be referred to by GPIO controller and offset in
GPIO lookup tables.
Add support for looking them up by line name.
Signed-off-by: Geert Uytterhoeven
---
If this is rejected, the GPIO Aggregator documentation and code must be
updated.
v3:
- New.
---
drivers/gpio/gpiolib.c | 12
include/linux/gpio/machine.h | 2 +-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d24a3d79dcfe69ad..cb608512ad6bbded 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4475,6 +4475,18 @@ static struct gpio_desc *gpiod_find(struct device *dev,
const char *con_id,
if (p->con_id && (!con_id || strcmp(p->con_id, con_id)))
continue;
+ if (p->chip_hwnum == (u16)-1) {
+ desc = gpio_name_to_desc(p->chip_label);
+ if (desc) {
+ *flags = p->flags;
+ return desc;
+ }
+
+ dev_warn(dev, "cannot find GPIO line %s, deferring\n",
+p->chip_label);
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+
chip = find_chip_by_name(p->chip_label);
if (!chip) {
diff --git a/include/linux/gpio/machine.h b/include/linux/gpio/machine.h
index 1ebe5be05d5f81fa..84c1c097e55eefaf 100644
--- a/include/linux/gpio/machine.h
+++ b/include/linux/gpio/machine.h
@@ -31,7 +31,7 @@ enum gpio_lookup_flags {
*/
struct gpiod_lookup {
const char *chip_label;
- u16 chip_hwnum;
+ u16 chip_hwnum; /* if -1, chip_label is named line */
const char *con_id;
unsigned int idx;
unsigned long flags;
--
2.17.1
[PATCH v3 7/7] MAINTAINERS: Add GPIO Aggregator/Repeater section
Add a maintainership section for the GPIO Aggregator/Repeater, covering documentation, Device Tree bindings, and driver source code. Signed-off-by: Geert Uytterhoeven --- Harish: Do you want to be listed as maintainer, too? v3: - New. --- MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index e5949b6827b72f2b..0f12ebdaa8faa76b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7043,6 +7043,14 @@ S: Maintained F: Documentation/firmware-guide/acpi/gpio-properties.rst F: drivers/gpio/gpiolib-acpi.c +GPIO AGGREGATOR/REPEATER +M: Geert Uytterhoeven +L: linux-g...@vger.kernel.org +S: Maintained +F: Documentation/admin-guide/gpio/gpio-aggregator.rst +F: Documentation/devicetree/bindings/gpio/gpio-repeater.yaml +F: drivers/gpio/gpio-aggregator.c + GPIO IR Transmitter M: Sean Young L: linux-me...@vger.kernel.org -- 2.17.1
[PATCH v3 2/7] gpiolib: Add support for gpiochipN-based table lookup
Currently GPIO controllers can only be referred to by label in GPIO
lookup tables.
Add support for looking them up by "gpiochipN" name, with "N" either the
corresponding GPIO device's ID number, or the GPIO controller's first
GPIO number.
Signed-off-by: Geert Uytterhoeven
---
If this is rejected, the GPIO Aggregator documentation must be updated.
The second variant is currently used by the legacy sysfs interface only,
so perhaps the chip->base check should be dropped?
v3:
- New.
---
drivers/gpio/gpiolib.c | 22 +-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index c9e47620d2434983..d24a3d79dcfe69ad 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1746,9 +1746,29 @@ static int gpiochip_match_name(struct gpio_chip *chip,
void *data)
return !strcmp(chip->label, name);
}
+static int gpiochip_match_id(struct gpio_chip *chip, void *data)
+{
+ int id = (uintptr_t)data;
+
+ return id == chip->base || id == chip->gpiodev->id;
+}
+
static struct gpio_chip *find_chip_by_name(const char *name)
{
- return gpiochip_find((void *)name, gpiochip_match_name);
+ struct gpio_chip *chip;
+ int id;
+
+ chip = gpiochip_find((void *)name, gpiochip_match_name);
+ if (chip)
+ return chip;
+
+ if (!str_has_prefix(name, GPIOCHIP_NAME))
+ return NULL;
+
+ if (kstrtoint(name + strlen(GPIOCHIP_NAME), 10, &id))
+ return NULL;
+
+ return gpiochip_find((void *)(uintptr_t)id, gpiochip_match_id);
}
#ifdef CONFIG_GPIOLIB_IRQCHIP
--
2.17.1
[PATCH v3 1/7] gpiolib: Add GPIOCHIP_NAME definition
The string literal "gpiochip" is used in several places.
Add a definition for it, and use it everywhere, to make sure everything
stays in sync.
Signed-off-by: Geert Uytterhoeven
---
v3:
- New.
---
drivers/gpio/gpiolib-sysfs.c | 7 +++
drivers/gpio/gpiolib.c | 4 ++--
drivers/gpio/gpiolib.h | 2 ++
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index fbf6b1a0a4fae6ce..23e3d335cd543d53 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -762,10 +762,9 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
parent = &gdev->dev;
/* use chip->base for the ID; it's already known to be unique */
- dev = device_create_with_groups(&gpio_class, parent,
- MKDEV(0, 0),
- chip, gpiochip_groups,
- "gpiochip%d", chip->base);
+ dev = device_create_with_groups(&gpio_class, parent, MKDEV(0, 0), chip,
+ gpiochip_groups, GPIOCHIP_NAME "%d",
+ chip->base);
if (IS_ERR(dev))
return PTR_ERR(dev);
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index dce0b31f4125a6b3..c9e47620d2434983 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1419,7 +1419,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip,
void *data,
ret = gdev->id;
goto err_free_gdev;
}
- dev_set_name(&gdev->dev, "gpiochip%d", gdev->id);
+ dev_set_name(&gdev->dev, GPIOCHIP_NAME "%d", gdev->id);
device_initialize(&gdev->dev);
dev_set_drvdata(&gdev->dev, gdev);
if (chip->parent && chip->parent->driver)
@@ -5105,7 +5105,7 @@ static int __init gpiolib_dev_init(void)
return ret;
}
- ret = alloc_chrdev_region(&gpio_devt, 0, GPIO_DEV_MAX, "gpiochip");
+ ret = alloc_chrdev_region(&gpio_devt, 0, GPIO_DEV_MAX, GPIOCHIP_NAME);
if (ret < 0) {
pr_err("gpiolib: failed to allocate char dev region\n");
bus_unregister(&gpio_bus_type);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index ca9bc1e4803c2979..a4a759920faa48ab 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -16,6 +16,8 @@
#include
#include
+#define GPIOCHIP_NAME "gpiochip"
+
/**
* struct gpio_device - internal state container for GPIO devices
* @id: numerical ID number for the GPIO chip
--
2.17.1
[PATCH v3 4/7] dt-bindings: gpio: Add gpio-repeater bindings
Add Device Tree bindings for a GPIO repeater, with optional translation
of physical signal properties. This is useful for describing explicitly
the presence of e.g. an inverter on a GPIO line, and was inspired by the
non-YAML gpio-inverter bindings by Harish Jenny K N
[1].
Note that this is different from a GPIO Nexus Node[2], which cannot do
physical signal property translation.
While an inverter can be described implicitly by exchanging the
GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags, this has its limitations.
Each GPIO line has only a single GPIO_ACTIVE_* flag, but applies to both
th provider and consumer sides:
1. The GPIO provider (controller) looks at the flags to know the
polarity, so it can translate between logical (active/not active)
and physical (high/low) signal levels.
2. While the signal polarity is usually fixed on the GPIO consumer
side (e.g. an LED is tied to either the supply voltage or GND),
it may be configurable on some devices, and both sides need to
agree. Hence the GPIO_ACTIVE_* flag as seen by the consumer must
match the actual polarity.
There exists a similar issue with interrupt flags, where both the
interrupt controller and the device generating the interrupt need
to agree, which breaks in the presence of a physical inverter not
described in DT (see e.g. [3]).
[1] "[PATCH V4 2/2] gpio: inverter: document the inverter bindings"
https://lore.kernel.org/linux-gpio/1561699236-18620-3-git-send-email-harish_kand...@mentor.com/
[2] Devicetree Specification v0.3-rc2, Section 2.5
https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.3-rc2
[3] "[PATCH] wlcore/wl18xx: Add invert-irq OF property for physically
inverted IRQ"
https://lore.kernel.org/linux-renesas-soc/20190607172958.20745-1-ero...@de.adit-jv.com/
Signed-off-by: Geert Uytterhoeven
---
v3:
- New.
---
.../bindings/gpio/gpio-repeater.yaml | 53 +++
1 file changed, 53 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-repeater.yaml
diff --git a/Documentation/devicetree/bindings/gpio/gpio-repeater.yaml
b/Documentation/devicetree/bindings/gpio/gpio-repeater.yaml
new file mode 100644
index ..efdee0c3be43f731
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-repeater.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/gpio-repeater.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GPIO Repeater
+
+maintainers:
+ - Harish Jenny K N
+ - Geert Uytterhoeven
+
+description:
+ This represents a repeater for one or more GPIOs, possibly including physical
+ signal property translation (e.g. polarity inversion).
+
+properties:
+ compatible:
+const: gpio-repeater
+
+ "#gpio-cells":
+const: 2
+
+ gpio-controller: true
+
+ gpios:
+description:
+ Phandle and specifier, one for each repeated GPIO.
+
+ gpio-line-names:
+description:
+ Strings defining the names of the GPIO lines going out of the GPIO
+ controller.
+
+required:
+ - compatible
+ - "#gpio-cells"
+ - gpio-controller
+ - gpios
+
+additionalProperties: false
+
+examples:
+ # Device node describing a polarity inverter for a single GPIO
+ - |
+#include
+
+inverter: gpio-repeater {
+compatible = "gpio-repeater";
+#gpio-cells = <2>;
+gpio-controller;
+gpios = <&gpio 95 GPIO_ACTIVE_LOW>;
+};
--
2.17.1
[PATCH v3 0/7] gpio: Add GPIO Aggregator/Repeater
.be/) [4] "[PATCH QEMU POC] Add a GPIO backend" (https://lore.kernel.org/linux-renesas-soc/20181003152521.23144-1-geert+rene...@glider.be/) [5] "Getting To Blinky: Virt Edition / Making device pass-through work on embedded ARM" (https://fosdem.org/2019/schedule/event/vai_getting_to_blinky/) Geert Uytterhoeven (7): gpiolib: Add GPIOCHIP_NAME definition gpiolib: Add support for gpiochipN-based table lookup gpiolib: Add support for GPIO line table lookup dt-bindings: gpio: Add gpio-repeater bindings gpio: Add GPIO Aggregator/Repeater driver docs: gpio: Add GPIO Aggregator/Repeater documentation MAINTAINERS: Add GPIO Aggregator/Repeater section .../admin-guide/gpio/gpio-aggregator.rst | 111 Documentation/admin-guide/gpio/index.rst | 1 + .../bindings/gpio/gpio-repeater.yaml | 53 ++ MAINTAINERS | 8 + drivers/gpio/Kconfig | 13 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-aggregator.c| 587 ++ drivers/gpio/gpiolib-sysfs.c | 7 +- drivers/gpio/gpiolib.c| 38 +- drivers/gpio/gpiolib.h| 2 + include/linux/gpio/machine.h | 2 +- 11 files changed, 815 insertions(+), 8 deletions(-) create mode 100644 Documentation/admin-guide/gpio/gpio-aggregator.rst create mode 100644 Documentation/devicetree/bindings/gpio/gpio-repeater.yaml create mode 100644 drivers/gpio/gpio-aggregator.c -- 2.17.1
[PATCH v3 6/7] docs: gpio: Add GPIO Aggregator/Repeater documentation
Document the GPIO Aggregator/Repeater, and the three typical use-cases.
Signed-off-by: Geert Uytterhoeven
---
v3:
- New.
---
.../admin-guide/gpio/gpio-aggregator.rst | 111 ++
Documentation/admin-guide/gpio/index.rst | 1 +
2 files changed, 112 insertions(+)
create mode 100644 Documentation/admin-guide/gpio/gpio-aggregator.rst
diff --git a/Documentation/admin-guide/gpio/gpio-aggregator.rst
b/Documentation/admin-guide/gpio/gpio-aggregator.rst
new file mode 100644
index ..826146e260253299
--- /dev/null
+++ b/Documentation/admin-guide/gpio/gpio-aggregator.rst
@@ -0,0 +1,111 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+GPIO Aggregator/Repeater
+
+
+The GPIO Aggregator/Repeater allows to aggregate GPIOs, and expose them as a
+new gpio_chip. This supports the following use cases.
+
+
+Aggregating GPIOs using Sysfs
+-
+
+GPIO controllers are exported to userspace using /dev/gpiochip* character
+devices. Access control to these devices is provided by standard UNIX file
+system permissions, on an all-or-nothing basis: either a GPIO controller is
+accessible for a user, or it is not.
+
+The GPIO Aggregator allows access control for individual GPIOs, by aggregating
+them into a new gpio_chip, which can be assigned to a group or user using
+standard UNIX file ownership and permissions. Furthermore, this simplifies and
+hardens exporting GPIOs to a virtual machine, as the VM can just grab the full
+GPIO controller, and no longer needs to care about which GPIOs to grab and
+which not, reducing the attack surface.
+
+Aggregated GPIO controllers are instantiated and destroyed by writing to
+write-only attribute files in sysfs.
+
+/sys/bus/platform/drivers/gpio-aggregator/
+
+ "new_device" ...
+ Userspace may ask the kernel to instantiate an aggregated GPIO
+ controller by writing a string describing the GPIOs to
+ aggregate to the "new_device" file, using the format
+
+ .. code-block:: none
+
+ [] [ ] ...
+
+ Where:
+
+ "" ...
+ is a GPIO line name,
+
+ "" ...
+ is a GPIO chip label or name, and
+
+ "" ...
+ is a comma-separated list of GPIO offsets and/or
+ GPIO offset ranges denoted by dashes.
+
+ Example: Instantiate a new GPIO aggregator by aggregating GPIO
+ 19 of "e6052000.gpio" and GPIOs 20-21 of "gpiochip2" into a new
+ gpio_chip:
+
+ .. code-block:: bash
+
+ echo 'e6052000.gpio 19 gpiochip2 20-21' > new_device
+
+ "delete_device" ...
+ Userspace may ask the kernel to destroy an aggregated GPIO
+ controller after use by writing its device name to the
+ "delete_device" file.
+
+ Example: Destroy the previously-created aggregated GPIO
+ controller "gpio-aggregator.0":
+
+ .. code-block:: bash
+
+ echo gpio-aggregator.0 > delete_device
+
+
+GPIO Repeater in Device Tree
+
+
+A GPIO Repeater is a node in a Device Tree representing a repeater for one or
+more GPIOs, possibly including physical signal property translation (e.g.
+polarity inversion). This allows to model e.g. inverters in DT.
+
+See Documentation/devicetree/bindings/gpio/gpio-repeater.yaml
+
+
+Generic GPIO Driver
+---
+
+The GPIO Aggregator can also be used as a generic driver for a simple
+GPIO-operated device described in DT, without a dedicated in-kernel driver.
+This is not unlike e.g. spidev, which allows to communicated with an SPI device
+from userspace.
+
+Binding a device to the GPIO Aggregator is performed either by modifying the
+gpio-aggregator driver, or by writing to the "driver_override" file in Sysfs.
+
+Example: If "frobnicator" is a GPIO-operated device described in DT, using its
+own compatible value::
+
+frobnicator {
+compatible = "myvendor,frobnicator";
+
+gpios = <&gpio2 19 GPIO_ACTIVE_HIGH>,
+<&gpio2 20 GPIO_ACTIVE_LOW>;
+};
+
+it can be bound to the GPIO Aggregator by either:
+
+1. Adding its compatible value to ``gpio_aggregator_dt_ids[]``,
+2. Binding manually using "driver_override":
+
+.. code-block:: bash
+
+echo gpio-aggregator >
/sys/bus/platform/devices/frobnicator/driver_override
+echo frobnicator > /sys/bus/platform/drivers/gpio-aggregator/bind
diff --git a/Documentation/admin-guide/gpio/index.rst
b/Documentation/admin-guide/gpio/index.rst
index a244ba4e87d5398a..ef2838638e9
[PATCH v3 5/7] gpio: Add GPIO Aggregator/Repeater driver
GPIO controllers are exported to userspace using /dev/gpiochip*
character devices. Access control to these devices is provided by
standard UNIX file system permissions, on an all-or-nothing basis:
either a GPIO controller is accessible for a user, or it is not.
Currently no mechanism exists to control access to individual GPIOs.
Hence add a GPIO driver to aggregate existing GPIOs, and expose them as
a new gpiochip.
This supports the following use cases:
1. Aggregating GPIOs using Sysfs
This is useful for implementing access control, and assigning a set
of GPIOs to a specific user or virtual machine.
2. GPIO Repeater in Device Tree
This supports modelling e.g. GPIO inverters in DT.
3. Generic GPIO Driver
This provides userspace access to a simple GPIO-operated device
described in DT, cfr. e.g. spidev for SPI-operated devices.
Signed-off-by: Geert Uytterhoeven
---
v3:
- Absorb GPIO forwarder,
- Integrate GPIO Repeater and Generic GPIO driver functionality,
- Use the aggregator parameters to create a GPIO lookup table instead
of an array of GPIO descriptors, which allows to simplify the code:
1. This removes the need for calling gpio_name_to_desc(),
gpiochip_find(), gpiochip_get_desc(), and gpiod_request(),
2. This allows the platform device to always use
devm_gpiod_get_index(), regardless of the origin of the GPIOs,
- Move parameter parsing from platform device probe to sysfs attribute
store, removing the need for platform data passing,
- Use more devm_*() functions to simplify cleanup,
- Add pr_fmt(),
- General refactoring.
v2:
- Add missing initialization of i in gpio_virt_agg_probe(),
- Update for removed .need_valid_mask field and changed
.init_valid_mask() signature,
- Drop "virtual", rename to gpio-aggregator,
- Drop bogus FIXME related to gpiod_set_transitory() expectations,
- Use new GPIO Forwarder Helper,
- Lift limit on the maximum number of GPIOs,
- Improve parsing:
- add support for specifying GPIOs by line name,
- add support for specifying GPIO chips by ID,
- add support for GPIO offset ranges,
- names and offset specifiers must be separated by whitespace,
- GPIO offsets must separated by spaces,
- Use str_has_prefix() and kstrtouint().
---
drivers/gpio/Kconfig | 13 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-aggregator.c | 587 +
3 files changed, 601 insertions(+)
create mode 100644 drivers/gpio/gpio-aggregator.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 8adffd42f8cb0559..36b6b57a6b05e906 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1507,6 +1507,19 @@ config GPIO_VIPERBOARD
endmenu
+config GPIO_AGGREGATOR
+ tristate "GPIO Aggregator/Repeater"
+ help
+ Say yes here to enable the GPIO Aggregator and repeater, which
+ provides a way to aggregate and/or repeat existing GPIOs into a new
+ GPIO device.
+ This can serve the following purposes:
+ 1. Assign a collection of GPIOs to a user, or export them to a
+ virtual machine,
+ 2. Support GPIOs that are connected to a physical inverter,
+ 3. Provide a generic driver for a GPIO-operated device, to be
+ controlled from userspace using the GPIO chardev interface.
+
config GPIO_MOCKUP
tristate "GPIO Testing Driver"
select IRQ_SIM
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 34eb8b2b12dd656c..f9971eeb1f32335f 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_GPIO_74XX_MMIO) += gpio-74xx-mmio.o
obj-$(CONFIG_GPIO_ADNP)+= gpio-adnp.o
obj-$(CONFIG_GPIO_ADP5520) += gpio-adp5520.o
obj-$(CONFIG_GPIO_ADP5588) += gpio-adp5588.o
+obj-$(CONFIG_GPIO_AGGREGATOR) += gpio-aggregator.o
obj-$(CONFIG_GPIO_ALTERA_A10SR)+= gpio-altera-a10sr.o
obj-$(CONFIG_GPIO_ALTERA) += gpio-altera.o
obj-$(CONFIG_GPIO_AMD8111) += gpio-amd8111.o
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
new file mode 100644
index ..873578c6f9683db8
--- /dev/null
+++ b/drivers/gpio/gpio-aggregator.c
@@ -0,0 +1,587 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// GPIO Aggregator and Repeater
+//
+// Copyright (C) 2019 Glider bvba
+
+#define DRV_NAME "gpio-aggregator"
+#define pr_fmt(fmt)DRV_NAME ": " fmt
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "gpiolib.h"
+
+
+ /*
+* GPIO Aggregator sysfs interface
+*/
+
+struct gpio_aggregator {
+ struct gpiod_lookup_table *lookups;
+ struct platform_d
Re: [PATCH v3 5/7] gpio: Add GPIO Aggregator/Repeater driver
Hi Eugeniu,
On Wed, Nov 27, 2019 at 3:15 PM Eugeniu Rosca wrote:
> On Wed, Nov 27, 2019 at 09:42:51AM +0100, Geert Uytterhoeven wrote:
> > +static bool isrange(const char *s)
> > +{
> > + size_t n = strlen(s);
>
> Cppcheck 1.40-18521-ge6d692d96058:
> drivers/gpio/gpio-aggregator.c:69:11: style: Variable 'n' is assigned a value
> that is never used. [unreadVariable]
>
> Smatch v0.5.0-6150-gc1ed13e4ee7b:
> drivers/gpio/gpio-aggregator.c:69 isrange() warn: unused return: n = strlen()
Correct, this is a remainder of code present temporarily during development.
Will drop.
(where are the days gcc itself warned about that?)
> > + aggr->lookups->dev_id = kasprintf(GFP_KERNEL, "%s.%d", DRV_NAME, id);
> > + if (!aggr->lookups) {
> > + res = -ENOMEM;
> > + goto remove_idr;
> > + }
>
> s/aggr->lookups/aggr->lookups->dev_id/ ?
Thanks, will fix.
> > +static int gpio_fwd_get_multiple(struct gpio_chip *chip, unsigned long
> > *mask,
> > + unsigned long *bits)
> > +{
> > + struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
> > + unsigned long *values, flags;
>
> gcc 9.2.1:
> warning: ‘flags’ may be used uninitialized in this function
> [-Wmaybe-uninitialized]
>
> [..]
>
> > +static void gpio_fwd_set_multiple(struct gpio_chip *chip, unsigned long
> > *mask,
> > + unsigned long *bits)
> > +{
> > + struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
> > + unsigned long *values, flags;
>
> gcc 9.2.1, same as above:
> warning: ‘flags’ may be used uninitialized in this function
> [-Wmaybe-uninitialized]
So newer gcc is (again) no longer smart enough to notice the check is
the same for initializer and user...
> Should these be silenced like in 2bf593f101f3ca ("xilinx_uartps.c:
> suppress "may be used uninitialised" warning") ?
TBH, I'm not a big fan of silencing false positives.
But if people like to see flags preinitialized to zero, that can be done...
> I plan to do some runtime testing soon.
Thanks, looking forward to the results!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3 5/7] gpio: Add GPIO Aggregator/Repeater driver
Hi Harish,
On Tue, Dec 3, 2019 at 6:42 AM Harish Jenny K N
wrote:
> > +static int gpio_aggregator_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct gpio_desc **descs;
> > + struct gpiochip_fwd *fwd;
> > + int i, n;
> > +
> > + n = gpiod_count(dev, NULL);
> > + if (n < 0)
> > + return n;
> > +
> > + descs = devm_kmalloc_array(dev, n, sizeof(*descs), GFP_KERNEL);
> > + if (!descs)
> > + return -ENOMEM;
> > +
> > + for (i = 0; i < n; i++) {
> > + descs[i] = devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS);
>
> can you please add this check as well as we need to return EPROBE_DEFER.
>
> if (desc[i] == ERR_PTR(-ENOENT))
> < return -EPROBE_DEFER;
So gpiod_get_index() nevers return -EPROBE_DEFER, but returns -ENOENT
instead?
How can a driver distinguish between "GPIO not found" and "gpiochip driver
not yet initialized"?
Worse, so the *_optional() variants will return NULL in both cases, too, so
the caller will always fall back to optional GPIO not present?
Or am I missing something?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine
CC arnd On Sun, Jun 5, 2022 at 9:32 AM Stafford Horne wrote: > On Sun, Jun 05, 2022 at 10:58:14AM +0900, Stafford Horne wrote: > > On Fri, Jun 03, 2022 at 09:05:09AM +0200, Geert Uytterhoeven wrote: > > > On Thu, Jun 2, 2022 at 9:59 PM Stafford Horne wrote: > > > > On Thu, Jun 02, 2022 at 09:08:52PM +0200, Geert Uytterhoeven wrote: > > > > > On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley wrote: > > > > > > On Fri, 27 May 2022 at 17:27, Stafford Horne > > > > > > wrote: > > > > > > > This patch add the OpenRISC virtual machine 'virt' for OpenRISC. > > > > > > > This > > > > > > > platform allows for a convenient CI platform for toolchain, > > > > > > > software > > > > > > > ports and the OpenRISC linux kernel port. > > > > > > > > > > > > > > Much of this has been sourced from the m68k and riscv virt > > > > > > > platforms. > > > > > > > > > > > I enabled the options: > > > > > > > > > > > > CONFIG_RTC_CLASS=y > > > > > > # CONFIG_RTC_SYSTOHC is not set > > > > > > # CONFIG_RTC_NVMEM is not set > > > > > > CONFIG_RTC_DRV_GOLDFISH=y > > > > > > > > > > > > But it didn't work. It seems the goldfish rtc model doesn't handle a > > > > > > big endian guest running on my little endian host. > > > > > > > > > > > > Doing this fixes it: > > > > > > > > > > > > -.endianness = DEVICE_NATIVE_ENDIAN, > > > > > > +.endianness = DEVICE_HOST_ENDIAN, > > > > > > > > > > > > [0.19] goldfish_rtc 96005000.rtc: registered as rtc0 > > > > > > [0.19] goldfish_rtc 96005000.rtc: setting system clock to > > > > > > 2022-06-02T11:16:04 UTC (1654168564) > > > > > > > > > > > > But literally no other model in the tree does this, so I suspect > > > > > > it's > > > > > > not the right fix. > > > > > > > > > > Goldfish devices are supposed to be little endian. > > > > > Unfortunately m68k got this wrong, cfr. > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad > > > > > Please don't duplicate this bad behavior for new architectures > > > > > > > > Thanks for the pointer, I just wired in the goldfish RTC because I > > > > wanted to > > > > play with it. I was not attached to it. I can either remove it our > > > > find another > > > > RTC. > > > > > > Sorry for being too unclear: the mistake was not to use the Goldfish > > > RTC, but to make its register accesses big-endian. > > > Using Goldfish devices as little-endian devices should be fine. > > > > OK, then I would think this patch would be needed on Goldfish. I tested > > this > > out and it seems to work: > > Sorry, it seems maybe I mis-understood this again. In Arnd's mail [1] he, at > the end, mentions. > > It might be a good idea to revisit the qemu implementation and make > sure that the extra byteswap is only inserted on m68k and not on > other targets, but hopefully there are no new targets based on goldfish > anymore and we don't need to care. > > So, it seems that in addition to my patch we would need something in m68k to > switch it back to 'native' (big) endian? > > Looking at the m68k kernel/qemu interface I see: > > Pre 5.19: >(data) <-- kernel(readl / little) <-- m68k qemu (native / big) - RTC/PIC >(data) <-- kernel(__raw_readl / big) <-- m68k qemu (native / big) - TTY > > 5.19: >(data) <-- kernel(gf_ioread32 / big) <-- m68k qemu (native / big) - all > > The new fixes to add gf_ioread32/gf_iowrite32 fix this for goldfish and m68k. > This wouldn't have been an issue for little-endian platforms where > readl/writel > were originally used. > > Why can't m68k switch to little-endian in qemu and the kernel? The m68k virt > platform is not that old, 1 year? Are there a lot of users that this would be > a big > problem? > > [1] > https://lore.kernel.org/lkml/CAK8P3a1oN8NrUjkh2X8jHQbyz42Xo6GSa=5n0gd6vqcxrjm...@mail.gmail.com/ > > -Stafford > > > Patch: > > > > diff
Re: [PATCH v2] m68k: virt: pass RNG seed via bootinfo block
On Sun, Jun 26, 2022 at 1:18 PM Jason A. Donenfeld wrote:
> This commit wires up bootinfo's RNG seed attribute so that Linux VMs can
> have their RNG seeded from the earliest possible time in boot, just like
> the "rng-seed" device tree property on those platforms. The link
> contains the corresponding Linux patch.
>
> Link: https://lore.kernel.org/lkml/20220626111509.330159-1-ja...@zx2c4.com/
> Based-on: <20220625152318.120849-1-ja...@zx2c4.com>
> Reviewed-by: Laurent Vivier
> Signed-off-by: Jason A. Donenfeld
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine
Hi Joel,
On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley wrote:
> On Fri, 27 May 2022 at 17:27, Stafford Horne wrote:
> > This patch add the OpenRISC virtual machine 'virt' for OpenRISC. This
> > platform allows for a convenient CI platform for toolchain, software
> > ports and the OpenRISC linux kernel port.
> >
> > Much of this has been sourced from the m68k and riscv virt platforms.
> I enabled the options:
>
> CONFIG_RTC_CLASS=y
> # CONFIG_RTC_SYSTOHC is not set
> # CONFIG_RTC_NVMEM is not set
> CONFIG_RTC_DRV_GOLDFISH=y
>
> But it didn't work. It seems the goldfish rtc model doesn't handle a
> big endian guest running on my little endian host.
>
> Doing this fixes it:
>
> -.endianness = DEVICE_NATIVE_ENDIAN,
> +.endianness = DEVICE_HOST_ENDIAN,
>
> [0.19] goldfish_rtc 96005000.rtc: registered as rtc0
> [0.19] goldfish_rtc 96005000.rtc: setting system clock to
> 2022-06-02T11:16:04 UTC (1654168564)
>
> But literally no other model in the tree does this, so I suspect it's
> not the right fix.
Goldfish devices are supposed to be little endian.
Unfortunately m68k got this wrong, cfr.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
Please don't duplicate this bad behavior for new architectures.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine
Hi Stafford,
On Thu, Jun 2, 2022 at 9:59 PM Stafford Horne wrote:
> On Thu, Jun 02, 2022 at 09:08:52PM +0200, Geert Uytterhoeven wrote:
> > On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley wrote:
> > > On Fri, 27 May 2022 at 17:27, Stafford Horne wrote:
> > > > This patch add the OpenRISC virtual machine 'virt' for OpenRISC. This
> > > > platform allows for a convenient CI platform for toolchain, software
> > > > ports and the OpenRISC linux kernel port.
> > > >
> > > > Much of this has been sourced from the m68k and riscv virt platforms.
> >
> > > I enabled the options:
> > >
> > > CONFIG_RTC_CLASS=y
> > > # CONFIG_RTC_SYSTOHC is not set
> > > # CONFIG_RTC_NVMEM is not set
> > > CONFIG_RTC_DRV_GOLDFISH=y
> > >
> > > But it didn't work. It seems the goldfish rtc model doesn't handle a
> > > big endian guest running on my little endian host.
> > >
> > > Doing this fixes it:
> > >
> > > -.endianness = DEVICE_NATIVE_ENDIAN,
> > > +.endianness = DEVICE_HOST_ENDIAN,
> > >
> > > [0.19] goldfish_rtc 96005000.rtc: registered as rtc0
> > > [0.19] goldfish_rtc 96005000.rtc: setting system clock to
> > > 2022-06-02T11:16:04 UTC (1654168564)
> > >
> > > But literally no other model in the tree does this, so I suspect it's
> > > not the right fix.
> >
> > Goldfish devices are supposed to be little endian.
> > Unfortunately m68k got this wrong, cfr.
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
> > Please don't duplicate this bad behavior for new architectures
>
> Thanks for the pointer, I just wired in the goldfish RTC because I wanted to
> play with it. I was not attached to it. I can either remove it our find
> another
> RTC.
Sorry for being too unclear: the mistake was not to use the Goldfish
RTC, but to make its register accesses big-endian.
Using Goldfish devices as little-endian devices should be fine.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH RFC] hw/sh4/sh7750: Add STBCR/STBCR2 register support
The new Linux SH7750 clock driver uses the registers for power-down
mode control, causing a crash:
byte read to SH7750_STBCR_A7 (0x1fc4) not supported
Aborted (core dumped)
Fix this by adding support for the Standby Control Registers STBCR and
STBCR2.
Signed-off-by: Geert Uytterhoeven
---
[RFC PATCH v3 12/35] drivers/clk/renesas: clk-sh7750.c SH7750/7751 CPG driver.
https://lore.kernel.org/all/a772e1b6de89af22057d3af31cc03dcad7964fc7.1697199949.git.ys...@users.sourceforge.jp
Accesses to CLKSTP00 and CLKSTCLK00 (0xfe0a/0x1e0a and
0xfe0a0008/0x1e0a0008) don't seem to cause any issues, although I can't
see immediately where they are handled?
---
hw/sh4/sh7750.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c
index ebe0fd96d94ca17b..deeb83b4540bbf2b 100644
--- a/hw/sh4/sh7750.c
+++ b/hw/sh4/sh7750.c
@@ -59,6 +59,9 @@ typedef struct SH7750State {
uint16_t bcr3;
uint32_t bcr4;
uint16_t rfcr;
+/* Power-Down Modes */
+uint8_t stbcr;
+uint8_t stbcr2;
/* PCMCIA controller */
uint16_t pcr;
/* IO ports */
@@ -219,7 +222,13 @@ static void ignore_access(const char *kind, hwaddr addr)
static uint32_t sh7750_mem_readb(void *opaque, hwaddr addr)
{
+SH7750State *s = opaque;
+
switch (addr) {
+case SH7750_STBCR_A7:
+return s->stbcr;
+case SH7750_STBCR2_A7:
+return s->stbcr2;
default:
error_access("byte read", addr);
abort();
@@ -318,14 +327,24 @@ static uint32_t sh7750_mem_readl(void *opaque, hwaddr
addr)
static void sh7750_mem_writeb(void *opaque, hwaddr addr,
uint32_t mem_value)
{
+SH7750State *s = opaque;
if (is_in_sdrmx(addr, 2) || is_in_sdrmx(addr, 3)) {
ignore_access("byte write", addr);
return;
}
-error_access("byte write", addr);
-abort();
+switch (addr) {
+case SH7750_STBCR_A7:
+s->stbcr = mem_value;
+return;
+case SH7750_STBCR2_A7:
+s->stbcr2 = mem_value;
+return;
+default:
+error_access("byte write", addr);
+abort();
+}
}
static void sh7750_mem_writew(void *opaque, hwaddr addr,
--
2.34.1
Re: [PATCH RFC] hw/sh4/sh7750: Add STBCR/STBCR2 register support
Hi Adrian,
On Wed, Oct 18, 2023 at 2:46 PM John Paul Adrian Glaubitz
wrote:
> On Wed, 2023-10-18 at 14:40 +0200, Geert Uytterhoeven wrote:
> > The new Linux SH7750 clock driver uses the registers for power-down
> > mode control, causing a crash:
> >
> > byte read to SH7750_STBCR_A7 (0x1fc4) not supported
> > Aborted (core dumped)
> >
> > Fix this by adding support for the Standby Control Registers STBCR and
> > STBCR2.
> >
> > Signed-off-by: Geert Uytterhoeven
>
> Is this supposed to be applied on top of Yoshinori's DT conversion series?
No, it's a patch for QEMU. Sorry for the confusion.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH RFC] hw/sh4/sh7750: Add STBCR/STBCR2 register support
Hi Sato-san,
On Thu, Oct 19, 2023 at 4:03 AM Yoshinori Sato
wrote:
> On Wed, 18 Oct 2023 21:40:23 +0900,
> Geert Uytterhoeven wrote:
> > The new Linux SH7750 clock driver uses the registers for power-down
> > mode control, causing a crash:
> >
> > byte read to SH7750_STBCR_A7 (0x1fc4) not supported
> > Aborted (core dumped)
> >
> > Fix this by adding support for the Standby Control Registers STBCR and
> > STBCR2.
>
> FRQCR is also not returning the correct value, so it needs to be fixed.
I knew there would be more, hence the RFC ;-)
> Here are my changes.
> https://gitlab.com/yoshinori.sato/qemu.git
>
> It include.
> - Minimal CPG support.
> - DT support
> - Add target LANDISK.
Thank you very much!
It would be a good idea to mention this is the cover letter of your
Linux patch series, so your test audience doesn't have to fix
already-solved problems...
BTW, your commit da64d6541226a516 ("hw/sh4: sh7750.c allow access
STBCR and STBCR2.") just ignores writes, and always returns zero
when reading. This may cause issues with Linux code relying on
clock_ops.is_enabled() to return correct data.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Qemu-devel] [PATCH v2 for-3.1] hw/arm/sysbus-fdt: Only call match_fn callback if the type matches
Hi Eric,
On Tue, Nov 6, 2018 at 7:42 PM Eric Auger wrote:
> Commit af7d64ede0b9 (hw/arm/sysbus-fdt: Allow device matching with DT
> compatible value) introduced a match_fn callback which gets called
> for each registered combo to check whether a sysbus device can be
> dynamically instantiated. However the callback gets called even if
> the device type does not match the binding combo typename field.
> This causes an assert when passing "-device ramfb" to the qemu
> command line as vfio_platform_match() gets called on a non
> vfio-platform device.
>
> To fix this regression, let's change the add_fdt_node() logic so
> that we first check the type and if the match_fn callback is defined,
> then we also call it.
>
> Binding combos only requesting a type check do not define the
> match_fn callback.
>
> Fixes: af7d64ede0b9 (hw/arm/sysbus-fdt: Allow device matching with
> DT compatible value)
>
> Signed-off-by: Eric Auger
> Reported-by: Thomas Huth
>
> ---
>
> v1 -> v2:
> - use "if (!iter->match_fn || iter->match_fn(sbdev, iter)) {"
> as suggested by Peter to avoid code duplication
> - mention the ramfb regression fixed by this patch in the
> commit message.
Thanks for looking into this!
After applying "hw/arm/sysbus-fdt: Add support for instantiating generic
devices", "-device vfio-platform,host=ee30.sata" still works fine.
Tested-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[Qemu-devel] [PATCH qemu v5 3/3] hw/arm/virt: Allow dynamic vfio-platform devices again
Allow the instantation of generic dynamic vfio-platform devices again,
without the need to create a new device-specific vfio type.
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Eric Auger
Tested-by: Eric Auger
---
v5:
- Drop reference to commit 6f2062b9758ebc64 ("hw/arm/virt: Allow only
supported dynamic sysbus devices"),
- Add Reviewed-by, Tested-by,
v4:
- s/sysbus/vfio-platform/ in patch description,
v3:
- Drop RFC state,
v2:
- Restrict from TYPE_SYS_BUS_DEVICE to TYPE_VFIO_PLATFORM, as
suggested by Eric Auger.
---
hw/arm/virt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 0b57f87abcbfd54b..e33f7776c72fa9d0 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1758,6 +1758,7 @@ static void virt_machine_class_init(ObjectClass *oc, void
*data)
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_CALXEDA_XGMAC);
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
+machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_PLATFORM);
mc->block_default_type = IF_VIRTIO;
mc->no_cdrom = 1;
mc->pci_allow_0_address = true;
--
2.17.1
[Qemu-devel] [PATCH qemu v5 2/3] hw/arm/sysbus-fdt: Allow device matching with DT compatible value
From: Auger Eric
Up to now we have relied on the device type to identify a device tree
node creation function. Since we would like the vfio-platform device to
be instantiable with different compatible strings we introduce the
capability to specialize the node creation depending on actual
compatible value.
NodeCreationPair is renamed into BindingEntry. The struct is enhanced
with compat and match_fn() fields. We introduce a new matching function
adapted to the vfio-platform generic device.
Soon, the AMD XGBE can be instantiated with either manner, i.e.:
-device vfio-amd-xgbe,host=e090.xgmac
or using the new option line:
-device vfio-platform,host=e090.xgmac
Signed-off-by: Eric Auger
[geert: Match using compatible values in sysfs instead of user-supplied
manufacturer/model options, reword]
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Eric Auger
Tested-by: Eric Auger
---
v5:
- s/instantiatable/instantiable/,
- Add Reviewed-by,
v4:
- Add Tested-by (with amd-xgbe),
- s/From now on/Soon/ in patch description,
v3:
- Match using the compatible values from sysfs instead of using
user-supplied manufacturer and model options,
- Reword patch description,
- Drop RFC state,
v2:
- No changes,
v1:
- No changes,
v0:
- Original version from Eric.
---
hw/arm/sysbus-fdt.c | 61 ++---
1 file changed, 47 insertions(+), 14 deletions(-)
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index 43d6a7bb48ddc351..0e24c803a1c2c734 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -50,11 +50,13 @@ typedef struct PlatformBusFDTData {
PlatformBusDevice *pbus;
} PlatformBusFDTData;
-/* struct that associates a device type name and a node creation function */
-typedef struct NodeCreationPair {
+/* struct that allows to match a device and create its FDT node */
+typedef struct BindingEntry {
const char *typename;
-int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque);
-} NodeCreationPair;
+const char *compat;
+int (*add_fn)(SysBusDevice *sbdev, void *opaque);
+bool (*match_fn)(SysBusDevice *sbdev, const struct BindingEntry *combo);
+} BindingEntry;
/* helpers */
@@ -413,6 +415,27 @@ static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev, void
*opaque)
return 0;
}
+/* DT compatible matching */
+static bool vfio_platform_match(SysBusDevice *sbdev,
+const BindingEntry *entry)
+{
+VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
+const char *compat;
+unsigned int n;
+
+for (n = vdev->num_compat, compat = vdev->compat; n > 0;
+ n--, compat += strlen(compat) + 1) {
+if (!strcmp(entry->compat, compat)) {
+return true;
+}
+}
+
+return false;
+}
+
+#define VFIO_PLATFORM_BINDING(compat, add_fn) \
+{TYPE_VFIO_PLATFORM, (compat), (add_fn), vfio_platform_match}
+
#endif /* CONFIG_LINUX */
static int no_fdt_node(SysBusDevice *sbdev, void *opaque)
@@ -420,14 +443,23 @@ static int no_fdt_node(SysBusDevice *sbdev, void *opaque)
return 0;
}
-/* list of supported dynamic sysbus devices */
-static const NodeCreationPair add_fdt_node_functions[] = {
+/* Device type based matching */
+static bool type_match(SysBusDevice *sbdev, const BindingEntry *entry)
+{
+return !strcmp(object_get_typename(OBJECT(sbdev)), entry->typename);
+}
+
+#define TYPE_BINDING(type, add_fn) {(type), NULL, (add_fn), type_match}
+
+/* list of supported dynamic sysbus bindings */
+static const BindingEntry bindings[] = {
#ifdef CONFIG_LINUX
-{TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node},
-{TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node},
+TYPE_BINDING(TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node),
+TYPE_BINDING(TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node),
+VFIO_PLATFORM_BINDING("amd,xgbe-seattle-v1a", add_amd_xgbe_fdt_node),
#endif
-{TYPE_RAMFB_DEVICE, no_fdt_node},
-{"", NULL}, /* last element */
+TYPE_BINDING(TYPE_RAMFB_DEVICE, no_fdt_node),
+TYPE_BINDING("", NULL), /* last element */
};
/* Generic Code */
@@ -446,10 +478,11 @@ static void add_fdt_node(SysBusDevice *sbdev, void
*opaque)
{
int i, ret;
-for (i = 0; i < ARRAY_SIZE(add_fdt_node_functions); i++) {
-if (!strcmp(object_get_typename(OBJECT(sbdev)),
-add_fdt_node_functions[i].typename)) {
-ret = add_fdt_node_functions[i].add_fdt_node_fn(sbdev, opaque);
+for (i = 0; i < ARRAY_SIZE(bindings); i++) {
+const BindingEntry *iter = &bindings[i];
+
+if (iter->match_fn(sbdev, iter)) {
+ret = iter->add_fn(sbdev, opaque);
assert(!ret);
return;
}
--
2.17.1
[Qemu-devel] [PATCH qemu v5 1/3] vfio/platform: Make the vfio-platform device non-abstract
From: Auger Eric
Up to now the vfio-platform device has been abstract and could not be
instantiated. The integration of a new vfio platform device required
creating a dummy derived device which only set the compatible string.
Following the few vfio-platform device integrations we have seen the
actual requested adaptation happens on device tree node creation
(sysbus-fdt).
Hence remove the abstract setting, and read the list of compatible
values from sysfs if not set by a derived device.
Update the amd-xgbe and calxeda-xgmac drivers to fill in the number of
compatible values, as there can now be more than one.
Note that sysbus-fdt does not support the instantiation of the
vfio-platform device yet.
Signed-off-by: Eric Auger
[geert: Rebase, set user_creatable=true, use compatible values in sysfs
instead of user-supplied manufacturer/model options, reword]
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Eric Auger
Tested-by: Eric Auger
---
v5:
- Fix path leak on error,
- Add Reviewed-by, Tested-by,
v4:
- Propagate g_file_get_contents() errors through errp,
v3:
- Read all compatible values from sysfs instead of using user-supplied
manufacturer and model options,
- Reword patch description,
- Drop RFC state,
v2:
- No changes,
v1:
- Rebase, Set user_creatable=true,
v0:
- Original version from Eric.
---
hw/vfio/amd-xgbe.c | 1 +
hw/vfio/calxeda-xgmac.c | 1 +
hw/vfio/platform.c | 25 -
include/hw/vfio/vfio-platform.h | 3 ++-
4 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/amd-xgbe.c b/hw/vfio/amd-xgbe.c
index 0c4ec4ba25170366..ee64a3b4a2e45bf5 100644
--- a/hw/vfio/amd-xgbe.c
+++ b/hw/vfio/amd-xgbe.c
@@ -20,6 +20,7 @@ static void amd_xgbe_realize(DeviceState *dev, Error **errp)
VFIOAmdXgbeDeviceClass *k = VFIO_AMD_XGBE_DEVICE_GET_CLASS(dev);
vdev->compat = g_strdup("amd,xgbe-seattle-v1a");
+vdev->num_compat = 1;
k->parent_realize(dev, errp);
}
diff --git a/hw/vfio/calxeda-xgmac.c b/hw/vfio/calxeda-xgmac.c
index 24cee6d06512c1b6..e7767c4b021be566 100644
--- a/hw/vfio/calxeda-xgmac.c
+++ b/hw/vfio/calxeda-xgmac.c
@@ -20,6 +20,7 @@ static void calxeda_xgmac_realize(DeviceState *dev, Error
**errp)
VFIOCalxedaXgmacDeviceClass *k = VFIO_CALXEDA_XGMAC_DEVICE_GET_CLASS(dev);
vdev->compat = g_strdup("calxeda,hb-xgmac");
+vdev->num_compat = 1;
k->parent_realize(dev, errp);
}
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 57c4a0ee2b58da70..64c1af653d145cc0 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -655,6 +655,28 @@ static void vfio_platform_realize(DeviceState *dev, Error
**errp)
goto out;
}
+if (!vdev->compat) {
+GError *gerr = NULL;
+gchar *contents;
+gsize length;
+char *path;
+
+path = g_strdup_printf("%s/of_node/compatible", vbasedev->sysfsdev);
+if (!g_file_get_contents(path, &contents, &length, &gerr)) {
+error_setg(errp, "%s", gerr->message);
+g_error_free(gerr);
+g_free(path);
+return;
+}
+g_free(path);
+vdev->compat = contents;
+for (vdev->num_compat = 0; length; vdev->num_compat++) {
+size_t skip = strlen(contents) + 1;
+contents += skip;
+length -= skip;
+}
+}
+
for (i = 0; i < vbasedev->num_regions; i++) {
if (vfio_region_mmap(vdev->regions[i])) {
error_report("%s mmap unsupported. Performance may be slow",
@@ -700,6 +722,8 @@ static void vfio_platform_class_init(ObjectClass *klass,
void *data)
dc->desc = "VFIO-based platform device assignment";
sbc->connect_irq_notifier = vfio_start_irqfd_injection;
set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+/* Supported by TYPE_VIRT_MACHINE */
+dc->user_creatable = true;
}
static const TypeInfo vfio_platform_dev_info = {
@@ -708,7 +732,6 @@ static const TypeInfo vfio_platform_dev_info = {
.instance_size = sizeof(VFIOPlatformDevice),
.class_init = vfio_platform_class_init,
.class_size = sizeof(VFIOPlatformDeviceClass),
-.abstract = true,
};
static void register_vfio_platform_dev_type(void)
diff --git a/include/hw/vfio/vfio-platform.h b/include/hw/vfio/vfio-platform.h
index 9baaa2db09b84f3e..0ee10b1d71ed2503 100644
--- a/include/hw/vfio/vfio-platform.h
+++ b/include/hw/vfio/vfio-platform.h
@@ -54,7 +54,8 @@ typedef struct VFIOPlatformDevice {
QLIST_HEAD(, VFIOINTp) intp_list; /* list of IRQs */
/* queue of pending IRQs */
QSIMPLEQ_HEAD(pending_intp_queue, VFIOINTp) pending_intp_queue;
-char *compat; /* compatibility string */
+char *compat; /* DT compatible values, separated by NUL */
+unsigned int num_compat; /* number of compatible val
[Qemu-devel] [PATCH qemu v5 0/3] vfio/sysbus-fdt: Prepare for Generic DT Pass-Through
Hi all,
This patch series prepares for exporting generic devices in DT using
vfio-platform, providing direct access from a QEMU+KVM guest to the
exported devices.
- Patches 1-2 (submitted before by Eric Auger) make the vfio-platform
device non-abstract, incl. matching using a compatible string.
- Patch 3 allows dynamic vfio-platform devices again, without needing to
create device-specific vfio types for each and every new device.
This will avoid having to write device-specific instantation methods for
each and every "simple" device using only a set of generic properties.
Devices that need more specialized handling will still be able provide
their own instantation methods.
Note that this series no longer contains "[PATCH 4/4] hw/arm/sysbus-fdt:
Add support for instantiating generic devices", following advice from Eric
Auger, as that patch needs more safeguards. Hence this series now contains
only preparative work.
Changes compared to v4 ("vfio/sysbus-fdt: Prepare for Generic DT Pass-Through",
http://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg01672.html):
- Add Reviewed-by, Tested-by,
- Fix path leak on error,
- s/instantiatable/instantiable/,
- Drop reference to commit 6f2062b9758ebc64 ("hw/arm/virt: Allow only
supported dynamic sysbus devices").
Changes compared to v3 ("hw/arm/sysbus-fdt: Generic DT Pass-Through"),
http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg05006.html):
- Propagate g_file_get_contents() errors through errp,
- Add Tested-by (with amd-xgbe),
- s/From now on/Soon/ in patch description,
- s/sysbus/vfio-platform/ in patch description,
- Postpone "[PATCH 4/4] hw/arm/sysbus-fdt: Add support for instantiating
generic devices".
Changes compared to v2 (not submitted to the mailing list):
- Use the compatible values from sysfs instead of user-supplied
manufacturer and model options,
- Replace "hw/arm/sysbus-fdt: Enable rcar-gen3-gpio dynamic
instatiation" by generic "hw/arm/sysbus-fdt: Add support for
instantiating generic devices",
- Reword patch descriptions,
- Drop RFC state,
- Drop "vfio: No-IOMMU mode support".
Changes compared to v1 ("R-Car Gen3 GPIO Pass-Through Prototype (QEMU)",
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg02716.html):
- Restrict dynamic sysbus devices to TYPE_VFIO_PLATFORM, as suggested
by Eric Auger.
This (plus the postponed "hw/arm/sysbus-fdt: Add support for instantiating
generic devices") has been tested on a Renesas Salvator-XS board with R-Car
H3 ES2.0 with SATA:
-device vfio-platform,host=ee30.sata
and GPIO (needs VFIO No-IOMMU support):
-device vfio-platform,host=e6055400.gpio
Thanks for applying!
Auger Eric (2):
vfio/platform: Make the vfio-platform device non-abstract
hw/arm/sysbus-fdt: Allow device matching with DT compatible value
Geert Uytterhoeven (1):
hw/arm/virt: Allow dynamic vfio-platform devices again
hw/arm/sysbus-fdt.c | 61 +
hw/arm/virt.c | 1 +
hw/vfio/amd-xgbe.c | 1 +
hw/vfio/calxeda-xgmac.c | 1 +
hw/vfio/platform.c | 25 +-
include/hw/vfio/vfio-platform.h | 3 +-
6 files changed, 76 insertions(+), 16 deletions(-)
--
2.17.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Qemu-devel] [PATCH qemu v2] hw/char/sh_serial: Add timeout handling to unbreak serial input
Hi Paolo,
On Tue, Sep 11, 2018 at 3:11 PM Paolo Bonzini wrote:
> On 05/09/2018 15:11, Geert Uytterhoeven wrote:
> > As of commit 18e8cf159177100e ("serial: sh-sci: increase RX FIFO trigger
> > defaults for (H)SCIF") in Linux v4.11-rc1, the serial console on the
> > QEMU SH4 target is broken: it delays serial input until enough data has
> > been received.
> >
> > Since aforementioned commit, the Linux SCIF driver programs the Receive
> > FIFO Data Count Trigger bits in the FIFO Control Register, to postpone
> > generating a receive interrupt until:
> > 1. At least the receive trigger count of bytes of data are available
> > in the receive FIFO, OR
> > 2. No further data has been received for at least 15 etu after the
> > last received data.
> >
> > While QEMU implements the former, it does not implement the latter.
> > Hence the receive interrupt is not generated until the former condition
> > is met.
> >
> > Fix this by adding basic timeout handling. As the QEMU SCIF emulation
> > ignores any serial speed programming, the timeout value used conforms to
> > a default speed of 9600 bps, which is fine for any interactive console.
> >
> > Reported-by: Rob Landley
> > Signed-off-by: Geert Uytterhoeven
> > Tested-by: Ulrich Hecht
> > Tested-by: Rob Landley
> > Tested-by: Rich Felker
> Queued, thanks.
Does that mean it should show up in qemu.git anytime soon?
Thanks, and have a nice weekend!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[Qemu-devel] [PATCH QEMU POC] Add a GPIO backend
This is a Proof-of-Concept for a GPIO backend, which allows to connect
virtual GPIOs on the guest to physical GPIOs on the host. This allows
the guest to control any external device connected to the physical
GPIOs.
Features and limitations:
- The backend uses libgpiod on Linux,
- For now only GPIO outputs are supported,
- The frontend is currently hardcoded to be the PL061 GPIO controller
on the arm virtual machine. As the generic qdev_connect_gpio_out()
API call is used, any virtual GPIO controller could do, though.
Future work:
- Adding a user-instantiable GPIO frontend (or is any of the existing
virtualized GPIO controllers already user-instantiable?),
- Proper frontend/backend interface using IDs,
- Adding a QEMU internal API for controlling multiple GPIOs at once,
- Defining an API for GPIO paravirtualization,
- ...
Example:
To connect the first three GPIOs of the virtual PL061 GPIO controller
to the GPIOs controlling the three LEDs on the Renesas Salvator-X(S)
board, add the following to your qemu command invocation:
-gpiodev e6055400.gpio,vgpios=0:1:2,gpios=11:12:13
After that, the guest can cycle through the three LEDs using:
for i in $(seq 504 506); do echo $i > /sys/class/gpio/export; done
while /bin/true; do
for i in $(seq 504 506); do
echo high > /sys/class/gpio/gpio$i/direction
sleep 1
echo low > /sys/class/gpio/gpio$i/direction
done
done
Signed-off-by: Geert Uytterhoeven
---
Thanks for your comments!
---
backends/Makefile.objs | 2 +
backends/gpiodev.c | 183 +++
configure| 29 +++
hw/arm/virt.c| 6 ++
include/sysemu/gpiodev.h | 11 +++
qemu-options.hx | 20 +
vl.c | 27 ++
7 files changed, 278 insertions(+)
create mode 100644 backends/gpiodev.c
create mode 100644 include/sysemu/gpiodev.h
diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 717fcbdae4715db1..2b5f68fedd40bea0 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -16,3 +16,5 @@ common-obj-$(call land,$(CONFIG_VHOST_USER),$(CONFIG_LINUX))
+= \
endif
common-obj-$(CONFIG_LINUX) += hostmem-memfd.o
+
+common-obj-$(CONFIG_GPIO) += gpiodev.o
diff --git a/backends/gpiodev.c b/backends/gpiodev.c
new file mode 100644
index ..8d90e150f5472463
--- /dev/null
+++ b/backends/gpiodev.c
@@ -0,0 +1,183 @@
+/*
+ * QEMU GPIO Backend
+ *
+ * Copyright (C) 2018 Glider bvba
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include
+#include
+
+#include "qemu/osdep.h"
+#include "qemu/config-file.h"
+#include "qemu/cutils.h"
+#include "qemu/error-report.h"
+#include "qemu/module.h"
+#include "qemu/option.h"
+
+#include "sysemu/gpiodev.h"
+
+#include "hw/irq.h"
+#include "hw/qdev-core.h"
+
+DeviceState *the_pl061_dev;
+
+static void gpio_irq_handler(void *opaque, int n, int level)
+{
+struct gpiod_line *line = opaque;
+int status;
+
+status = gpiod_line_set_value(line, level);
+if (status < 0) {
+struct gpiod_chip *chip = gpiod_line_get_chip(line);
+
+error_report("%s/%s: Cannot set GPIO line %u: %s",
+ gpiod_chip_name(chip), gpiod_chip_label(chip),
+ gpiod_line_offset(line), strerror(errno));
+}
+}
+
+static int gpio_connect_line(unsigned int vgpio, struct gpiod_chip *chip,
+ unsigned int gpio)
+{
+const char *name = gpiod_chip_name(chip);
+const char *label = gpiod_chip_label(chip);
+struct gpiod_line *line;
+qemu_irq irq;
+int status;
+
+if (!the_pl061_dev) {
+error_report("PL061 GPIO controller not available");
+return -1;
+}
+
+line = gpiod_chip_get_line(chip, gpio);
+if (!line) {
+error_report("%s/%s: Cannot obtain GPIO line %u: %s", name, label,
+ gpio, strerror(errno));
+return -1;
+}
+
+status = gpiod_line_request_output(line, "qemu", 0);
+if (status < 0) {
+error_report("%s/%s: Cannot request GPIO line %u for output: %s", name,
+ label, gpio, strerror(errno));
+return -1;
+}
+
+irq = qemu_allocate_irq(gpio_irq_handler, line, 0);
+qdev_connect_gpio_out(the_pl061_dev, vgpio, irq);
+
+info_report("%s/%s: Connected PL061 GPIO %u to GPIO line %u", name, label,
+vgpio, gpio);
+return 0;
+}
+
+static int gpio_count_gpios(const char *opt)
+{
+unsigned int len = 0;
+unsigned int n = 0;
+
+do {
+switch (*opt) {
+case '0' ... '9':
+len++;
+break;
+
+case ':':
+case '\0':
+if (!len) {
+retu
[Qemu-devel] [PATCH QEMU v5] hw/arm/sysbus-fdt: Add support for instantiating generic devices
Add a fallback for instantiating generic devices without a type-specific
or compatible-specific instantiation method. This will be used when no
other match is found.
Generic device instantiation avoids having to write device-specific
instantiation methods for each and every "simple" device using only a
set of generic properties. Devices that need more specialized handling
can still provide their own instantiation methods.
The generic instantiation method creates a device node with remapped
"reg" and (optional) "interrupts" properties, and copies properties from
the host, if deemed appropriate:
- In general, properties without phandles are safe to be copied.
Unfortunately, the FDT does not carry type information, hence an
explicit whitelist is used, which can be extended when needed.
- Properties related to power management (power and/or clock domain),
isolation, and pin control, are handled by the host, and must not to
be copied.
Devices nodes with subnodes are rejected, as subnodes cannot easily be
handled in a generic way.
This has been tested on a Renesas Salvator-XS board (R-Car H3 ES2.0)
with SATA, using:
-device vfio-platform,host=ee300000.sata
Signed-off-by: Geert Uytterhoeven
---
Note: Also tested with GPIO, which needs "vfio: No-IOMMU mode support":
-device vfio-platform,host=e6055400.gpio
v5:
- Replace copying of a fixed list of properties (and ignoring all
others), by scanning all properties on the host, and deciding if
they need to be ignored, copied, or rejected,
- Reject device nodes with subnodes,
- Handle edge interrupts,
v3:
- New.
---
hw/arm/sysbus-fdt.c | 238
1 file changed, 238 insertions(+)
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index ad698d4832c694be..52de8c9a040c282a 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -28,6 +28,9 @@
#ifdef CONFIG_LINUX
#include
#endif
+#ifdef CONFIG_FNMATCH
+#include
+#endif
#include "hw/arm/sysbus-fdt.h"
#include "qemu/error-report.h"
#include "sysemu/device_tree.h"
@@ -415,6 +418,232 @@ static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev,
void *opaque)
return 0;
}
+enum GenericPropertyAction {
+PROP_IGNORE,
+PROP_WARN,
+PROP_COPY,
+PROP_REJECT,
+};
+
+static struct {
+const char *name;
+enum GenericPropertyAction action;
+} generic_properties[] = {
+{ "name",PROP_IGNORE }, /* handled automatically */
+{ "phandle", PROP_IGNORE }, /* not needed for the generic case */
+
+/* The following are copied and remapped by dedicated code */
+{ "reg", PROP_IGNORE },
+{ "interrupts", PROP_IGNORE },
+
+/* The following are handled by the host */
+{ "power-domains", PROP_IGNORE }, /* power management (+ opt. clocks) */
+{ "iommus", PROP_IGNORE }, /* isolation */
+{ "resets", PROP_IGNORE }, /* isolation */
+{ "pinctrl-*", PROP_IGNORE }, /* pin control */
+
+/* Ignoring the following may or may not work, hence the warning */
+{ "gpio-ranges", PROP_WARN }, /* no support for pinctrl yet */
+{ "dmas",PROP_WARN }, /* no support for external DMACs yet */
+
+/* The following are irrelevant, as corresponding specifiers are ignored */
+{ "clock-names", PROP_IGNORE },
+{ "reset-names", PROP_IGNORE },
+{ "dma-names", PROP_IGNORE },
+
+/* Whitelist of properties not taking phandles, and thus safe to copy */
+{ "compatible", PROP_COPY },
+{ "status", PROP_COPY },
+{ "reg-names", PROP_COPY },
+{ "interrupt-names", PROP_COPY },
+{ "#*-cells",PROP_COPY },
+};
+
+#ifndef CONFIG_FNMATCH
+/* Fall back to exact string matching instead of allowing wildcards */
+static inline int fnmatch(const char *pattern, const char *string, int flags)
+{
+return strcmp(pattern, string);
+}
+#endif
+
+static enum GenericPropertyAction get_generic_property_action(const char *name)
+{
+unsigned int i;
+
+for (i = 0; i < ARRAY_SIZE(generic_properties); i++) {
+if (!fnmatch(generic_properties[i].name, name, 0)) {
+return generic_properties[i].action;
+}
+}
+
+/*
+ * Unfortunately DT properties do not carry type information,
+ * so we have to assume everything else contains a phandle,
+ * and must be rejected
+ */
+return PROP_REJECT;
+}
+
+/**
+ * copy_generic_node
+ *
+ * Copy the generic part of a DT node from the host
+ */
+static void copy_generic_node(void *host_fdt, void *guest_fdt, char *host_path,
+ char *guest_path)
+{
+int node, prop, len;
+
Re: [Qemu-devel] [PATCH QEMU v5] hw/arm/sysbus-fdt: Add support for instantiating generic devices
Hi Eric,
Thanks for your comments!
On Wed, Jan 9, 2019 at 4:56 PM Auger Eric wrote:
> On 1/3/19 10:42 AM, Geert Uytterhoeven wrote:
> > Add a fallback for instantiating generic devices without a type-specific
> > or compatible-specific instantiation method. This will be used when no
> > other match is found.
> >
> > Generic device instantiation avoids having to write device-specific
> > instantiation methods for each and every "simple" device using only a
> > set of generic properties. Devices that need more specialized handling
> > can still provide their own instantiation methods.
> >
> > The generic instantiation method creates a device node with remapped
> > "reg" and (optional) "interrupts" properties, and copies properties from
> > the host, if deemed appropriate:
> > - In general, properties without phandles are safe to be copied.
> > Unfortunately, the FDT does not carry type information, hence an
> > explicit whitelist is used, which can be extended when needed.
> > - Properties related to power management (power and/or clock domain),
> > isolation, and pin control, are handled by the host, and must not to
> > be copied.
> >
> > Devices nodes with subnodes are rejected, as subnodes cannot easily be
> > handled in a generic way.
> >
> > This has been tested on a Renesas Salvator-XS board (R-Car H3 ES2.0)
> > with SATA, using:
> >
> > -device vfio-platform,host=ee30.sata
> >
> > Signed-off-by: Geert Uytterhoeven
> > ---
> > Note: Also tested with GPIO, which needs "vfio: No-IOMMU mode support":
> >
> > -device vfio-platform,host=e6055400.gpio
> >
> > v5:
> > - Replace copying of a fixed list of properties (and ignoring all
> > others), by scanning all properties on the host, and deciding if
> > they need to be ignored, copied, or rejected,
> > - Reject device nodes with subnodes,
> > - Handle edge interrupts,
> >
> > v3:
> > - New.
> > --- a/hw/arm/sysbus-fdt.c
> > +++ b/hw/arm/sysbus-fdt.c
> > +static struct {
> > +const char *name;
> > +enum GenericPropertyAction action;
> > +} generic_properties[] = {
> > +{ "name",PROP_IGNORE }, /* handled automatically */
> > +{ "phandle", PROP_IGNORE }, /* not needed for the generic case
> > */
> > +
> > +/* The following are copied and remapped by dedicated code */
> > +{ "reg", PROP_IGNORE },
> > +{ "interrupts", PROP_IGNORE },
> Shouldn't interrupt-parent be safely ignored as remapped?
Probably. Typically interrupt-parent is present at a higher level.
And interrupts-extended should be ignored, too.
> > +
> > +/* The following are handled by the host */
> > +{ "power-domains", PROP_IGNORE }, /* power management (+ opt.
> > clocks) */
> > +{ "iommus", PROP_IGNORE }, /* isolation */
> > +{ "resets", PROP_IGNORE }, /* isolation */
> > +{ "pinctrl-*", PROP_IGNORE }, /* pin control */
> > +
> > +/* Ignoring the following may or may not work, hence the warning */
> > +{ "gpio-ranges", PROP_WARN }, /* no support for pinctrl yet */
> > +{ "dmas",PROP_WARN }, /* no support for external DMACs
> > yet */
> I would be tempted to simply reject things that may not work.
I kept gpio-ranges, as it works with my rcar-gpio proof of concept
(depends on with no-iommu support).
Without dmas, drivers are supposed to fall back to PIO. If a driver
doesn't support that, it will complain.
> > +
> > +/* The following are irrelevant, as corresponding specifiers are
> > ignored */
> > +{ "clock-names", PROP_IGNORE },
> > +{ "reset-names", PROP_IGNORE },
> > +{ "dma-names", PROP_IGNORE },
> > +
> > +/* Whitelist of properties not taking phandles, and thus safe to copy
> > */
> > +{ "compatible", PROP_COPY },
> > +{ "status", PROP_COPY },
> > +{ "reg-names", PROP_COPY },
> > +{ "interrupt-names", PROP_COPY },
> > +{ "#*-cells",PROP_COPY },
> > +};
> > +
> > +#ifndef CONFIG_FNMATCH
> > +/* Fall back to exact string matching instead of allowing wildcards */
> > +static inline int fnmatch(const char *pattern, const char *string, int
> > flags)
> >
Re: [Qemu-devel] [PATCH QEMU v5] hw/arm/sysbus-fdt: Add support for instantiating generic devices
Hi Peter,
Thanks for your comments!
On Wed, Jan 9, 2019 at 5:03 PM Peter Maydell wrote:
> On Wed, 9 Jan 2019 at 15:55, Auger Eric wrote:
> > On 1/3/19 10:42 AM, Geert Uytterhoeven wrote:
> > > Add a fallback for instantiating generic devices without a type-specific
> > > or compatible-specific instantiation method. This will be used when no
> > > other match is found.
> > >
> > > Generic device instantiation avoids having to write device-specific
> > > instantiation methods for each and every "simple" device using only a
> > > set of generic properties. Devices that need more specialized handling
> > > can still provide their own instantiation methods.
>
> > > +/* Ignoring the following may or may not work, hence the warning */
> > > +{ "gpio-ranges", PROP_WARN }, /* no support for pinctrl yet */
> > > +{ "dmas",PROP_WARN }, /* no support for external DMACs
> > > yet */
> > I would be tempted to simply reject things that may not work.
>
> More generally, this whole feature seems to be "allow things that
> might not work", isn't it? Otherwise we could just have explicit
I can remove the two PROP_WARN properties above from the list, if you prefer.
Exporting rcar-sata still works fine after that.
> whitelists for the devices we want to allow passthrough of and
> that we've tested to work.
In the end, this will become a loong list (SoC x devices)...
> I have to say I'm not really very enthusiastic about
> enhancing this to allow random device passthrough,
> because it encourages further use of this. If people
> want hardware that can be passed through they should
> put it behind a bus that can be probed and can go
> behind an IOMMU, ie pci or some equivalent. That
> is a supportable hardware mechanism. All this
> machinery feels very heath-robinson...
As no-iommu suppport is not upstream (in Qemu; it is upstream in Linux,
perhaps it should be removed?), all devices using DMA require being
behind an IOMMU.
Reality is that on embedded, on-SoC devices are usually not on a PCI bus.
But IOMMUs are present, and virtualization is wanted.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[Qemu-devel] [PATCH qemu v2] hw/char/sh_serial: Add timeout handling to unbreak serial input
As of commit 18e8cf159177100e ("serial: sh-sci: increase RX FIFO trigger
defaults for (H)SCIF") in Linux v4.11-rc1, the serial console on the
QEMU SH4 target is broken: it delays serial input until enough data has
been received.
Since aforementioned commit, the Linux SCIF driver programs the Receive
FIFO Data Count Trigger bits in the FIFO Control Register, to postpone
generating a receive interrupt until:
1. At least the receive trigger count of bytes of data are available
in the receive FIFO, OR
2. No further data has been received for at least 15 etu after the
last received data.
While QEMU implements the former, it does not implement the latter.
Hence the receive interrupt is not generated until the former condition
is met.
Fix this by adding basic timeout handling. As the QEMU SCIF emulation
ignores any serial speed programming, the timeout value used conforms to
a default speed of 9600 bps, which is fine for any interactive console.
Reported-by: Rob Landley
Signed-off-by: Geert Uytterhoeven
Tested-by: Ulrich Hecht
Tested-by: Rob Landley
Tested-by: Rich Felker
---
v2:
- Add Tested-by,
- Fix spelling in patch description.
---
hw/char/sh_serial.c | 20
1 file changed, 20 insertions(+)
diff --git a/hw/char/sh_serial.c b/hw/char/sh_serial.c
index 373a40595fd975d1..12831561a6c8b137 100644
--- a/hw/char/sh_serial.c
+++ b/hw/char/sh_serial.c
@@ -29,6 +29,7 @@
#include "hw/sh4/sh.h"
#include "chardev/char-fe.h"
#include "qapi/error.h"
+#include "qemu/timer.h"
//#define DEBUG_SERIAL
@@ -63,6 +64,8 @@ typedef struct {
int rtrg;
CharBackend chr;
+QEMUTimer *fifo_timeout_timer;
+uint64_t etu; /* Elementary Time Unit (ns) */
qemu_irq eri;
qemu_irq rxi;
@@ -314,6 +317,16 @@ static int sh_serial_can_receive1(void *opaque)
return sh_serial_can_receive(s);
}
+static void sh_serial_timeout_int(void *opaque)
+{
+sh_serial_state *s = opaque;
+
+s->flags |= SH_SERIAL_FLAG_RDF;
+if (s->scr & (1 << 6) && s->rxi) {
+qemu_set_irq(s->rxi, 1);
+}
+}
+
static void sh_serial_receive1(void *opaque, const uint8_t *buf, int size)
{
sh_serial_state *s = opaque;
@@ -330,8 +343,12 @@ static void sh_serial_receive1(void *opaque, const uint8_t
*buf, int size)
if (s->rx_cnt >= s->rtrg) {
s->flags |= SH_SERIAL_FLAG_RDF;
if (s->scr & (1 << 6) && s->rxi) {
+timer_del(s->fifo_timeout_timer);
qemu_set_irq(s->rxi, 1);
}
+} else {
+timer_mod(s->fifo_timeout_timer,
+qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 15 * s->etu);
}
}
}
@@ -402,6 +419,9 @@ void sh_serial_init(MemoryRegion *sysmem,
sh_serial_event, NULL, s, NULL, true);
}
+s->fifo_timeout_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+ sh_serial_timeout_int, s);
+s->etu = NANOSECONDS_PER_SECOND / 9600;
s->eri = eri_source;
s->rxi = rxi_source;
s->txi = txi_source;
--
2.17.1
[Qemu-devel] [PATCH v4 2/3] hw/arm/sysbus-fdt: Allow device matching with DT compatible value
From: Auger Eric
Up to now we have relied on the device type to identify a device tree
node creation function. Since we would like the vfio-platform device to
be instantiatable with different compatible strings we introduce the
capability to specialize the node creation depending on actual
compatible value.
NodeCreationPair is renamed into BindingEntry. The struct is enhanced
with compat and match_fn() fields. We introduce a new matching function
adapted to the vfio-platform generic device.
Soon, the AMD XGBE can be instantiated with either manner, i.e.:
-device vfio-amd-xgbe,host=e090.xgmac
or using the new option line:
-device vfio-platform,host=e090.xgmac
Signed-off-by: Eric Auger
[geert: Match using compatible values in sysfs instead of user-supplied
manufacturer/model options, reword]
Signed-off-by: Geert Uytterhoeven
Tested-by: Eric Auger
---
v4:
- Add Tested-by (with amd-xgbe),
- s/From now on/Soon/ in patch description,
v3:
- Match using the compatible values from sysfs instead of using
user-supplied manufacturer and model options,
- Reword patch description,
- Drop RFC state,
v2:
- No changes,
v1:
- No changes,
v0:
- Original version from Eric.
---
hw/arm/sysbus-fdt.c | 61 ++---
1 file changed, 47 insertions(+), 14 deletions(-)
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index 43d6a7bb48ddc351..0e24c803a1c2c734 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -50,11 +50,13 @@ typedef struct PlatformBusFDTData {
PlatformBusDevice *pbus;
} PlatformBusFDTData;
-/* struct that associates a device type name and a node creation function */
-typedef struct NodeCreationPair {
+/* struct that allows to match a device and create its FDT node */
+typedef struct BindingEntry {
const char *typename;
-int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque);
-} NodeCreationPair;
+const char *compat;
+int (*add_fn)(SysBusDevice *sbdev, void *opaque);
+bool (*match_fn)(SysBusDevice *sbdev, const struct BindingEntry *combo);
+} BindingEntry;
/* helpers */
@@ -413,6 +415,27 @@ static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev, void
*opaque)
return 0;
}
+/* DT compatible matching */
+static bool vfio_platform_match(SysBusDevice *sbdev,
+const BindingEntry *entry)
+{
+VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
+const char *compat;
+unsigned int n;
+
+for (n = vdev->num_compat, compat = vdev->compat; n > 0;
+ n--, compat += strlen(compat) + 1) {
+if (!strcmp(entry->compat, compat)) {
+return true;
+}
+}
+
+return false;
+}
+
+#define VFIO_PLATFORM_BINDING(compat, add_fn) \
+{TYPE_VFIO_PLATFORM, (compat), (add_fn), vfio_platform_match}
+
#endif /* CONFIG_LINUX */
static int no_fdt_node(SysBusDevice *sbdev, void *opaque)
@@ -420,14 +443,23 @@ static int no_fdt_node(SysBusDevice *sbdev, void *opaque)
return 0;
}
-/* list of supported dynamic sysbus devices */
-static const NodeCreationPair add_fdt_node_functions[] = {
+/* Device type based matching */
+static bool type_match(SysBusDevice *sbdev, const BindingEntry *entry)
+{
+return !strcmp(object_get_typename(OBJECT(sbdev)), entry->typename);
+}
+
+#define TYPE_BINDING(type, add_fn) {(type), NULL, (add_fn), type_match}
+
+/* list of supported dynamic sysbus bindings */
+static const BindingEntry bindings[] = {
#ifdef CONFIG_LINUX
-{TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node},
-{TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node},
+TYPE_BINDING(TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node),
+TYPE_BINDING(TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node),
+VFIO_PLATFORM_BINDING("amd,xgbe-seattle-v1a", add_amd_xgbe_fdt_node),
#endif
-{TYPE_RAMFB_DEVICE, no_fdt_node},
-{"", NULL}, /* last element */
+TYPE_BINDING(TYPE_RAMFB_DEVICE, no_fdt_node),
+TYPE_BINDING("", NULL), /* last element */
};
/* Generic Code */
@@ -446,10 +478,11 @@ static void add_fdt_node(SysBusDevice *sbdev, void
*opaque)
{
int i, ret;
-for (i = 0; i < ARRAY_SIZE(add_fdt_node_functions); i++) {
-if (!strcmp(object_get_typename(OBJECT(sbdev)),
-add_fdt_node_functions[i].typename)) {
-ret = add_fdt_node_functions[i].add_fdt_node_fn(sbdev, opaque);
+for (i = 0; i < ARRAY_SIZE(bindings); i++) {
+const BindingEntry *iter = &bindings[i];
+
+if (iter->match_fn(sbdev, iter)) {
+ret = iter->add_fn(sbdev, opaque);
assert(!ret);
return;
}
--
2.17.1
[Qemu-devel] [PATCH v4 0/3] vfio/sysbus-fdt: Prepare for Generic DT Pass-Through
Hi all,
This patch series prepares for exporting generic devices in DT using
vfio-platform, providing direct access from a QEMU+KVM guest to the
exported devices.
- Patches 1-2 (submitted before by Eric Auger) make the vfio-platform
device non-abstract, incl. matching using a compatible string.
- Patch 3 allows dynamic vfio-platform devices again, without needing to
create device-specific vfio types for each and every new device.
This will avoid having to write device-specific instantation methods for
each and every "simple" device using only a set of generic properties.
Devices that need more specialized handling will still be able provide
their own instantation methods.
Note that this series no longer contains "[PATCH 4/4] hw/arm/sysbus-fdt:
Add support for instantiating generic devices", following advice from Eric
Auger, as that patch needs more safeguards. Hence this series now contains
only preparative work.
Changes compared to v4 ("hw/arm/sysbus-fdt: Generic DT Pass-Through"),
http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg05006.html):
- Propagate g_file_get_contents() errors through errp,
- Add Tested-by (with amd-xgbe),
- s/From now on/Soon/ in patch description,
- s/sysbus/vfio-platform/ in patch description,
- Postpone "[PATCH 4/4] hw/arm/sysbus-fdt: Add support for instantiating
generic devices".
Changes compared to v2 (not submitted to the mailing list):
- Use the compatible values from sysfs instead of user-supplied
manufacturer and model options,
- Replace "hw/arm/sysbus-fdt: Enable rcar-gen3-gpio dynamic
instatiation" by generic "hw/arm/sysbus-fdt: Add support for
instantiating generic devices",
- Reword patch descriptions,
- Drop RFC state,
- Drop "vfio: No-IOMMU mode support".
Changes compared to v1 ("R-Car Gen3 GPIO Pass-Through Prototype (QEMU)",
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg02716.html):
- Restrict dynamic sysbus devices to TYPE_VFIO_PLATFORM, as suggested
by Eric Auger.
Thanks!
For testing, this patch series, and the postponed "hw/arm/sysbus-fdt: Add
support for instantiating generic devices" are available in the
topic/rcar3-virt-gpio-passthrough-v4 branch of my git repository at
https://github.com/geertu/qemu.git.
This has been tested on a Renesas Salvator-XS board with R-Car H3 ES2.0
with SATA:
-device vfio-platform,host=ee30.sata
and GPIO (needs VFIO No-IOMMU support):
-device vfio-platform,host=e6055400.gpio
Thanks for applying!
Auger Eric (2):
vfio/platform: Make the vfio-platform device non-abstract
hw/arm/sysbus-fdt: Allow device matching with DT compatible value
Geert Uytterhoeven (1):
hw/arm/virt: Allow dynamic vfio-platform devices again
hw/arm/sysbus-fdt.c | 61 +
hw/arm/virt.c | 1 +
hw/vfio/amd-xgbe.c | 1 +
hw/vfio/calxeda-xgmac.c | 1 +
hw/vfio/platform.c | 24 -
include/hw/vfio/vfio-platform.h | 3 +-
6 files changed, 75 insertions(+), 16 deletions(-)
--
2.17.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[Qemu-devel] [PATCH v4 1/3] vfio/platform: Make the vfio-platform device non-abstract
From: Auger Eric
Up to now the vfio-platform device has been abstract and could not be
instantiated. The integration of a new vfio platform device required
creating a dummy derived device which only set the compatible string.
Following the few vfio-platform device integrations we have seen the
actual requested adaptation happens on device tree node creation
(sysbus-fdt).
Hence remove the abstract setting, and read the list of compatible
values from sysfs if not set by a derived device.
Update the amd-xgbe and calxeda-xgmac drivers to fill in the number of
compatible values, as there can now be more than one.
Note that sysbus-fdt does not support the instantiation of the
vfio-platform device yet.
Signed-off-by: Eric Auger
[geert: Rebase, set user_creatable=true, use compatible values in sysfs
instead of user-supplied manufacturer/model options, reword]
Signed-off-by: Geert Uytterhoeven
---
v4:
- Propagate g_file_get_contents() errors through errp,
v3:
- Read all compatible values from sysfs instead of using user-supplied
manufacturer and model options,
- Reword patch description,
- Drop RFC state,
v2:
- No changes,
v1:
- Rebase, Set user_creatable=true,
v0:
- Original version from Eric.
---
hw/vfio/amd-xgbe.c | 1 +
hw/vfio/calxeda-xgmac.c | 1 +
hw/vfio/platform.c | 24 +++-
include/hw/vfio/vfio-platform.h | 3 ++-
4 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/amd-xgbe.c b/hw/vfio/amd-xgbe.c
index 0c4ec4ba25170366..ee64a3b4a2e45bf5 100644
--- a/hw/vfio/amd-xgbe.c
+++ b/hw/vfio/amd-xgbe.c
@@ -20,6 +20,7 @@ static void amd_xgbe_realize(DeviceState *dev, Error **errp)
VFIOAmdXgbeDeviceClass *k = VFIO_AMD_XGBE_DEVICE_GET_CLASS(dev);
vdev->compat = g_strdup("amd,xgbe-seattle-v1a");
+vdev->num_compat = 1;
k->parent_realize(dev, errp);
}
diff --git a/hw/vfio/calxeda-xgmac.c b/hw/vfio/calxeda-xgmac.c
index 24cee6d06512c1b6..e7767c4b021be566 100644
--- a/hw/vfio/calxeda-xgmac.c
+++ b/hw/vfio/calxeda-xgmac.c
@@ -20,6 +20,7 @@ static void calxeda_xgmac_realize(DeviceState *dev, Error
**errp)
VFIOCalxedaXgmacDeviceClass *k = VFIO_CALXEDA_XGMAC_DEVICE_GET_CLASS(dev);
vdev->compat = g_strdup("calxeda,hb-xgmac");
+vdev->num_compat = 1;
k->parent_realize(dev, errp);
}
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 57c4a0ee2b58da70..894333c602eb6fd2 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -655,6 +655,27 @@ static void vfio_platform_realize(DeviceState *dev, Error
**errp)
goto out;
}
+if (!vdev->compat) {
+GError *gerr = NULL;
+gchar *contents;
+gsize length;
+char *path;
+
+path = g_strdup_printf("%s/of_node/compatible", vbasedev->sysfsdev);
+if (!g_file_get_contents(path, &contents, &length, &gerr)) {
+error_setg(errp, "%s", gerr->message);
+g_error_free(gerr);
+return;
+}
+g_free(path);
+vdev->compat = contents;
+for (vdev->num_compat = 0; length; vdev->num_compat++) {
+size_t skip = strlen(contents) + 1;
+contents += skip;
+length -= skip;
+}
+}
+
for (i = 0; i < vbasedev->num_regions; i++) {
if (vfio_region_mmap(vdev->regions[i])) {
error_report("%s mmap unsupported. Performance may be slow",
@@ -700,6 +721,8 @@ static void vfio_platform_class_init(ObjectClass *klass,
void *data)
dc->desc = "VFIO-based platform device assignment";
sbc->connect_irq_notifier = vfio_start_irqfd_injection;
set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+/* Supported by TYPE_VIRT_MACHINE */
+dc->user_creatable = true;
}
static const TypeInfo vfio_platform_dev_info = {
@@ -708,7 +731,6 @@ static const TypeInfo vfio_platform_dev_info = {
.instance_size = sizeof(VFIOPlatformDevice),
.class_init = vfio_platform_class_init,
.class_size = sizeof(VFIOPlatformDeviceClass),
-.abstract = true,
};
static void register_vfio_platform_dev_type(void)
diff --git a/include/hw/vfio/vfio-platform.h b/include/hw/vfio/vfio-platform.h
index 9baaa2db09b84f3e..0ee10b1d71ed2503 100644
--- a/include/hw/vfio/vfio-platform.h
+++ b/include/hw/vfio/vfio-platform.h
@@ -54,7 +54,8 @@ typedef struct VFIOPlatformDevice {
QLIST_HEAD(, VFIOINTp) intp_list; /* list of IRQs */
/* queue of pending IRQs */
QSIMPLEQ_HEAD(pending_intp_queue, VFIOINTp) pending_intp_queue;
-char *compat; /* compatibility string */
+char *compat; /* DT compatible values, separated by NUL */
+unsigned int num_compat; /* number of compatible values */
uint32_t mmap_timeout; /* delay to re-enable mmaps after interrupt */
QEMUTimer *mmap_timer; /* allows fast-path resume after IRQ hit */
QemuMutex intp_mutex; /* protect the intp_list IRQ state */
--
2.17.1
[Qemu-devel] [PATCH v4 3/3] hw/arm/virt: Allow dynamic vfio-platform devices again
Allow the instantation of generic dynamic vfio-platform devices again,
without the need to create a new device-specific vfio type.
This is more or less a partial revert of commit 6f2062b9758ebc64
("hw/arm/virt: Allow only supported dynamic sysbus devices").
Signed-off-by: Geert Uytterhoeven
---
v4:
- s/sysbus/vfio-platform/ in patch description,
v3:
- Drop RFC state,
v2:
- Restrict from TYPE_SYS_BUS_DEVICE to TYPE_VFIO_PLATFORM, as
suggested by Eric Auger.
---
hw/arm/virt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 0b57f87abcbfd54b..e33f7776c72fa9d0 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1758,6 +1758,7 @@ static void virt_machine_class_init(ObjectClass *oc, void
*data)
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_CALXEDA_XGMAC);
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
+machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_PLATFORM);
mc->block_default_type = IF_VIRTIO;
mc->no_cdrom = 1;
mc->pci_allow_0_address = true;
--
2.17.1
[Qemu-devel] [PATCH] hw/arm/sysbus-fdt: Fix assertion in copy_properties_from_host()
When copy_properties_from_host() ignores the error for an optional
property, it frees the error, but fails to reset it.
Hence if two or more optional properties are missing, an assertion is
triggered:
util/error.c:57: error_setv: Assertion `*errp == NULL' failed.
Fis this by resetting err to NULL after ignoring the error.
Fixes: 9481cf2e5f2f2bb6 ("hw/arm/sysbus-fdt: helpers for clock node generation")
Signed-off-by: Geert Uytterhoeven
---
hw/arm/sysbus-fdt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index 0d4c75702c3ddfae..43d6a7bb48ddc351 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -107,6 +107,7 @@ static void copy_properties_from_host(HostProperty *props,
int nb_props,
/* mandatory property not found: bail out */
exit(1);
}
+err = NULL;
}
}
}
--
2.17.1
[Qemu-devel] [PATCH v3 3/4] hw/arm/virt: Allow dynamic sysbus devices again
Allow the instantation of generic dynamic sysbus devices again, without
the need to create a new device-specific vfio type.
This is more or less a partial revert of commit 6f2062b9758ebc64
("hw/arm/virt: Allow only supported dynamic sysbus devices").
Signed-off-by: Geert Uytterhoeven
---
v3:
- Drop RFC state,
v2:
- Restrict from TYPE_SYS_BUS_DEVICE to TYPE_VFIO_PLATFORM, as
suggested by Eric Auger.
---
hw/arm/virt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 281ddcdf6e26236d..24c1d8e28c7c4285 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1724,6 +1724,7 @@ static void virt_machine_class_init(ObjectClass *oc, void
*data)
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_CALXEDA_XGMAC);
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
+machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_PLATFORM);
mc->block_default_type = IF_VIRTIO;
mc->no_cdrom = 1;
mc->pci_allow_0_address = true;
--
2.17.1
[Qemu-devel] [PATCH v3 4/4] hw/arm/sysbus-fdt: Add support for instantiating generic devices
Add a fallback for instantiating generic devices without a type-specific
or compatible-specific instantation method. This will be used when no
other match is found.
The generic instantation method creates a device node with "reg" and
(optional) "interrupts" properties, and copies the "compatible"
property and other optional properties from the host.
For now the following optional properties are copied, if found:
- "#gpio-cells",
- "gpio-controller",
- "#interrupt-cells",
- "interrupt-controller",
- "interrupt-names".
More can be added when needed.
This avoids having to write device-specific instantation methods for
each and every "simple" device using only a set of generic properties.
Devices that need more specialized handling can still provide their
own instantation method.
This has been tested on a Renesas Salvator-XS board with R-Car H3 ES2.0
with SATA:
-device vfio-platform,host=ee30.sata
and GPIO (needs VFIO No-IOMMU support):
-device vfio-platform,host=e6055400.gpio
Signed-off-by: Geert Uytterhoeven
---
v3:
- New.
---
hw/arm/sysbus-fdt.c | 102
1 file changed, 102 insertions(+)
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index 0e24c803a1c2c734..8040af00ccf131d7 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -415,6 +415,99 @@ static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev, void
*opaque)
return 0;
}
+static HostProperty generic_copied_properties[] = {
+{"compatible", false},
+{"#gpio-cells", true},
+{"gpio-controller", true},
+{"#interrupt-cells", true},
+{"interrupt-controller", true},
+{"interrupt-names", true},
+};
+
+/**
+ * add_generic_fdt_node
+ *
+ * Generates a generic DT node by copying properties from the host
+ */
+static int add_generic_fdt_node(SysBusDevice *sbdev, void *opaque)
+{
+PlatformBusFDTData *data = opaque;
+VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
+const char *parent_node = data->pbus_node_name;
+void *guest_fdt = data->fdt, *host_fdt;
+VFIODevice *vbasedev = &vdev->vbasedev;
+char **node_path, *node_name, *dt_name;
+PlatformBusDevice *pbus = data->pbus;
+int i, irq_number;
+uint32_t *reg_attr, *irq_attr;
+uint64_t mmio_base;
+
+host_fdt = load_device_tree_from_sysfs();
+
+dt_name = sysfs_to_dt_name(vbasedev->name);
+if (!dt_name) {
+error_report("%s incorrect sysfs device name %s", __func__,
+ vbasedev->name);
+exit(1);
+}
+node_path = qemu_fdt_node_path(host_fdt, dt_name, vdev->compat,
+ &error_fatal);
+if (!node_path || !node_path[0]) {
+error_report("%s unable to retrieve node path for %s/%s", __func__,
+ dt_name, vdev->compat);
+exit(1);
+}
+
+if (node_path[1]) {
+error_report("%s more than one node matching %s/%s!", __func__,
dt_name,
+ vdev->compat);
+exit(1);
+}
+
+mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
+node_name = g_strdup_printf("%s/%s@%" PRIx64, parent_node,
+ vbasedev->name, mmio_base);
+qemu_fdt_add_subnode(guest_fdt, node_name);
+
+/* Copy generic properties */
+copy_properties_from_host(generic_copied_properties,
+ ARRAY_SIZE(generic_copied_properties), host_fdt,
+ guest_fdt, node_path[0], node_name);
+
+/* Copy reg (remapped) */
+reg_attr = g_new(uint32_t, vbasedev->num_regions * 2);
+for (i = 0; i < vbasedev->num_regions; i++) {
+mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, i);
+reg_attr[2 * i] = cpu_to_be32(mmio_base);
+reg_attr[2 * i + 1] = cpu_to_be32(
+memory_region_size(vdev->regions[i]->mem));
+}
+qemu_fdt_setprop(guest_fdt, node_name, "reg", reg_attr,
+ vbasedev->num_regions * 2 * sizeof(uint32_t));
+
+/* Copy interrupts (remapped) */
+if (vbasedev->num_irqs) {
+irq_attr = g_new(uint32_t, vbasedev->num_irqs * 3);
+for (i = 0; i < vbasedev->num_irqs; i++) {
+irq_number = platform_bus_get_irqn(pbus, sbdev , i)
+ + data->irq_start;
+irq_attr[3 * i] = cpu_to_be32(GIC_FDT_IRQ_TYPE_SPI);
+irq_attr[3 * i + 1] = cpu_to_be32(irq_number);
+irq_attr[3 * i + 2] = cpu_to_be32(GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+}
+qemu_fdt_setprop(guest_fdt, node_name, "interrupts",
+ irq_attr, vbasedev->num_irqs * 3 * sizeof(uint32_t));
+g_free(irq_attr);
+}
[Qemu-devel] [PATCH v3 1/4] vfio/platform: Make the vfio-platform device non-abstract
From: Auger Eric
Up to now the vfio-platform device has been abstract and could not be
instantiated. The integration of a new vfio platform device required
creating a dummy derived device which only set the compatible string.
Following the few vfio-platform device integrations we have seen the
actual requested adaptation happens on device tree node creation
(sysbus-fdt).
Hence remove the abstract setting, and read the list of compatible
values from sysfs if not set by a derived device.
Update the amd-xgbe and calxeda-xgmac drivers to fill in the number of
compatible values, as there can now be more than one.
Note that sysbus-fdt does not support the instantiation of the
vfio-platform device yet.
Signed-off-by: Eric Auger
[geert: Rebase, set user_creatable=true, use compatible values in sysfs
instead of user-supplied manufacturer/model options, reword]
Signed-off-by: Geert Uytterhoeven
---
v3:
- Read all compatible values from sysfs instead of using user-supplied
manufacturer and model options,
- Reword patch description,
- Drop RFC state,
v2:
- No changes,
v1:
- Rebase, Set user_creatable=true,
v0:
- Original version from Eric.
---
hw/vfio/amd-xgbe.c | 1 +
hw/vfio/calxeda-xgmac.c | 1 +
hw/vfio/platform.c | 22 +-
include/hw/vfio/vfio-platform.h | 3 ++-
4 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/amd-xgbe.c b/hw/vfio/amd-xgbe.c
index 0c4ec4ba25170366..ee64a3b4a2e45bf5 100644
--- a/hw/vfio/amd-xgbe.c
+++ b/hw/vfio/amd-xgbe.c
@@ -20,6 +20,7 @@ static void amd_xgbe_realize(DeviceState *dev, Error **errp)
VFIOAmdXgbeDeviceClass *k = VFIO_AMD_XGBE_DEVICE_GET_CLASS(dev);
vdev->compat = g_strdup("amd,xgbe-seattle-v1a");
+vdev->num_compat = 1;
k->parent_realize(dev, errp);
}
diff --git a/hw/vfio/calxeda-xgmac.c b/hw/vfio/calxeda-xgmac.c
index 24cee6d06512c1b6..e7767c4b021be566 100644
--- a/hw/vfio/calxeda-xgmac.c
+++ b/hw/vfio/calxeda-xgmac.c
@@ -20,6 +20,7 @@ static void calxeda_xgmac_realize(DeviceState *dev, Error
**errp)
VFIOCalxedaXgmacDeviceClass *k = VFIO_CALXEDA_XGMAC_DEVICE_GET_CLASS(dev);
vdev->compat = g_strdup("calxeda,hb-xgmac");
+vdev->num_compat = 1;
k->parent_realize(dev, errp);
}
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 57c4a0ee2b58da70..e264555cd5dafb16 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -655,6 +655,25 @@ static void vfio_platform_realize(DeviceState *dev, Error
**errp)
goto out;
}
+if (!vdev->compat) {
+gchar *contents;
+gsize length;
+char *tmp;
+
+tmp = g_strdup_printf("%s/of_node/compatible", vbasedev->sysfsdev);
+if (!g_file_get_contents(tmp, &contents, &length, NULL)) {
+error_report("failed to load \"%s\"", tmp);
+exit(1);
+}
+g_free(tmp);
+vdev->compat = contents;
+for (vdev->num_compat = 0; length; vdev->num_compat++) {
+size_t skip = strlen(contents) + 1;
+contents += skip;
+length -= skip;
+}
+}
+
for (i = 0; i < vbasedev->num_regions; i++) {
if (vfio_region_mmap(vdev->regions[i])) {
error_report("%s mmap unsupported. Performance may be slow",
@@ -700,6 +719,8 @@ static void vfio_platform_class_init(ObjectClass *klass,
void *data)
dc->desc = "VFIO-based platform device assignment";
sbc->connect_irq_notifier = vfio_start_irqfd_injection;
set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+/* Supported by TYPE_VIRT_MACHINE */
+dc->user_creatable = true;
}
static const TypeInfo vfio_platform_dev_info = {
@@ -708,7 +729,6 @@ static const TypeInfo vfio_platform_dev_info = {
.instance_size = sizeof(VFIOPlatformDevice),
.class_init = vfio_platform_class_init,
.class_size = sizeof(VFIOPlatformDeviceClass),
-.abstract = true,
};
static void register_vfio_platform_dev_type(void)
diff --git a/include/hw/vfio/vfio-platform.h b/include/hw/vfio/vfio-platform.h
index 9baaa2db09b84f3e..0ee10b1d71ed2503 100644
--- a/include/hw/vfio/vfio-platform.h
+++ b/include/hw/vfio/vfio-platform.h
@@ -54,7 +54,8 @@ typedef struct VFIOPlatformDevice {
QLIST_HEAD(, VFIOINTp) intp_list; /* list of IRQs */
/* queue of pending IRQs */
QSIMPLEQ_HEAD(pending_intp_queue, VFIOINTp) pending_intp_queue;
-char *compat; /* compatibility string */
+char *compat; /* DT compatible values, separated by NUL */
+unsigned int num_compat; /* number of compatible values */
uint32_t mmap_timeout; /* delay to re-enable mmaps after interrupt */
QEMUTimer *mmap_timer; /* allows fast-path resume after IRQ hit */
QemuMutex intp_mutex; /* protect the intp_list IRQ state */
--
2.17.1
[Qemu-devel] [PATCH v3 0/4] hw/arm/sysbus-fdt: Generic DT Pass-Through
Hi all,
This patch series allows to export generic devices in DT using
vfio-platform, providing direct access from a QEMU+KVM guest to the
exported devices.
- Patches 1-2 (submitted before by Eric Auger) make the vfio-platform
device non-abstract, incl. matching using a compatible string.
- Patch 3 allows dynamic sysbus devices again, without needing to
create device-specific vfio types for each and every new device.
- Patch 4 adds code to instantiate device nodes for generic DT
devices, copying a set of generic properties from the host.
This avoids having to write device-specific instantation methods for
each and every "simple" device using only a set of generic properties.
Devices that need more specialized handling can still provide their own
instantation method.
Changes compared to v2 (not submitted to the mailing list):
- Use the compatible values from sysfs instead of user-supplied
manufacturer and model options,
- Replace "hw/arm/sysbus-fdt: Enable rcar-gen3-gpio dynamic
instatiation" by generic "hw/arm/sysbus-fdt: Add support for
instantiating generic devices",
- Reword patch descriptions,
- Drop RFC state,
- Drop "vfio: No-IOMMU mode support".
Changes compared to v1 ("R-Car Gen3 GPIO Pass-Through Prototype (QEMU)",
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg02716.html):
- Restrict dynamic sysbus devices to TYPE_VFIO_PLATFORM, as suggested
by Eric Auger.
For proper operation, this depends on "hw/arm/sysbus-fdt: Fix assertion
in copy_properties_from_host()"
(https://lists.gnu.org/archive/html/qemu-devel/2018-07/msg04922.html).
Thanks!
For testing, this patch series and all prerequisites are available in
the topic/rcar3-virt-gpio-passthrough-v3 branch of my git repository at
https://github.com/geertu/qemu.git.
This has been tested on a Renesas Salvator-XS board with R-Car H3 ES2.0
with SATA:
-device vfio-platform,host=ee30.sata
and GPIO (needs VFIO No-IOMMU support):
-device vfio-platform,host=e6055400.gpio
Thanks!
Auger Eric (2):
vfio/platform: Make the vfio-platform device non-abstract
hw/arm/sysbus-fdt: Allow device matching with DT compatible value
Geert Uytterhoeven (2):
hw/arm/virt: Allow dynamic sysbus devices again
hw/arm/sysbus-fdt: Add support for instantiating generic devices
hw/arm/sysbus-fdt.c | 163 +---
hw/arm/virt.c | 1 +
hw/vfio/amd-xgbe.c | 1 +
hw/vfio/calxeda-xgmac.c | 1 +
hw/vfio/platform.c | 22 -
include/hw/vfio/vfio-platform.h | 3 +-
6 files changed, 175 insertions(+), 16 deletions(-)
--
2.17.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[Qemu-devel] [PATCH v3 2/4] hw/arm/sysbus-fdt: Allow device matching with DT compatible value
From: Auger Eric
Up to now we have relied on the device type to identify a device tree
node creation function. Since we would like the vfio-platform device to
be instantiatable with different compatible strings we introduce the
capability to specialize the node creation depending on actual
compatible value.
NodeCreationPair is renamed into BindingEntry. The struct is enhanced
with compat and match_fn() fields. We introduce a new matching function
adapted to the vfio-platform generic device.
>From now on, the AMD XGBE can be instantiated with either manner, i.e.:
-device vfio-amd-xgbe,host=e090.xgmac
or using the new option line:
-device vfio-platform,host=e090.xgmac
Signed-off-by: Eric Auger
[geert: Match using compatible values in sysfs instead of user-supplied
manufacturer/model options, reword]
Signed-off-by: Geert Uytterhoeven
---
Not tested with amd-xgbe due to lack of hardware.
v3:
- Match using the compatible values from sysfs instead of using
user-supplied manufacturer and model options,
- Reword patch description,
- Drop RFC state,
v2:
- No changes,
v1:
- No changes,
v0:
- Original version from Eric.
---
hw/arm/sysbus-fdt.c | 61 ++---
1 file changed, 47 insertions(+), 14 deletions(-)
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index 43d6a7bb48ddc351..0e24c803a1c2c734 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -50,11 +50,13 @@ typedef struct PlatformBusFDTData {
PlatformBusDevice *pbus;
} PlatformBusFDTData;
-/* struct that associates a device type name and a node creation function */
-typedef struct NodeCreationPair {
+/* struct that allows to match a device and create its FDT node */
+typedef struct BindingEntry {
const char *typename;
-int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque);
-} NodeCreationPair;
+const char *compat;
+int (*add_fn)(SysBusDevice *sbdev, void *opaque);
+bool (*match_fn)(SysBusDevice *sbdev, const struct BindingEntry *combo);
+} BindingEntry;
/* helpers */
@@ -413,6 +415,27 @@ static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev, void
*opaque)
return 0;
}
+/* DT compatible matching */
+static bool vfio_platform_match(SysBusDevice *sbdev,
+const BindingEntry *entry)
+{
+VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
+const char *compat;
+unsigned int n;
+
+for (n = vdev->num_compat, compat = vdev->compat; n > 0;
+ n--, compat += strlen(compat) + 1) {
+if (!strcmp(entry->compat, compat)) {
+return true;
+}
+}
+
+return false;
+}
+
+#define VFIO_PLATFORM_BINDING(compat, add_fn) \
+{TYPE_VFIO_PLATFORM, (compat), (add_fn), vfio_platform_match}
+
#endif /* CONFIG_LINUX */
static int no_fdt_node(SysBusDevice *sbdev, void *opaque)
@@ -420,14 +443,23 @@ static int no_fdt_node(SysBusDevice *sbdev, void *opaque)
return 0;
}
-/* list of supported dynamic sysbus devices */
-static const NodeCreationPair add_fdt_node_functions[] = {
+/* Device type based matching */
+static bool type_match(SysBusDevice *sbdev, const BindingEntry *entry)
+{
+return !strcmp(object_get_typename(OBJECT(sbdev)), entry->typename);
+}
+
+#define TYPE_BINDING(type, add_fn) {(type), NULL, (add_fn), type_match}
+
+/* list of supported dynamic sysbus bindings */
+static const BindingEntry bindings[] = {
#ifdef CONFIG_LINUX
-{TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node},
-{TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node},
+TYPE_BINDING(TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node),
+TYPE_BINDING(TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node),
+VFIO_PLATFORM_BINDING("amd,xgbe-seattle-v1a", add_amd_xgbe_fdt_node),
#endif
-{TYPE_RAMFB_DEVICE, no_fdt_node},
-{"", NULL}, /* last element */
+TYPE_BINDING(TYPE_RAMFB_DEVICE, no_fdt_node),
+TYPE_BINDING("", NULL), /* last element */
};
/* Generic Code */
@@ -446,10 +478,11 @@ static void add_fdt_node(SysBusDevice *sbdev, void
*opaque)
{
int i, ret;
-for (i = 0; i < ARRAY_SIZE(add_fdt_node_functions); i++) {
-if (!strcmp(object_get_typename(OBJECT(sbdev)),
-add_fdt_node_functions[i].typename)) {
-ret = add_fdt_node_functions[i].add_fdt_node_fn(sbdev, opaque);
+for (i = 0; i < ARRAY_SIZE(bindings); i++) {
+const BindingEntry *iter = &bindings[i];
+
+if (iter->match_fn(sbdev, iter)) {
+ret = iter->add_fn(sbdev, opaque);
assert(!ret);
return;
}
--
2.17.1
[Qemu-devel] [PATCH QEMU] hw/char/sh_serial: Add timeout handling to unbreak serial input
As of commit 18e8cf159177100e ("serial: sh-sci: increase RX FIFO trigger
defaults for (H)SCIF") in Linux v4.11-rc1, the serial console on the
QEMU SH4 target is broken: it delays serial input until enough data has
been received.
Since aformentioned commit, the Linux SCIF driver programs the Receive
FIFO Data Count Trigger bits in the FIFO Control Register, to postpone
generating a receive interrupt until:
1. At least the receive trigger count of bytes of data are available
in the receive FIFO, OR
2. No further data has been received for at least 15 etu after the
last received data.
While QEMU implements the former, it does not implement the latter.
Hence the receive interrupt is not generated until the former condition
is met.
Fix this by adding basic timeout handling. As the QEMU SCIF emulation
ignores any serial speed programming, the timeout value used conforms to
a default speed of 9600 bps, which is fine for any interative console.
Reported-by: Rob Landley
Signed-off-by: Geert Uytterhoeven
---
hw/char/sh_serial.c | 20
1 file changed, 20 insertions(+)
diff --git a/hw/char/sh_serial.c b/hw/char/sh_serial.c
index 373a40595fd975d1..12831561a6c8b137 100644
--- a/hw/char/sh_serial.c
+++ b/hw/char/sh_serial.c
@@ -29,6 +29,7 @@
#include "hw/sh4/sh.h"
#include "chardev/char-fe.h"
#include "qapi/error.h"
+#include "qemu/timer.h"
//#define DEBUG_SERIAL
@@ -63,6 +64,8 @@ typedef struct {
int rtrg;
CharBackend chr;
+QEMUTimer *fifo_timeout_timer;
+uint64_t etu; /* Elementary Time Unit (ns) */
qemu_irq eri;
qemu_irq rxi;
@@ -314,6 +317,16 @@ static int sh_serial_can_receive1(void *opaque)
return sh_serial_can_receive(s);
}
+static void sh_serial_timeout_int(void *opaque)
+{
+sh_serial_state *s = opaque;
+
+s->flags |= SH_SERIAL_FLAG_RDF;
+if (s->scr & (1 << 6) && s->rxi) {
+qemu_set_irq(s->rxi, 1);
+}
+}
+
static void sh_serial_receive1(void *opaque, const uint8_t *buf, int size)
{
sh_serial_state *s = opaque;
@@ -330,8 +343,12 @@ static void sh_serial_receive1(void *opaque, const uint8_t
*buf, int size)
if (s->rx_cnt >= s->rtrg) {
s->flags |= SH_SERIAL_FLAG_RDF;
if (s->scr & (1 << 6) && s->rxi) {
+timer_del(s->fifo_timeout_timer);
qemu_set_irq(s->rxi, 1);
}
+} else {
+timer_mod(s->fifo_timeout_timer,
+qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 15 * s->etu);
}
}
}
@@ -402,6 +419,9 @@ void sh_serial_init(MemoryRegion *sysmem,
sh_serial_event, NULL, s, NULL, true);
}
+s->fifo_timeout_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+ sh_serial_timeout_int, s);
+s->etu = NANOSECONDS_PER_SECOND / 9600;
s->eri = eri_source;
s->rxi = rxi_source;
s->txi = txi_source;
--
2.17.1
Re: [Qemu-devel] [PATCH v3 1/4] vfio/platform: Make the vfio-platform device non-abstract
Hi Eric,
On Tue, Aug 7, 2018 at 4:18 PM Auger Eric wrote:
> On 07/25/2018 04:34 PM, Geert Uytterhoeven wrote:
> > From: Auger Eric
> >
> > Up to now the vfio-platform device has been abstract and could not be
> > instantiated. The integration of a new vfio platform device required
> > creating a dummy derived device which only set the compatible string.
> >
> > Following the few vfio-platform device integrations we have seen the
> > actual requested adaptation happens on device tree node creation
> > (sysbus-fdt).
> >
> > Hence remove the abstract setting, and read the list of compatible
> > values from sysfs if not set by a derived device.
> >
> > Update the amd-xgbe and calxeda-xgmac drivers to fill in the number of
> > compatible values, as there can now be more than one.
> >
> > Note that sysbus-fdt does not support the instantiation of the
> > vfio-platform device yet.
> >
> > Signed-off-by: Eric Auger
> > [geert: Rebase, set user_creatable=true, use compatible values in sysfs
> > instead of user-supplied manufacturer/model options, reword]
> > Signed-off-by: Geert Uytterhoeven
> > ---
> > v3:
> > - Read all compatible values from sysfs instead of using user-supplied
> > manufacturer and model options,
> > - Reword patch description,
> > - Drop RFC state,
> >
> > v2:
> > - No changes,
> >
> > v1:
> > - Rebase, Set user_creatable=true,
> >
> > v0:
> > - Original version from Eric.
> > --- a/hw/vfio/platform.c
> > +++ b/hw/vfio/platform.c
> > @@ -655,6 +655,25 @@ static void vfio_platform_realize(DeviceState *dev,
> > Error **errp)
> > goto out;
> > }
> >
> > +if (!vdev->compat) {
> > +gchar *contents;
> > +gsize length;
> > +char *tmp;
> > +
> > +tmp = g_strdup_printf("%s/of_node/compatible", vbasedev->sysfsdev);
> > +if (!g_file_get_contents(tmp, &contents, &length, NULL)) {
> > +error_report("failed to load \"%s\"", tmp);
> > +exit(1);
> You should set errp instead so that the error gets properly propagated.
Thanks, will do.
> > --- a/include/hw/vfio/vfio-platform.h
> > +++ b/include/hw/vfio/vfio-platform.h
> > @@ -54,7 +54,8 @@ typedef struct VFIOPlatformDevice {
> > QLIST_HEAD(, VFIOINTp) intp_list; /* list of IRQs */
> > /* queue of pending IRQs */
> > QSIMPLEQ_HEAD(pending_intp_queue, VFIOINTp) pending_intp_queue;
> > -char *compat; /* compatibility string */
> > +char *compat; /* DT compatible values, separated by NUL */
> by NULL characters?
"NUL" is the character ('\0'), "NULL" is the pointer.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Qemu-devel] [PATCH v3 4/4] hw/arm/sysbus-fdt: Add support for instantiating generic devices
Hi Eric,
On Tue, Aug 7, 2018 at 4:19 PM Auger Eric wrote:
> On 07/25/2018 04:34 PM, Geert Uytterhoeven wrote:
> > Add a fallback for instantiating generic devices without a type-specific
> > or compatible-specific instantation method. This will be used when no
> > other match is found.
> >
> > The generic instantation method creates a device node with "reg" and
> > (optional) "interrupts" properties, and copies the "compatible"
> > property and other optional properties from the host.
> > For now the following optional properties are copied, if found:
> > - "#gpio-cells",
> > - "gpio-controller",
> > - "#interrupt-cells",
> > - "interrupt-controller",
> > - "interrupt-names".
> >
> > More can be added when needed.
> >
> > This avoids having to write device-specific instantation methods for
> instantiation
Thanks, will fix.
> > each and every "simple" device using only a set of generic properties.
> > Devices that need more specialized handling can still provide their
> > own instantation method.
Arghl, one more.
> > --- a/hw/arm/sysbus-fdt.c
> > +++ b/hw/arm/sysbus-fdt.c
> > @@ -415,6 +415,99 @@ static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev,
> > void *opaque)
> > return 0;
> > }
> >
> > +static HostProperty generic_copied_properties[] = {
> > +{"compatible", false},
> > +{"#gpio-cells", true},
> > +{"gpio-controller", true},
> > +{"#interrupt-cells", true},
> > +{"interrupt-controller", true},
> > +{"interrupt-names", true},
>
> I think we would need to enumerate the other source properties which
> were not copied to the guest fdt and either warn the userspace those
> were omitted or fail. We may end up generating an incomplete guest dt
> node that may not work properly.
The list above is quite generic, so it is expected that some of the optional
properties (marked "true") cannot be copied. Hence warning about them
will be noisy, and confuse users. Failing is definitely the wrong thing
to do.
If the host lacks a property that is mandatory for a specific device, the
device won't have worked on the host before neither, right?
The major issue remains that an incomplete guest DT node may be generated
if the list above lacks certain needed properties, or if subnodes are
needed.
I expect the guest OS driver would complain about the missing parts, though.
In that case, either the list should be extended, or a device-specific
instantiation method should be written.
> > +/**
> > + * add_generic_fdt_node
> > + *
> > + * Generates a generic DT node by copying properties from the host
> > + */
> > +static int add_generic_fdt_node(SysBusDevice *sbdev, void *opaque)
> > +{
> > +/* Copy interrupts (remapped) */
> > +if (vbasedev->num_irqs) {
> > +irq_attr = g_new(uint32_t, vbasedev->num_irqs * 3);
> > +for (i = 0; i < vbasedev->num_irqs; i++) {
> > +irq_number = platform_bus_get_irqn(pbus, sbdev , i)
> > + + data->irq_start;
> > +irq_attr[3 * i] = cpu_to_be32(GIC_FDT_IRQ_TYPE_SPI);
> > +irq_attr[3 * i + 1] = cpu_to_be32(irq_number);
> > +irq_attr[3 * i + 2] = cpu_to_be32(GIC_FDT_IRQ_FLAGS_LEVEL_HI);
>
> I think this part is not generic enough. How can you assume the IRQs are
> level or edge? Can't you copy the source interrupt properties and just
> overwrite the IRQ number?
I have to check that. I wouldn't be surprised if the virtualized GIC supports
only a subset of all possible flags and/or remaps interrupt types too.
I see the AMD XGBE driver looks at VFIO_IRQ_INFO_AUTOMASKED to choose
between GIC_FDT_IRQ_FLAGS_LEVEL_HI and GIC_FDT_IRQ_FLAGS_EDGE_LO_HI.
The VFIO_IRQ_INFO_* don't seem to provide more details.
There are also no users of GIC_FDT_IRQ_FLAGS_LEVEL_LO and
GIC_FDT_IRQ_FLAGS_EDGE_HI_LO in the Qemu sources...
So probably following the AMD XGBE example is the way forward?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Qemu-devel] [PATCH v3 4/4] hw/arm/sysbus-fdt: Add support for instantiating generic devices
Hi Eric,
On Tue, Aug 7, 2018 at 7:21 PM Auger Eric wrote:
> On 08/07/2018 05:32 PM, Geert Uytterhoeven wrote:
> > On Tue, Aug 7, 2018 at 4:19 PM Auger Eric wrote:
> >> On 07/25/2018 04:34 PM, Geert Uytterhoeven wrote:
> >>> Add a fallback for instantiating generic devices without a type-specific
> >>> or compatible-specific instantation method. This will be used when no
> >>> other match is found.
> >>>
> >>> The generic instantation method creates a device node with "reg" and
> >>> (optional) "interrupts" properties, and copies the "compatible"
> >>> property and other optional properties from the host.
> >>> --- a/hw/arm/sysbus-fdt.c
> >>> +++ b/hw/arm/sysbus-fdt.c
> >>> @@ -415,6 +415,99 @@ static int add_amd_xgbe_fdt_node(SysBusDevice
> >>> *sbdev, void *opaque)
> >>> return 0;
> >>> }
> >>>
> >>> +static HostProperty generic_copied_properties[] = {
> >>> +{"compatible", false},
> >>> +{"#gpio-cells", true},
> >>> +{"gpio-controller", true},
> >>> +{"#interrupt-cells", true},
> >>> +{"interrupt-controller", true},
> >>> +{"interrupt-names", true},
> >>
> >> I think we would need to enumerate the other source properties which
> >> were not copied to the guest fdt and either warn the userspace those
> >> were omitted or fail. We may end up generating an incomplete guest dt
> >> node that may not work properly.
> >
> > The list above is quite generic, so it is expected that some of the optional
> > properties (marked "true") cannot be copied. Hence warning about them
> > will be noisy, and confuse users. Failing is definitely the wrong thing
> > to do.
>
> I was not talking about those listed here and optional. Those ones are
> taken care of. I was rather talking about potential other ones, present
> on host side and not copied on guest side. For instance, let's say your
> host dt node has a clocks property. You will attempt to create a guest
> dt node without this property and obviously the device will not work
> properly on guest side. The end user attempting this assignment does not
> get any warning on guest launch. Maybe the driver on guest side will be
> cool enough to issue a warning but we cannot really rely on this
> assumption. So from a maintenance point of view, it looks not
> manageable. I think we should checl all props found in the host dt node
> are considered and copied into the guest node. Otherwise this means the
> host dt node does not belong to the category of a "simple" one and thus
> cannot use this creation function.
It depends. And that makes it difficult to come up with a sensible
detection system for notifying the user, while avoiding too many false
positives and negatives.
Properties like "clocks" typically use phandles, which means the node
they're pointing to should be copied, too, possibly involving rewriting
like with interrupts. Hence I think this should be left to a
device-specific instantiation method.
Furthermore, depending on the SoC, some DT properties should be ignored,
and must not be copied.
Examples are:
1. "power-domains", and optionally "clocks", when the device is part of a
power and/or clock domain.
Power management must be handled by the host, cfr. commit
415eb9fc0e23071f ("vfio: platform: Fix using devices in PM Domains",
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=415eb9fc0e23071f).
That is another reason why we are replacing explicit clock handling
for power management by Runtime PM in the individual drivers, cfr.
e.g. commit 1ecd34ddf63ef1d4 ("ata: sata_rcar: Add rudimentary Runtime
PM support") in linux-next
(https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=1ecd34ddf63ef1d4).
(the first reason is abstracting power management, which may differ
among SoCs using the same IP core).
2. "resets", pointing to a reset controller, as reset must be handled by
the host's vfio driver to restore the device to a pristine state
before/after virtualization.
See also "[PATCH v3 2/2] vfio: platform: Add generic DT reset support"
(https://www.spinics.net/lists/devicetree/msg223516.html).
> > If the host lacks a property that is mandatory for a specific device, the
> > device won't have worked on the host before neither, right?
> >
> > The major issue remains that an incomplet
Re: [Qemu-devel] [PATCH v3 4/4] hw/arm/sysbus-fdt: Add support for instantiating generic devices
Hi Eric,
On Wed, Aug 8, 2018 at 3:16 PM Auger Eric wrote:
> On 08/08/2018 02:59 PM, Geert Uytterhoeven wrote:
> > On Tue, Aug 7, 2018 at 7:21 PM Auger Eric wrote:
> >> On 08/07/2018 05:32 PM, Geert Uytterhoeven wrote:
> >>> On Tue, Aug 7, 2018 at 4:19 PM Auger Eric wrote:
> >>>> On 07/25/2018 04:34 PM, Geert Uytterhoeven wrote:
> >>>>> Add a fallback for instantiating generic devices without a type-specific
> >>>>> or compatible-specific instantation method. This will be used when no
> >>>>> other match is found.
> >>>>>
> >>>>> The generic instantation method creates a device node with "reg" and
> >>>>> (optional) "interrupts" properties, and copies the "compatible"
> >>>>> property and other optional properties from the host.
[...]
> Wouldn't it make sense to try getting 1-3 upstream separately as this
> last patch seems more controversial and independent?
I agree. I will rework and resubmit after my holidays.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[Qemu-devel] [PATCH] device_tree: Increase FDT_MAX_SIZE to 128 KiB
It is not uncommon for a contemporary FDT to be larger than 64 KiB,
leading to failures loading the device tree from sysfs:
qemu-system-aarch64: qemu_fdt_setprop: Couldn't set ...: FDT_ERR_NOSPACE
For reference, the largest arm64 DTB created from the Linux sources is
70 KiB large (93 KiB when built with symbols/fixup support).
Signed-off-by: Geert Uytterhoeven
---
device_tree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/device_tree.c b/device_tree.c
index a24ddff02bdd857c..1ba9b8e0a49e6bbc 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -29,7 +29,7 @@
#include
-#define FDT_MAX_SIZE 0x1
+#define FDT_MAX_SIZE 0x2
void *create_device_tree(int *sizep)
{
--
2.7.4
[Qemu-devel] [PATCH v2] device_tree: Increase FDT_MAX_SIZE to 1 MiB
It is not uncommon for a contemporary FDT to be larger than 64 KiB,
leading to failures loading the device tree from sysfs:
qemu-system-aarch64: qemu_fdt_setprop: Couldn't set ...: FDT_ERR_NOSPACE
Hence increase the limit to 1 MiB, like on PPC.
For reference, the largest arm64 DTB created from the Linux sources is
70 KiB large (93 KiB when built with symbols/fixup support).
Signed-off-by: Geert Uytterhoeven
---
v2:
- Enlarge from 128 KiB to 1 MiB, as suggested by Peter Maydell.
---
device_tree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/device_tree.c b/device_tree.c
index a24ddff02bdd857c..9eb5fae7381fe885 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -29,7 +29,7 @@
#include
-#define FDT_MAX_SIZE 0x1
+#define FDT_MAX_SIZE 0x10
void *create_device_tree(int *sizep)
{
--
2.7.4
Re: [Qemu-devel] [PATCH/RFC 3/5] hw/arm/virt: Allow dynamic sysbus devices again
Hi Eric,
On Wed, Feb 14, 2018 at 11:37 AM, Auger Eric wrote:
> On 09/02/18 16:17, Geert Uytterhoeven wrote:
>> Allow the instantation of generic dynamic sysbus devices again, without
>> the need to create a new device-specific vfio type.
>>
>> This is a partial revert of commit 6f2062b9758ebc64 ("hw/arm/virt:
>> Allow only supported dynamic sysbus devices").
>>
>> Not-Yet-Signed-off-by: Geert Uytterhoeven
>> ---
>> hw/arm/virt.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index b334c82edae9fb1f..fa83784fc08ed076 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -1596,6 +1596,7 @@ static void virt_machine_class_init(ObjectClass *oc,
>> void *data)
>> mc->max_cpus = 255;
>> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_CALXEDA_XGMAC);
>> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
>> +machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SYS_BUS_DEVICE);
> Couldn't you limit that to TYPE_VFIO_PLATFORM instead?
Yep, that works. Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[Qemu-devel] [PATCH qemu v3] device_tree: Increase FDT_MAX_SIZE to 1 MiB
It is not uncommon for a contemporary FDT to be larger than 64 KiB,
leading to failures loading the device tree from sysfs:
qemu-system-aarch64: qemu_fdt_setprop: Couldn't set ...: FDT_ERR_NOSPACE
Hence increase the limit to 1 MiB, like on PPC.
For reference, the largest arm64 DTB created from the Linux sources is
ca. 75 KiB large (100 KiB when built with symbols/fixup support).
Signed-off-by: Geert Uytterhoeven
---
v3:
- Update example size figures,
v2:
- Enlarge from 128 KiB to 1 MiB, as suggested by Peter Maydell.
---
device_tree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/device_tree.c b/device_tree.c
index 19458b32bf81e55e..52c3358a55838d33 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -29,7 +29,7 @@
#include
-#define FDT_MAX_SIZE 0x1
+#define FDT_MAX_SIZE 0x10
void *create_device_tree(int *sizep)
{
--
2.7.4
Re: [Qemu-devel] [PATCH/RFC 3/5] hw/arm/virt: Allow dynamic sysbus devices again
Hi Peter,
On Fri, Feb 9, 2018 at 4:27 PM, Peter Maydell wrote:
> On 9 February 2018 at 15:17, Geert Uytterhoeven
> wrote:
>> Allow the instantation of generic dynamic sysbus devices again, without
>> the need to create a new device-specific vfio type.
>>
>> This is a partial revert of commit 6f2062b9758ebc64 ("hw/arm/virt:
>> Allow only supported dynamic sysbus devices").
>>
>> Not-Yet-Signed-off-by: Geert Uytterhoeven
>> ---
>> hw/arm/virt.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index b334c82edae9fb1f..fa83784fc08ed076 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -1596,6 +1596,7 @@ static void virt_machine_class_init(ObjectClass *oc,
>> void *data)
>> mc->max_cpus = 255;
>> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_CALXEDA_XGMAC);
>> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
>> +machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SYS_BUS_DEVICE);
>> mc->block_default_type = IF_VIRTIO;
>> mc->no_cdrom = 1;
>> mc->pci_allow_0_address = true;
>
> This needs a lot of justification. Dynamic sysbus is not supposed
> to be for plugging any random sysbus device in, it's for vfio,
> which needs special casing anyway to work right. (Overall it's
Sure. Is there a way to limit this to vfio devices?
> a terrible hack -- in an ideal world all vfio would use pci
> or another probeable bus.)
What about vfio-platform, which is my use case?
On DT-based systems, platform devices are described very well in DT (even
better than what's provided by probing PCI IDs and BARs ;-)
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Qemu-devel] [PATCH/RFC 4/5] vfio: No-IOMMU mode support
Hi Alex,
On Fri, Feb 9, 2018 at 4:50 PM, Alex Williamson
wrote:
> On Fri, 9 Feb 2018 16:17:35 +0100
> Geert Uytterhoeven wrote:
>> From: Xiao Feng Ren
>>
>> Add qemu support for the newly introduced VFIO No-IOMMU driver.
>>
>> We need to add special handling for:
>> - Group character device is /dev/vfio/noiommu-$GROUP.
>> - No-IOMMU does not rely on a memory listener.
>> - No IOMMU will be set for its group, so no need to call
>> vfio_kvm_device_add_group.
>>
>> Signed-off-by: Xiao Feng Ren
>> [geert: Rebase]
>> Signed-off-by: Geert Uytterhoeven
>> ---
>> hw/vfio/common.c | 61
>> ++-
>> include/hw/vfio/vfio-common.h | 2 ++
>> 2 files changed, 50 insertions(+), 13 deletions(-)
>
> NAK. I'm opposed to no-iommu support in QEMU in general, but accepting
> vfio devices with no-iommu (which provide no DMA translation!!!)
> transparently as if they might actually work like a regular vfio device
> is absolutely unacceptable. Without DMA translation and isolation, you
> might want to think about another interface, I'm not keen on the idea
What if the device cannot do DMA?
There are other devices that cannot do DMA, but that can be useful to
access from a guest in an embedded system.
E.g. smart ADC monitor blocks that monitor and average several ADCs in an
automotive environment (drivers/iio/adc/rcar-gyroadc.c).
Should all such devices be paravirtualized?
> of corrupting vfio support in order to blink some LEDs. Thanks,
This GPIO+LED prototype is just a proof-of-concept. There's no plan to
upstream it.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[Qemu-devel] [PATCH/RFC 3/6] clk: renesas: r8a7795: Mark the GPIO6 clock critical
The GPIO6 block will be exported to a guest. As long as the guest won't
manage its module clock, it must be kept running by the host.
Not-Signed-off-by: Geert Uytterhoeven
---
TODO: Find a way to manage module clocks using PM Domains and Runtime
PM on the guest.
---
drivers/clk/renesas/r8a7795-cpg-mssr.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c
b/drivers/clk/renesas/r8a7795-cpg-mssr.c
index b1d9f48eae9e6ad4..6fbfd5973a889b7d 100644
--- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
@@ -271,6 +271,7 @@ static struct mssr_mod_clk r8a7795_mod_clks[] __initdata = {
static const unsigned int r8a7795_crit_mod_clks[] __initconst = {
MOD_CLK_ID(408),/* INTC-AP (GIC) */
+ MOD_CLK_ID(906),/* GPIO6 */
};
--
2.7.4
[Qemu-devel] [PATCH/RFC 0/6] R-Car Gen3 GPIO Pass-Through Prototype (Linux)
Hi all,
This RFC patch series is the Linux side of a GPIO Pass-Through prototype
for Renesas R-Car platforms using vfio-platform. Together with its
counterpart for QEMU, it provides direct access from a QEMU+KVM guest to
a GPIO controller in an R-Car Gen3 SoC. This allows the guest to
control the LEDs on a Renesas Salvator-X(S) board.
This patch series is not meant to be upstreamed as-is. Indeed, for
various reasons (e.g. security, as the different GPIOs on the same GPIO
controller may control different parts of the system) access to GPIOs is
better not implemented using Device Pass-Through, but by
paravirtualization. Yet, this is still a simple and valuable
proof-of-concept, which can serve as a basis for the future development
of Pass-Through support for more complex platform devices on R-Car Gen3
SoCs.
This patch series consists of two parts:
1. Patches 1-4 are Linux host patches.
They provide workarounds for missing virtualization platform
support (vfio reset, IOMMU group, clock domain), and a defconfig
update for testing.
These allow a GPIO controller to be unbound from its host driver,
and rebound to vfio-platform, for pass-through to a guest:
echo e6055400.gpio > /sys/bus/platform/drivers/gpio_rcar/unbind
echo vfio-platform > \
/sys/bus/platform/devices/e6055400.gpio/driver_override
echo e6055400.gpio > /sys/bus/platform/drivers/vfio-platform/bind
2. Patches 5-6 are Linux guest patches.
They provide workarounds for missing pass-through support (clock,
interrupt), and a guest defconfig for testing.
These allow the gpio-rcar driver to bind to a pass-through GPIO
device, and thus control the LEDs from the guest.
Several questions and TODOs are appended to the individual patches.
Please see https://elinux.org/R-Car/Virtualization/VFIO for full usage
instructions of this prototype.
Thanks for your comments!
Geert Uytterhoeven (6):
vfio: platform: Allow runtime override of reset_required
vfio: Ignore real IOMMUs if CONFIG_VFIO_NOIOMMU=y
clk: renesas: r8a7795: Mark the GPIO6 clock critical
arm64: renesas_defconfig: Enable VFIO_PLATFORM and VFIO_NOIOMMU
gpio: rcar: Add virtualization workarounds
arm64: Add virt_defconfig
arch/arm64/configs/renesas_defconfig | 2 +
arch/arm64/configs/virt_defconfig | 722 +
drivers/clk/renesas/r8a7795-cpg-mssr.c | 1 +
drivers/gpio/Kconfig | 2 +-
drivers/gpio/gpio-rcar.c | 28 +-
drivers/vfio/platform/vfio_platform.c | 2 +-
drivers/vfio/vfio.c| 6 +-
7 files changed, 744 insertions(+), 19 deletions(-)
create mode 100644 arch/arm64/configs/virt_defconfig
--
2.7.4
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[Qemu-devel] [PATCH/RFC 1/5] vfio/platform: make the vfio-platform device non abstract
From: Auger Eric
Up to now the vfio-platform device has been abstract and could not be
instantiated. The integration of a new vfio platform device required
to create a dummy derived device which only set the compatibility
string.
Following the few vfio-platform device integration we have seen
the actual requested adaptation happens on device tree node creation
(sysbus-fdt).
So this patch removes the abstract setting and defines 2 new options,
manufacturer and model that are used to build a compatibility string.
This latter will be used to match the device tree node creation
function
sysbus-fdt does not support the instantiation of the vfio-platform
device yet.
Signed-off-by: Eric Auger
[geert: Rebase, Set user_creatable=true]
Signed-off-by: Geert Uytterhoeven
---
hw/vfio/platform.c | 20 ++--
include/hw/vfio/vfio-platform.h | 2 ++
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 0d4bc0aae8891435..32bed1974e23c569 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -637,7 +637,20 @@ static void vfio_platform_realize(DeviceState *dev, Error
**errp)
VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(dev);
SysBusDevice *sbdev = SYS_BUS_DEVICE(dev);
VFIODevice *vbasedev = &vdev->vbasedev;
-int i, ret;
+int i, ret = -EINVAL;
+
+if (!vdev->compat) {
+if (!vdev->model) {
+error_setg(errp, "no usable compatible string");
+goto out;
+}
+if (!vdev->manufacturer) {
+vdev->compat = g_strdup(vdev->model);
+} else {
+vdev->compat = g_strjoin(",", vdev->manufacturer,
+ vdev->model, NULL);
+}
+}
vbasedev->type = VFIO_DEVICE_TYPE_PLATFORM;
vbasedev->dev = dev;
@@ -681,6 +694,8 @@ static const VMStateDescription vfio_platform_vmstate = {
static Property vfio_platform_dev_properties[] = {
DEFINE_PROP_STRING("host", VFIOPlatformDevice, vbasedev.name),
DEFINE_PROP_STRING("sysfsdev", VFIOPlatformDevice, vbasedev.sysfsdev),
+DEFINE_PROP_STRING("manufacturer", VFIOPlatformDevice, manufacturer),
+DEFINE_PROP_STRING("model", VFIOPlatformDevice, model),
DEFINE_PROP_BOOL("x-no-mmap", VFIOPlatformDevice, vbasedev.no_mmap, false),
DEFINE_PROP_UINT32("mmap-timeout-ms", VFIOPlatformDevice,
mmap_timeout, 1100),
@@ -699,6 +714,8 @@ static void vfio_platform_class_init(ObjectClass *klass,
void *data)
dc->desc = "VFIO-based platform device assignment";
sbc->connect_irq_notifier = vfio_start_irqfd_injection;
set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+/* Supported by TYPE_VIRT_MACHINE */
+dc->user_creatable = true;
}
static const TypeInfo vfio_platform_dev_info = {
@@ -707,7 +724,6 @@ static const TypeInfo vfio_platform_dev_info = {
.instance_size = sizeof(VFIOPlatformDevice),
.class_init = vfio_platform_class_init,
.class_size = sizeof(VFIOPlatformDeviceClass),
-.abstract = true,
};
static void register_vfio_platform_dev_type(void)
diff --git a/include/hw/vfio/vfio-platform.h b/include/hw/vfio/vfio-platform.h
index 9baaa2db09b84f3e..31b9a9800f7d9d00 100644
--- a/include/hw/vfio/vfio-platform.h
+++ b/include/hw/vfio/vfio-platform.h
@@ -55,6 +55,8 @@ typedef struct VFIOPlatformDevice {
/* queue of pending IRQs */
QSIMPLEQ_HEAD(pending_intp_queue, VFIOINTp) pending_intp_queue;
char *compat; /* compatibility string */
+char *manufacturer; /* manufacturer (1st part of the compatible property)
*/
+char *model; /* model (2d part of the compatible property) */
uint32_t mmap_timeout; /* delay to re-enable mmaps after interrupt */
QEMUTimer *mmap_timer; /* allows fast-path resume after IRQ hit */
QemuMutex intp_mutex; /* protect the intp_list IRQ state */
--
2.7.4
[Qemu-devel] [PATCH/RFC 6/6] arm64: Add virt_defconfig
Add a simple defconfig for virtualized arm64 machines, based on an OpenWRT config. This expects "initramfs.cpio" to exist, which can be extracted from e.g. an OpenWRT image using binwalk. CONFIG_GPIO_RCAR is enabled for testing GPIO pass-through on R-Car Gen3. Not-Signed-off-by: Geert Uytterhoeven --- Not intended for upstream merge. --- arch/arm64/configs/virt_defconfig | 722 ++ 1 file changed, 722 insertions(+) create mode 100644 arch/arm64/configs/virt_defconfig diff --git a/arch/arm64/configs/virt_defconfig b/arch/arm64/configs/virt_defconfig new file mode 100644 index ..4477d75d8e982b37 --- /dev/null +++ b/arch/arm64/configs/virt_defconfig @@ -0,0 +1,722 @@ +CONFIG_LOCALVERSION="-arm64-virt" +CONFIG_SYSVIPC=y +CONFIG_POSIX_MQUEUE=y +# CONFIG_CROSS_MEMORY_ATTACH is not set +CONFIG_NO_HZ=y +CONFIG_HIGH_RES_TIMERS=y +CONFIG_BSD_PROCESS_ACCT=y +CONFIG_BSD_PROCESS_ACCT_V3=y +CONFIG_RCU_EXPERT=y +CONFIG_RCU_FANOUT=32 +CONFIG_IKCONFIG=y +CONFIG_IKCONFIG_PROC=y +CONFIG_RELAY=y +CONFIG_BLK_DEV_INITRD=y +CONFIG_INITRAMFS_SOURCE="initramfs.cpio" +# CONFIG_RD_GZIP is not set +# CONFIG_RD_BZIP2 is not set +# CONFIG_RD_LZMA is not set +# CONFIG_RD_XZ is not set +# CONFIG_RD_LZO is not set +# CONFIG_RD_LZ4 is not set +CONFIG_CC_OPTIMIZE_FOR_SIZE=y +# CONFIG_SYSFS_SYSCALL is not set +# CONFIG_FHANDLE is not set +# CONFIG_AIO is not set +# CONFIG_ADVISE_SYSCALLS is not set +CONFIG_BPF_SYSCALL=y +CONFIG_EMBEDDED=y +# CONFIG_VM_EVENT_COUNTERS is not set +# CONFIG_SLUB_DEBUG is not set +# CONFIG_COMPAT_BRK is not set +CONFIG_CC_STACKPROTECTOR_REGULAR=y +CONFIG_MODULES=y +CONFIG_MODULE_UNLOAD=y +# CONFIG_BLK_DEV_BSG is not set +CONFIG_PARTITION_ADVANCED=y +# CONFIG_IOSCHED_DEADLINE is not set +# CONFIG_IOSCHED_CFQ is not set +CONFIG_ARCH_VEXPRESS=y +CONFIG_PCI=y +# CONFIG_CAVIUM_ERRATUM_22375 is not set +# CONFIG_CAVIUM_ERRATUM_23154 is not set +CONFIG_NR_CPUS=4 +CONFIG_PREEMPT_VOLUNTARY=y +CONFIG_HZ_100=y +# CONFIG_COMPACTION is not set +CONFIG_ZSMALLOC=m +CONFIG_CMDLINE="console=ttyAMA0" +# CONFIG_EFI is not set +# CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set +CONFIG_COMPAT=y +# CONFIG_SUSPEND is not set +CONFIG_NET=y +CONFIG_PACKET=y +CONFIG_UNIX=y +CONFIG_XFRM_USER=m +CONFIG_NET_KEY=m +CONFIG_INET=y +CONFIG_IP_MULTICAST=y +CONFIG_IP_ADVANCED_ROUTER=y +CONFIG_IP_MULTIPLE_TABLES=y +CONFIG_IP_ROUTE_MULTIPATH=y +CONFIG_IP_ROUTE_VERBOSE=y +CONFIG_NET_IPIP=m +CONFIG_NET_IPGRE_DEMUX=m +CONFIG_NET_IPGRE=m +CONFIG_NET_IPGRE_BROADCAST=y +CONFIG_IP_MROUTE=y +CONFIG_IP_MROUTE_MULTIPLE_TABLES=y +CONFIG_SYN_COOKIES=y +CONFIG_NET_IPVTI=m +CONFIG_INET_AH=m +CONFIG_INET_ESP=m +CONFIG_INET_IPCOMP=m +CONFIG_INET_XFRM_MODE_TRANSPORT=m +CONFIG_INET_XFRM_MODE_TUNNEL=m +CONFIG_INET_XFRM_MODE_BEET=m +# CONFIG_INET_DIAG is not set +CONFIG_TCP_CONG_ADVANCED=y +# CONFIG_TCP_CONG_BIC is not set +# CONFIG_TCP_CONG_WESTWOOD is not set +# CONFIG_TCP_CONG_HTCP is not set +CONFIG_INET6_AH=m +CONFIG_INET6_ESP=m +CONFIG_INET6_IPCOMP=m +CONFIG_INET6_XFRM_MODE_TRANSPORT=m +CONFIG_INET6_XFRM_MODE_TUNNEL=m +CONFIG_INET6_XFRM_MODE_BEET=m +CONFIG_IPV6_VTI=m +CONFIG_IPV6_SIT=m +CONFIG_IPV6_SIT_6RD=y +CONFIG_IPV6_GRE=m +CONFIG_IPV6_MULTIPLE_TABLES=y +CONFIG_IPV6_SUBTREES=y +CONFIG_IPV6_MROUTE=y +CONFIG_NETFILTER=y +CONFIG_BRIDGE_NETFILTER=y +# CONFIG_NETFILTER_INGRESS is not set +CONFIG_NF_CONNTRACK=m +CONFIG_NF_CONNTRACK_ZONES=y +CONFIG_NF_CONNTRACK_EVENTS=y +# CONFIG_NF_CT_PROTO_DCCP is not set +# CONFIG_NF_CT_PROTO_SCTP is not set +# CONFIG_NF_CT_PROTO_UDPLITE is not set +CONFIG_NF_CONNTRACK_AMANDA=m +CONFIG_NF_CONNTRACK_FTP=m +CONFIG_NF_CONNTRACK_H323=m +CONFIG_NF_CONNTRACK_IRC=m +CONFIG_NF_CONNTRACK_SNMP=m +CONFIG_NF_CONNTRACK_PPTP=m +CONFIG_NF_CONNTRACK_SIP=m +CONFIG_NF_CONNTRACK_TFTP=m +CONFIG_NF_CT_NETLINK=m +CONFIG_NF_TABLES=m +CONFIG_NF_TABLES_INET=m +CONFIG_NFT_EXTHDR=m +CONFIG_NFT_META=m +CONFIG_NFT_CT=m +CONFIG_NFT_COUNTER=m +CONFIG_NFT_LOG=m +CONFIG_NFT_LIMIT=m +CONFIG_NFT_MASQ=m +CONFIG_NFT_REDIR=m +CONFIG_NFT_NAT=m +CONFIG_NFT_REJECT=m +CONFIG_NFT_HASH=m +CONFIG_NETFILTER_XT_MARK=m +CONFIG_NETFILTER_XT_CONNMARK=m +CONFIG_NETFILTER_XT_SET=m +CONFIG_NETFILTER_XT_TARGET_CHECKSUM=m +CONFIG_NETFILTER_XT_TARGET_CLASSIFY=m +CONFIG_NETFILTER_XT_TARGET_CT=m +CONFIG_NETFILTER_XT_TARGET_DSCP=m +CONFIG_NETFILTER_XT_TARGET_HL=m +CONFIG_NETFILTER_XT_TARGET_LED=m +CONFIG_NETFILTER_XT_TARGET_LOG=m +CONFIG_NETFILTER_XT_TARGET_NFLOG=m +CONFIG_NETFILTER_XT_TARGET_NFQUEUE=m +CONFIG_NETFILTER_XT_TARGET_TPROXY=m +CONFIG_NETFILTER_XT_TARGET_TCPMSS=m +CONFIG_NETFILTER_XT_MATCH_ADDRTYPE=m +CONFIG_NETFILTER_XT_MATCH_CLUSTER=m +CONFIG_NETFILTER_XT_MATCH_COMMENT=m +CONFIG_NETFILTER_XT_MATCH_CONNBYTES=m +CONFIG_NETFILTER_XT_MATCH_CONNLIMIT=m +CONFIG_NETFILTER_XT_MATCH_CONNTRACK=m +CONFIG_NETFILTER_XT_MATCH_DSCP=m +CONFIG_NETFILTER_XT_MATCH_ECN=m +CONFIG_NETFILTER_XT_MATCH_ESP=m +CONFIG_NETFILTER_XT_MATCH_HASHLIMIT=m +CONFIG_NETFILTER_XT_MATCH_
[Qemu-devel] [PATCH/RFC 2/6] vfio: Ignore real IOMMUs if CONFIG_VFIO_NOIOMMU=y
Allow use of the No-IOMMU mode even if a real IOMMU is present. This is useful in case the device is not part of an actual IOMMU group. Not-Signed-off-by: Geert Uytterhoeven --- Question: - Some devices (e.g. rcar-gpio) don't use DMA, so why do they need an IOMMU group? - Even devices using DMA may do so using a separate DMAC module, pointed to by "dmas" DT properties (e.g. sh-sci), hence they may be not part of an IOMMU group. - How to know which devices do DMA? --- drivers/vfio/vfio.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 2bc3705a99bd2f1a..7736c3fbe468a3da 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -129,9 +129,13 @@ struct iommu_group *vfio_iommu_group_get(struct device *dev) * bus. We set iommudata simply to be able to identify these groups * as special use and for reclamation later. */ - if (group || !noiommu || iommu_present(dev->bus)) + if (group || !noiommu) return group; + if (iommu_present(dev->bus)) + dev_warn(dev, "iommu present (%ps), ignoring\n", +dev->bus->iommu_ops); + group = iommu_group_alloc(); if (IS_ERR(group)) return NULL; -- 2.7.4
[Qemu-devel] [PATCH/RFC 1/6] vfio: platform: Allow runtime override of reset_required
Currently reset_required can be configured using a module parameter. But it cannot be overridden at runtime through sysfs, as the parameter is read-only. Make it writeable for root, as this is useful if vfio-platform is builtin, so the following works: echo 0 > /sys/module/vfio_platform/parameters/reset_required Not-Signed-off-by: Geert Uytterhoeven --- TODO: Implement a vfio reset driver instead. --- drivers/vfio/platform/vfio_platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c index 6561751a1063ee29..ef89146deddcea80 100644 --- a/drivers/vfio/platform/vfio_platform.c +++ b/drivers/vfio/platform/vfio_platform.c @@ -24,7 +24,7 @@ #define DRIVER_DESC "VFIO for platform devices - User Level meta-driver" static bool reset_required = true; -module_param(reset_required, bool, 0444); +module_param(reset_required, bool, 0644); MODULE_PARM_DESC(reset_required, "override reset requirement (default: 1)"); /* probing devices from the linux platform bus */ -- 2.7.4
[Qemu-devel] [PATCH/RFC 5/5] hw/arm/sysbus-fdt: Enable rcar-gen3-gpio dynamic instantiation
Allow the instantiation of a Renesas R-Car Gen3 GPIO controller device
from the QEMU command line:
-device vfio-platform,host=,manufacturer=renesas,model=rcar-gen3-gpio
-device vfio-platform,sysfsdev=,manufacturer=renesas,model=rcar-gen3-gpio
A specialized device tree node is created for the guest, containing
compatible, reg, gpio-controller, and #gpio-cells properties.
Not-Yet-Signed-off-by: Geert Uytterhoeven
---
Question:
- Why do we need the manufacturer=foo,model=bar syntax? Can't this
just be extracted from the host DT?
TODO:
- Copy properties from the host DT, as add_amd_xgbe_fdt_node() does,
- Make this more generic?
---
hw/arm/sysbus-fdt.c | 47 +++
1 file changed, 47 insertions(+)
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index c5d4fd5604c28118..428175f343d9f3b9 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -416,6 +416,52 @@ static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev, void
*opaque)
return 0;
}
+/**
+ * add_rcar_gpio_fdt_node
+ *
+ * Generates a simple node with following properties:
+ * compatible string, regs, #gpio-cells, gpio-controller
+ */
+static int add_rcar_gpio_fdt_node(SysBusDevice *sbdev, void *opaque)
+{
+PlatformBusFDTData *data = opaque;
+PlatformBusDevice *pbus = data->pbus;
+void *fdt = data->fdt;
+const char *parent_node = data->pbus_node_name;
+int compat_str_len, i;
+char *nodename;
+uint32_t *reg_attr;
+uint64_t mmio_base;
+VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
+VFIODevice *vbasedev = &vdev->vbasedev;
+
+mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
+nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node,
+ vbasedev->name, mmio_base);
+qemu_fdt_add_subnode(fdt, nodename);
+
+compat_str_len = strlen(vdev->compat) + 1;
+qemu_fdt_setprop(fdt, nodename, "compatible",
+ vdev->compat, compat_str_len);
+
+qemu_fdt_setprop(fdt, nodename, "gpio-controller", NULL, 0);
+qemu_fdt_setprop_cells(fdt, nodename, "#gpio-cells", 2);
+
+reg_attr = g_new(uint32_t, vbasedev->num_regions * 2);
+for (i = 0; i < vbasedev->num_regions; i++) {
+mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, i);
+reg_attr[2 * i] = cpu_to_be32(mmio_base);
+reg_attr[2 * i + 1] = cpu_to_be32(
+memory_region_size(vdev->regions[i]->mem));
+}
+qemu_fdt_setprop(fdt, nodename, "reg", reg_attr,
+ vbasedev->num_regions * 2 * sizeof(uint32_t));
+
+g_free(reg_attr);
+g_free(nodename);
+return 0;
+}
+
/* manufacturer/model matching */
static bool vfio_platform_match(SysBusDevice *sbdev,
const BindingEntry *entry)
@@ -454,6 +500,7 @@ static const BindingEntry bindings[] = {
TYPE_BINDING(TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node),
TYPE_BINDING(TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node),
VFIO_PLATFORM_BINDING("amd", "xgbe-seattle-v1a", add_amd_xgbe_fdt_node),
+VFIO_PLATFORM_BINDING("renesas", "rcar-gen3-gpio", add_rcar_gpio_fdt_node),
#endif
TYPE_BINDING("", NULL), /* last element */
};
--
2.7.4
[Qemu-devel] [PATCH/RFC 3/5] hw/arm/virt: Allow dynamic sysbus devices again
Allow the instantation of generic dynamic sysbus devices again, without
the need to create a new device-specific vfio type.
This is a partial revert of commit 6f2062b9758ebc64 ("hw/arm/virt:
Allow only supported dynamic sysbus devices").
Not-Yet-Signed-off-by: Geert Uytterhoeven
---
hw/arm/virt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b334c82edae9fb1f..fa83784fc08ed076 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1596,6 +1596,7 @@ static void virt_machine_class_init(ObjectClass *oc, void
*data)
mc->max_cpus = 255;
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_CALXEDA_XGMAC);
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
+machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SYS_BUS_DEVICE);
mc->block_default_type = IF_VIRTIO;
mc->no_cdrom = 1;
mc->pci_allow_0_address = true;
--
2.7.4
[Qemu-devel] [PATCH/RFC 4/6] arm64: renesas_defconfig: Enable VFIO_PLATFORM and VFIO_NOIOMMU
Enable VFIO_PLATFORM for platform device pass-through. Enable VFIO_NOIOMMU for devices not part of an IOMMU group. Not-Signed-off-by: Geert Uytterhoeven --- Not intended for upstream merge. --- arch/arm64/configs/renesas_defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/configs/renesas_defconfig b/arch/arm64/configs/renesas_defconfig index 386211464efa4885..959aa9de8792ae2d 100644 --- a/arch/arm64/configs/renesas_defconfig +++ b/arch/arm64/configs/renesas_defconfig @@ -259,7 +259,9 @@ CONFIG_DMADEVICES=y CONFIG_RCAR_DMAC=y CONFIG_RENESAS_USB_DMAC=y CONFIG_VFIO=y +CONFIG_VFIO_NOIOMMU=y CONFIG_VFIO_PCI=y +CONFIG_VFIO_PLATFORM=y CONFIG_VIRTIO_PCI=y CONFIG_VIRTIO_BALLOON=y CONFIG_VIRTIO_MMIO=y -- 2.7.4
[Qemu-devel] [PATCH/RFC 2/5] hw/arm/sysbus-fdt: Allow device matching with compat string
From: Auger Eric
Up to now we have relied on the device type to identify a device tree
node creation function. Since we would like the VFIO-PLATFORM
device to be instantiable with different compatibility strings
we introduce the capability to specialize the node creation depending
on a manufacturer/model combo.
NodeCreationPair is renamed into BindingEntry. The struct is enhanced
with manufacturer, model and match_fn() fields. We introduce a new
matching function adapted to vfio-platform generic device.
>From now on, the AMD XGBE can be instantiated with either manner, ie:
-device vfio-amd-xgbe,host=e090.xgmac
or new option line:
-device
vfio-platform,host=e090.xgmac,manufacturer=amd,model=xgbe-seattle-v1a
Signed-off-by: Eric Auger
Signed-off-by: Geert Uytterhoeven
---
hw/arm/sysbus-fdt.c | 61 +
1 file changed, 48 insertions(+), 13 deletions(-)
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index d68e3dcdbd260895..c5d4fd5604c28118 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -58,11 +58,14 @@ typedef struct PlatformBusFDTNotifierParams {
ARMPlatformBusFDTParams *fdt_params;
} PlatformBusFDTNotifierParams;
-/* struct that associates a device type name and a node creation function */
-typedef struct NodeCreationPair {
+/* struct that allows to match a device and create its FDT node */
+typedef struct BindingEntry {
const char *typename;
-int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque);
-} NodeCreationPair;
+const char *manufacturer;
+const char *model;
+int (*add_fn)(SysBusDevice *sbdev, void *opaque);
+bool (*match_fn)(SysBusDevice *sbdev, const struct BindingEntry *combo);
+} BindingEntry;
/* helpers */
@@ -413,15 +416,46 @@ static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev,
void *opaque)
return 0;
}
+/* manufacturer/model matching */
+static bool vfio_platform_match(SysBusDevice *sbdev,
+const BindingEntry *entry)
+{
+VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
+
+if (strcmp(entry->model, vdev->model)) {
+return false;
+}
+
+if (entry->manufacturer) {
+if (!vdev->manufacturer) {
+return false;
+}
+return !strcmp(entry->manufacturer, vdev->manufacturer);
+}
+return true;
+}
+
+#define VFIO_PLATFORM_BINDING(manuf, model, add_fn) \
+{TYPE_VFIO_PLATFORM, (manuf), (model), (add_fn), vfio_platform_match}
+
#endif /* CONFIG_LINUX */
-/* list of supported dynamic sysbus devices */
-static const NodeCreationPair add_fdt_node_functions[] = {
+/* Device type based matching */
+static bool type_match(SysBusDevice *sbdev, const BindingEntry *entry)
+{
+return !strcmp(object_get_typename(OBJECT(sbdev)), entry->typename);
+}
+
+#define TYPE_BINDING(type, add_fn) {(type), NULL, NULL, (add_fn), type_match}
+
+/* list of supported dynamic sysbus bindings */
+static const BindingEntry bindings[] = {
#ifdef CONFIG_LINUX
-{TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node},
-{TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node},
+TYPE_BINDING(TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node),
+TYPE_BINDING(TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node),
+VFIO_PLATFORM_BINDING("amd", "xgbe-seattle-v1a", add_amd_xgbe_fdt_node),
#endif
-{"", NULL}, /* last element */
+TYPE_BINDING("", NULL), /* last element */
};
/* Generic Code */
@@ -440,10 +474,11 @@ static void add_fdt_node(SysBusDevice *sbdev, void
*opaque)
{
int i, ret;
-for (i = 0; i < ARRAY_SIZE(add_fdt_node_functions); i++) {
-if (!strcmp(object_get_typename(OBJECT(sbdev)),
-add_fdt_node_functions[i].typename)) {
-ret = add_fdt_node_functions[i].add_fdt_node_fn(sbdev, opaque);
+for (i = 0; i < ARRAY_SIZE(bindings); i++) {
+const BindingEntry *iter = &bindings[i];
+
+if (iter->match_fn(sbdev, iter)) {
+ret = iter->add_fn(sbdev, opaque);
assert(!ret);
return;
}
--
2.7.4
[Qemu-devel] [PATCH/RFC 4/5] vfio: No-IOMMU mode support
From: Xiao Feng Ren
Add qemu support for the newly introduced VFIO No-IOMMU driver.
We need to add special handling for:
- Group character device is /dev/vfio/noiommu-$GROUP.
- No-IOMMU does not rely on a memory listener.
- No IOMMU will be set for its group, so no need to call
vfio_kvm_device_add_group.
Signed-off-by: Xiao Feng Ren
[geert: Rebase]
Signed-off-by: Geert Uytterhoeven
---
hw/vfio/common.c | 61 ++-
include/hw/vfio/vfio-common.h | 2 ++
2 files changed, 50 insertions(+), 13 deletions(-)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index f895e3c3359af779..2ee94a3304202a74 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1019,6 +1019,33 @@ static int vfio_connect_container(VFIOGroup *group,
AddressSpace *as,
container->fd = fd;
QLIST_INIT(&container->giommu_list);
QLIST_INIT(&container->hostwin_list);
+container->noiommu = group->noiommu;
+
+if (container->noiommu) {
+ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
+if (ret) {
+error_report("vfio: failed to set group container: %m");
+ret = -errno;
+goto free_container_exit;
+}
+
+ret = ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_NOIOMMU_IOMMU);
+if (!ret) {
+error_report("vfio: No available IOMMU models");
+ret = -EINVAL;
+goto free_container_exit;
+}
+
+ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_NOIOMMU_IOMMU);
+if (ret) {
+error_report("vfio: failed to set iommu for container: %m");
+ret = -errno;
+goto free_container_exit;
+}
+
+goto listener_register;
+}
+
if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU) ||
ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU)) {
bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU);
@@ -1151,15 +1178,16 @@ static int vfio_connect_container(VFIOGroup *group,
AddressSpace *as,
group->container = container;
QLIST_INSERT_HEAD(&container->group_list, group, container_next);
-container->listener = vfio_memory_listener;
-
-memory_listener_register(&container->listener, container->space->as);
-
-if (container->error) {
-ret = container->error;
-error_setg_errno(errp, -ret,
- "memory listener initialization failed for
container");
-goto listener_release_exit;
+listener_register:
+if (!container->noiommu) {
+container->listener = vfio_memory_listener;
+memory_listener_register(&container->listener, container->space->as);
+if (container->error) {
+ret = container->error;
+error_setg_errno(errp, -ret,
+"memory listener initialization failed for container");
+goto listener_release_exit;
+}
}
container->initialized = true;
@@ -1169,7 +1197,9 @@ listener_release_exit:
QLIST_REMOVE(group, container_next);
QLIST_REMOVE(container, next);
vfio_kvm_device_del_group(group);
-vfio_listener_release(container);
+if (!container->noiommu) {
+vfio_listener_release(container);
+}
free_container_exit:
g_free(container);
@@ -1195,7 +1225,7 @@ static void vfio_disconnect_container(VFIOGroup *group)
* since unset may destroy the backend container if it's the last
* group.
*/
-if (QLIST_EMPTY(&container->group_list)) {
+if (QLIST_EMPTY(&container->group_list) && !container->noiommu) {
vfio_listener_release(container);
}
@@ -1249,8 +1279,13 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as,
Error **errp)
snprintf(path, sizeof(path), "/dev/vfio/%d", groupid);
group->fd = qemu_open(path, O_RDWR);
if (group->fd < 0) {
-error_setg_errno(errp, errno, "failed to open %s", path);
-goto free_group_exit;
+snprintf(path, sizeof(path), "/dev/vfio/noiommu-%d", groupid);
+group->fd = qemu_open(path, O_RDWR);
+if (group->fd < 0) {
+error_setg_errno(errp, errno, "failed to open %s", path);
+goto free_group_exit;
+}
+group->noiommu = 1;
}
if (ioctl(group->fd, VFIO_GROUP_GET_STATUS, &status)) {
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index f3a2ac9fee02066f..aca2e33efb9b118c 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -77,6 +77,7 @@ struct VFIOGroup;
typedef struct VFIOContainer {
VFIOAddressSpace *space;
int fd; /* /dev/vfio/vfio, empowered by the attached groups */
+bool noiommu;
MemoryListener listener;
MemoryListener prereg_lis
[Qemu-devel] [PATCH/RFC 0/5] R-Car Gen3 GPIO Pass-Through Prototype (QEMU)
Hi all,
This RFC patch series is the QEMU side of a GPIO Pass-Through prototype
for Renesas R-Car platforms using vfio-platform. Together with its
counterpart for Linux, it provides direct access from a QEMU+KVM guest
to a GPIO controller in an R-Car Gen3 SoC. This allows the guest to
control the LEDs on a Renesas Salvator-X(S) board.
This patch series is not meant to be upstreamed as-is. Indeed, for
various reasons (e.g. security, as the different GPIOs on the same GPIO
controller may control different parts of the system) access to GPIOs is
better not implemented using Device Pass-Through, but by
paravirtualization. Yet, this is still a simple and valuable
proof-of-concept, which can serve as a basis for the future development
of Pass-Through support for more complex platform devices on R-Car Gen3
SoCs.
- Patches 1-2 (submitted before by Eric Auger) make the vfio-platform
device non-abstract, incl. matching using a compatible string.
- Patch 3 allows dynamic sysbus devices again, without needing to
create device-specific vfio types for each and every new device.
- Patch 4 (submitted before by Xiao Feng Ren) adds support for the
VFIO No-IOMMU mode, which was added to Linux two years ago (in
v4.4).
- Patch 5 adds code to instantiate device nodes for Renesas R-Car Gen3
GPIO controllers.
Several questions and TODOs are appended to the individual patches.
Please see https://elinux.org/R-Car/Virtualization/VFIO for full usage
instructions of this prototype.
Thanks for your comments!
Auger Eric (2):
vfio/platform: make the vfio-platform device non abstract
hw/arm/sysbus-fdt: Allow device matching with compat string
Geert Uytterhoeven (2):
hw/arm/virt: Allow dynamic sysbus devices again
hw/arm/sysbus-fdt: Enable rcar-gen3-gpio dynamic instatiation
Xiao Feng Ren (1):
vfio: No-IOMMU mode support
hw/arm/sysbus-fdt.c | 108 +++-
hw/arm/virt.c | 1 +
hw/vfio/common.c| 61 ++-
hw/vfio/platform.c | 20 +++-
include/hw/vfio/vfio-common.h | 2 +
include/hw/vfio/vfio-platform.h | 2 +
6 files changed, 166 insertions(+), 28 deletions(-)
--
2.7.4
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[Qemu-devel] [PATCH/RFC 5/6] gpio: rcar: Add virtualization workarounds
Allow to enable the driver if virtualization is enabled.
Handle the absence of clocks and interrupts, to support guests that
don't provide these yet.
Not-Signed-off-by: Geert Uytterhoeven
---
To be dropped once clocks and interrupts are supported on the guest.
---
drivers/gpio/Kconfig | 2 +-
drivers/gpio/gpio-rcar.c | 28
2 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d6a8e851ad13b8e6..50eeaa3bb0749b84 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -386,7 +386,7 @@ config GPIO_PXA
config GPIO_RCAR
tristate "Renesas R-Car GPIO"
- depends on ARCH_RENESAS || COMPILE_TEST
+ depends on ARCH_RENESAS || VIRTIO_MMIO || COMPILE_TEST
select GPIOLIB_IRQCHIP
help
Say yes here to support GPIO on Renesas R-Car SoCs.
diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
index e76de57dd617d7e2..bc205b7fbb30e892 100644
--- a/drivers/gpio/gpio-rcar.c
+++ b/drivers/gpio/gpio-rcar.c
@@ -442,22 +442,16 @@ static int gpio_rcar_probe(struct platform_device *pdev)
p->clk = devm_clk_get(dev, NULL);
if (IS_ERR(p->clk)) {
- if (p->needs_clk) {
- dev_err(dev, "unable to get clock\n");
- ret = PTR_ERR(p->clk);
- goto err0;
- }
+ if (p->needs_clk)
+ dev_warn(dev, "missing clock, ignoring\n");
p->clk = NULL;
}
pm_runtime_enable(dev);
irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (!irq) {
- dev_err(dev, "missing IRQ\n");
- ret = -EINVAL;
- goto err0;
- }
+ if (!irq)
+ dev_warn(dev, "missing IRQ, ignoring\n");
io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
p->base = devm_ioremap_resource(dev, io);
@@ -502,12 +496,14 @@ static int gpio_rcar_probe(struct platform_device *pdev)
goto err1;
}
- p->irq_parent = irq->start;
- if (devm_request_irq(dev, irq->start, gpio_rcar_irq_handler,
-IRQF_SHARED, name, p)) {
- dev_err(dev, "failed to request IRQ\n");
- ret = -ENOENT;
- goto err1;
+ if (irq) {
+ p->irq_parent = irq->start;
+ if (devm_request_irq(dev, irq->start, gpio_rcar_irq_handler,
+IRQF_SHARED, name, p)) {
+ dev_err(dev, "failed to request IRQ\n");
+ ret = -ENOENT;
+ goto err1;
+ }
}
dev_info(dev, "driving %d GPIOs\n", npins);
--
2.7.4
[PATCH v6 0/8] gpio: Add GPIO Aggregator
gpioset gpiochip12 0=1 1=0 # LED6 ON, LED7 OFF
$ gpioset gpiochip13 0=1 # LED8 ON
$ gpioset gpiochip13 0=0 # LED8 OFF
- Destroy aggregators:
$ echo gpio-aggregator.0 \
> /sys/bus/platform/drivers/gpio-aggregator/delete_device
$ echo gpio-aggregator.1 \
> /sys/bus/platform/drivers/gpio-aggregator/delete_device
To ease testing, I have pushed this series to the
topic/gpio-aggregator-v6 branch of my renesas-drivers repository at
git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git.
Kbuild test robot reported no issues.
Thanks!
References:
[1] "[PATCH V4 2/2] gpio: inverter: document the inverter bindings"
(https://lore.kernel.org/r/1561699236-18620-3-git-send-email-harish_kand...@mentor.com/)
[2] "[PATCH v5 0/5] gpio: Add GPIO Aggregator"
(https://lore.kernel.org/r/20200218151812.7816-1-geert+rene...@glider.be/)
[3] "[PATCH v4 0/5] gpio: Add GPIO Aggregator"
(https://lore.kernel.org/r/20200115181523.23556-1-geert+rene...@glider.be)
[4] "[PATCH v3 0/7] gpio: Add GPIO Aggregator/Repeater"
(https://lore.kernel.org/r/20191127084253.16356-1-geert+rene...@glider.be/)
[5] "[PATCH/RFC v2 0/5] gpio: Add GPIO Aggregator Driver"
(https://lore.kernel.org/r/20190911143858.13024-1-geert+rene...@glider.be/)
[6] "[PATCH RFC] gpio: Add Virtual Aggregator GPIO Driver"
(https://lore.kernel.org/r/20190705160536.12047-1-geert+rene...@glider.be/)
[7] "[PATCH QEMU POC] Add a GPIO backend"
(https://lore.kernel.org/r/20181003152521.23144-1-geert+rene...@glider.be/)
[8] "Getting To Blinky: Virt Edition / Making device pass-through
work on embedded ARM"
(https://fosdem.org/2019/schedule/event/vai_getting_to_blinky/)
Geert Uytterhoeven (8):
ARM: integrator: impd1: Use GPIO_LOOKUP() helper macro
i2c: i801: Use GPIO_LOOKUP() helper macro
mfd: sm501: Use GPIO_LOOKUP_IDX() helper macro
gpiolib: Add support for GPIO lookup by line name
gpiolib: Introduce gpiod_set_config()
gpio: Add GPIO Aggregator
docs: gpio: Add GPIO Aggregator documentation
MAINTAINERS: Add GPIO Aggregator section
.../admin-guide/gpio/gpio-aggregator.rst | 111
Documentation/admin-guide/gpio/index.rst | 1 +
Documentation/driver-api/gpio/board.rst | 10 +-
MAINTAINERS | 7 +
arch/arm/mach-integrator/impd1.c | 11 +-
drivers/gpio/Kconfig | 12 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-aggregator.c| 568 ++
drivers/gpio/gpiolib.c| 50 +-
drivers/i2c/busses/i2c-i801.c | 6 +-
drivers/mfd/sm501.c | 24 +-
include/linux/gpio/consumer.h | 8 +
include/linux/gpio/machine.h | 15 +-
13 files changed, 776 insertions(+), 48 deletions(-)
create mode 100644 Documentation/admin-guide/gpio/gpio-aggregator.rst
create mode 100644 drivers/gpio/gpio-aggregator.c
--
2.17.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH v6 8/8] MAINTAINERS: Add GPIO Aggregator section
Add a maintainership section for the GPIO Aggregator, covering documentation and driver source code. Signed-off-by: Geert Uytterhoeven Reviewed-by: Eugeniu Rosca Tested-by: Eugeniu Rosca --- v6: - No changes, v5: - Add Reviewed-by, Tested-by, v4: - Drop controversial GPIO repeater, v3: - New. --- MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index fcd79fc38928fafc..1fad69b956df1162 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7127,6 +7127,13 @@ F: Documentation/firmware-guide/acpi/gpio-properties.rst F: drivers/gpio/gpiolib-acpi.c F: drivers/gpio/gpiolib-acpi.h +GPIO AGGREGATOR +M: Geert Uytterhoeven +L: linux-g...@vger.kernel.org +S: Maintained +F: Documentation/admin-guide/gpio/gpio-aggregator.rst +F: drivers/gpio/gpio-aggregator.c + GPIO IR Transmitter M: Sean Young L: linux-me...@vger.kernel.org -- 2.17.1
[PATCH v6 5/8] gpiolib: Introduce gpiod_set_config()
The GPIO Aggregator will need a method to forward a .set_config() call
to its parent gpiochip. This requires obtaining the gpio_chip and
offset for a given gpio_desc. While gpiod_to_chip() is public,
gpio_chip_hwgpio() is not, so there is currently no method to obtain the
needed GPIO offset parameter.
Hence introduce a public gpiod_set_config() helper, which invokes the
.set_config() callback through a gpio_desc pointer, like is done for
most other gpio_chip callbacks.
Rewrite the existing gpiod_set_debounce() helper as a wrapper around
gpiod_set_config(), to avoid duplication.
Signed-off-by: Geert Uytterhoeven
---
v6:
- New.
---
drivers/gpio/gpiolib.c| 28 ++--
include/linux/gpio/consumer.h | 8
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index c756602e249c052e..30ea75e972b5a3b1 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3478,6 +3478,26 @@ int gpiod_direction_output(struct gpio_desc *desc, int
value)
}
EXPORT_SYMBOL_GPL(gpiod_direction_output);
+/**
+ * gpiod_set_config - sets @config for a GPIO
+ * @desc: descriptor of the GPIO for which to set the configuration
+ * @config: Same packed config format as generic pinconf
+ *
+ * Returns:
+ * 0 on success, %-ENOTSUPP if the controller doesn't support setting the
+ * configuration.
+ */
+int gpiod_set_config(struct gpio_desc *desc, unsigned long config)
+{
+ struct gpio_chip *chip;
+
+ VALIDATE_DESC(desc);
+ chip = desc->gdev->chip;
+
+ return gpio_do_set_config(chip, gpio_chip_hwgpio(desc), config);
+}
+EXPORT_SYMBOL_GPL(gpiod_set_config);
+
/**
* gpiod_set_debounce - sets @debounce time for a GPIO
* @desc: descriptor of the GPIO for which to set debounce time
@@ -3489,14 +3509,10 @@ EXPORT_SYMBOL_GPL(gpiod_direction_output);
*/
int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
{
- struct gpio_chip*chip;
- unsigned long config;
-
- VALIDATE_DESC(desc);
- chip = desc->gdev->chip;
+ unsigned long config;
config = pinconf_to_config_packed(PIN_CONFIG_INPUT_DEBOUNCE, debounce);
- return gpio_do_set_config(chip, gpio_chip_hwgpio(desc), config);
+ return gpiod_set_config(desc, config);
}
EXPORT_SYMBOL_GPL(gpiod_set_debounce);
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 0a72fccf60fff230..901aab89d025f3ff 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -157,6 +157,7 @@ int gpiod_set_raw_array_value_cansleep(unsigned int
array_size,
struct gpio_array *array_info,
unsigned long *value_bitmap);
+int gpiod_set_config(struct gpio_desc *desc, unsigned long config);
int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce);
int gpiod_set_transitory(struct gpio_desc *desc, bool transitory);
void gpiod_toggle_active_low(struct gpio_desc *desc);
@@ -473,6 +474,13 @@ static inline int
gpiod_set_raw_array_value_cansleep(unsigned int array_size,
return 0;
}
+static inline int gpiod_set_config(struct gpio_desc *desc, unsigned long
config)
+{
+ /* GPIO can never have been requested */
+ WARN_ON(desc);
+ return -ENOSYS;
+}
+
static inline int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
{
/* GPIO can never have been requested */
--
2.17.1
[PATCH v6 4/8] gpiolib: Add support for GPIO lookup by line name
Currently a GPIO lookup table can only refer to a specific GPIO by a
tuple, consisting of a GPIO controller label and a GPIO offset inside
the controller.
However, a GPIO may also carry a line name, defined by DT or ACPI.
If present, the line name is the most use-centric way to refer to a
GPIO. Hence add support for looking up GPIOs by line name.
Implement this by reusing the existing gpiod_lookup infrastructure.
Rename gpiod_lookup.chip_label to gpiod_lookup.key, to make it clear
that this field can have two meanings, and update the kerneldoc and
GPIO_LOOKUP*() macros.
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Ulrich Hecht
Reviewed-by: Eugeniu Rosca
Tested-by: Eugeniu Rosca
---
v6:
- Update Documentation/driver-api/gpio/board.rst,
- Reword rationale,
v5:
- Add Reviewed-by, Tested-by,
v4:
- Add Reviewed-by,
- Rename gpiod_lookup.chip_label.
- Use U16_MAX instead of (u16)-1,
v3:
- New.
---
Documentation/driver-api/gpio/board.rst | 10 ++
drivers/gpio/gpiolib.c | 22 +-
include/linux/gpio/machine.h| 15 ---
3 files changed, 31 insertions(+), 16 deletions(-)
diff --git a/Documentation/driver-api/gpio/board.rst
b/Documentation/driver-api/gpio/board.rst
index ce91518bf9f48ded..0ad1f8cacf5e5d26 100644
--- a/Documentation/driver-api/gpio/board.rst
+++ b/Documentation/driver-api/gpio/board.rst
@@ -113,13 +113,15 @@ files that desire to do so need to include the following
header::
GPIOs are mapped by the means of tables of lookups, containing instances of the
gpiod_lookup structure. Two macros are defined to help declaring such
mappings::
- GPIO_LOOKUP(chip_label, chip_hwnum, con_id, flags)
- GPIO_LOOKUP_IDX(chip_label, chip_hwnum, con_id, idx, flags)
+ GPIO_LOOKUP(key, chip_hwnum, con_id, flags)
+ GPIO_LOOKUP_IDX(key, chip_hwnum, con_id, idx, flags)
where
- - chip_label is the label of the gpiod_chip instance providing the GPIO
- - chip_hwnum is the hardware number of the GPIO within the chip
+ - key is either the label of the gpiod_chip instance providing the GPIO, or
+the GPIO line name
+ - chip_hwnum is the hardware number of the GPIO within the chip, or U16_MAX
+to indicate that key is a GPIO line name
- con_id is the name of the GPIO function from the device point of view. It
can be NULL, in which case it will match any function.
- idx is the index of the GPIO within the function.
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 8d7366f4451fe695..c756602e249c052e 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4643,7 +4643,7 @@ static struct gpio_desc *gpiod_find(struct device *dev,
const char *con_id,
if (!table)
return desc;
- for (p = &table->table[0]; p->chip_label; p++) {
+ for (p = &table->table[0]; p->key; p++) {
struct gpio_chip *chip;
/* idx must always match exactly */
@@ -4654,18 +4654,30 @@ static struct gpio_desc *gpiod_find(struct device *dev,
const char *con_id,
if (p->con_id && (!con_id || strcmp(p->con_id, con_id)))
continue;
- chip = find_chip_by_name(p->chip_label);
+ if (p->chip_hwnum == U16_MAX) {
+ desc = gpio_name_to_desc(p->key);
+ if (desc) {
+ *flags = p->flags;
+ return desc;
+ }
+
+ dev_warn(dev, "cannot find GPIO line %s, deferring\n",
+p->key);
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+
+ chip = find_chip_by_name(p->key);
if (!chip) {
/*
* As the lookup table indicates a chip with
-* p->chip_label should exist, assume it may
+* p->key should exist, assume it may
* still appear later and let the interested
* consumer be probed again or let the Deferred
* Probe infrastructure handle the error.
*/
dev_warn(dev, "cannot find GPIO chip %s, deferring\n",
-p->chip_label);
+p->key);
return ERR_PTR(-EPROBE_DEFER);
}
@@ -4696,7 +4708,7 @@ static int platform_gpio_count(struct device *dev, const
char *con_id)
if (!table)
return -ENOENT;
- for (p = &table->table[0]; p->chip_label; p++) {
+ for (p = &table->table[0]; p->key; p++) {
if ((con_id && p->con_id && !strcmp(con_id, p->con_id)) ||
(!co
[PATCH v6 7/8] docs: gpio: Add GPIO Aggregator documentation
Document the GPIO Aggregator, and the two typical use-cases.
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Ulrich Hecht
Reviewed-by: Eugeniu Rosca
Tested-by: Eugeniu Rosca
---
v6:
- Fix "allows" without object:
-> provides a mechanism to aggregate GPIOs,
-> provides access control for a set of one or more GPIOs,
-> allows the user to communicate,
- Drop "gpiochipN" support,
- Extend example,
v5:
- Add Reviewed-by, Tested-by,
- Fix inconsistent indentation,
v4:
- Add Reviewed-by,
- Drop controversial GPIO repeater,
- Clarify industrial control use case,
- Fix typo s/communicated/communicate/,
- Replace abstract frobnicator example by concrete door example with
gpio-line-names,
v3:
- New.
---
.../admin-guide/gpio/gpio-aggregator.rst | 111 ++
Documentation/admin-guide/gpio/index.rst | 1 +
2 files changed, 112 insertions(+)
create mode 100644 Documentation/admin-guide/gpio/gpio-aggregator.rst
diff --git a/Documentation/admin-guide/gpio/gpio-aggregator.rst
b/Documentation/admin-guide/gpio/gpio-aggregator.rst
new file mode 100644
index ..5cd1e7221756504c
--- /dev/null
+++ b/Documentation/admin-guide/gpio/gpio-aggregator.rst
@@ -0,0 +1,111 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+GPIO Aggregator
+===
+
+The GPIO Aggregator provides a mechanism to aggregate GPIOs, and expose them as
+a new gpio_chip. This supports the following use cases.
+
+
+Aggregating GPIOs using Sysfs
+-
+
+GPIO controllers are exported to userspace using /dev/gpiochip* character
+devices. Access control to these devices is provided by standard UNIX file
+system permissions, on an all-or-nothing basis: either a GPIO controller is
+accessible for a user, or it is not.
+
+The GPIO Aggregator provides access control for a set of one or more GPIOs, by
+aggregating them into a new gpio_chip, which can be assigned to a group or user
+using standard UNIX file ownership and permissions. Furthermore, this
+simplifies and hardens exporting GPIOs to a virtual machine, as the VM can just
+grab the full GPIO controller, and no longer needs to care about which GPIOs to
+grab and which not, reducing the attack surface.
+
+Aggregated GPIO controllers are instantiated and destroyed by writing to
+write-only attribute files in sysfs.
+
+/sys/bus/platform/drivers/gpio-aggregator/
+
+ "new_device" ...
+ Userspace may ask the kernel to instantiate an aggregated GPIO
+ controller by writing a string describing the GPIOs to
+ aggregate to the "new_device" file, using the format
+
+ .. code-block:: none
+
+ [] [ ] ...
+
+ Where:
+
+ "" ...
+ is a GPIO line name,
+
+ "" ...
+ is a GPIO chip label, and
+
+ "" ...
+ is a comma-separated list of GPIO offsets and/or
+ GPIO offset ranges denoted by dashes.
+
+ Example: Instantiate a new GPIO aggregator by aggregating GPIO
+ line 19 of "e6052000.gpio" and GPIO lines 20-21 of
+ "e605.gpio" into a new gpio_chip:
+
+ .. code-block:: sh
+
+ $ echo 'e6052000.gpio 19 e605.gpio 20-21' > new_device
+
+ "delete_device" ...
+ Userspace may ask the kernel to destroy an aggregated GPIO
+ controller after use by writing its device name to the
+ "delete_device" file.
+
+ Example: Destroy the previously-created aggregated GPIO
+ controller, assumed to be "gpio-aggregator.0":
+
+ .. code-block:: sh
+
+ $ echo gpio-aggregator.0 > delete_device
+
+
+Generic GPIO Driver
+---
+
+The GPIO Aggregator can also be used as a generic driver for a simple
+GPIO-operated device described in DT, without a dedicated in-kernel driver.
+This is useful in industrial control, and is not unlike e.g. spidev, which
+allows the user to communicate with an SPI device from userspace.
+
+Binding a device to the GPIO Aggregator is performed either by modifying the
+gpio-aggregator driver, or by writing to the "driver_override" file in Sysfs.
+
+Example: If "door" is a GPIO-operated device described in DT, using its own
+compatible value::
+
+ door {
+ compatible = "myvendor,mydoor";
+
+ gpios = <&gpio2 19 GPIO_ACTIVE_HIGH>,
+ <&gpio2 20 GPIO_ACTIVE_LOW>;
+ gpio-line-names = "open", "lock";
+ };
+
+it can be bound to the GPIO Aggregator by either:
+
+1. Adding its compatibl
[PATCH v6 2/8] i2c: i801: Use GPIO_LOOKUP() helper macro
i801_add_mux() fills in the GPIO lookup table by manually populating an
array of gpiod_lookup structures. Use the existing GPIO_LOOKUP() helper
macro instead, to relax a dependency on the gpiod_lookup structure's
member names.
Signed-off-by: Geert Uytterhoeven
Cc: Jean Delvare
Cc: linux-...@vger.kernel.org
---
While this patch is a dependency for "[PATCH v6 4/8] gpiolib: Add
support for GPIO lookup by line name", it can be applied independently.
But an Acked-by would be nice, too.
Cover letter and full series at
https://lore.kernel.org/r/20200324135328.5796-1-geert+rene...@glider.be/
v6:
- New.
---
drivers/i2c/busses/i2c-i801.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index ca4f096fef749302..8e64a71bea684cc7 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1444,9 +1444,9 @@ static int i801_add_mux(struct i801_priv *priv)
return -ENOMEM;
lookup->dev_id = "i2c-mux-gpio";
for (i = 0; i < mux_config->n_gpios; i++) {
- lookup->table[i].chip_label = mux_config->gpio_chip;
- lookup->table[i].chip_hwnum = mux_config->gpios[i];
- lookup->table[i].con_id = "mux";
+ lookup->table[i] = (struct gpiod_lookup)
+ GPIO_LOOKUP(mux_config->gpio_chip,
+ mux_config->gpios[i], "mux", 0);
}
gpiod_add_lookup_table(lookup);
priv->lookup = lookup;
--
2.17.1
[PATCH v6 1/8] ARM: integrator: impd1: Use GPIO_LOOKUP() helper macro
impd1_probe() fills in the GPIO lookup table by manually populating an array of gpiod_lookup structures. Use the existing GPIO_LOOKUP() helper macro instead, to relax a dependency on the gpiod_lookup structure's member names. Signed-off-by: Geert Uytterhoeven Cc: linux-arm-ker...@lists.infradead.org --- While this patch is a dependency for "[PATCH v6 4/8] gpiolib: Add support for GPIO lookup by line name", it can be applied independently. But an Acked-by would be nice, too. Cover letter and full series at https://lore.kernel.org/r/20200324135328.5796-1-geert+rene...@glider.be/ v6: - New. --- arch/arm/mach-integrator/impd1.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/arch/arm/mach-integrator/impd1.c b/arch/arm/mach-integrator/impd1.c index 1ecbea5331d6ed8b..6f875ded841924d8 100644 --- a/arch/arm/mach-integrator/impd1.c +++ b/arch/arm/mach-integrator/impd1.c @@ -410,13 +410,10 @@ static int __ref impd1_probe(struct lm_device *dev) * 5 = Key lower right */ /* We need the two MMCI GPIO entries */ - lookup->table[0].chip_label = chipname; - lookup->table[0].chip_hwnum = 3; - lookup->table[0].con_id = "wp"; - lookup->table[1].chip_label = chipname; - lookup->table[1].chip_hwnum = 4; - lookup->table[1].con_id = "cd"; - lookup->table[1].flags = GPIO_ACTIVE_LOW; + lookup->table[0] = (struct gpiod_lookup) + GPIO_LOOKUP(chipname, 3, "wp", 0); + lookup->table[1] = (struct gpiod_lookup) + GPIO_LOOKUP(chipname, 4, "cd", GPIO_ACTIVE_LOW); gpiod_add_lookup_table(lookup); } -- 2.17.1
[PATCH v6 3/8] mfd: sm501: Use GPIO_LOOKUP_IDX() helper macro
i801_add_mux() fills in the GPIO lookup table by manually populating an array of gpiod_lookup structures. Use the existing GPIO_LOOKUP_IDX() helper macro instead, to relax a dependency on the gpiod_lookup structure's member names. Signed-off-by: Geert Uytterhoeven Cc: Lee Jones --- While this patch is a dependency for "[PATCH v6 4/8] gpiolib: Add support for GPIO lookup by line name", it can be applied independently. But an Acked-by would be nice, too. Cover letter and full series at https://lore.kernel.org/r/20200324135328.5796-1-geert+rene...@glider.be/ v6: - New. --- drivers/mfd/sm501.c | 24 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c index e49787e6bb93e5c8..ccd62b963952814e 100644 --- a/drivers/mfd/sm501.c +++ b/drivers/mfd/sm501.c @@ -1145,22 +1145,14 @@ static int sm501_register_gpio_i2c_instance(struct sm501_devdata *sm, return -ENOMEM; lookup->dev_id = "i2c-gpio"; - if (iic->pin_sda < 32) - lookup->table[0].chip_label = "SM501-LOW"; - else - lookup->table[0].chip_label = "SM501-HIGH"; - lookup->table[0].chip_hwnum = iic->pin_sda % 32; - lookup->table[0].con_id = NULL; - lookup->table[0].idx = 0; - lookup->table[0].flags = GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN; - if (iic->pin_scl < 32) - lookup->table[1].chip_label = "SM501-LOW"; - else - lookup->table[1].chip_label = "SM501-HIGH"; - lookup->table[1].chip_hwnum = iic->pin_scl % 32; - lookup->table[1].con_id = NULL; - lookup->table[1].idx = 1; - lookup->table[1].flags = GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN; + lookup->table[0] = (struct gpiod_lookup) + GPIO_LOOKUP_IDX(iic->pin_sda < 32 ? "SM501-LOW" : "SM501-HIGH", + iic->pin_sda % 32, NULL, 0, + GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN); + lookup->table[1] = (struct gpiod_lookup) + GPIO_LOOKUP_IDX(iic->pin_scl < 32 ? "SM501-LOW" : "SM501-HIGH", + iic->pin_scl % 32, NULL, 1, + GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN); gpiod_add_lookup_table(lookup); icd = dev_get_platdata(&pdev->dev); -- 2.17.1
[PATCH v6 6/8] gpio: Add GPIO Aggregator
GPIO controllers are exported to userspace using /dev/gpiochip* character devices. Access control to these devices is provided by standard UNIX file system permissions, on an all-or-nothing basis: either a GPIO controller is accessible for a user, or it is not. Currently no mechanism exists to control access to individual GPIOs. Hence add a GPIO driver to aggregate existing GPIOs, and expose them as a new gpiochip. This supports the following use cases: - Aggregating GPIOs using Sysfs This is useful for implementing access control, and assigning a set of GPIOs to a specific user or virtual machine. - Generic GPIO Driver This is useful for industrial control, where it can provide userspace access to a simple GPIO-operated device described in DT, cfr. e.g. spidev for SPI-operated devices. Signed-off-by: Geert Uytterhoeven Reviewed-by: Eugeniu Rosca Tested-by: Eugeniu Rosca --- v6: - Use gpiod_to_chip() instead of open-coding, - Drop debug print of gpio_desc.label, as it usually just points to the GPIO Aggregator itself, - Drop no longer needed #include "gpiolib.h", - Fix missing offset translation in gpio_fwd_set_config(), - Use GPIO_LOOKUP_IDX() to populate struct gpiod_lookup, v5: - Add Reviewed-by, Tested-by, v4: - Remove unused assignment to n in isrange(), - Check correct pointer after aggr->lookups->dev_id allocation, - Preinitialize flags to 0 in gpio_fwd_[gs]et_multiple() to avoid may-be-used-uninitialized warning, - Drop controversial GPIO repeater, - Update for gpiod_lookup.chip_label rename, - Use %pe to format error pointers, - Use U16_MAX instead of (u16)-1, - Correct comment indentation, - Use skip_spaces() helper, - Rename a and b to first_index resp. last_index, - Add comment to tmp[] use, - Improve Kconfig help text, - Include for gpiod_[gs]et_*(), - Drop unneeded valid_mask handling, - Add comment about sleeping and .set_config() support, v3: - Absorb GPIO forwarder, - Integrate GPIO Repeater and Generic GPIO driver functionality, - Use the aggregator parameters to create a GPIO lookup table instead of an array of GPIO descriptors, which allows to simplify the code: 1. This removes the need for calling gpio_name_to_desc(), gpiochip_find(), gpiochip_get_desc(), and gpiod_request(), 2. This allows the platform device to always use devm_gpiod_get_index(), regardless of the origin of the GPIOs, - Move parameter parsing from platform device probe to sysfs attribute store, removing the need for platform data passing, - Use more devm_*() functions to simplify cleanup, - Add pr_fmt(), - General refactoring, v2: - Add missing initialization of i in gpio_virt_agg_probe(), - Update for removed .need_valid_mask field and changed .init_valid_mask() signature, - Drop "virtual", rename to gpio-aggregator, - Drop bogus FIXME related to gpiod_set_transitory() expectations, - Use new GPIO Forwarder Helper, - Lift limit on the maximum number of GPIOs, - Improve parsing: - add support for specifying GPIOs by line name, - add support for specifying GPIO chips by ID, - add support for GPIO offset ranges, - names and offset specifiers must be separated by whitespace, - GPIO offsets must separated by spaces, - Use str_has_prefix() and kstrtouint(). --- drivers/gpio/Kconfig | 12 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-aggregator.c | 568 + 3 files changed, 581 insertions(+) create mode 100644 drivers/gpio/gpio-aggregator.c diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 6234ccc90e7eb4a1..6ddc7353a46afdf6 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -1541,6 +1541,18 @@ config GPIO_VIPERBOARD endmenu +config GPIO_AGGREGATOR + tristate "GPIO Aggregator" + help + Say yes here to enable the GPIO Aggregator, which provides a way to + aggregate existing GPIO lines into a new virtual GPIO chip. + This can serve the following purposes: + - Assign permissions for a collection of GPIO lines to a user, + - Export a collection of GPIO lines to a virtual machine, + - Provide a generic driver for a GPIO-operated device in an + industrial control context, to be operated from userspace using + the GPIO chardev interface. + config GPIO_MOCKUP tristate "GPIO Testing Driver" select IRQ_SIM diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index b2cfc21a97f3e52b..65bf3940e33cf734 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -25,6 +25,7 @@ obj-$(CONFIG_GPIO_74XX_MMIO) += gpio-74xx-mmio.o obj-$(CONFIG_GPIO_ADNP)+= gpio-adnp.o obj-$(CONFIG_GPIO_ADP5520) += gpio-adp5520.o obj-$(CONFIG_GPIO_ADP5588) +
Re: [PATCH v6 5/8] gpiolib: Introduce gpiod_set_config()
Hi Linus,
On Thu, Mar 26, 2020 at 10:26 PM Linus Walleij wrote:
> On Tue, Mar 24, 2020 at 2:57 PM Geert Uytterhoeven
> wrote:
> > The GPIO Aggregator will need a method to forward a .set_config() call
> > to its parent gpiochip. This requires obtaining the gpio_chip and
> > offset for a given gpio_desc. While gpiod_to_chip() is public,
> > gpio_chip_hwgpio() is not, so there is currently no method to obtain the
> > needed GPIO offset parameter.
> >
> > Hence introduce a public gpiod_set_config() helper, which invokes the
> > .set_config() callback through a gpio_desc pointer, like is done for
> > most other gpio_chip callbacks.
> >
> > Rewrite the existing gpiod_set_debounce() helper as a wrapper around
> > gpiod_set_config(), to avoid duplication.
> >
> > Signed-off-by: Geert Uytterhoeven
> > ---
> > v6:
> > - New.
>
> This is nice, I tried to actually just apply this (you also sent some
> two cleanups that I tried to apply) byt Yue's cleanup patch
> commit d18fddff061d2796525e6d4a958cb3d30aed8efd
> "gpiolib: Remove duplicated function gpio_do_set_config()"
> makes none of them apply :/
/me confused.
That commit was reverted later, so it shouldn't matter.
I have just verified, and both my full series and just this single
patch, do apply fine to all of current gpio/for-next, linus/master, and
next-20200327. They even apply fine to gpio/for-next before or after
the two cleanups I sent, too.
What am I missing?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH QEMU v2 4/5] ARM: PL061: Add gpiodev support
Make the PL061 GPIO controller user-creatable, and allow the user to tie
a newly created instance to a gpiochip on the host.
To create a new GPIO controller, the QEMU command line must be augmented
with:
-device pl061,host=
with the name or label of the gpiochip on the host.
Signed-off-by: Geert Uytterhoeven
---
v2:
- New.
---
hw/gpio/pl061.c | 35 +++
qemu-options.hx | 9 +
2 files changed, 44 insertions(+)
diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
index 74ba733a8a5e8ca5..98204f9a586ae8c8 100644
--- a/hw/gpio/pl061.c
+++ b/hw/gpio/pl061.c
@@ -12,11 +12,14 @@
#include "hw/arm/fdt.h"
#include "hw/gpio/pl061.h"
#include "hw/irq.h"
+#include "hw/qdev-properties.h"
#include "hw/sysbus.h"
#include "migration/vmstate.h"
+#include "qapi/error.h"
#include "qemu/log.h"
#include "qemu/module.h"
#include "sysemu/device_tree.h"
+#include "sysemu/gpiodev.h"
//#define DEBUG_PL061 1
@@ -41,6 +44,9 @@ static const uint8_t pl061_id_luminary[12] =
typedef struct PL061State {
SysBusDevice parent_obj;
+#ifdef CONFIG_GPIODEV
+char *host;
+#endif
MemoryRegion iomem;
uint32_t locked;
uint32_t data;
@@ -370,10 +376,39 @@ static void pl061_init(Object *obj)
qdev_init_gpio_out(dev, s->out, 8);
}
+#ifdef CONFIG_GPIODEV
+static Property pl061_properties[] = {
+DEFINE_PROP_STRING("host", PL061State, host),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void pl061_realize(DeviceState *dev, Error **errp)
+{
+PL061State *s = PL061(dev);
+
+if (!dev->opts) {
+/* Not created by user */
+return;
+}
+
+if (!s->host) {
+error_setg(errp, "'host' property is required");
+return;
+}
+
+qemu_gpiodev_add(dev, s->host, 8, errp);
+}
+#endif /* CONFIG_GPIODEV */
+
static void pl061_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+#ifdef CONFIG_GPIODEV
+device_class_set_props(dc, pl061_properties);
+dc->realize = pl061_realize;
+dc->user_creatable = true;
+#endif
dc->vmsd = &vmstate_pl061;
dc->reset = &pl061_reset;
}
diff --git a/qemu-options.hx b/qemu-options.hx
index 292d4e7c0cef6097..182de7fb63923b38 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -875,6 +875,15 @@ SRST
``-device isa-ipmi-bt,bmc=id[,ioport=val][,irq=val]``
Like the KCS interface, but defines a BT interface. The default port
is 0xe4 and the default interrupt is 5.
+
+#ifdef CONFIG_GPIODEV
+``-device pl061,host=gpiochip``
+Add a PL061 GPIO controller, and map its virtual GPIO lines to a GPIO
+controller on the host.
+
+``host=gpiochip``
+The name or label of the GPIO controller on the host.
+#endif
ERST
DEF("name", HAS_ARG, QEMU_OPTION_name,
--
2.17.1
[PATCH QEMU v2 3/5] Add a GPIO backend using libgpiod
Add a GPIO controller backend, to connect virtual GPIOs on the guest to
physical GPIOs on the host. This allows the guest to control any
external device connected to the physical GPIOs.
Features and limitations:
- The backend uses libgpiod on Linux,
- For now only GPIO outputs are supported,
- The number of GPIO lines mapped is limited to the number of GPIO
lines available on the virtual GPIO controller.
Future work:
- GPIO inputs,
- GPIO line configuration,
- Optimizations for controlling multiple GPIO lines at once,
- ...
Signed-off-by: Geert Uytterhoeven
---
v2:
- Drop vgpios and gpios parameters, and map the full gpiochip instead,
- Replace hardcoded PL061 instance by multiple dynamic instances,
registered through qemu_gpiodev_add().
---
MAINTAINERS | 6 +++
backends/Makefile.objs | 2 +
backends/gpiodev.c | 94
configure| 28
include/sysemu/gpiodev.h | 12 +
5 files changed, 142 insertions(+)
create mode 100644 backends/gpiodev.c
create mode 100644 include/sysemu/gpiodev.h
diff --git a/MAINTAINERS b/MAINTAINERS
index e760f65270d29d5d..a70af47430083d14 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -607,6 +607,12 @@ F: include/hw/arm/digic.h
F: hw/*/digic*
F: include/hw/*/digic*
+GPIO device backend
+M: Geert Uytterhoeven
+S: Supported
+F: backends/gpiodev.c
+F: include/sysemu/gpiodev.h
+
Goldfish RTC
M: Anup Patel
M: Alistair Francis
diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 28a847cd571d96ed..ee658e797454119a 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -21,3 +21,5 @@ common-obj-$(CONFIG_LINUX) += hostmem-memfd.o
common-obj-$(CONFIG_GIO) += dbus-vmstate.o
dbus-vmstate.o-cflags = $(GIO_CFLAGS)
dbus-vmstate.o-libs = $(GIO_LIBS)
+
+common-obj-$(CONFIG_GPIODEV) += gpiodev.o
diff --git a/backends/gpiodev.c b/backends/gpiodev.c
new file mode 100644
index ..df1bd0113c7b2985
--- /dev/null
+++ b/backends/gpiodev.c
@@ -0,0 +1,94 @@
+/*
+ * QEMU GPIO Backend
+ *
+ * Copyright (C) 2018-2020 Glider bv
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include
+#include
+
+#include "qemu/osdep.h"
+#include "qemu/config-file.h"
+#include "qemu/cutils.h"
+#include "qemu/error-report.h"
+#include "qemu/module.h"
+#include "qemu/option.h"
+#include "qapi/error.h"
+
+#include "sysemu/gpiodev.h"
+
+#include "hw/irq.h"
+#include "hw/qdev-core.h"
+
+static void gpiodev_irq_handler(void *opaque, int n, int level)
+{
+struct gpiod_line *line = opaque;
+int status;
+
+status = gpiod_line_set_value(line, level);
+if (status < 0) {
+struct gpiod_chip *chip = gpiod_line_get_chip(line);
+
+error_report("%s/%s: Cannot set GPIO line %u: %s",
+ gpiod_chip_name(chip), gpiod_chip_label(chip),
+ gpiod_line_offset(line), strerror(errno));
+}
+}
+
+static int gpiodev_map_line(DeviceState *dev, struct gpiod_chip *chip,
+unsigned int gpio, Error **errp)
+{
+struct gpiod_line *line;
+qemu_irq irq;
+int status;
+
+line = gpiod_chip_get_line(chip, gpio);
+if (!line) {
+error_setg(errp, "Cannot obtain GPIO line %u: %s", gpio,
+ strerror(errno));
+return -1;
+}
+
+status = gpiod_line_request_output(line, "qemu", 0);
+if (status < 0) {
+error_setg(errp, "Cannot request GPIO line %u for output: %s", gpio,
+ strerror(errno));
+return status;
+}
+
+irq = qemu_allocate_irq(gpiodev_irq_handler, line, 0);
+qdev_connect_gpio_out(dev, gpio, irq);
+return 0;
+}
+
+void qemu_gpiodev_add(DeviceState *dev, const char *name, unsigned int maxgpio,
+ Error **errp)
+{
+struct gpiod_chip *chip;
+unsigned int i, n;
+int status;
+
+chip = gpiod_chip_open_lookup(name);
+if (!chip) {
+error_setg(errp, "Cannot open GPIO chip %s: %s", name,
+ strerror(errno));
+return;
+}
+
+n = gpiod_chip_num_lines(chip);
+if (n > maxgpio) {
+warn_report("Last %u GPIO line(s) will not be mapped", n - maxgpio);
+n = maxgpio;
+}
+
+for (i = 0; i < n; i++) {
+status = gpiodev_map_line(dev, chip, i, errp);
+if (status < 0) {
+return;
+}
+}
+
+info_report("Mapped %u GPIO lines", n);
+}
diff --git a/configure b/configure
index 23b5e93752b6a259..8b133402ef727c8e 100755
--- a/configure
+++ b/configure
@@ -509,6 +509,7 @@ libpmem=""
default_devices="yes"
plugins="no"
fuzzing="no"
+gpio=""
supported_cpu="no"
supported_os="no"
@@ -1601,6 +1602,10 @@ f
[PATCH QEMU v2 0/5] Add a GPIO backend
Hi all,
This patch series adds a GPIO controller backend, to connect virtual
GPIOs on the guest to physical GPIOs on the host, and enables support
for this using user-creatable PL061 GPIO controller instances. This
allows the guest to control any external device connected to the
physical GPIOs.
While this can be used with an upstream Linux kernel (e.g. using a
dedicated GPIO controller connected to an external bus), proper
isolation and assignment of GPIOs to virtual machines depends on the
GPIO Aggregator[1], which has not been accepted in Linux upstream yet.
Aggregating GPIOs and exposing them as a new gpiochip was suggested in
response to my proof-of-concept for GPIO virtualization with QEMU[2][3].
Features and limitations:
- The backend uses libgpiod on Linux,
- For now only GPIO outputs are supported,
- The number of GPIO lines mapped is limited to the number of GPIO
lines available on the virtual GPIO controller (i.e. 8 on PL061).
Future work:
- GPIO inputs,
- GPIO line configuration,
- Optimizations for controlling multiple GPIO lines at once,
- ...
This series contains 5 patches:
- The first two patches refactor the existing code for reuse,
- The third patch adds a gneric GPIO backend using libgpiod,
- The fourth patch adds gpiodev support to the PL061 driver,
- The fifth patch adds dynamic PL061 support to ARM virt.
Changes compared to v1[2]:
- Drop vgpios and gpios parameters, and map the full gpiochip instead,
- Replace the single hardcoded PL061 instance (created by ARM virt) by
multiple dynamically created instances, one per imported GPIO
controller.
For testing, I have pushed this series to the topic/gpio-backend-v2
branch of my git repository at https://github.com/geertu/qemu.git.
Sample session on the Renesas Salvator-XS development board:
- Unbind keyboard (shared with LEDs) from gpio-keys driver:
host$ echo keys > /sys/bus/platform/drivers/gpio-keys/unbind
- Aggregate GPIO lines connected to LEDs into a new virtual GPIO chip:
host$ echo e6055400.gpio 11-13 \
> /sys/bus/platform/drivers/gpio-aggregator/new_device
gpio-aggregator gpio-aggregator.0: 0 => gpio-371
gpio-aggregator gpio-aggregator.0: 1 => gpio-372
gpio-aggregator gpio-aggregator.0: 2 => gpio-373
gpiochip_find_base: found new base at 343
gpio gpiochip10: (gpio-aggregator.0): added GPIO chardev (254:10)
gpiochip_setup_dev: registered GPIOs 343 to 345 on device: gpiochip10
(gpio-aggregator.0)
- Adjust permissions on /dev/gpiochip10 (optional)
- Launch QEMU:
host$ aarch64-softmmu/qemu-system-aarch64 -enable-kvm -M virt \
-cpu cortex-a57 -m 1024 -nographic -kernel /path/to/Image \
-device pl061,host=gpio-aggregator.0
...
pl061_gpio c00.gpio: PL061 GPIO chip registered
...
- Control LEDs:
guest$ gpioset gpiochip0 0=0 1=1 # LED4 OFF, LED5 ON
guest$ gpioset gpiochip0 0=1 1=0 # LED4 ON, LED5 OFF
Thanks for your comments!
[1] "[PATCH v6 0/8] gpio: Add GPIO Aggregator"
(https://lore.kernel.org/linux-gpio/20200324135328.5796-1-geert+rene...@glider.be/)
[2] "[PATCH QEMU POC] Add a GPIO backend"
(https://lore.kernel.org/linux-gpio/20181003152521.23144-1-geert+rene...@glider.be/)
[3] "Getting To Blinky: Virt Edition / Making device pass-through work
on embedded ARM"
(https://fosdem.org/2019/schedule/event/vai_getting_to_blinky/)
Geert Uytterhoeven (5):
ARM: PL061: Move TYPE_PL061 to hw/gpio/pl061.h
ARM: PL061: Extract pl061_create_fdt()
Add a GPIO backend using libgpiod
ARM: PL061: Add gpiodev support
hw/arm/virt: Add dynamic PL061 GPIO support
MAINTAINERS | 7 +++
backends/Makefile.objs | 2 +
backends/gpiodev.c | 94
configure| 28
hw/arm/sysbus-fdt.c | 18
hw/arm/virt.c| 21 ++---
hw/gpio/pl061.c | 79 -
include/hw/gpio/pl061.h | 23 ++
include/sysemu/gpiodev.h | 12 +
qemu-options.hx | 9
10 files changed, 275 insertions(+), 18 deletions(-)
create mode 100644 backends/gpiodev.c
create mode 100644 include/hw/gpio/pl061.h
create mode 100644 include/sysemu/gpiodev.h
--
2.17.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH QEMU v2 1/5] ARM: PL061: Move TYPE_PL061 to hw/gpio/pl061.h
Move the definition of TYPE_PL061 to a new header file, so it can be
used outside the driver.
Signed-off-by: Geert Uytterhoeven
---
v2:
- New.
---
MAINTAINERS | 1 +
hw/gpio/pl061.c | 2 +-
include/hw/gpio/pl061.h | 16
3 files changed, 18 insertions(+), 1 deletion(-)
create mode 100644 include/hw/gpio/pl061.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 8cbc1fac2bfcec86..e760f65270d29d5d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -538,6 +538,7 @@ F: hw/dma/pl080.c
F: include/hw/dma/pl080.h
F: hw/dma/pl330.c
F: hw/gpio/pl061.c
+F: include/hw/gpio/pl061.h
F: hw/input/pl050.c
F: hw/intc/pl190.c
F: hw/sd/pl181.c
diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
index 2a828260bdb0b946..e776c09e474216ef 100644
--- a/hw/gpio/pl061.c
+++ b/hw/gpio/pl061.c
@@ -9,6 +9,7 @@
*/
#include "qemu/osdep.h"
+#include "hw/gpio/pl061.h"
#include "hw/irq.h"
#include "hw/sysbus.h"
#include "migration/vmstate.h"
@@ -33,7 +34,6 @@ static const uint8_t pl061_id[12] =
static const uint8_t pl061_id_luminary[12] =
{ 0x00, 0x00, 0x00, 0x00, 0x61, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1 };
-#define TYPE_PL061 "pl061"
#define PL061(obj) OBJECT_CHECK(PL061State, (obj), TYPE_PL061)
typedef struct PL061State {
diff --git a/include/hw/gpio/pl061.h b/include/hw/gpio/pl061.h
new file mode 100644
index ..78cc40c52679dc4e
--- /dev/null
+++ b/include/hw/gpio/pl061.h
@@ -0,0 +1,16 @@
+/*
+ * Arm PrimeCell PL061 General Purpose IO with additional Luminary Micro
+ * Stellaris bits.
+ *
+ * Copyright (c) 2007 CodeSourcery.
+ * Written by Paul Brook
+ *
+ * This code is licensed under the GPL.
+ */
+
+#ifndef PL061_GPIO_H
+#define PL061_GPIO_H
+
+#define TYPE_PL061 "pl061"
+
+#endif /* PL061_GPIO_H */
--
2.17.1
[PATCH QEMU v2 5/5] hw/arm/virt: Add dynamic PL061 GPIO support
Add support for dynamically instantiating PL061 GPIO controller
instances in ARM virt. Device tree nodes are created dynamically.
Signed-off-by: Geert Uytterhoeven
---
v2:
- New.
---
hw/arm/sysbus-fdt.c | 18 ++
hw/arm/virt.c | 1 +
2 files changed, 19 insertions(+)
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index 6b6906f4cfc36198..493583218d176d0a 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -32,6 +32,7 @@
#include "sysemu/device_tree.h"
#include "sysemu/tpm.h"
#include "hw/platform-bus.h"
+#include "hw/gpio/pl061.h"
#include "hw/vfio/vfio-platform.h"
#include "hw/vfio/vfio-calxeda-xgmac.h"
#include "hw/vfio/vfio-amd-xgbe.h"
@@ -468,6 +469,22 @@ static int add_tpm_tis_fdt_node(SysBusDevice *sbdev, void
*opaque)
return 0;
}
+/*
+ * add_pl061_node: Create a DT node for a PL061 GPIO controller
+ */
+static int add_pl061_node(SysBusDevice *sbdev, void *opaque)
+{
+PlatformBusFDTData *data = opaque;
+PlatformBusDevice *pbus = data->pbus;
+void *fdt = data->fdt;
+
+pl061_create_fdt(fdt, data->pbus_node_name, 1,
+ platform_bus_get_mmio_addr(pbus, sbdev, 0), 0x1000,
+ platform_bus_get_irqn(pbus, sbdev, 0) + data->irq_start,
+ qemu_fdt_get_phandle(fdt, "/apb-pclk"));
+return 0;
+}
+
static int no_fdt_node(SysBusDevice *sbdev, void *opaque)
{
return 0;
@@ -489,6 +506,7 @@ static const BindingEntry bindings[] = {
VFIO_PLATFORM_BINDING("amd,xgbe-seattle-v1a", add_amd_xgbe_fdt_node),
#endif
TYPE_BINDING(TYPE_TPM_TIS_SYSBUS, add_tpm_tis_fdt_node),
+TYPE_BINDING(TYPE_PL061, add_pl061_node),
TYPE_BINDING(TYPE_RAMFB_DEVICE, no_fdt_node),
TYPE_BINDING("", NULL), /* last element */
};
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index c88c8850fbe00bdb..191594db09422d54 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2178,6 +2178,7 @@ static void virt_machine_class_init(ObjectClass *oc, void
*data)
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_PLATFORM);
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
+machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PL061);
mc->block_default_type = IF_VIRTIO;
mc->no_cdrom = 1;
mc->pci_allow_0_address = true;
--
2.17.1
[PATCH QEMU v2 2/5] ARM: PL061: Extract pl061_create_fdt()
Move the code to create the DT node for the PL061 GPIO controller from
hw/arm/virt.c to the PL061 driver, so it can be reused.
While at it, make the created node comply with the PL061 Device Tree
bindings:
- Use generic node name "gpio" instead of "pl061",
- Add missing "#interrupt-cells" and "interrupt-controller"
properties.
Signed-off-by: Geert Uytterhoeven
---
v2:
- New.
---
hw/arm/virt.c | 20 +++-
hw/gpio/pl061.c | 42 +
include/hw/gpio/pl061.h | 7 +++
3 files changed, 52 insertions(+), 17 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7dc96abf72cf2b9a..c88c8850fbe00bdb 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -40,6 +40,7 @@
#include "hw/arm/primecell.h"
#include "hw/arm/virt.h"
#include "hw/block/flash.h"
+#include "hw/gpio/pl061.h"
#include "hw/vfio/vfio-calxeda-xgmac.h"
#include "hw/vfio/vfio-amd-xgbe.h"
#include "hw/display/ramfb.h"
@@ -807,30 +808,16 @@ static void virt_powerdown_req(Notifier *n, void *opaque)
static void create_gpio(const VirtMachineState *vms)
{
-char *nodename;
DeviceState *pl061_dev;
hwaddr base = vms->memmap[VIRT_GPIO].base;
hwaddr size = vms->memmap[VIRT_GPIO].size;
int irq = vms->irqmap[VIRT_GPIO];
-const char compat[] = "arm,pl061\0arm,primecell";
pl061_dev = sysbus_create_simple("pl061", base,
qdev_get_gpio_in(vms->gic, irq));
-uint32_t phandle = qemu_fdt_alloc_phandle(vms->fdt);
-nodename = g_strdup_printf("/pl061@%" PRIx64, base);
-qemu_fdt_add_subnode(vms->fdt, nodename);
-qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
- 2, base, 2, size);
-qemu_fdt_setprop(vms->fdt, nodename, "compatible", compat, sizeof(compat));
-qemu_fdt_setprop_cell(vms->fdt, nodename, "#gpio-cells", 2);
-qemu_fdt_setprop(vms->fdt, nodename, "gpio-controller", NULL, 0);
-qemu_fdt_setprop_cells(vms->fdt, nodename, "interrupts",
- GIC_FDT_IRQ_TYPE_SPI, irq,
- GIC_FDT_IRQ_FLAGS_LEVEL_HI);
-qemu_fdt_setprop_cell(vms->fdt, nodename, "clocks", vms->clock_phandle);
-qemu_fdt_setprop_string(vms->fdt, nodename, "clock-names", "apb_pclk");
-qemu_fdt_setprop_cell(vms->fdt, nodename, "phandle", phandle);
+uint32_t phandle = pl061_create_fdt(vms->fdt, "", 2, base, size, irq,
+vms->clock_phandle);
gpio_key_dev = sysbus_create_simple("gpio-key", -1,
qdev_get_gpio_in(pl061_dev, 3));
@@ -846,7 +833,6 @@ static void create_gpio(const VirtMachineState *vms)
KEY_POWER);
qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff",
"gpios", phandle, 3, 0);
-g_free(nodename);
}
static void create_virtio_devices(const VirtMachineState *vms)
diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
index e776c09e474216ef..74ba733a8a5e8ca5 100644
--- a/hw/gpio/pl061.c
+++ b/hw/gpio/pl061.c
@@ -9,12 +9,14 @@
*/
#include "qemu/osdep.h"
+#include "hw/arm/fdt.h"
#include "hw/gpio/pl061.h"
#include "hw/irq.h"
#include "hw/sysbus.h"
#include "migration/vmstate.h"
#include "qemu/log.h"
#include "qemu/module.h"
+#include "sysemu/device_tree.h"
//#define DEBUG_PL061 1
@@ -397,3 +399,43 @@ static void pl061_register_types(void)
}
type_init(pl061_register_types)
+
+/*
+ * pl061_create_fdt: Create a DT node for a PL061 GPIO controller
+ * @fdt: device tree blob
+ * @parent: name of the parent node
+ * @n_cells: value of #address-cells and #size-cells
+ * @base: base address of the controller's register block
+ * @size: size of the controller's register block
+ * @irq: interrupt number
+ * @clock: phandle of the apb-pclk clock
+ *
+ * Return value: a phandle referring to the created DT node.
+ *
+ * See the DT Binding Documentation in the Linux kernel source tree:
+ * Documentation/devicetree/bindings/gpio/pl061-gpio.yaml
+ */
+uint32_t pl061_create_fdt(void *fdt, const char *parent, unsigned int n_cells,
+ hwaddr base, hwaddr size, int irq, uint32_t clock)
+{
+char *nodename = g_strdup_printf("%s/gpio@%" PRIx64, parent, base);
+static const char compat[] = "arm,pl061\0arm,primecell";
+uint32_t phandle = qemu_fdt_alloc_phandle(fdt);
+
+qemu_fdt_add_subnode(fdt, nodename);
+qemu_fdt_setprop_sized_cells(fdt, nodename, "reg", n_cel
Re: [PATCH QEMU v2 3/5] Add a GPIO backend using libgpiod
Hi Philippe,
Thanks for your comments!
On Thu, Apr 23, 2020 at 11:28 AM Philippe Mathieu-Daudé wrote:
> On 4/23/20 11:01 AM, Geert Uytterhoeven wrote:
> > Add a GPIO controller backend, to connect virtual GPIOs on the guest to
> > physical GPIOs on the host. This allows the guest to control any
> > external device connected to the physical GPIOs.
> >
> > Features and limitations:
> > - The backend uses libgpiod on Linux,
> > - For now only GPIO outputs are supported,
> > - The number of GPIO lines mapped is limited to the number of GPIO
> > lines available on the virtual GPIO controller.
> >
> > Future work:
> > - GPIO inputs,
> > - GPIO line configuration,
> > - Optimizations for controlling multiple GPIO lines at once,
> > - ...
> >
> > Signed-off-by: Geert Uytterhoeven
> > --- /dev/null
> > +++ b/backends/gpiodev.c
> > @@ -0,0 +1,94 @@
> > +/*
> > + * QEMU GPIO Backend
> > + *
> > + * Copyright (C) 2018-2020 Glider bv
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include
>
> probably not needed.
It is indeed included by one of the other header files.
What is the QEMU policy w.r.t. that?
>
> > +#include
>
> Please move this one...
>
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/config-file.h"
> > +#include "qemu/cutils.h"
I forgot to remove the two above...
> > +#include "qemu/error-report.h"
> > +#include "qemu/module.h"
> > +#include "qemu/option.h"
... and the two above.
> > +#include "qapi/error.h"
> > +
> > +#include "sysemu/gpiodev.h"
> > +
> > +#include "hw/irq.h"
> > +#include "hw/qdev-core.h"
>
> ... here:
>
> #include
OK.
> > --- a/configure
> > +++ b/configure
> > @@ -509,6 +509,7 @@ libpmem=""
> > default_devices="yes"
> > plugins="no"
> > fuzzing="no"
> > +gpio=""
>
> Maybe name this feature 'libgpiod'?
Makes sense.
> >
> > supported_cpu="no"
> > supported_os="no"
> > @@ -1601,6 +1602,10 @@ for opt do
> >;;
> >--gdb=*) gdb_bin="$optarg"
> >;;
> > + --disable-gpio) gpio="no"
> > + ;;
> > + --enable-gpio) gpio="yes"
>
> Ditto: --enable-libgpiod, because else it seems rather confusing.
OK.
> > --- /dev/null
> > +++ b/include/sysemu/gpiodev.h
> > @@ -0,0 +1,12 @@
> > +/*
> > + * QEMU GPIO Backend
> > + *
> > + * Copyright (C) 2018-2020 Glider bv
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include "qemu/typedefs.h"
>
> "qemu/typedefs.h" not needed in includes.
While removing that works, it does mean the header file is no longer
self-contained:
include/sysemu/gpiodev.h:10:23: error: unknown type name ‘DeviceState’
> > +
> > +void qemu_gpiodev_add(DeviceState *dev, const char *name, unsigned int
> > maxgpio,
> > + Error **errp);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH QEMU v2 4/5] ARM: PL061: Add gpiodev support
Hi Philippe,
On Thu, Apr 23, 2020 at 12:08 PM Philippe Mathieu-Daudé wrote:
> On 4/23/20 11:33 AM, Philippe Mathieu-Daudé wrote:
> > On 4/23/20 11:01 AM, Geert Uytterhoeven wrote:
> >> Make the PL061 GPIO controller user-creatable, and allow the user to tie
> >> a newly created instance to a gpiochip on the host.
> >>
> >> To create a new GPIO controller, the QEMU command line must be augmented
> >> with:
> >>
> >> -device pl061,host=
> >>
> >> with the name or label of the gpiochip on the host.
> >>
> >> Signed-off-by: Geert Uytterhoeven
> >> --- a/hw/gpio/pl061.c
> >> +++ b/hw/gpio/pl061.c
> >> @@ -12,11 +12,14 @@
> >> #include "hw/arm/fdt.h"
> >> #include "hw/gpio/pl061.h"
> >> #include "hw/irq.h"
> >> +#include "hw/qdev-properties.h"
> >> #include "hw/sysbus.h"
> >> #include "migration/vmstate.h"
> >> +#include "qapi/error.h"
> >> #include "qemu/log.h"
> >> #include "qemu/module.h"
> >> #include "sysemu/device_tree.h"
> >> +#include "sysemu/gpiodev.h"
> >>
> >> //#define DEBUG_PL061 1
> >>
> >> @@ -41,6 +44,9 @@ static const uint8_t pl061_id_luminary[12] =
> >> typedef struct PL061State {
> >> SysBusDevice parent_obj;
> >>
> >> +#ifdef CONFIG_GPIODEV
> >> +char *host;
> >> +#endif
> >> MemoryRegion iomem;
> >> uint32_t locked;
> >> uint32_t data;
> >> @@ -370,10 +376,39 @@ static void pl061_init(Object *obj)
> >> qdev_init_gpio_out(dev, s->out, 8);
> >>
> >> +#ifdef CONFIG_GPIODEV
> >> +static Property pl061_properties[] = {
> >> +DEFINE_PROP_STRING("host", PL061State, host),
> >> +DEFINE_PROP_END_OF_LIST(),
> >> +};
> >> +
> >> +static void pl061_realize(DeviceState *dev, Error **errp)
> >> +{
> >> +PL061State *s = PL061(dev);
> >> +
> >> +if (!dev->opts) {
> >> +/* Not created by user */
> >> +return;
> >> +}
> >> +
> >> +if (!s->host) {
> >> +error_setg(errp, "'host' property is required");
> >> +return;
> >> +}
> >> +
> >> +qemu_gpiodev_add(dev, s->host, 8, errp);
> >> +}
> >> +#endif /* CONFIG_GPIODEV */
> >> +
> >> static void pl061_class_init(ObjectClass *klass, void *data)
> >> {
> >> DeviceClass *dc = DEVICE_CLASS(klass);
> >>
> >> +#ifdef CONFIG_GPIODEV
> >> +device_class_set_props(dc, pl061_properties);
> >> +dc->realize = pl061_realize;
> >> +dc->user_creatable = true;
> >> +#endif
> >> dc->vmsd = &vmstate_pl061;
> >> dc->reset = &pl061_reset;
> >> }
> >> diff --git a/qemu-options.hx b/qemu-options.hx
> >> index 292d4e7c0cef6097..182de7fb63923b38 100644
> >> --- a/qemu-options.hx
> >> +++ b/qemu-options.hx
> >> @@ -875,6 +875,15 @@ SRST
> >> ``-device isa-ipmi-bt,bmc=id[,ioport=val][,irq=val]``
> >> Like the KCS interface, but defines a BT interface. The default port
> >> is 0xe4 and the default interrupt is 5.
> >> +
> >> +#ifdef CONFIG_GPIODEV
> >> +``-device pl061,host=gpiochip``
> >> +Add a PL061 GPIO controller, and map its virtual GPIO lines to a GPIO
> >> +controller on the host.
> >> +
> >> +``host=gpiochip``
> >> +The name or label of the GPIO controller on the host.
> >> +#endif
> >> ERST
> >>
> >> DEF("name", HAS_ARG, QEMU_OPTION_name,
> >>
> >
> > Instead of restricting this to the pl061, it would be cleaner you add a
> > GPIO_PLUGGABLE_INTERFACE (or GPIO_BINDABLE_INTERFACE or better name),
> > and have TYPE_PL061 implement it.
>
> IOW your backend should consume devices implementing this generic interface.
Thanks for the suggestion! I had a look into implementing this.
Please pardon my QEMU illiteracy, but I fail to see how adding an
interface, and letting frontends like pl061.c implement it, will help:
- The backend never has to call directly into the frontend: all
communication is done indirectly, using existing core qemu irq and
qdev gpio calls.
- The frontend has to call into the backend once, at initializ
Re: [PATCH v3 0/7] gpio: Add GPIO Aggregator/Repeater
Hi Eugeniu,
On Sat, Jan 18, 2020 at 2:46 AM Eugeniu Rosca wrote:
> On Wed, Nov 27, 2019 at 09:42:46AM +0100, Geert Uytterhoeven wrote:
> > - Create aggregators:
> >
> > $ echo e6052000.gpio 19,20 \
> > > /sys/bus/platform/drivers/gpio-aggregator/new_device
> The only unexpected thing is seeing below messages (where gpiochip99 and
> gpiochip22 are inexisting gpiochip names, mistakenly provided on command
> line prior to passing the correct name):
>
> root@rcar-gen3:~# echo gpiochip6 12-13 >
> /sys/bus/platform/drivers/gpio-aggregator/new_device
> [ 915.572905] gpio-aggregator gpio-aggregator.0: cannot find GPIO chip
> gpiochip99, deferring
> [ 915.584224] gpio-aggregator gpio-aggregator.2: cannot find GPIO chip
> gpiochip99, deferring
> [ 915.865281] gpio-aggregator gpio-aggregator.29: cannot find GPIO chip
> gpiochip22, deferring
>
> Obviously, in the above case, due to a typo in the names, the gpio
> chips will never be found, no matter how long gpio-aggregator defers
Indeed, that is expected behavior: you have created platform devices
referring to resources that are not available.
> their probing. Unfortunately, the driver will continuously emit those
> messages, upon each successfully created/aggregated gpiochip. I built
That is expected behavior, too: every time the driver core manages to
bind a device to a driver, it will retry all previously deferred probes,
in the hope they can be satisfied by the just bound device.
Note that you can destroy these bogus devices, using e.g.
# echo gpio-aggregator.0 > \
/sys/bus/platform/drivers/gpio-aggregator/delete_device
> gpio-aggregator as a loadable module, if that's relevant.
Modular or non-modular shouldn't matter w.r.t. this behavior.
Although unloading the module should get rid of the cruft.
> Another comment is that, while the series _does_ allow specifying
> gpio lines in the DTS (this would require a common compatible string
> in gpio_aggregator_dt_ids[] and in the DTS node) and while those lines
> are indeed exposed to userspace, based on my testing, these same gpio
> lines are marked as "used/reserved" by the kernel. This means that
> operating on those gpio pins from userspace will not be possible.
> For instance, gpioget/gpioset return "Device or resource busy":
>
> gpioget: error reading GPIO values: Device or resource busy
> gpioset: error setting the GPIO line values: Device or resource busy
>
> I guess Harish will be unhappy about that, as his expectation was that
> upon merging gpio-aggregator with gpio-inverter, he will be able to
> describe GPIO polarity and names in DTS without "hogging" the pins.
> Perhaps this can be supplemented via an add-on patch later on?
When aggregating GPIO lines, the original GPIO lines are indeed marked
used/reserved, so you cannot use them from userspace.
However, you are expected to use them through the newly created virtual
gpiochip representing the aggregated GPIO lines.
You can try this using the "door" example in
Documentation/admin-guide/gpio/gpio-aggregator.rst, after replacing
gpio2 {19,20} by gpio6 {12,13} to suit your H3ULCB.
> For the whole series (leaving the above findings to your discretion):
>
> Reviewed-by: Eugeniu Rosca
> Tested-by: Eugeniu Rosca
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Qemu-devel] [PATCH RFC] gpio: Add Virtual Aggregator GPIO Driver
On Tue, Jul 9, 2019 at 4:59 PM Bartosz Golaszewski
wrote:
> pon., 8 lip 2019 o 12:24 Geert Uytterhoeven napisał(a):
> > On Mon, Jul 8, 2019 at 11:45 AM Bartosz Golaszewski
> > wrote:
> > > pt., 5 lip 2019 o 18:05 Geert Uytterhoeven
> > > napisał(a):
> > > > +static int gpio_virt_agg_set_config(struct gpio_chip *chip,
> > > > + unsigned int offset, unsigned long
> > > > config)
> > > > +{
> > > > + struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
> > > > +
> > > > + chip = priv->desc[offset]->gdev->chip;
> > > > + if (chip->set_config)
> > > > + return chip->set_config(chip, offset, config);
> > > > +
> > > > + // FIXME gpiod_set_transitory() expects success if not
> > > > implemented
> >
> > BTW, do you have a comment about this FIXME?
>
> Ha! Interesting. I'll give it a thought and respond elsewhere as it's
> a different subject.
>
> > > > + return -ENOTSUPP;
Upon closer look, this turns out to be a red herring: gpiod_set_transitory()
converts -ENOTSUPP to zero, so there is no issue.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Qemu-devel] [PATCH RFC] gpio: Add Virtual Aggregator GPIO Driver
Hi Bartosz,
On Fri, Jul 12, 2019 at 11:27 AM Bartosz Golaszewski
wrote:
> wt., 9 lip 2019 o 17:59 Geert Uytterhoeven napisał(a):
> > On Tue, Jul 9, 2019 at 4:59 PM Bartosz Golaszewski
> > wrote:
> > > pon., 8 lip 2019 o 12:24 Geert Uytterhoeven
> > > napisał(a):
> > > > On Mon, Jul 8, 2019 at 11:45 AM Bartosz Golaszewski
> > > > wrote:
> > > > > pt., 5 lip 2019 o 18:05 Geert Uytterhoeven
> > > > > napisał(a):
> > > > > > GPIO controllers are exported to userspace using /dev/gpiochip*
> > > > > > character devices. Access control to these devices is provided by
> > > > > > standard UNIX file system permissions, on an all-or-nothing basis:
> > > > > > either a GPIO controller is accessible for a user, or it is not.
> > > > > > Currently no mechanism exists to control access to individual GPIOs.
> > > > > >
> > > > > > Hence add a virtual GPIO driver to aggregate existing GPIOs (up to
> > > > > > 32),
> > > > > > and expose them as a new gpiochip. This is useful for implementing
> > > > > > access control, and assigning a set of GPIOs to a specific user.
> > > > > > Furthermore, it would simplify and harden exporting GPIOs to a
> > > > > > virtual
> > > > > > machine, as the VM can just grab the full virtual GPIO controller,
> > > > > > and
> > > > > > no longer needs to care about which GPIOs to grab and which not,
> > > > > > reducing the attack surface.
> > > > > >
> > > > > > Virtual GPIO controllers are instantiated by writing to the
> > > > > > "new_device"
> > > > > > attribute file in sysfs:
> > > > > >
> > > > > > $ echo " [ ...]"
> > > > > >"[, [ ...]] ...]"
> > > > > > > /sys/bus/platform/drivers/gpio-virt-agg/new_device
> > > > > >
> > > > > > Likewise, virtual GPIO controllers can be destroyed after use:
> > > > > >
> > > > > > $ echo gpio-virt-agg. \
> > > > > > > /sys/bus/platform/drivers/gpio-virt-agg/delete_device
> >
> > > Am I doing it right? I'm trying to create a device and am only getting
> > > this:
> > >
> > > # echo gpiochip2 23 > new_device
> > > [ 707.507039] gpio-virt-agg gpio-virt-agg.0: Cannot find gpiochip
> > > gpiochip2
> > >
> > > gpiochip2 *does* exist in the system.
> >
> > Please try the name of the platform device instead.
> > I.e. for my koelsch (R-Car M2-W), it needs "e6052000.gpio" instead
> > of "gpiochip2".
> >
> > Probably the driver should match on both.
> >
> > > I see. I'll try to review it more thoroughly once I get to play with
> > > it. So far I'm stuck on creating the virtual chip.
>
> This is not a show-stopper but one thing that's bothering me in this
> is that lines used by the aggregator are considered 'used' in regard
> to the original chip. I'm wondering how much effort would it take to
> have them be 'muxed' into two (real and virtual) chips at once.
Is that really what you want?
If a GPIO is aggregated with othrs, it's intended to be used only through
the aggregator, isn't it?
> Other than that - seems to works pretty nice other than the matching
> by chip name and by line names.
Thanks, working on that...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[Qemu-devel] [PATCH/RFC v2 2/5] gpio: Export gpiochip_get_desc() to modular GPIO code
Export the gpiochip_get_desc() symbol, so modular GPIO driver can make
use of this function again.
This is a partial revert of commit 1bd6b601fe196b6f ("gpio: make
gpiochip_get_desc() gpiolib-private").
Signed-off-by: Geert Uytterhoeven
---
ERROR: "gpiochip_get_desc" [drivers/gpio/gpio-aggregator.ko] undefined!
---
drivers/gpio/gpiolib.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index dbecf266be5a5a2a..c2cf01fb72a9de87 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -95,6 +95,7 @@ static inline void desc_set_label(struct gpio_desc *d, const
char *label)
{
d->label = label;
}
+EXPORT_SYMBOL_GPL(gpiochip_get_desc);
/**
* gpio_to_desc - Convert a GPIO number to its descriptor
--
2.17.1
[Qemu-devel] [PATCH/RFC v2 5/5] gpio: Add GPIO Aggregator Driver
GPIO controllers are exported to userspace using /dev/gpiochip*
character devices. Access control to these devices is provided by
standard UNIX file system permissions, on an all-or-nothing basis:
either a GPIO controller is accessible for a user, or it is not.
Currently no mechanism exists to control access to individual GPIOs.
Hence add a GPIO driver to aggregate existing GPIOs, and expose them as
a new gpiochip. This is useful for implementing access control, and
assigning a set of GPIOs to a specific user.
Furthermore, this simplifies and hardens exporting GPIOs to a virtual
machine, as the VM can just grab the full GPIO controller, and no longer
needs to care about which GPIOs to grab and which not, reducing the
attack surface.
Aggregated GPIO controllers are instantiated by writing to the
"new_device" attribute file in sysfs:
$ echo [] [ ] ...
> /sys/bus/platform/drivers/gpio-aggregator/new_device
Where is a GPIO line name, is a GPIO chip label or
name, and is a comma-separated list of GPIO offsets and/or
GPIO offset ranges.
Likewise, aggregated GPIO controllers can be destroyed after use:
$ echo gpio-aggregator. \
> /sys/bus/platform/drivers/gpio-aggregator/delete_device
Signed-off-by: Geert Uytterhoeven
---
v2:
- Add missing initialization of i in gpio_virt_agg_probe(),
- Update for removed .need_valid_mask field and changed
.init_valid_mask() signature,
- Drop "virtual", rename to gpio-aggregator,
- Drop bogus FIXME related to gpiod_set_transitory() expectations,
- Use new GPIO Forwarder Helper,
- Lift limit on the maximum number of GPIOs,
- Improve parsing:
- add support for specifying GPIOs by line name,
- add support for specifying GPIO chips by ID,
- add support for GPIO offset ranges,
- names and offset specifiers must be separated by whitespace,
- GPIO offsets must separated by spaces,
- Use str_has_prefix() and kstrtouint().
I didn't use argv_split(), as it doesn't support quoted strings yet,
and GPIO line names can contain spaces. Perhaps I should enhance
argv_split() first, and use that?
---
drivers/gpio/Kconfig | 9 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-aggregator.c | 333 +
3 files changed, 343 insertions(+)
create mode 100644 drivers/gpio/gpio-aggregator.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 29d3ce8debcca1f6..058aa68fd7015e7c 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1483,6 +1483,15 @@ config GPIO_VIPERBOARD
endmenu
+config GPIO_AGGREGATOR
+ tristate "GPIO Aggregator"
+ select GPIOLIB_FWD
+ help
+ This enabled the GPIO Aggregator, which provides a way to aggregate
+ existing GPIOs into a new GPIO device.
+ This is useful for assigning a collection of GPIOs to a user, or
+ exported them to a virtual machine.
+
config GPIO_MOCKUP
tristate "GPIO Testing Driver"
select IRQ_SIM
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 8a0e685c92b69855..2ec9128bcfefa40a 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_GPIO_74XX_MMIO) += gpio-74xx-mmio.o
obj-$(CONFIG_GPIO_ADNP)+= gpio-adnp.o
obj-$(CONFIG_GPIO_ADP5520) += gpio-adp5520.o
obj-$(CONFIG_GPIO_ADP5588) += gpio-adp5588.o
+obj-$(CONFIG_GPIO_AGGREGATOR) += gpio-aggregator.o
obj-$(CONFIG_GPIO_ALTERA_A10SR)+= gpio-altera-a10sr.o
obj-$(CONFIG_GPIO_ALTERA) += gpio-altera.o
obj-$(CONFIG_GPIO_AMD8111) += gpio-amd8111.o
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
new file mode 100644
index ..42485735bd823e02
--- /dev/null
+++ b/drivers/gpio/gpio-aggregator.c
@@ -0,0 +1,333 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// GPIO Aggregator
+//
+// Copyright (C) 2019 Glider bvba
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "gpiolib.h"
+#include "gpiolib-fwd.h"
+
+#define DRV_NAME "gpio-aggregator"
+
+struct gpio_aggregator {
+ struct platform_device *pdev;
+};
+
+static DEFINE_MUTEX(gpio_aggregator_lock); /* protects idr */
+static DEFINE_IDR(gpio_aggregator_idr);
+
+static int gpiochip_match_label(struct gpio_chip *chip, void *data)
+{
+ return !strcmp(chip->label, data);
+}
+
+static struct gpio_chip *gpiochip_find_by_label(const char *label)
+{
+ return gpiochip_find((void *)label, gpiochip_match_label);
+}
+
+static int gpiochip_match_id(struct gpio_chip *chip, void *data)
+{
+ unsigned int id = (uintptr_t)data;
+
+ return id == chip->base || id == chip->gpiodev->id;
+}
+
+static struct gpio_chip *gpiochip_find_by_id(const char *id)
+{
+ unsign
[Qemu-devel] [PATCH/RFC v2 3/5] gpio: Export gpio_name_to_desc() to modular GPIO code
Make gpio_name_to_desc() global, and export its symbol, so modular GPIO
driver can make use of this function.
This will be used by the GPIO Aggregator.
Signed-off-by: Geert Uytterhoeven
---
drivers/gpio/gpiolib.c | 3 ++-
drivers/gpio/gpiolib.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index c2cf01fb72a9de87..c0caafd111e6ff51 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -288,7 +288,7 @@ static int gpiodev_add_to_list(struct gpio_device *gdev)
/*
* Convert a GPIO name to its descriptor
*/
-static struct gpio_desc *gpio_name_to_desc(const char * const name)
+struct gpio_desc *gpio_name_to_desc(const char * const name)
{
struct gpio_device *gdev;
unsigned long flags;
@@ -315,6 +315,7 @@ static struct gpio_desc *gpio_name_to_desc(const char *
const name)
return NULL;
}
+EXPORT_SYMBOL_GPL(gpio_name_to_desc);
/*
* Takes the names from gc->names and checks if they are all unique. If they
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index b8b10a409c7b9c65..240d2868a3024b52 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -117,6 +117,7 @@ struct gpio_desc {
const char *name;
};
+struct gpio_desc *gpio_name_to_desc(const char * const name);
int gpiod_request(struct gpio_desc *desc, const char *label);
void gpiod_free(struct gpio_desc *desc);
int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
--
2.17.1
[Qemu-devel] [PATCH/RFC v2 1/5] gpio: Export gpiod_{request, free}() to modular GPIO code
Export the gpiod_request() and gpiod_free() symbols, so modular GPIO
library code can make use of these functions.
Signed-off-by: Geert Uytterhoeven
---
ERROR: "gpiod_free" [drivers/gpio/gpiolib-fwd.ko] undefined!
ERROR: "gpiod_request" [drivers/gpio/gpiolib-fwd.ko] undefined!
Alternatives:
- Force gpiolib-fwd builtin,
- Call gpio_{,request,free}(desc_to_gpio(...)) instead, as the legacy
functions are exported,
- Call gpiod_put() instead of gpiod_free(), as the former is a simple
exported wrapper around the latter. Unfortunately there's no such
alternative for gpiod_request().
---
drivers/gpio/gpiolib.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f1c1b5d4b00b40a7..dbecf266be5a5a2a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2733,6 +2733,7 @@ int gpiod_request(struct gpio_desc *desc, const char
*label)
return ret;
}
+EXPORT_SYMBOL_GPL(gpiod_request);
static bool gpiod_free_commit(struct gpio_desc *desc)
{
@@ -2777,6 +2778,7 @@ void gpiod_free(struct gpio_desc *desc)
WARN_ON(extra_checks);
}
}
+EXPORT_SYMBOL_GPL(gpiod_free);
/**
* gpiochip_is_requested - return string iff signal was requested
--
2.17.1
