Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND] irqchip: gicv3-its: Don't assume GICv3 hardware supports 16bit INTID
From: Shanker Donthineni @ 2017-04-30 14:54 UTC (permalink / raw)
  To: linux-arm-kernel

The current ITS driver is assuming every ITS hardware implementation
supports minimum of 16bit INTID. But this is not true, as per GICv3
specification, INTID field is IMPLEMENTATION DEFINED in the range of
14-24 bits. We might see an unpredictable system behavior on systems
where hardware support less than 16bits and software tries to use
64K LPI interrupts.

On Qualcomm Datacenter Technologies QDF2400 platform, boot log shows
confusing information about number of LPI chunks as shown below. The
QDF2400 ITS hardware supports 24bit INTID.

This patch allocates the memory resources for PEND/PROP tables based
on discoverable value which is specified in GITS_TYPER.IDbits. Also
taking this opportunity to increase number of LPI/MSI(x) to 128K if
the hardware is capable, and show log message that reflects the
correct number of LPI chunks.

ITS at 0xff7efe0000: allocated 524288 Devices @3c0400000 (indirect, esz 8, psz 64K, shr 1)
ITS at 0xff7efe0000: allocated 8192 Interrupt Collections @3c0130000 (flat, esz 8, psz 64K, shr 1)
ITS at 0xff7efe0000: allocated 8192 Virtual CPUs @3c0140000 (flat, esz 8, psz 64K, shr 1)
ITS: Allocated 524032 chunks for LPIs
PCI/MSI: ITS at 0xff7efe0000 domain created
Platform MSI: ITS at 0xff7efe0000 domain created

Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
---
 drivers/irqchip/irq-gic-v3-its.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 72e56f03..6c24e3c 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -687,9 +687,11 @@ static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg)
  */
 #define IRQS_PER_CHUNK_SHIFT	5
 #define IRQS_PER_CHUNK		(1 << IRQS_PER_CHUNK_SHIFT)
+#define ITS_MAX_LPI_NRBITS	(17) /* 128K LPIs */
 
 static unsigned long *lpi_bitmap;
 static u32 lpi_chunks;
+static u32 lpi_nrbits;
 static DEFINE_SPINLOCK(lpi_lock);
 
 static int its_lpi_to_chunk(int lpi)
@@ -785,26 +787,19 @@ static void its_lpi_free(struct event_lpi_map *map)
 }
 
 /*
- * We allocate 64kB for PROPBASE. That gives us@most 64K LPIs to
+ * We allocate memory for PROPBASE to cover 2 ^ lpi_nrbits LPIs to
  * deal with (one configuration byte per interrupt). PENDBASE has to
  * be 64kB aligned (one bit per LPI, plus 8192 bits for SPI/PPI/SGI).
  */
-#define LPI_PROPBASE_SZ		SZ_64K
-#define LPI_PENDBASE_SZ		(LPI_PROPBASE_SZ / 8 + SZ_1K)
-
-/*
- * This is how many bits of ID we need, including the useless ones.
- */
-#define LPI_NRBITS		ilog2(LPI_PROPBASE_SZ + SZ_8K)
 
 #define LPI_PROP_DEFAULT_PRIO	0xa0
 
 static int __init its_alloc_lpi_tables(void)
 {
+	u32 size = ALIGN(BIT(lpi_nrbits), SZ_64K);
 	phys_addr_t paddr;
 
-	gic_rdists->prop_page = alloc_pages(GFP_NOWAIT,
-					   get_order(LPI_PROPBASE_SZ));
+	gic_rdists->prop_page = alloc_pages(GFP_NOWAIT, get_order(size));
 	if (!gic_rdists->prop_page) {
 		pr_err("Failed to allocate PROPBASE\n");
 		return -ENOMEM;
@@ -816,10 +811,10 @@ static int __init its_alloc_lpi_tables(void)
 	/* Priority 0xa0, Group-1, disabled */
 	memset(page_address(gic_rdists->prop_page),
 	       LPI_PROP_DEFAULT_PRIO | LPI_PROP_GROUP1,
-	       LPI_PROPBASE_SZ);
+	       size);
 
 	/* Make sure the GIC will observe the written configuration */
-	gic_flush_dcache_to_poc(page_address(gic_rdists->prop_page), LPI_PROPBASE_SZ);
+	gic_flush_dcache_to_poc(page_address(gic_rdists->prop_page), size);
 
 	return 0;
 }
@@ -1091,12 +1086,14 @@ static void its_cpu_init_lpis(void)
 	pend_page = gic_data_rdist()->pend_page;
 	if (!pend_page) {
 		phys_addr_t paddr;
+		u32 size;
 		/*
-		 * The pending pages have to be at least 64kB aligned,
-		 * hence the 'max(LPI_PENDBASE_SZ, SZ_64K)' below.
+		 * The pending pages have to be at least 64kB aligned
+		 * hence the 'ALIGN(BIT(lpi_nrbits)/8, SZ_64K)' below.
 		 */
+		size = ALIGN(BIT(lpi_nrbits)/8, SZ_64K);
 		pend_page = alloc_pages(GFP_NOWAIT | __GFP_ZERO,
-					get_order(max(LPI_PENDBASE_SZ, SZ_64K)));
+					get_order(size));
 		if (!pend_page) {
 			pr_err("Failed to allocate PENDBASE for CPU%d\n",
 			       smp_processor_id());
@@ -1104,7 +1101,7 @@ static void its_cpu_init_lpis(void)
 		}
 
 		/* Make sure the GIC will observe the zero-ed page */
-		gic_flush_dcache_to_poc(page_address(pend_page), LPI_PENDBASE_SZ);
+		gic_flush_dcache_to_poc(page_address(pend_page), size);
 
 		paddr = page_to_phys(pend_page);
 		pr_info("CPU%d: using LPI pending table @%pa\n",
@@ -1126,7 +1123,7 @@ static void its_cpu_init_lpis(void)
 	val = (page_to_phys(gic_rdists->prop_page) |
 	       GICR_PROPBASER_InnerShareable |
 	       GICR_PROPBASER_RaWaWb |
-	       ((LPI_NRBITS - 1) & GICR_PROPBASER_IDBITS_MASK));
+	       ((lpi_nrbits - 1) & GICR_PROPBASER_IDBITS_MASK));
 
 	gicr_write_propbaser(val, rbase + GICR_PROPBASER);
 	tmp = gicr_read_propbaser(rbase + GICR_PROPBASER);
@@ -1897,9 +1894,10 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 		return -ENXIO;
 	}
 
+	lpi_nrbits = min_t(u32, rdists->id_bits, ITS_MAX_LPI_NRBITS);
 	gic_rdists = rdists;
 	its_alloc_lpi_tables();
-	its_lpi_init(rdists->id_bits);
+	its_lpi_init(lpi_nrbits);
 
 	return 0;
 }
-- 
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply related

* [PATCH 0/4] TI Bluetooth serdev support
From: Adam Ford @ 2017-04-30 15:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170405183005.20570-1-robh@kernel.org>

On Wed, Apr 5, 2017 at 1:30 PM, Rob Herring <robh@kernel.org> wrote:
> This series adds serdev support to the HCI LL protocol used on TI BT
> modules and enables support on HiKey board with with the WL1835 module.
> With this the custom TI UIM daemon and btattach are no longer needed.

Without UIM daemon, what instruction do you use to load the BT firmware?

I was thinking 'hciattach' but I was having trouble.  I was hoping you
might have some insight.

 hciattach -t 30 -s 115200 /dev/ttymxc1 texas 3000000 flow  Just
returns a timeout.

I modified my i.MX6 device tree per the binding documentation and
setup the regulators and enable GPIO pins.

adam
>
> The series is available on this git branch[1]. Patch 2 is just clean-up
> and can be applied independently. Patch 3 is dependent on the series
> "Nokia H4+ support". I'd suggest both series are merged thru the BT tree.
>
> Rob
>
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git ti-bluetooth
>
> Rob Herring (4):
>   dt-bindings: net: Add TI WiLink shared transport binding
>   bluetooth: hci_uart: remove unused hci_uart_init_tty
>   bluetooth: hci_uart: add LL protocol serdev driver support
>   arm64: dts: hikey: add WL1835 Bluetooth device node
>
>  .../devicetree/bindings/net/ti,wilink-st.txt       |  35 +++
>  arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts     |   5 +
>  drivers/bluetooth/hci_ldisc.c                      |  19 --
>  drivers/bluetooth/hci_ll.c                         | 261 ++++++++++++++++++++-
>  drivers/bluetooth/hci_uart.h                       |   1 -
>  5 files changed, 300 insertions(+), 21 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/ti,wilink-st.txt
>
> --
> 2.10.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 0/4] TI Bluetooth serdev support
From: Sebastian Reichel @ 2017-04-30 16:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAHCN7xJUxZm1qKAxT0kaK6qoDg+HWOJK7sTH-q9za4HJuUwe8Q@mail.gmail.com>

Hi,

On Sun, Apr 30, 2017 at 10:14:20AM -0500, Adam Ford wrote:
> On Wed, Apr 5, 2017 at 1:30 PM, Rob Herring <robh@kernel.org> wrote:
> > This series adds serdev support to the HCI LL protocol used on TI BT
> > modules and enables support on HiKey board with with the WL1835 module.
> > With this the custom TI UIM daemon and btattach are no longer needed.
> 
> Without UIM daemon, what instruction do you use to load the BT firmware?
> 
> I was thinking 'hciattach' but I was having trouble.  I was hoping you
> might have some insight.
> 
>  hciattach -t 30 -s 115200 /dev/ttymxc1 texas 3000000 flow  Just
> returns a timeout.
> 
> I modified my i.MX6 device tree per the binding documentation and
> setup the regulators and enable GPIO pins.

If you configured everything correctly no userspace interaction is
required. The driver should request the firmware automatically once
you power up the bluetooth device.

Apart from DT changes make sure, that the following options are
enabled and check dmesg for any hints.

CONFIG_SERIAL_DEV_BUS
CONFIG_SERIAL_DEV_CTRL_TTYPORT
CONFIG_BT_HCIUART
CONFIG_BT_HCIUART_LL

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170430/f8f82e8d/attachment.sig>

^ permalink raw reply

* [PATCH] iio: stm32 trigger: Add support for TRGO2 triggers
From: Jonathan Cameron @ 2017-04-30 17:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <baaa0dd4-67fe-da83-1431-1e026ec687cb@st.com>

On 28/04/17 15:52, Fabrice Gasnier wrote:
> On 04/27/2017 07:49 AM, Jonathan Cameron wrote:
>> On 26/04/17 09:55, Benjamin Gaignard wrote:
>>> 2017-04-26 10:17 GMT+02:00 Fabrice Gasnier <fabrice.gasnier@st.com>:
>>>> Add support for TRGO2 trigger that can be found on STM32F7.
>>>> Add additional master modes supported by TRGO2.
>> These additional modes would benefit from more information in the
>> ABI docs.  Otherwise patch seems fine, though this may win
>> the award for hardest hardware to come up with a generic
>> interface for... 
>>>> Register additional "tim[1/8]_trgo2" triggers for timer1 & timer8.
>>>> Detect TRGO2 timer capability (master mode selection 2).
>>>>
>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>>> ---
>>>>  .../ABI/testing/sysfs-bus-iio-timer-stm32          |  15 +++
>>>>  drivers/iio/trigger/stm32-timer-trigger.c          | 113 ++++++++++++++++++---
>>>>  include/linux/iio/timer/stm32-timer-trigger.h      |   2 +
>>>>  include/linux/mfd/stm32-timers.h                   |   2 +
>>>>  4 files changed, 118 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>> index 230020e..47647b4 100644
>>>> --- a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>>>> @@ -16,6 +16,21 @@ Description:
>>>>                 - "OC2REF"    : OC2REF signal is used as trigger output.
>>>>                 - "OC3REF"    : OC3REF signal is used as trigger output.
>>>>                 - "OC4REF"    : OC4REF signal is used as trigger output.
>>>> +               Additional modes (on TRGO2 only):
>>>> +               - "OC5REF"    : OC5REF signal is used as trigger output.
>>>> +               - "OC6REF"    : OC6REF signal is used as trigger output.
>>>> +               - "compare_pulse_OC4REF":
>>>> +                 OC4REF rising or falling edges generate pulses.
>> I'd like this to be fairly understandable without resorting to reading the
>> datasheet.  As I understand it you get a fixed term pulse on both edges
>> of the waveform?  Perhaps this calls for some ascii art :)
> 
> Hi Jonathan,
> 
> If you feel like it needs more documentation, I'd rather prefer to add
> reference or link to the datasheet... That will be more accurate,
> up-to-date (e.g. like RM0385 pdf). Does this sound ok ? Or...
Datasheet is good, but give it 10 years and chances are it will disappear
into a black hole, whereas the hardware might still be in use by someone.
Some of the hardware I use is at least that old. Frankly this laptop is
getting close ;)
> 
> Just in case, I prepared some ascii art, hope it clarify things.
> I'm wondering if this is best place to put it ?
> Shouldn't this be added in source code, instead of ABI Doc ?
Could be either, but arguably the ABI docs should be all that a
userspace developer should need to see.  This isn't an internal
detail afterall.
> Maybe I can skip 1st part of it, heading boxes? (only example is enough?
> or not...)
> 
> +-----------+   +-------------+            +---------+
> | Prescaler +-> | Counter     |        +-> | Master  | TRGO(2)
> +-----------+   +--+--------+-+        |-> | Control +-->
>                    |        |          ||  +---------+
>                 +--v--------+-+ OCxREF ||  +---------+
>                 | Chx compare +----------> | Output  | ChX
>                 +-----------+-+         |  | Control +-->
>                       .     |           |  +---------+
>                       .     |           |    .
>                 +-----------v-+ OC6REF  |    .
>                 | Ch6 compare +---------+>
>                 +-------------+
> 
> Example with: "compare_pulse_OC4REF_r_or_OC6REF_r":
> 
>                 X
>               X   X
>             X .   . X
>           X   .   .   X
>         X     .   .     X
> count X .     .   .     . X
>         .     .   .     .
>         .     .   .     .
>         +---------------+
> OC4REF  |     .   .     |
>       +-+     .   .     +-+
>         .     +---+     .
> OC6REF  .     |   |     .
>       +-------+   +-------+
>         +-+   +-+
> TRGO2   | |   | |
>       +-+ +---+ +---------+
This is good stuff so I'd put it in the ABI docs.

Jonathan
> 
> 
> side note: this isn't my house ;-)
:)
> 
> Please advise,
> Thanks,
> Fabrice
> 
> 
>>>> +               - "compare_pulse_OC6REF":
>>>> +                 OC6REF rising or falling edges generate pulses.
>>>> +               - "compare_pulse_OC4REF_r_or_OC6REF_r":
>>>> +                 OC4REF or OC6REF rising edges generate pulses.
>>>> +               - "compare_pulse_OC4REF_r_or_OC6REF_f":
>>>> +                 OC4REF rising or OC6REF falling edges generate pulses.
>>>> +               - "compare_pulse_OC5REF_r_or_OC6REF_r":
>>>> +                 OC5REF or OC6REF rising edges generate pulses.
>>>> +               - "compare_pulse_OC5REF_r_or_OC6REF_f":
>>>> +                 OC5REF rising or OC6REF falling edges generate pulses.
>>>>
>>>>  What:          /sys/bus/iio/devices/triggerX/master_mode
>>>>  KernelVersion: 4.11
>>>> diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
>>>> index 0f1a2cf..a0031b7 100644
>>>> --- a/drivers/iio/trigger/stm32-timer-trigger.c
>>>> +++ b/drivers/iio/trigger/stm32-timer-trigger.c
>>>> @@ -14,19 +14,19 @@
>>>>  #include <linux/module.h>
>>>>  #include <linux/platform_device.h>
>>>>
>>>> -#define MAX_TRIGGERS 6
>>>> +#define MAX_TRIGGERS 7
>>>>  #define MAX_VALIDS 5
>>>>
>>>>  /* List the triggers created by each timer */
>>>>  static const void *triggers_table[][MAX_TRIGGERS] = {
>>>> -       { TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4,},
>>>> +       { TIM1_TRGO, TIM1_TRGO2, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4,},
>>>>         { TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4,},
>>>>         { TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4,},
>>>>         { TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4,},
>>>>         { TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4,},
>>>>         { TIM6_TRGO,},
>>>>         { TIM7_TRGO,},
>>>> -       { TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4,},
>>>> +       { TIM8_TRGO, TIM8_TRGO2, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4,},
>>>>         { TIM9_TRGO, TIM9_CH1, TIM9_CH2,},
>>>>         { }, /* timer 10 */
>>>>         { }, /* timer 11 */
>>>> @@ -56,9 +56,16 @@ struct stm32_timer_trigger {
>>>>         u32 max_arr;
>>>>         const void *triggers;
>>>>         const void *valids;
>>>> +       bool has_trgo2;
>>>>  };
>>>>
>>>> +static bool stm32_timer_is_trgo2_name(const char *name)
>>>> +{
>>>> +       return !!strstr(name, "trgo2");
>>>> +}
>>>> +
>>>>  static int stm32_timer_start(struct stm32_timer_trigger *priv,
>>>> +                            struct iio_trigger *trig,
>>>>                              unsigned int frequency)
>>>>  {
>>>>         unsigned long long prd, div;
>>>> @@ -102,7 +109,12 @@ static int stm32_timer_start(struct stm32_timer_trigger *priv,
>>>>         regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
>>>>
>>>>         /* Force master mode to update mode */
>>>> -       regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS, 0x20);
>>>> +       if (stm32_timer_is_trgo2_name(trig->name))
>>>> +               regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS2,
>>>> +                                  0x2 << TIM_CR2_MMS2_SHIFT);
>>>> +       else
>>>> +               regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS,
>>>> +                                  0x2 << TIM_CR2_MMS_SHIFT);
>>>>
>>>>         /* Make sure that registers are updated */
>>>>         regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
>>>> @@ -150,7 +162,7 @@ static ssize_t stm32_tt_store_frequency(struct device *dev,
>>>>         if (freq == 0) {
>>>>                 stm32_timer_stop(priv);
>>>>         } else {
>>>> -               ret = stm32_timer_start(priv, freq);
>>>> +               ret = stm32_timer_start(priv, trig, freq);
>>>>                 if (ret)
>>>>                         return ret;
>>>>         }
>>>> @@ -183,6 +195,9 @@ static IIO_DEV_ATTR_SAMP_FREQ(0660,
>>>>                               stm32_tt_read_frequency,
>>>>                               stm32_tt_store_frequency);
>>>>
>>>> +#define MASTER_MODE_MAX                7
>>>> +#define MASTER_MODE2_MAX       15
>>>> +
>>>>  static char *master_mode_table[] = {
>>>>         "reset",
>>>>         "enable",
>>>> @@ -191,7 +206,16 @@ static IIO_DEV_ATTR_SAMP_FREQ(0660,
>>>>         "OC1REF",
>>>>         "OC2REF",
>>>>         "OC3REF",
>>>> -       "OC4REF"
>>>> +       "OC4REF",
>>>> +       /* Master mode selection 2 only */
>>>> +       "OC5REF",
>>>> +       "OC6REF",
>>>> +       "compare_pulse_OC4REF",
>>>> +       "compare_pulse_OC6REF",
>>>> +       "compare_pulse_OC4REF_r_or_OC6REF_r",
>>>> +       "compare_pulse_OC4REF_r_or_OC6REF_f",
>>>> +       "compare_pulse_OC5REF_r_or_OC6REF_r",
>>>> +       "compare_pulse_OC5REF_r_or_OC6REF_f",
>>>>  };
>>>>
>>>>  static ssize_t stm32_tt_show_master_mode(struct device *dev,
>>>> @@ -199,10 +223,15 @@ static ssize_t stm32_tt_show_master_mode(struct device *dev,
>>>>                                          char *buf)
>>>>  {
>>>>         struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
>>>> +       struct iio_trigger *trig = to_iio_trigger(dev);
>>>>         u32 cr2;
>>>>
>>>>         regmap_read(priv->regmap, TIM_CR2, &cr2);
>>>> -       cr2 = (cr2 & TIM_CR2_MMS) >> TIM_CR2_MMS_SHIFT;
>>>> +
>>>> +       if (stm32_timer_is_trgo2_name(trig->name))
>>>> +               cr2 = (cr2 & TIM_CR2_MMS2) >> TIM_CR2_MMS2_SHIFT;
>>>> +       else
>>>> +               cr2 = (cr2 & TIM_CR2_MMS) >> TIM_CR2_MMS_SHIFT;
>>>>
>>>>         return snprintf(buf, PAGE_SIZE, "%s\n", master_mode_table[cr2]);
>>>>  }
>>>> @@ -212,13 +241,25 @@ static ssize_t stm32_tt_store_master_mode(struct device *dev,
>>>>                                           const char *buf, size_t len)
>>>>  {
>>>>         struct stm32_timer_trigger *priv = dev_get_drvdata(dev);
>>>> +       struct iio_trigger *trig = to_iio_trigger(dev);
>>>> +       u32 mask, shift, master_mode_max;
>>>>         int i;
>>>>
>>>> -       for (i = 0; i < ARRAY_SIZE(master_mode_table); i++) {
>>>> +       if (stm32_timer_is_trgo2_name(trig->name)) {
>>>> +               mask = TIM_CR2_MMS2;
>>>> +               shift = TIM_CR2_MMS2_SHIFT;
>>>> +               master_mode_max = MASTER_MODE2_MAX;
>>>> +       } else {
>>>> +               mask = TIM_CR2_MMS;
>>>> +               shift = TIM_CR2_MMS_SHIFT;
>>>> +               master_mode_max = MASTER_MODE_MAX;
>>>> +       }
>>>> +
>>>> +       for (i = 0; i <= master_mode_max; i++) {
>>>>                 if (!strncmp(master_mode_table[i], buf,
>>>>                              strlen(master_mode_table[i]))) {
>>>> -                       regmap_update_bits(priv->regmap, TIM_CR2,
>>>> -                                          TIM_CR2_MMS, i << TIM_CR2_MMS_SHIFT);
>>>> +                       regmap_update_bits(priv->regmap, TIM_CR2, mask,
>>>> +                                          i << shift);
>>>>                         /* Make sure that registers are updated */
>>>>                         regmap_update_bits(priv->regmap, TIM_EGR,
>>>>                                            TIM_EGR_UG, TIM_EGR_UG);
>>>> @@ -229,8 +270,31 @@ static ssize_t stm32_tt_store_master_mode(struct device *dev,
>>>>         return -EINVAL;
>>>>  }
>>>>
>>>> -static IIO_CONST_ATTR(master_mode_available,
>>>> -       "reset enable update compare_pulse OC1REF OC2REF OC3REF OC4REF");
>>>> +static ssize_t stm32_tt_show_master_mode_avail(struct device *dev,
>>>> +                                              struct device_attribute *attr,
>>>> +                                              char *buf)
>>>> +{
>>>> +       struct iio_trigger *trig = to_iio_trigger(dev);
>>>> +       unsigned int i, master_mode_max;
>>>> +       size_t len = 0;
>>>> +
>>>> +       if (stm32_timer_is_trgo2_name(trig->name))
>>>> +               master_mode_max = MASTER_MODE2_MAX;
>>>> +       else
>>>> +               master_mode_max = MASTER_MODE_MAX;
>>>> +
>>>> +       for (i = 0; i <= master_mode_max; i++)
>>>> +               len += scnprintf(buf + len, PAGE_SIZE - len,
>>>> +                       "%s ", master_mode_table[i]);
>>>> +
>>>> +       /* replace trailing space by newline */
>>>> +       buf[len - 1] = '\n';
>>>> +
>>>> +       return len;
>>>> +}
>>>> +
>>>> +static IIO_DEVICE_ATTR(master_mode_available, 0444,
>>>> +                      stm32_tt_show_master_mode_avail, NULL, 0);
>>>>
>>>>  static IIO_DEVICE_ATTR(master_mode, 0660,
>>>>                        stm32_tt_show_master_mode,
>>>> @@ -240,7 +304,7 @@ static IIO_DEVICE_ATTR(master_mode, 0660,
>>>>  static struct attribute *stm32_trigger_attrs[] = {
>>>>         &iio_dev_attr_sampling_frequency.dev_attr.attr,
>>>>         &iio_dev_attr_master_mode.dev_attr.attr,
>>>> -       &iio_const_attr_master_mode_available.dev_attr.attr,
>>>> +       &iio_dev_attr_master_mode_available.dev_attr.attr,
>>>>         NULL,
>>>>  };
>>>>
>>>> @@ -264,6 +328,12 @@ static int stm32_setup_iio_triggers(struct stm32_timer_trigger *priv)
>>>>
>>>>         while (cur && *cur) {
>>>>                 struct iio_trigger *trig;
>>>> +               bool cur_is_trgo2 = stm32_timer_is_trgo2_name(*cur);
>>>> +
>>>> +               if (cur_is_trgo2 && !priv->has_trgo2) {
>>>> +                       cur++;
>>>> +                       continue;
>>>> +               }
>>>>
>>>>                 trig = devm_iio_trigger_alloc(priv->dev, "%s", *cur);
>>>>                 if  (!trig)
>>>> @@ -277,7 +347,7 @@ static int stm32_setup_iio_triggers(struct stm32_timer_trigger *priv)
>>>>                  * should only be available on trgo trigger which
>>>>                  * is always the first in the list.
>>>>                  */
>>>> -               if (cur == priv->triggers)
>>>> +               if (cur == priv->triggers || cur_is_trgo2)
>>>>                         trig->dev.groups = stm32_trigger_attr_groups;
>>>>
>>>>                 iio_trigger_set_drvdata(trig, priv);
>>>> @@ -584,6 +654,20 @@ bool is_stm32_timer_trigger(struct iio_trigger *trig)
>>>>  }
>>>>  EXPORT_SYMBOL(is_stm32_timer_trigger);
>>>>
>>>> +static void stm32_timer_detect_trgo2(struct stm32_timer_trigger *priv)
>>>> +{
>>>> +       u32 val;
>>>> +
>>>> +       /*
>>>> +        * Master mode selection 2 bits can only be written and read back when
>>>> +        * timer supports it.
>>>> +        */
>>>> +       regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS2, TIM_CR2_MMS2);
>>>> +       regmap_read(priv->regmap, TIM_CR2, &val);
>>>> +       regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS2, 0);
>>>> +       priv->has_trgo2 = !!val;
>>>> +}
>>>> +
>>>>  static int stm32_timer_trigger_probe(struct platform_device *pdev)
>>>>  {
>>>>         struct device *dev = &pdev->dev;
>>>> @@ -614,6 +698,7 @@ static int stm32_timer_trigger_probe(struct platform_device *pdev)
>>>>         priv->max_arr = ddata->max_arr;
>>>>         priv->triggers = triggers_table[index];
>>>>         priv->valids = valids_table[index];
>>>> +       stm32_timer_detect_trgo2(priv);
>>>>
>>>>         ret = stm32_setup_iio_triggers(priv);
>>>>         if (ret)
>>>> diff --git a/include/linux/iio/timer/stm32-timer-trigger.h b/include/linux/iio/timer/stm32-timer-trigger.h
>>>> index 55535ae..fa7d786 100644
>>>> --- a/include/linux/iio/timer/stm32-timer-trigger.h
>>>> +++ b/include/linux/iio/timer/stm32-timer-trigger.h
>>>> @@ -10,6 +10,7 @@
>>>>  #define _STM32_TIMER_TRIGGER_H_
>>>>
>>>>  #define TIM1_TRGO      "tim1_trgo"
>>>> +#define TIM1_TRGO2     "tim1_trgo2"
>>>>  #define TIM1_CH1       "tim1_ch1"
>>>>  #define TIM1_CH2       "tim1_ch2"
>>>>  #define TIM1_CH3       "tim1_ch3"
>>>> @@ -44,6 +45,7 @@
>>>>  #define TIM7_TRGO      "tim7_trgo"
>>>>
>>>>  #define TIM8_TRGO      "tim8_trgo"
>>>> +#define TIM8_TRGO2     "tim8_trgo2"
>>>>  #define TIM8_CH1       "tim8_ch1"
>>>>  #define TIM8_CH2       "tim8_ch2"
>>>>  #define TIM8_CH3       "tim8_ch3"
>>>> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
>>>> index 4a0abbc..ce7346e 100644
>>>> --- a/include/linux/mfd/stm32-timers.h
>>>> +++ b/include/linux/mfd/stm32-timers.h
>>>> @@ -34,6 +34,7 @@
>>>>  #define TIM_CR1_DIR    BIT(4)  /* Counter Direction       */
>>>>  #define TIM_CR1_ARPE   BIT(7)  /* Auto-reload Preload Ena */
>>>>  #define TIM_CR2_MMS    (BIT(4) | BIT(5) | BIT(6)) /* Master mode selection */
>>>> +#define TIM_CR2_MMS2   GENMASK(23, 20) /* Master mode selection 2 */
>>>>  #define TIM_SMCR_SMS   (BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
>>>>  #define TIM_SMCR_TS    (BIT(4) | BIT(5) | BIT(6)) /* Trigger selection */
>>>>  #define TIM_DIER_UIE   BIT(0)  /* Update interrupt        */
>>>> @@ -60,6 +61,7 @@
>>>>
>>>>  #define MAX_TIM_PSC            0xFFFF
>>>>  #define TIM_CR2_MMS_SHIFT      4
>>>> +#define TIM_CR2_MMS2_SHIFT     20
>>>>  #define TIM_SMCR_TS_SHIFT      4
>>>>  #define TIM_BDTR_BKF_MASK      0xF
>>>>  #define TIM_BDTR_BKF_SHIFT     16
>>>> --
>>>> 1.9.1
>>>>
>>>
>>> Acked-by: Benjamin Gaiganrd <benjamin.gaignard@linaro.org>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>> the body of a message to majordomo at vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* [PATCH 1/3] ARM: dts: rockchip: Move cros-ec-sbs to rk3288-veyron-chromebook-sbs
From: Paul Kocialkowski @ 2017-04-30 18:30 UTC (permalink / raw)
  To: linux-arm-kernel

This moves the cros-ec-sbs dtsi to a new rk3288-veyron-chromebook-sbs
dtsi since it only concerns rk3288 veyron Chromebooks.

Other Chromebooks (such as the tegra124 nyans) also have sbs batteries
and don't use this dtsi, that only makes sense when used with
rk3288-veyron-chromebook anyway.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 .../boot/dts/{cros-ec-sbs.dtsi => rk3288-veyron-chromebook-sbs.dtsi}    | 0
 arch/arm/boot/dts/rk3288-veyron-jaq.dts                                 | 2 +-
 arch/arm/boot/dts/rk3288-veyron-jerry.dts                               | 2 +-
 arch/arm/boot/dts/rk3288-veyron-pinky.dts                               | 2 +-
 arch/arm/boot/dts/rk3288-veyron-speedy.dts                              | 2 +-
 5 files changed, 4 insertions(+), 4 deletions(-)
 rename arch/arm/boot/dts/{cros-ec-sbs.dtsi => rk3288-veyron-chromebook-sbs.dtsi} (100%)

diff --git a/arch/arm/boot/dts/cros-ec-sbs.dtsi b/arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
similarity index 100%
rename from arch/arm/boot/dts/cros-ec-sbs.dtsi
rename to arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
diff --git a/arch/arm/boot/dts/rk3288-veyron-jaq.dts b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
index d33f5763c39c..f217a978e47a 100644
--- a/arch/arm/boot/dts/rk3288-veyron-jaq.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
@@ -45,7 +45,7 @@
 /dts-v1/;
 
 #include "rk3288-veyron-chromebook.dtsi"
-#include "cros-ec-sbs.dtsi"
+#include "rk3288-veyron-chromebook-sbs.dtsi"
 
 / {
 	model = "Google Jaq";
diff --git a/arch/arm/boot/dts/rk3288-veyron-jerry.dts b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
index cdea751f2a8c..bec607574165 100644
--- a/arch/arm/boot/dts/rk3288-veyron-jerry.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
@@ -44,7 +44,7 @@
 
 /dts-v1/;
 #include "rk3288-veyron-chromebook.dtsi"
-#include "cros-ec-sbs.dtsi"
+#include "rk3288-veyron-chromebook-sbs.dtsi"
 
 / {
 	model = "Google Jerry";
diff --git a/arch/arm/boot/dts/rk3288-veyron-pinky.dts b/arch/arm/boot/dts/rk3288-veyron-pinky.dts
index 995cff42fa43..c81ad5bf1121 100644
--- a/arch/arm/boot/dts/rk3288-veyron-pinky.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-pinky.dts
@@ -44,7 +44,7 @@
 
 /dts-v1/;
 #include "rk3288-veyron-chromebook.dtsi"
-#include "cros-ec-sbs.dtsi"
+#include "rk3288-veyron-chromebook-sbs.dtsi"
 
 / {
 	model = "Google Pinky";
diff --git a/arch/arm/boot/dts/rk3288-veyron-speedy.dts b/arch/arm/boot/dts/rk3288-veyron-speedy.dts
index cc0b78cefe34..8aea9c3ff6e2 100644
--- a/arch/arm/boot/dts/rk3288-veyron-speedy.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-speedy.dts
@@ -44,7 +44,7 @@
 
 /dts-v1/;
 #include "rk3288-veyron-chromebook.dtsi"
-#include "cros-ec-sbs.dtsi"
+#include "rk3288-veyron-chromebook-sbs.dtsi"
 
 / {
 	model = "Google Speedy";
-- 
2.12.2

^ permalink raw reply related

* [PATCH 2/3] ARM: dts: rockchip: List charger as power supply for sbs battery
From: Paul Kocialkowski @ 2017-04-30 18:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170430183054.24563-1-contact@paulk.fr>

This lists the GPIO charger node as power supply for the battery, which
in turns allows receiving external power notifications and synchronizing
the charging state as soon as possible.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi | 1 +
 arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi     | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi b/arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
index 71f5c5ecce46..8e4d2b9a35e1 100644
--- a/arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
@@ -48,5 +48,6 @@
 		reg = <0xb>;
 		sbs,i2c-retry-count = <2>;
 		sbs,poll-retry-count = <1>;
+		power-supplies = <&charger>;
 	};
 };
diff --git a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
index d752a315f884..fd4a3886c94b 100644
--- a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
@@ -99,7 +99,7 @@
 		pwm-delay-us = <10000>;
 	};
 
-	gpio-charger {
+	charger: gpio-charger {
 		compatible = "gpio-charger";
 		charger-type = "mains";
 		gpios = <&gpio0 RK_PB0 GPIO_ACTIVE_HIGH>;
-- 
2.12.2

^ permalink raw reply related

* [PATCH 3/3] ARM: dts: rockchip: List charger as power supply for minnie
From: Paul Kocialkowski @ 2017-04-30 18:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170430183054.24563-1-contact@paulk.fr>

This lists the GPIO charger node as power supply for the battery, which
in turns allows receiving external power notifications and synchronizing
the charging state as soon as possible.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/boot/dts/rk3288-veyron-minnie.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/rk3288-veyron-minnie.dts b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
index 544de6027aaa..3f97f33bd783 100644
--- a/arch/arm/boot/dts/rk3288-veyron-minnie.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
@@ -151,6 +151,7 @@
 	battery: bq27500 at 55 {
 		compatible = "ti,bq27500";
 		reg = <0x55>;
+		power-supplies = <&charger>;
 	};
 };
 
-- 
2.12.2

^ permalink raw reply related

* [PATCH v5 16/22] KVM: arm64: vgic-its: Add infrastructure for table lookup
From: Christoffer Dall @ 2017-04-30 19:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492164934-988-17-git-send-email-eric.auger@redhat.com>

On Fri, Apr 14, 2017 at 12:15:28PM +0200, Eric Auger wrote:
> Add a generic lookup_table() helper whose role consists in
> scanning a contiguous table located in guest RAM and applying
> a callback on each entry. Entries can be handled as linked lists
> since the callback may return an offset to the next entry and
> also tell that an entry is the last one.
> 
> Helper functions also are added to compute the device/event ID
> offset to the next DTE/ITE.
> 
> compute_next_devid_offset, compute_next_eventid_offset and
> lookup_table will become static in subsequent patches
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> v4 -> v5:
> - use kvm_read_guest
> 
> v3 -> v4:
> - remove static to avoid compilation warning
> - correct size computation in looup_table()
> - defines now encode the number of bits used for devid and eventid offsets
> - use BIT() - 1 to encode the max offets
> ---
>  virt/kvm/arm/vgic/vgic-its.c | 93 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index 56c5123..c22b35d 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -195,6 +195,8 @@ static struct its_ite *find_ite(struct vgic_its *its, u32 device_id,
>  
>  #define VITS_TYPER_IDBITS 16
>  #define VITS_TYPER_DEVBITS 16
> +#define VITS_DTE_MAX_DEVID_OFFSET	(BIT(14) - 1)
> +#define VITS_ITE_MAX_EVENTID_OFFSET	(BIT(16) - 1)
>  
>  /*
>   * Finds and returns a collection in the ITS collection table.
> @@ -1674,6 +1676,97 @@ int vgic_its_attr_regs_access(struct kvm_device *dev,
>  	return ret;
>  }
>  
> +u32 compute_next_devid_offset(struct list_head *h, struct its_device *dev)
> +{
> +	struct list_head *e = &dev->dev_list;
> +	struct its_device *next;
> +	u32 next_offset;
> +
> +	if (e->next == h)
> +		return 0;
> +	next = list_entry(e->next, struct its_device, dev_list);
> +	next_offset = next->device_id - dev->device_id;
> +
> +	return min_t(u32, next_offset, VITS_DTE_MAX_DEVID_OFFSET);
> +}
> +
> +u32 compute_next_eventid_offset(struct list_head *h, struct its_ite *ite)
> +{
> +	struct list_head *e = &ite->ite_list;
> +	struct its_ite *next;
> +	u32 next_offset;
> +
> +	if (e->next == h)
> +		return 0;
> +	next = list_entry(e->next, struct its_ite, ite_list);
> +	next_offset = next->event_id - ite->event_id;
> +
> +	return min_t(u32, next_offset, VITS_ITE_MAX_EVENTID_OFFSET);
> +}
> +
> +/**
> + * entry_fn_t - Callback called on a table entry restore path
> + * @its: its handle
> + * @id: id of the entry
> + * @entry: pointer to the entry
> + * @opaque: pointer to an opaque data
> + * @next_offset: minimal ID offset to the next entry. 0 if this
> + * entry is the last one, 1 if the entry is invalid, >= 1 if an
> + * entry's next_offset field was truly decoded
> + *
> + * Return: < 0 on error, 0 otherwise
> + */
> +typedef int (*entry_fn_t)(struct vgic_its *its, u32 id, void *entry,
> +			  void *opaque, u32 *next_offset);

Just noticed.  All the table entries are 64-bit long at this point,
right?  So why not make entry a u64 * instead?  Could we end up with
some endianness mess with using void pointers the way it is now?

Thanks,
-Christoffer

> +
> +/**
> + * lookup_table - scan a contiguous table in guest RAM and applies a function
> + * to each entry
> + *
> + * @its: its handle
> + * @base: base gpa of the table
> + * @size: size of the table in bytes
> + * @esz: entry size in bytes
> + * @start_id: first entry's ID
> + * @fn: function to apply on each entry
> + *
> + * Return: < 0 on error, 1 if last element identified, 0 otherwise
> + */
> +int lookup_table(struct vgic_its *its, gpa_t base, int size, int esz,
> +		 int start_id, entry_fn_t fn, void *opaque)
> +{
> +	void *entry = kzalloc(esz, GFP_KERNEL);
> +	struct kvm *kvm = its->dev->kvm;
> +	unsigned long len = size;
> +	u32 id = start_id;
> +	gpa_t gpa = base;
> +	int ret;
> +
> +	while (len > 0) {
> +		u32 next_offset;
> +		size_t byte_offset;
> +
> +		ret = kvm_read_guest(kvm, gpa, entry, esz);
> +		if (ret)
> +			goto out;
> +
> +		ret = fn(its, id, entry, opaque, &next_offset);
> +		if (ret < 0 || (!ret && !next_offset))
> +			goto out;
> +
> +		byte_offset = next_offset * esz;
> +		id += next_offset;
> +		gpa += byte_offset;
> +		len -= byte_offset;
> +	}
> +	kfree(entry);
> +	return 0;
> +
> +out:
> +	kfree(entry);
> +	return (ret < 0 ? ret : 1);
> +}
> +
>  /**
>   * vgic_its_save_device_tables - Save the device table and all ITT
>   * into guest RAM
> -- 
> 2.5.5
> 

^ permalink raw reply

* [PATCH v5 16/22] KVM: arm64: vgic-its: Add infrastructure for table lookup
From: Christoffer Dall @ 2017-04-30 19:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492164934-988-17-git-send-email-eric.auger@redhat.com>

On Fri, Apr 14, 2017 at 12:15:28PM +0200, Eric Auger wrote:
> Add a generic lookup_table() helper whose role consists in
> scanning a contiguous table located in guest RAM and applying
> a callback on each entry. Entries can be handled as linked lists
> since the callback may return an offset to the next entry and
> also tell that an entry is the last one.
> 
> Helper functions also are added to compute the device/event ID
> offset to the next DTE/ITE.
> 
> compute_next_devid_offset, compute_next_eventid_offset and
> lookup_table will become static in subsequent patches
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> v4 -> v5:
> - use kvm_read_guest
> 
> v3 -> v4:
> - remove static to avoid compilation warning
> - correct size computation in looup_table()
> - defines now encode the number of bits used for devid and eventid offsets
> - use BIT() - 1 to encode the max offets
> ---
>  virt/kvm/arm/vgic/vgic-its.c | 93 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index 56c5123..c22b35d 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -195,6 +195,8 @@ static struct its_ite *find_ite(struct vgic_its *its, u32 device_id,
>  
>  #define VITS_TYPER_IDBITS 16
>  #define VITS_TYPER_DEVBITS 16
> +#define VITS_DTE_MAX_DEVID_OFFSET	(BIT(14) - 1)
> +#define VITS_ITE_MAX_EVENTID_OFFSET	(BIT(16) - 1)
>  
>  /*
>   * Finds and returns a collection in the ITS collection table.
> @@ -1674,6 +1676,97 @@ int vgic_its_attr_regs_access(struct kvm_device *dev,
>  	return ret;
>  }
>  
> +u32 compute_next_devid_offset(struct list_head *h, struct its_device *dev)
> +{
> +	struct list_head *e = &dev->dev_list;
> +	struct its_device *next;
> +	u32 next_offset;
> +
> +	if (e->next == h)
> +		return 0;
> +	next = list_entry(e->next, struct its_device, dev_list);
> +	next_offset = next->device_id - dev->device_id;
> +
> +	return min_t(u32, next_offset, VITS_DTE_MAX_DEVID_OFFSET);
> +}
> +
> +u32 compute_next_eventid_offset(struct list_head *h, struct its_ite *ite)
> +{
> +	struct list_head *e = &ite->ite_list;
> +	struct its_ite *next;
> +	u32 next_offset;
> +
> +	if (e->next == h)
> +		return 0;
> +	next = list_entry(e->next, struct its_ite, ite_list);
> +	next_offset = next->event_id - ite->event_id;
> +
> +	return min_t(u32, next_offset, VITS_ITE_MAX_EVENTID_OFFSET);
> +}
> +
> +/**
> + * entry_fn_t - Callback called on a table entry restore path
> + * @its: its handle
> + * @id: id of the entry
> + * @entry: pointer to the entry
> + * @opaque: pointer to an opaque data
> + * @next_offset: minimal ID offset to the next entry. 0 if this
> + * entry is the last one, 1 if the entry is invalid, >= 1 if an

eh, also, did you mean -1 if the entry is invalid?

Thanks,
-Christoffer

^ permalink raw reply

* [PATCH v5 16/22] KVM: arm64: vgic-its: Add infrastructure for table lookup
From: Christoffer Dall @ 2017-04-30 20:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492164934-988-17-git-send-email-eric.auger@redhat.com>

On Fri, Apr 14, 2017 at 12:15:28PM +0200, Eric Auger wrote:
> Add a generic lookup_table() helper whose role consists in
> scanning a contiguous table located in guest RAM and applying
> a callback on each entry. Entries can be handled as linked lists
> since the callback may return an offset to the next entry and
> also tell that an entry is the last one.
> 
> Helper functions also are added to compute the device/event ID
> offset to the next DTE/ITE.
> 
> compute_next_devid_offset, compute_next_eventid_offset and
> lookup_table will become static in subsequent patches
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> v4 -> v5:
> - use kvm_read_guest
> 
> v3 -> v4:
> - remove static to avoid compilation warning
> - correct size computation in looup_table()
> - defines now encode the number of bits used for devid and eventid offsets
> - use BIT() - 1 to encode the max offets
> ---
>  virt/kvm/arm/vgic/vgic-its.c | 93 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index 56c5123..c22b35d 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -195,6 +195,8 @@ static struct its_ite *find_ite(struct vgic_its *its, u32 device_id,
>  
>  #define VITS_TYPER_IDBITS 16
>  #define VITS_TYPER_DEVBITS 16
> +#define VITS_DTE_MAX_DEVID_OFFSET	(BIT(14) - 1)
> +#define VITS_ITE_MAX_EVENTID_OFFSET	(BIT(16) - 1)
>  
>  /*
>   * Finds and returns a collection in the ITS collection table.
> @@ -1674,6 +1676,97 @@ int vgic_its_attr_regs_access(struct kvm_device *dev,
>  	return ret;
>  }
>  
> +u32 compute_next_devid_offset(struct list_head *h, struct its_device *dev)
> +{
> +	struct list_head *e = &dev->dev_list;
> +	struct its_device *next;
> +	u32 next_offset;
> +
> +	if (e->next == h)
> +		return 0;
> +	next = list_entry(e->next, struct its_device, dev_list);
> +	next_offset = next->device_id - dev->device_id;
> +
> +	return min_t(u32, next_offset, VITS_DTE_MAX_DEVID_OFFSET);
> +}
> +
> +u32 compute_next_eventid_offset(struct list_head *h, struct its_ite *ite)
> +{
> +	struct list_head *e = &ite->ite_list;
> +	struct its_ite *next;
> +	u32 next_offset;
> +
> +	if (e->next == h)
> +		return 0;
> +	next = list_entry(e->next, struct its_ite, ite_list);
> +	next_offset = next->event_id - ite->event_id;
> +
> +	return min_t(u32, next_offset, VITS_ITE_MAX_EVENTID_OFFSET);

nit: in both of these functions you can avoid the extra e variable and
maybe make this slightly more readable by using these two calls instead:

list_is_last(&ite->ite_list, h);
next = list_next_entry(ite, ite_list);

Thanks,
-Christoffer

^ permalink raw reply

* [PATCH v5 19/22] KVM: arm64: vgic-its: ITT save and restore
From: Christoffer Dall @ 2017-04-30 20:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492164934-988-20-git-send-email-eric.auger@redhat.com>

On Fri, Apr 14, 2017 at 12:15:31PM +0200, Eric Auger wrote:
> Introduce routines to save and restore device ITT and their
> interrupt table entries (ITE).
> 
> The routines will be called on device table save and
> restore. They will become static in subsequent patches.

Why this bottom-up approach?  Couldn't you start by having the patch
that restores the device table and define the static functions that
return an error there, and then fill them in with subsequent patches
(liek this one)?

That would have the added benefit of being able to tell how things are
designed to be called.

> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> v4 -> v5:
> - ITE are now sorted by eventid on the flush
> - rename *flush* into *save*
> - use macros for shits and masks
> - pass ite_esz to vgic_its_save_ite
> 
> v3 -> v4:
> - lookup_table and compute_next_eventid_offset become static in this
>   patch
> - remove static along with vgic_its_flush/restore_itt to avoid
>   compilation warnings
> - next field only computed with a shift (mask removed)
> - handle the case where the last element has not been found
> 
> v2 -> v3:
> - add return 0 in vgic_its_restore_ite (was in subsequent patch)
> 
> v2: creation
> ---
>  virt/kvm/arm/vgic/vgic-its.c | 128 ++++++++++++++++++++++++++++++++++++++++++-
>  virt/kvm/arm/vgic/vgic.h     |   4 ++
>  2 files changed, 129 insertions(+), 3 deletions(-)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index 35b2ca1..b02fc3f 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -23,6 +23,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/list.h>
>  #include <linux/uaccess.h>
> +#include <linux/list_sort.h>
>  
>  #include <linux/irqchip/arm-gic-v3.h>
>  
> @@ -1695,7 +1696,7 @@ u32 compute_next_devid_offset(struct list_head *h, struct its_device *dev)
>  	return min_t(u32, next_offset, VITS_DTE_MAX_DEVID_OFFSET);
>  }
>  
> -u32 compute_next_eventid_offset(struct list_head *h, struct its_ite *ite)
> +static u32 compute_next_eventid_offset(struct list_head *h, struct its_ite *ite)
>  {
>  	struct list_head *e = &ite->ite_list;
>  	struct its_ite *next;
> @@ -1737,8 +1738,8 @@ typedef int (*entry_fn_t)(struct vgic_its *its, u32 id, void *entry,
>   *
>   * Return: < 0 on error, 1 if last element identified, 0 otherwise
>   */
> -int lookup_table(struct vgic_its *its, gpa_t base, int size, int esz,
> -		 int start_id, entry_fn_t fn, void *opaque)
> +static int lookup_table(struct vgic_its *its, gpa_t base, int size, int esz,
> +			int start_id, entry_fn_t fn, void *opaque)
>  {
>  	void *entry = kzalloc(esz, GFP_KERNEL);
>  	struct kvm *kvm = its->dev->kvm;
> @@ -1773,6 +1774,127 @@ int lookup_table(struct vgic_its *its, gpa_t base, int size, int esz,
>  }
>  
>  /**
> + * vgic_its_save_ite - Save an interrupt translation entry at @gpa
> + */
> +static int vgic_its_save_ite(struct vgic_its *its, struct its_device *dev,
> +			      struct its_ite *ite, gpa_t gpa, int ite_esz)
> +{
> +	struct kvm *kvm = its->dev->kvm;
> +	u32 next_offset;
> +	u64 val;
> +
> +	next_offset = compute_next_eventid_offset(&dev->itt_head, ite);
> +	val = ((u64)next_offset << KVM_ITS_ITE_NEXT_SHIFT) |
> +	       ((u64)ite->lpi << KVM_ITS_ITE_PINTID_SHIFT) |
> +		ite->collection->collection_id;
> +	val = cpu_to_le64(val);
> +	return kvm_write_guest(kvm, gpa, &val, ite_esz);
> +}
> +
> +/**
> + * vgic_its_restore_ite - restore an interrupt translation entry
> + * @event_id: id used for indexing
> + * @ptr: pointer to the ITE entry
> + * @opaque: pointer to the its_device
> + * @next: id offset to the next entry
> + */
> +static int vgic_its_restore_ite(struct vgic_its *its, u32 event_id,
> +				void *ptr, void *opaque, u32 *next)
> +{
> +	struct its_device *dev = (struct its_device *)opaque;
> +	struct its_collection *collection;
> +	struct kvm *kvm = its->dev->kvm;
> +	u64 val, *p = (u64 *)ptr;

nit: initializations on separate line (and possible do that just above
assigning val).

> +	struct vgic_irq *irq;
> +	u32 coll_id, lpi_id;
> +	struct its_ite *ite;
> +	int ret;
> +
> +	val = *p;
> +	*next = 1;
> +
> +	val = le64_to_cpu(val);
> +
> +	coll_id = val & KVM_ITS_ITE_ICID_MASK;
> +	lpi_id = (val & KVM_ITS_ITE_PINTID_MASK) >> KVM_ITS_ITE_PINTID_SHIFT;
> +
> +	if (!lpi_id)
> +		return 0;

are all non-zero LPI IDs valid?  Don't we have a wrapper that tests if
the ID is valid?

(looks like it's possible to add LPIs with the INTID range of SPIs, SGIs
and PPIs here)

> +
> +	*next = val >> KVM_ITS_ITE_NEXT_SHIFT;

Don't we need to validate this somehow since it will presumably be used
to forward a pointer somehow by the caller?

> +
> +	collection = find_collection(its, coll_id);
> +	if (!collection)
> +		return -EINVAL;
> +
> +	ret = vgic_its_alloc_ite(dev, &ite, collection,
> +				  lpi_id, event_id);
> +	if (ret)
> +		return ret;
> +
> +	irq = vgic_add_lpi(kvm, lpi_id);
> +	if (IS_ERR(irq))
> +		return PTR_ERR(irq);
> +	ite->irq = irq;
> +
> +	/* restore the configuration of the LPI */
> +	ret = update_lpi_config(kvm, irq, NULL);
> +	if (ret)
> +		return ret;
> +
> +	update_affinity_ite(kvm, ite);
> +	return 0;
> +}
> +
> +static int vgic_its_ite_cmp(void *priv, struct list_head *a,
> +			    struct list_head *b)
> +{
> +	struct its_ite *itea = container_of(a, struct its_ite, ite_list);
> +	struct its_ite *iteb = container_of(b, struct its_ite, ite_list);
> +
> +	if (itea->event_id < iteb->event_id)
> +		return -1;
> +	else
> +		return 1;
> +}
> +
> +int vgic_its_save_itt(struct vgic_its *its, struct its_device *device)
> +{
> +	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
> +	gpa_t base = device->itt_addr;
> +	struct its_ite *ite;
> +	int ret, ite_esz = abi->ite_esz;

nit: initializations on separate line

> +
> +	list_sort(NULL, &device->itt_head, vgic_its_ite_cmp);
> +
> +	list_for_each_entry(ite, &device->itt_head, ite_list) {
> +		gpa_t gpa = base + ite->event_id * ite_esz;
> +
> +		ret = vgic_its_save_ite(its, device, ite, gpa, ite_esz);
> +		if (ret)
> +			return ret;
> +	}
> +	return 0;
> +}
> +
> +int vgic_its_restore_itt(struct vgic_its *its, struct its_device *dev)
> +{
> +	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
> +	gpa_t base = dev->itt_addr;
> +	int ret, ite_esz = abi->ite_esz;
> +	size_t max_size = BIT_ULL(dev->nb_eventid_bits) * ite_esz;

nit: initializations on separate line

> +
> +	ret =  lookup_table(its, base, max_size, ite_esz, 0,
> +			    vgic_its_restore_ite, dev);

nit: extra white space

> +
> +	if (ret < 0)
> +		return ret;
> +
> +	/* if the last element has not been found we are in trouble */
> +	return ret ? 0 : -EINVAL;

hmm, these are values potentially created by the guest in guest RAM,
right?  So do we really abort migration and return an error to userspace
in this case?

Also, this comment doesn't really tell me what this situation is and how
we handle things...

> +}
> +
> +/**
>   * vgic_its_save_device_tables - Save the device table and all ITT
>   * into guest RAM
>   */
> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
> index 56e57c1..ce57fbd 100644
> --- a/virt/kvm/arm/vgic/vgic.h
> +++ b/virt/kvm/arm/vgic/vgic.h
> @@ -81,6 +81,10 @@
>  #define KVM_ITS_CTE_VALID_MASK		BIT_ULL(63)
>  #define KVM_ITS_CTE_RDBASE_SHIFT	16
>  #define KVM_ITS_CTE_ICID_MASK		GENMASK_ULL(15, 0)
> +#define KVM_ITS_ITE_NEXT_SHIFT		48
> +#define KVM_ITS_ITE_PINTID_SHIFT	16
> +#define KVM_ITS_ITE_PINTID_MASK		GENMASK_ULL(47, 16)
> +#define KVM_ITS_ITE_ICID_MASK		GENMASK_ULL(15, 0)
>  
>  static inline bool irq_is_pending(struct vgic_irq *irq)
>  {
> -- 
> 2.5.5
> 

Thanks,
-Christoffer

^ permalink raw reply

* [PATCH 1/3] ARM: dts: rockchip: Move cros-ec-sbs to rk3288-veyron-chromebook-sbs
From: Heiko Stuebner @ 2017-04-30 20:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170430183054.24563-1-contact@paulk.fr>

Hi Paul,

Am Sonntag, 30. April 2017, 20:30:52 CEST schrieb Paul Kocialkowski:
> This moves the cros-ec-sbs dtsi to a new rk3288-veyron-chromebook-sbs
> dtsi since it only concerns rk3288 veyron Chromebooks.
> 
> Other Chromebooks (such as the tegra124 nyans) also have sbs batteries
> and don't use this dtsi, that only makes sense when used with
> rk3288-veyron-chromebook anyway.

That isn't true. The gru series (rk3399-based) also uses the
sbs-battery [0]. And while it is currently limited to Rockchip-based
Chromebooks it is nevertheless used on more than one platform, so
the probability is high that it will be used in future series as well.

Also, it might be nice to also include some Chromeos people (there should
be some in the git logs, like Brian who submitted the Gru patches), as they
might be able to provide more detailed input.


Heiko

[0] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi#n886

> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  .../boot/dts/{cros-ec-sbs.dtsi => rk3288-veyron-chromebook-sbs.dtsi}    | 0
>  arch/arm/boot/dts/rk3288-veyron-jaq.dts                                 | 2 +-
>  arch/arm/boot/dts/rk3288-veyron-jerry.dts                               | 2 +-
>  arch/arm/boot/dts/rk3288-veyron-pinky.dts                               | 2 +-
>  arch/arm/boot/dts/rk3288-veyron-speedy.dts                              | 2 +-
>  5 files changed, 4 insertions(+), 4 deletions(-)
>  rename arch/arm/boot/dts/{cros-ec-sbs.dtsi => rk3288-veyron-chromebook-sbs.dtsi} (100%)
> 
> diff --git a/arch/arm/boot/dts/cros-ec-sbs.dtsi b/arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
> similarity index 100%
> rename from arch/arm/boot/dts/cros-ec-sbs.dtsi
> rename to arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
> diff --git a/arch/arm/boot/dts/rk3288-veyron-jaq.dts b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> index d33f5763c39c..f217a978e47a 100644
> --- a/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> +++ b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> @@ -45,7 +45,7 @@
>  /dts-v1/;
>  
>  #include "rk3288-veyron-chromebook.dtsi"
> -#include "cros-ec-sbs.dtsi"
> +#include "rk3288-veyron-chromebook-sbs.dtsi"
>  
>  / {
>  	model = "Google Jaq";
> diff --git a/arch/arm/boot/dts/rk3288-veyron-jerry.dts b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
> index cdea751f2a8c..bec607574165 100644
> --- a/arch/arm/boot/dts/rk3288-veyron-jerry.dts
> +++ b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
> @@ -44,7 +44,7 @@
>  
>  /dts-v1/;
>  #include "rk3288-veyron-chromebook.dtsi"
> -#include "cros-ec-sbs.dtsi"
> +#include "rk3288-veyron-chromebook-sbs.dtsi"
>  
>  / {
>  	model = "Google Jerry";
> diff --git a/arch/arm/boot/dts/rk3288-veyron-pinky.dts b/arch/arm/boot/dts/rk3288-veyron-pinky.dts
> index 995cff42fa43..c81ad5bf1121 100644
> --- a/arch/arm/boot/dts/rk3288-veyron-pinky.dts
> +++ b/arch/arm/boot/dts/rk3288-veyron-pinky.dts
> @@ -44,7 +44,7 @@
>  
>  /dts-v1/;
>  #include "rk3288-veyron-chromebook.dtsi"
> -#include "cros-ec-sbs.dtsi"
> +#include "rk3288-veyron-chromebook-sbs.dtsi"
>  
>  / {
>  	model = "Google Pinky";
> diff --git a/arch/arm/boot/dts/rk3288-veyron-speedy.dts b/arch/arm/boot/dts/rk3288-veyron-speedy.dts
> index cc0b78cefe34..8aea9c3ff6e2 100644
> --- a/arch/arm/boot/dts/rk3288-veyron-speedy.dts
> +++ b/arch/arm/boot/dts/rk3288-veyron-speedy.dts
> @@ -44,7 +44,7 @@
>  
>  /dts-v1/;
>  #include "rk3288-veyron-chromebook.dtsi"
> -#include "cros-ec-sbs.dtsi"
> +#include "rk3288-veyron-chromebook-sbs.dtsi"
>  
>  / {
>  	model = "Google Speedy";
> 

^ permalink raw reply

* [PATCH v5 20/22] KVM: arm64: vgic-its: Device table save/restore
From: Christoffer Dall @ 2017-04-30 20:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492164934-988-21-git-send-email-eric.auger@redhat.com>

On Fri, Apr 14, 2017 at 12:15:32PM +0200, Eric Auger wrote:
> This patch saves the device table entries into guest RAM.
> Both flat table and 2 stage tables are supported. DeviceId
> indexing is used.
> 
> For each device listed in the device table, we also save
> the translation table using the vgic_its_save/restore_itt
> routines.
> 
> On restore, devices are re-allocated and their ite are
> re-built.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> v4 -> v5:
> - sort the device list by deviceid on device table save
> - use defines for shifts and masks
> - use abi->dte_esz
> - clatify entry sizes for L1 and L2 tables
> 
> v3 -> v4:
> - use the new proto for its_alloc_device
> - compute_next_devid_offset, vgic_its_flush/restore_itt
>   become static in this patch
> - change in the DTE entry format with the introduction of the
>   valid bit and next field width decrease; ittaddr encoded
>   on its full range
> - fix handle_l1_entry entry handling
> - correct vgic_its_table_restore error handling
> 
> v2 -> v3:
> - fix itt_addr bitmask in vgic_its_restore_dte
> - addition of return 0 in vgic_its_restore_ite moved to
>   the ITE related patch
> 
> v1 -> v2:
> - use 8 byte format for DTE and ITE
> - support 2 stage format
> - remove kvm parameter
> - ITT flush/restore moved in a separate patch
> - use deviceid indexing
> ---
>  virt/kvm/arm/vgic/vgic-its.c | 183 +++++++++++++++++++++++++++++++++++++++++--
>  virt/kvm/arm/vgic/vgic.h     |   7 ++
>  2 files changed, 185 insertions(+), 5 deletions(-)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index b02fc3f..86dfc6c 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -1682,7 +1682,8 @@ int vgic_its_attr_regs_access(struct kvm_device *dev,
>  	return ret;
>  }
>  
> -u32 compute_next_devid_offset(struct list_head *h, struct its_device *dev)
> +static u32 compute_next_devid_offset(struct list_head *h,
> +				     struct its_device *dev)
>  {
>  	struct list_head *e = &dev->dev_list;
>  	struct its_device *next;
> @@ -1858,7 +1859,7 @@ static int vgic_its_ite_cmp(void *priv, struct list_head *a,
>  		return 1;
>  }
>  
> -int vgic_its_save_itt(struct vgic_its *its, struct its_device *device)
> +static int vgic_its_save_itt(struct vgic_its *its, struct its_device *device)
>  {
>  	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
>  	gpa_t base = device->itt_addr;
> @@ -1877,7 +1878,7 @@ int vgic_its_save_itt(struct vgic_its *its, struct its_device *device)
>  	return 0;
>  }
>  
> -int vgic_its_restore_itt(struct vgic_its *its, struct its_device *dev)
> +static int vgic_its_restore_itt(struct vgic_its *its, struct its_device *dev)
>  {
>  	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
>  	gpa_t base = dev->itt_addr;
> @@ -1895,12 +1896,161 @@ int vgic_its_restore_itt(struct vgic_its *its, struct its_device *dev)
>  }
>  
>  /**
> + * vgic_its_save_dte - Save a device table entry at a given GPA
> + *
> + * @its: ITS handle
> + * @dev: ITS device
> + * @ptr: GPA
> + */
> +static int vgic_its_save_dte(struct vgic_its *its, struct its_device *dev,
> +			     gpa_t ptr, int dte_esz)
> +{
> +	struct kvm *kvm = its->dev->kvm;
> +	u64 val, itt_addr_field;
> +	int ret;
> +	u32 next_offset;
> +
> +	itt_addr_field = dev->itt_addr >> 8;
> +	next_offset = compute_next_devid_offset(&its->device_list, dev);
> +	val = (1ULL << KVM_ITS_DTE_VALID_SHIFT |
> +	       ((u64)next_offset << KVM_ITS_DTE_NEXT_SHIFT) |

I think this implies that the next field in your ABI points to the next
offset, regardless of whether or not this is in a a level 2 or lavel 1
table.  See more comments on this below (I reviewed this patch from the
bottom up).

I have a feeling this wasn't tested with 2 level device tables.  Could
that be true?

> +	       (itt_addr_field << KVM_ITS_DTE_ITTADDR_SHIFT) |
> +		(dev->nb_eventid_bits - 1));
> +	val = cpu_to_le64(val);
> +	ret = kvm_write_guest(kvm, ptr, &val, dte_esz);
> +	return ret;
> +}
> +
> +/**
> + * vgic_its_restore_dte - restore a device table entry
> + *
> + * @its: its handle
> + * @id: device id the DTE corresponds to
> + * @ptr: kernel VA where the 8 byte DTE is located
> + * @opaque: unused
> + * @next: offset to the next valid device id
> + *
> + * Return: < 0 on error, 0 otherwise
> + */
> +static int vgic_its_restore_dte(struct vgic_its *its, u32 id,
> +				void *ptr, void *opaque, u32 *next)
> +{
> +	struct its_device *dev;
> +	gpa_t itt_addr;
> +	u8 nb_eventid_bits;
> +	u64 entry = *(u64 *)ptr;
> +	bool valid;
> +	int ret;
> +
> +	entry = le64_to_cpu(entry);
> +
> +	valid = entry >> KVM_ITS_DTE_VALID_SHIFT;
> +	nb_eventid_bits = (entry & KVM_ITS_DTE_SIZE_MASK) + 1;
> +	itt_addr = ((entry & KVM_ITS_DTE_ITTADDR_MASK)
> +			>> KVM_ITS_DTE_ITTADDR_SHIFT) << 8;
> +	*next = 1;
> +
> +	if (!valid)
> +		return 0;
> +
> +	/* dte entry is valid */
> +	*next = (entry & KVM_ITS_DTE_NEXT_MASK) >> KVM_ITS_DTE_NEXT_SHIFT;
> +
> +	ret = vgic_its_alloc_device(its, &dev, id,
> +				    itt_addr, nb_eventid_bits);
> +	if (ret)
> +		return ret;
> +	ret = vgic_its_restore_itt(its, dev);
> +
> +	return ret;
> +}
> +
> +static int vgic_its_device_cmp(void *priv, struct list_head *a,
> +			       struct list_head *b)
> +{
> +	struct its_device *deva = container_of(a, struct its_device, dev_list);
> +	struct its_device *devb = container_of(b, struct its_device, dev_list);
> +
> +	if (deva->device_id < devb->device_id)
> +		return -1;
> +	else
> +		return 1;
> +}
> +
> +/**
>   * vgic_its_save_device_tables - Save the device table and all ITT
>   * into guest RAM
> + *
> + * L1/L2 handling is hidden by vgic_its_check_id() helper which directly
> + * returns the GPA of the device entry
>   */
>  static int vgic_its_save_device_tables(struct vgic_its *its)
>  {
> -	return -ENXIO;
> +	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
> +	struct its_device *dev;
> +	int dte_esz = abi->dte_esz;
> +	u64 baser;
> +
> +	baser = its->baser_device_table;
> +
> +	list_sort(NULL, &its->device_list, vgic_its_device_cmp);
> +
> +	list_for_each_entry(dev, &its->device_list, dev_list) {
> +		int ret;
> +		gpa_t eaddr;
> +
> +		if (!vgic_its_check_id(its, baser,
> +				       dev->device_id, &eaddr))
> +			return -EINVAL;
> +
> +		ret = vgic_its_save_itt(its, dev);
> +		if (ret)
> +			return ret;
> +
> +		ret = vgic_its_save_dte(its, dev, eaddr, dte_esz);
> +		if (ret)
> +			return ret;
> +	}
> +	return 0;
> +}
> +
> +/**
> + * handle_l1_entry - callback used for L1 entries (2 stage case)
> + *
> + * @its: its handle
> + * @id: id

IIUC, this is actually the index of the entry in the L1 table.  I think
this should be clarified.

> + * @addr: kernel VA
> + * @opaque: unused
> + * @next_offset: offset to the next L1 entry: 0 if the last element
> + * was found, 1 otherwise
> + */
> +static int handle_l1_entry(struct vgic_its *its, u32 id, void *addr,
> +			   void *opaque, u32 *next_offset)

nit: shouldn't this be called handle_l1_device_table_entry ?

> +{
> +	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
> +	int l2_start_id = id * (SZ_64K / GITS_LVL1_ENTRY_SIZE);

Hmmm, is this not actually supposed to be (SZ_64K / abi->dte_esz) ?

> +	u64 entry = *(u64 *)addr;
> +	int ret, ite_esz = abi->ite_esz;

Should this be ite_esz or dte_esz?

> +	gpa_t gpa;

nit: put declarations with initialization on separate lines.

> +
> +	entry = le64_to_cpu(entry);
> +	*next_offset = 1;

I think you could attach a comment here saying that level 1 tables have
to be scanned entirely.

But this also reminds me.  Does that mean that the next field in the DTE
in your documented ABI format points to the next DTE within that level-2
table, or does it point across to different level-2 tables?  I think
this needs to be clarified in the ABI unless I'm missing something.

> +
> +	if (!(entry & BIT_ULL(63)))
> +		return 0;
> +
> +	gpa = entry & GENMASK_ULL(51, 16);

Can you define the bit fields for the level-1 entries as well please?

> +
> +	ret = lookup_table(its, gpa, SZ_64K, ite_esz,
> +			   l2_start_id, vgic_its_restore_dte, NULL);
> +
> +	if (ret == 1) {
> +		/* last entry was found in this L2 table */

maybe you should define these return codes for you table scan function,
and you wouldn't have to have a separate comment and it would be
generally easier to follow the code.

> +		*next_offset = 0;
> +		ret = 0;
> +	}
> +
> +	return ret;
>  }
>  
>  /**
> @@ -1909,7 +2059,30 @@ static int vgic_its_save_device_tables(struct vgic_its *its)
>   */
>  static int vgic_its_restore_device_tables(struct vgic_its *its)
>  {
> -	return -ENXIO;
> +	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
> +	u64 baser = its->baser_device_table;
> +	int l1_esz, ret, l1_tbl_size = GITS_BASER_NR_PAGES(baser) * SZ_64K;

nit: put this initialization on a separate line.

> +	gpa_t l1_gpa;
> +
> +	l1_gpa = BASER_ADDRESS(baser);
> +	if (!l1_gpa)
> +		return 0;

I think you meant to check the valid bit here.

> +
> +	if (baser & GITS_BASER_INDIRECT) {
> +		l1_esz = 8;
> +		ret = lookup_table(its, l1_gpa, l1_tbl_size, l1_esz, 0,
> +				   handle_l1_entry, NULL);
> +	} else {
> +		l1_esz = abi->dte_esz;
> +		ret = lookup_table(its, l1_gpa, l1_tbl_size, l1_esz, 0,
> +				   vgic_its_restore_dte, NULL);
> +	}
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	/* if last element was not found we have an issue here */

same comment as other patch

> +	return ret ? 0 : -EINVAL;
>  }
>  
>  static int vgic_its_save_cte(struct vgic_its *its,
> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
> index ce57fbd..9bc52ef 100644
> --- a/virt/kvm/arm/vgic/vgic.h
> +++ b/virt/kvm/arm/vgic/vgic.h
> @@ -85,6 +85,13 @@
>  #define KVM_ITS_ITE_PINTID_SHIFT	16
>  #define KVM_ITS_ITE_PINTID_MASK		GENMASK_ULL(47, 16)
>  #define KVM_ITS_ITE_ICID_MASK		GENMASK_ULL(15, 0)
> +#define KVM_ITS_DTE_VALID_SHIFT		63
> +#define KVM_ITS_DTE_VALID_MASK		BIT_ULL(63)
> +#define KVM_ITS_DTE_NEXT_SHIFT		49
> +#define KVM_ITS_DTE_NEXT_MASK		GENMASK_ULL(62, 49)
> +#define KVM_ITS_DTE_ITTADDR_SHIFT	5
> +#define KVM_ITS_DTE_ITTADDR_MASK	GENMASK_ULL(48, 5)
> +#define KVM_ITS_DTE_SIZE_MASK		GENMASK_ULL(4, 0)
>  
>  static inline bool irq_is_pending(struct vgic_irq *irq)
>  {
> -- 
> 2.5.5
> 
Thanks,
-Christoffer

^ permalink raw reply

* [PATCH 1/3] ARM: dts: rockchip: Move cros-ec-sbs to rk3288-veyron-chromebook-sbs
From: Paul Kocialkowski @ 2017-04-30 20:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2369975.a4dWayqU5d@phil>

Le dimanche 30 avril 2017 ? 22:37 +0200, Heiko Stuebner a ?crit?:
> Hi Paul,
> 
> Am Sonntag, 30. April 2017, 20:30:52 CEST schrieb Paul Kocialkowski:
> > This moves the cros-ec-sbs dtsi to a new rk3288-veyron-chromebook-sbs
> > dtsi since it only concerns rk3288 veyron Chromebooks.
> > 
> > Other Chromebooks (such as the tegra124 nyans) also have sbs batteries
> > and don't use this dtsi, that only makes sense when used with
> > rk3288-veyron-chromebook anyway.
> 
> That isn't true. The gru series (rk3399-based) also uses the
> sbs-battery [0]. And while it is currently limited to Rockchip-based
> Chromebooks it is nevertheless used on more than one platform, so
> the probability is high that it will be used in future series as well.

That's good to know, but as pointed out, other cros devices are using a sbs
battery without this header, so such a generic name isn't really a good fit.

Note that &charger has to be defined (after my subsequent patches), which it is
for devices that also include rk3288-veyron-chromebook, but not necessarily
others.

Overall, I think having one -sbs dtsi file makes sense here because there is
already a rk3288-veyron-chromebook dtsi that veyron chromebooks use. That file
cannot contain the battery bindings because minnie has a different one and it
would be a bit silly to copy it over all devices. That definitely makes sense.

As for other devices, I don't see why we should have a separate include file for
the battery instead of having it in the device's dts.?I think this should be the
case on gru/kevin.

Also maybe not *all* gru-based devices will turn out to use a SBS battery, so it
seems early to include this header in the gru dtsi. One last point, gru/kevin
currently don't define a charger, which will break my subsequent patch (that is
however needed for the veyrons that use this file).

To me, it seems that there's little advantage and major drawbacks in keeping
this file the way it is.

> Also, it might be nice to also include some Chromeos people (there should
> be some in the git logs, like Brian who submitted the Gru patches), as they
> might be able to provide more detailed input.

That's a good point, thanks for including them.

> 
> Heiko
> 
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/a
> rch/arm64/boot/dts/rockchip/rk3399-gru.dtsi#n886
> 
> > 
> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > ---
> > ?.../boot/dts/{cros-ec-sbs.dtsi => rk3288-veyron-chromebook-sbs.dtsi}????| 0
> > ?arch/arm/boot/dts/rk3288-veyron-jaq.dts?????????????????????????????????| 2
> > +-
> > ?arch/arm/boot/dts/rk3288-veyron-jerry.dts???????????????????????????????| 2
> > +-
> > ?arch/arm/boot/dts/rk3288-veyron-pinky.dts???????????????????????????????| 2
> > +-
> > ?arch/arm/boot/dts/rk3288-veyron-speedy.dts??????????????????????????????| 2
> > +-
> > ?5 files changed, 4 insertions(+), 4 deletions(-)
> > ?rename arch/arm/boot/dts/{cros-ec-sbs.dtsi => rk3288-veyron-chromebook-
> > sbs.dtsi} (100%)
> > 
> > diff --git a/arch/arm/boot/dts/cros-ec-sbs.dtsi b/arch/arm/boot/dts/rk3288-
> > veyron-chromebook-sbs.dtsi
> > similarity index 100%
> > rename from arch/arm/boot/dts/cros-ec-sbs.dtsi
> > rename to arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
> > diff --git a/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> > b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> > index d33f5763c39c..f217a978e47a 100644
> > --- a/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> > +++ b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> > @@ -45,7 +45,7 @@
> > ?/dts-v1/;
> > ?
> > ?#include "rk3288-veyron-chromebook.dtsi"
> > -#include "cros-ec-sbs.dtsi"
> > +#include "rk3288-veyron-chromebook-sbs.dtsi"
> > ?
> > ?/ {
> > ?	model = "Google Jaq";
> > diff --git a/arch/arm/boot/dts/rk3288-veyron-jerry.dts
> > b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
> > index cdea751f2a8c..bec607574165 100644
> > --- a/arch/arm/boot/dts/rk3288-veyron-jerry.dts
> > +++ b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
> > @@ -44,7 +44,7 @@
> > ?
> > ?/dts-v1/;
> > ?#include "rk3288-veyron-chromebook.dtsi"
> > -#include "cros-ec-sbs.dtsi"
> > +#include "rk3288-veyron-chromebook-sbs.dtsi"
> > ?
> > ?/ {
> > ?	model = "Google Jerry";
> > diff --git a/arch/arm/boot/dts/rk3288-veyron-pinky.dts
> > b/arch/arm/boot/dts/rk3288-veyron-pinky.dts
> > index 995cff42fa43..c81ad5bf1121 100644
> > --- a/arch/arm/boot/dts/rk3288-veyron-pinky.dts
> > +++ b/arch/arm/boot/dts/rk3288-veyron-pinky.dts
> > @@ -44,7 +44,7 @@
> > ?
> > ?/dts-v1/;
> > ?#include "rk3288-veyron-chromebook.dtsi"
> > -#include "cros-ec-sbs.dtsi"
> > +#include "rk3288-veyron-chromebook-sbs.dtsi"
> > ?
> > ?/ {
> > ?	model = "Google Pinky";
> > diff --git a/arch/arm/boot/dts/rk3288-veyron-speedy.dts
> > b/arch/arm/boot/dts/rk3288-veyron-speedy.dts
> > index cc0b78cefe34..8aea9c3ff6e2 100644
> > --- a/arch/arm/boot/dts/rk3288-veyron-speedy.dts
> > +++ b/arch/arm/boot/dts/rk3288-veyron-speedy.dts
> > @@ -44,7 +44,7 @@
> > ?
> > ?/dts-v1/;
> > ?#include "rk3288-veyron-chromebook.dtsi"
> > -#include "cros-ec-sbs.dtsi"
> > +#include "rk3288-veyron-chromebook-sbs.dtsi"
> > ?
> > ?/ {
> > ?	model = "Google Speedy";
> > 
> 
> 
-- 
Paul Kocialkowski, developer of free digital technology and hardware support

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170430/3d522370/attachment-0001.sig>

^ permalink raw reply

* [PATCH v5 21/22] KVM: arm64: vgic-its: Fix pending table sync
From: Christoffer Dall @ 2017-04-30 21:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492164934-988-22-git-send-email-eric.auger@redhat.com>

On Fri, Apr 14, 2017 at 12:15:33PM +0200, Eric Auger wrote:
> In its_sync_lpi_pending_table() we currently ignore the
> target_vcpu of the LPIs. We sync the pending bit found in
> the vcpu pending table even if the LPI is not targeting it.
> 
> Also in vgic_its_cmd_handle_invall() we are supposed to
> read the config table data for the LPIs associated to the
> collection ID. At the moment we refresh all LPI config
> information.
> 
> This patch passes a vpcu to vgic_copy_lpi_list() so that
> this latter returns a snapshot of the LPIs targeting this
> CPU and only those.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  virt/kvm/arm/vgic/vgic-its.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index 86dfc6c..be848be 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -252,13 +252,13 @@ static int update_lpi_config(struct kvm *kvm, struct vgic_irq *irq,
>  }
>  
>  /*
> - * Create a snapshot of the current LPI list, so that we can enumerate all
> - * LPIs without holding any lock.
> - * Returns the array length and puts the kmalloc'ed array into intid_ptr.
> + * Create a snapshot of the current LPIs targeting @vcpu, so that we can
> + * enumerate those LPIs without holding any lock.
> + * Returns their number and puts the kmalloc'ed array into intid_ptr.
>   */
> -static int vgic_copy_lpi_list(struct kvm *kvm, u32 **intid_ptr)
> +static int vgic_copy_lpi_list(struct kvm_vcpu *vcpu, u32 **intid_ptr)
>  {
> -	struct vgic_dist *dist = &kvm->arch.vgic;
> +	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
>  	struct vgic_irq *irq;
>  	u32 *intids;
>  	int irq_count = dist->lpi_list_count, i = 0;
> @@ -277,14 +277,14 @@ static int vgic_copy_lpi_list(struct kvm *kvm, u32 **intid_ptr)
>  	spin_lock(&dist->lpi_list_lock);
>  	list_for_each_entry(irq, &dist->lpi_list_head, lpi_list) {
>  		/* We don't need to "get" the IRQ, as we hold the list lock. */
> -		intids[i] = irq->intid;
> -		if (++i == irq_count)
> -			break;
> +		if (irq->target_vcpu != vcpu)
> +			continue;
> +		intids[i++] = irq->intid;

were we checking the ++i == irq_count condition for no good reason
before since we can just drop it now?

>  	}
>  	spin_unlock(&dist->lpi_list_lock);
>  
>  	*intid_ptr = intids;
> -	return irq_count;
> +	return i;
>  }
>  
>  /*
> @@ -333,7 +333,7 @@ static u32 max_lpis_propbaser(u64 propbaser)
>  }
>  
>  /*
> - * Scan the whole LPI pending table and sync the pending bit in there
> + * Sync the pending table pending bit of LPIs targeting @vcpu
>   * with our own data structures. This relies on the LPI being
>   * mapped before.
>   */
> @@ -346,7 +346,7 @@ static int its_sync_lpi_pending_table(struct kvm_vcpu *vcpu)
>  	u32 *intids;
>  	int nr_irqs, i;
>  
> -	nr_irqs = vgic_copy_lpi_list(vcpu->kvm, &intids);
> +	nr_irqs = vgic_copy_lpi_list(vcpu, &intids);
>  	if (nr_irqs < 0)
>  		return nr_irqs;
>  
> @@ -1027,7 +1027,7 @@ static int vgic_its_cmd_handle_invall(struct kvm *kvm, struct vgic_its *its,
>  
>  	vcpu = kvm_get_vcpu(kvm, collection->target_addr);
>  
> -	irq_count = vgic_copy_lpi_list(kvm, &intids);
> +	irq_count = vgic_copy_lpi_list(vcpu, &intids);
>  	if (irq_count < 0)
>  		return irq_count;
>  
> -- 
> 2.5.5
> 

Assuming that it's ok to remove the irq_count check above, the rest of
this patch looks good to me.

Thanks,
-Christoffer

^ permalink raw reply

* [PATCH v5 22/22] KVM: arm64: vgic-v3: KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES
From: Christoffer Dall @ 2017-04-30 21:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492164934-988-23-git-send-email-eric.auger@redhat.com>

On Fri, Apr 14, 2017 at 12:15:34PM +0200, Eric Auger wrote:
> This patch adds a new attribute to GICV3 KVM device
> KVM_DEV_ARM_VGIC_GRP_CTRL group. This Allows the userspace to

nit: allows (lowercase)
nit: s/the userspace/userspace/

> flush all GICR pending tables into guest RAM. At the moment
> we do not offer any restore control as the sync is implicit.

I would probably remove the last sentence here.

> 
> As we need the PENDBASER_ADDRESS() in vgic-v3, let's move its
> definition in the irqchip header. We restore the full length
> of the field, ie [51:16]. Same for PROPBASER_ADDRESS with full
> field length of [51:12].
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> v4 -> v5:
> - move pending table save code/ctrl into VGICv3 instead of ITS
> - remove useless gpa_t cast
> - use the same optimization as in its_sync_lpi_pending_table
> 
> v3 -> v4:
> - remove the wrong comment about locking
> - pass kvm struct instead of its handle
> - add comment about restore method
> - remove GITR_PENDABASER.PTZ check
> - continue if target_vcpu == NULL
> - new locking strategy
> 
> v1 -> v2:
> - do not care about the 1st KB which should be zeroed according to
>   the spec.
> ---
>  arch/arm/include/uapi/asm/kvm.h     |  1 +
>  arch/arm64/include/uapi/asm/kvm.h   |  1 +
>  include/linux/irqchip/arm-gic-v3.h  |  2 ++
>  virt/kvm/arm/vgic/vgic-its.c        |  6 ++---
>  virt/kvm/arm/vgic/vgic-kvm-device.c | 20 ++++++++++++++
>  virt/kvm/arm/vgic/vgic-v3.c         | 54 +++++++++++++++++++++++++++++++++++++
>  virt/kvm/arm/vgic/vgic.h            |  1 +
>  7 files changed, 81 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index 8e6563c..78fe803 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -202,6 +202,7 @@ struct kvm_arch_memory_slot {
>  #define   KVM_DEV_ARM_VGIC_CTRL_INIT		0
>  #define   KVM_DEV_ARM_ITS_SAVE_TABLES		1
>  #define   KVM_DEV_ARM_ITS_RESTORE_TABLES	2
> +#define   KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES	3
>  
>  /* KVM_IRQ_LINE irq field index values */
>  #define KVM_ARM_IRQ_TYPE_SHIFT		24
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 1e35115..8a3758a 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -222,6 +222,7 @@ struct kvm_arch_memory_slot {
>  #define   KVM_DEV_ARM_VGIC_CTRL_INIT		0
>  #define   KVM_DEV_ARM_ITS_SAVE_TABLES           1
>  #define   KVM_DEV_ARM_ITS_RESTORE_TABLES        2
> +#define   KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES	3
>  
>  /* Device Control API on vcpu fd */
>  #define KVM_ARM_VCPU_PMU_V3_CTRL	0
> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
> index 0c6798c..9d3932f 100644
> --- a/include/linux/irqchip/arm-gic-v3.h
> +++ b/include/linux/irqchip/arm-gic-v3.h
> @@ -158,6 +158,7 @@
>  #define GICR_PROPBASER_RaWaWb	GIC_BASER_CACHEABILITY(GICR_PROPBASER, INNER, RaWaWb)
>  
>  #define GICR_PROPBASER_IDBITS_MASK			(0x1f)
> +#define GICR_PROPBASER_ADDRESS(x)    ((x) & GENMASK_ULL(51, 12))
>  
>  #define GICR_PENDBASER_SHAREABILITY_SHIFT		(10)
>  #define GICR_PENDBASER_INNER_CACHEABILITY_SHIFT		(7)
> @@ -183,6 +184,7 @@
>  #define GICR_PENDBASER_RaWaWb	GIC_BASER_CACHEABILITY(GICR_PENDBASER, INNER, RaWaWb)
>  
>  #define GICR_PENDBASER_PTZ				BIT_ULL(62)
> +#define GICR_PENDBASER_ADDRESS(x)    ((x) & GENMASK_ULL(51, 16))
>  
>  /*
>   * Re-Distributor registers, offsets from SGI_base
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index be848be..745c245 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -189,8 +189,6 @@ static struct its_ite *find_ite(struct vgic_its *its, u32 device_id,
>   */
>  #define BASER_ADDRESS(x)	((x) & GENMASK_ULL(47, 16))
>  #define CBASER_ADDRESS(x)	((x) & GENMASK_ULL(47, 12))
> -#define PENDBASER_ADDRESS(x)	((x) & GENMASK_ULL(47, 16))
> -#define PROPBASER_ADDRESS(x)	((x) & GENMASK_ULL(47, 12))
>  
>  #define GIC_LPI_OFFSET 8192
>  
> @@ -227,7 +225,7 @@ static struct its_collection *find_collection(struct vgic_its *its, int coll_id)
>  static int update_lpi_config(struct kvm *kvm, struct vgic_irq *irq,
>  			     struct kvm_vcpu *filter_vcpu)
>  {
> -	u64 propbase = PROPBASER_ADDRESS(kvm->arch.vgic.propbaser);
> +	u64 propbase = GICR_PROPBASER_ADDRESS(kvm->arch.vgic.propbaser);
>  	u8 prop;
>  	int ret;
>  
> @@ -339,7 +337,7 @@ static u32 max_lpis_propbaser(u64 propbaser)
>   */
>  static int its_sync_lpi_pending_table(struct kvm_vcpu *vcpu)
>  {
> -	gpa_t pendbase = PENDBASER_ADDRESS(vcpu->arch.vgic_cpu.pendbaser);
> +	gpa_t pendbase = GICR_PENDBASER_ADDRESS(vcpu->arch.vgic_cpu.pendbaser);
>  	struct vgic_irq *irq;
>  	int last_byte_offset = -1;
>  	int ret = 0;
> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
> index 859bfa8..d48743c 100644
> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
> @@ -580,6 +580,24 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
>  		reg = tmp32;
>  		return vgic_v3_attr_regs_access(dev, attr, &reg, true);
>  	}
> +	case KVM_DEV_ARM_VGIC_GRP_CTRL: {
> +		int ret;
> +
> +		switch (attr->attr) {
> +		case KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES:
> +			mutex_lock(&dev->kvm->lock);
> +
> +			if (!lock_all_vcpus(dev->kvm)) {
> +				mutex_unlock(&dev->kvm->lock);
> +				return -EBUSY;
> +			}
> +			ret = vgic_v3_save_pending_tables(dev->kvm);
> +			unlock_all_vcpus(dev->kvm);
> +			mutex_unlock(&dev->kvm->lock);
> +			return ret;
> +		}
> +		break;
> +	}
>  	}
>  	return -ENXIO;
>  }
> @@ -658,6 +676,8 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
>  		switch (attr->attr) {
>  		case KVM_DEV_ARM_VGIC_CTRL_INIT:
>  			return 0;
> +		case KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES:
> +			return 0;
>  		}
>  	}
>  	return -ENXIO;
> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
> index be0f4c3..1f0977f 100644
> --- a/virt/kvm/arm/vgic/vgic-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-v3.c
> @@ -15,6 +15,7 @@
>  #include <linux/irqchip/arm-gic-v3.h>
>  #include <linux/kvm.h>
>  #include <linux/kvm_host.h>
> +//#include <linux/bitops.h>
>  #include <kvm/arm_vgic.h>
>  #include <asm/kvm_mmu.h>
>  #include <asm/kvm_asm.h>
> @@ -252,6 +253,59 @@ void vgic_v3_enable(struct kvm_vcpu *vcpu)
>  	vgic_v3->vgic_hcr = ICH_HCR_EN;
>  }
>  
> +/**
> + * vgic_its_save_pending_tables - Save the pending tables into guest RAM
> + * kvm lock and all vcpu lock must be held
> + */
> +int vgic_v3_save_pending_tables(struct kvm *kvm)
> +{
> +	struct vgic_dist *dist = &kvm->arch.vgic;
> +	int last_byte_offset = -1;
> +	struct vgic_irq *irq;
> +	int ret;
> +
> +	list_for_each_entry(irq, &dist->lpi_list_head, lpi_list) {
> +		int byte_offset, bit_nr;
> +		struct kvm_vcpu *vcpu;
> +		gpa_t pendbase, ptr;
> +		bool stored;
> +		u8 val;
> +
> +		vcpu = irq->target_vcpu;
> +		if (!vcpu)
> +			continue;
> +
> +		pendbase = GICR_PENDBASER_ADDRESS(vcpu->arch.vgic_cpu.pendbaser);
> +
> +		byte_offset = irq->intid / BITS_PER_BYTE;
> +		bit_nr = irq->intid % BITS_PER_BYTE;
> +		ptr = pendbase + byte_offset;
> +
> +		if (byte_offset != last_byte_offset) {
> +			ret = kvm_read_guest(kvm, ptr, &val, 1);
> +			if (ret)
> +				return ret;
> +			last_byte_offset = byte_offset;
> +		}
> +
> +		stored = val & bit_nr;

didn't you mean 'stored = val & (1 << bit_nr)' ?

> +		if (stored == irq->pending_latch)
> +			continue;
> +
> +		if (irq->pending_latch)
> +			val |= 1 << bit_nr;
> +		else
> +			val &= ~(1 << bit_nr);
> +
> +		ret = kvm_write_guest(kvm, ptr, &val, 1);
> +		if (ret)
> +			return ret;
> +	}

This loop could probably be written a bit more efficiently and
simplicity by reading the memory one word at a time (and remembering to
do le64_to_cpu) and then doing something like:

		if (irq->pending_latch)
			old = __test_and_set_bit(bit_nr, &val);
		else
			old = __test_and_clear_bit(bit_nr, &val);

		if (old != val) {
			tmp = cpu_to_le64(val);
			ret = kvm_write_guest(kvm, ptr, &tmp,
					      sizeof(unsigned long));
			if (ret)
				retur ret;
		}

Further, you could also detect when the word_offset changes and write
back the entire word with all its changes then, but you'd also have to
check at the end of the loop then.  Not sure that's worth the
optimization or easier to read than what you heave.  It's up to you.

Thanks,
-Christoffer

> +
> +	return 0;
> +}
> +
> +
>  /* check for overlapping regions and for regions crossing the end of memory */
>  static bool vgic_v3_check_base(struct kvm *kvm)
>  {
> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
> index 9bc52ef..535c2fc 100644
> --- a/virt/kvm/arm/vgic/vgic.h
> +++ b/virt/kvm/arm/vgic/vgic.h
> @@ -177,6 +177,7 @@ void vgic_v3_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
>  void vgic_v3_enable(struct kvm_vcpu *vcpu);
>  int vgic_v3_probe(const struct gic_kvm_info *info);
>  int vgic_v3_map_resources(struct kvm *kvm);
> +int vgic_v3_save_pending_tables(struct kvm *kvm);
>  int vgic_register_redist_iodevs(struct kvm *kvm, gpa_t dist_base_address);
>  
>  int vgic_register_its_iodevs(struct kvm *kvm);
> -- 
> 2.5.5
> 

^ permalink raw reply

* [PATCH] scsi: fas216: Add __printf validation, fix fallout
From: Joe Perches @ 2017-04-30 21:50 UTC (permalink / raw)
  To: linux-arm-kernel

__printf makes the compiler check format and arguments.

Fix fallout.

Miscellanea:

o Convert formats to const char *
o Use vsprintf extension %pV instead of a static buffer.
o Add newline to logging and remove now unnecessary printk("\n")
o Use pr_cont where appropriate

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/scsi/arm/fas216.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
index 24388795ee9a..112bec886192 100644
--- a/drivers/scsi/arm/fas216.c
+++ b/drivers/scsi/arm/fas216.c
@@ -289,17 +289,20 @@ static char fas216_target(FAS216_Info *info)
 		return 'H';
 }
 
-static void
-fas216_do_log(FAS216_Info *info, char target, char *fmt, va_list ap)
+__printf(3, 0) static void
+fas216_do_log(FAS216_Info *info, char target, const char *fmt, va_list ap)
 {
-	static char buf[1024];
+	struct va_format vaf;
+
+	vaf.fmt = fmt;
+	vaf.va = &ap;
 
-	vsnprintf(buf, sizeof(buf), fmt, ap);
-	printk("scsi%d.%c: %s", info->host->host_no, target, buf);
+	printk("scsi%d.%c: %pV\n", info->host->host_no, target, &vaf);
 }
 
+__printf(4, 5)
 static void fas216_log_command(FAS216_Info *info, int level,
-			       struct scsi_cmnd *SCpnt, char *fmt, ...)
+			       struct scsi_cmnd *SCpnt, const char *fmt, ...)
 {
 	va_list args;
 
@@ -313,8 +316,9 @@ static void fas216_log_command(FAS216_Info *info, int level,
 	scsi_print_command(SCpnt);
 }
 
-static void
-fas216_log_target(FAS216_Info *info, int level, int target, char *fmt, ...)
+__printf(4, 5) static void
+fas216_log_target(FAS216_Info *info, int level, int target,
+		  const char *fmt, ...)
 {
 	va_list args;
 
@@ -329,11 +333,10 @@ fas216_log_target(FAS216_Info *info, int level, int target, char *fmt, ...)
 	va_start(args, fmt);
 	fas216_do_log(info, target, fmt, args);
 	va_end(args);
-
-	printk("\n");
 }
 
-static void fas216_log(FAS216_Info *info, int level, char *fmt, ...)
+__printf(3, 4)
+static void fas216_log(FAS216_Info *info, int level, const char *fmt, ...)
 {
 	va_list args;
 
@@ -343,8 +346,6 @@ static void fas216_log(FAS216_Info *info, int level, char *fmt, ...)
 	va_start(args, fmt);
 	fas216_do_log(info, fas216_target(info), fmt, args);
 	va_end(args);
-
-	printk("\n");
 }
 
 #define PH_SIZE	32
@@ -431,7 +432,7 @@ fas216_get_last_msg(FAS216_Info *info, int pos)
 	}
 
 	fas216_log(info, LOG_MESSAGES,
-		"Message: %04x found at position %02x\n", packed_msg, pos);
+		   "Message: %04x found at position %02x", packed_msg, pos);
 
 	return packed_msg;
 }
@@ -725,7 +726,7 @@ static void fas216_cleanuptransfer(FAS216_Info *info)
 	fifo = fas216_readb(info, REG_CFIS) & CFIS_CF;
 
 	fas216_log(info, LOG_BUFFER, "cleaning up from previous "
-		   "transfer: length 0x%06x, residual 0x%x, fifo %d",
+		   "transfer: length 0x%06lx, residual 0x%lx, fifo %ld",
 		   total, residual, fifo);
 
 	/*
@@ -1144,8 +1145,8 @@ static void fas216_parse_message(FAS216_Info *info, unsigned char *message, int
 	fas216_log(info, 0, "unrecognised message, rejecting");
 	printk("scsi%d.%c: message was", info->host->host_no, fas216_target(info));
 	for (i = 0; i < msglen; i++)
-		printk("%s%02X", i & 31 ? " " : "\n  ", message[i]);
-	printk("\n");
+		pr_cont("%s%02X", i & 31 ? " " : "\n  ", message[i]);
+	pr_cont("\n");
 
 	/*
 	 * Something strange seems to be happening here -
@@ -1582,7 +1583,7 @@ static void fas216_funcdone_intr(FAS216_Info *info, unsigned int stat, unsigned
 	default:
 		fas216_log(info, 0, "internal phase %s for function done?"
 			"  What do I do with this?",
-			fas216_target(info), fas216_drv_phase(info));
+			fas216_drv_phase(info));
 	}
 }
 
@@ -1642,7 +1643,7 @@ irqreturn_t fas216_intr(FAS216_Info *info)
 			fas216_bus_reset(info);
 			scsi_report_bus_reset(info->host, 0);
 		} else if (inst & INST_ILLEGALCMD) {
-			fas216_log(info, LOG_ERROR, "illegal command given\n");
+			fas216_log(info, LOG_ERROR, "illegal command given");
 			fas216_dumpstate(info);
 			print_debug_list();
 		} else if (inst & INST_DISCONNECT)
-- 
2.10.0.rc2.1.g053435c

^ permalink raw reply related

* [patch] autogain support for bayer10 format (was Re: [patch] propagating controls in libv4l2)
From: Pavel Machek @ 2017-04-30 22:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170426132337.GA6482@amd>

On Wed 2017-04-26 15:23:37, Pavel Machek wrote:
> Hi!
> 
> > > > I don't see why it would be hard to open files or have threads inside
> > > > a library. There are several libraries that do that already, specially
> > > > the ones designed to be used on multimidia apps.  
> > > 
> > > Well, This is what the libv4l2 says:
> > > 
> > >    This file implements libv4l2, which offers v4l2_ prefixed versions
> > >    of
> > >       open/close/etc. The API is 100% the same as directly opening
> > >    /dev/videoX
> > >       using regular open/close/etc, the big difference is that format
> > >    conversion
> > >    
> > > but if I open additional files in v4l2_open(), API is no longer the
> > > same, as unix open() is defined to open just one file descriptor.
> > > 
> > > Now. There is autogain support in libv4lconvert, but it expects to use
> > > same fd for camera and for the gain... which does not work with
> > > subdevs.
> > > 
> > > Of course, opening subdevs by name like this is not really
> > > acceptable. But can you suggest a method that is?
> > 
> > There are two separate things here:
> > 
> > 1) Autofoucs for a device that doesn't use subdev API
> > 2) libv4l2 support for devices that require MC and subdev API
> 
> Actually there are three: 0) autogain. Unfortunately, I need autogain
> first before autofocus has a chance...
> 
> And that means... bayer10 support for autogain.
> 
> Plus, I changed avg_lum to long long. Quick calculation tells me int
> could overflow with few megapixel sensor.
> 
> Oh, btw http://ytse.tricolour.net/docs/LowLightOptimization.html no
> longer works.

Can I get some comments here? Patch will need fixup (constants need
adjusting), but is style/design acceptable?

Thanks,
								Pavel

> diff --git a/lib/libv4lconvert/processing/autogain.c b/lib/libv4lconvert/processing/autogain.c
> index c6866d6..0b52d0f 100644
> --- a/lib/libv4lconvert/processing/autogain.c
> +++ b/lib/libv4lconvert/processing/autogain.c
> @@ -68,6 +71,41 @@ static void autogain_adjust(struct v4l2_queryctrl *ctrl, int *value,
>  	}
>  }
>  
> +static int get_luminosity_bayer10(uint16_t *buf, const struct v4l2_format *fmt)
> +{
> +	long long avg_lum = 0;
> +	int x, y;
> +	
> +	buf += fmt->fmt.pix.height * fmt->fmt.pix.bytesperline / 4 +
> +		fmt->fmt.pix.width / 4;
> +
> +	for (y = 0; y < fmt->fmt.pix.height / 2; y++) {
> +		for (x = 0; x < fmt->fmt.pix.width / 2; x++)
> +			avg_lum += *buf++;
> +		buf += fmt->fmt.pix.bytesperline - fmt->fmt.pix.width / 2;
> +	}
> +	avg_lum /= fmt->fmt.pix.height * fmt->fmt.pix.width / 4;
> +	avg_lum /= 4;
> +	return avg_lum;
> +}
> +
> +static int get_luminosity_bayer8(unsigned char *buf, const struct v4l2_format *fmt)
> +{
> +	long long avg_lum = 0;
> +	int x, y;
> +	
> +	buf += fmt->fmt.pix.height * fmt->fmt.pix.bytesperline / 4 +
> +		fmt->fmt.pix.width / 4;
> +
> +	for (y = 0; y < fmt->fmt.pix.height / 2; y++) {
> +		for (x = 0; x < fmt->fmt.pix.width / 2; x++)
> +			avg_lum += *buf++;
> +		buf += fmt->fmt.pix.bytesperline - fmt->fmt.pix.width / 2;
> +	}
> +	avg_lum /= fmt->fmt.pix.height * fmt->fmt.pix.width / 4;
> +	return avg_lum;
> +}
> +
>  /* auto gain and exposure algorithm based on the knee algorithm described here:
>  http://ytse.tricolour.net/docs/LowLightOptimization.html */
>  static int autogain_calculate_lookup_tables(
> @@ -100,17 +142,16 @@ static int autogain_calculate_lookup_tables(
>  	switch (fmt->fmt.pix.pixelformat) {
> +	case V4L2_PIX_FMT_SGBRG10:
> +	case V4L2_PIX_FMT_SGRBG10:
> +	case V4L2_PIX_FMT_SBGGR10:
> +	case V4L2_PIX_FMT_SRGGB10:
> +		avg_lum = get_luminosity_bayer10((void *) buf, fmt);
> +		break;
> +
>  	case V4L2_PIX_FMT_SGBRG8:
>  	case V4L2_PIX_FMT_SGRBG8:
>  	case V4L2_PIX_FMT_SBGGR8:
>  	case V4L2_PIX_FMT_SRGGB8:
> -		buf += fmt->fmt.pix.height * fmt->fmt.pix.bytesperline / 4 +
> -			fmt->fmt.pix.width / 4;
> -
> -		for (y = 0; y < fmt->fmt.pix.height / 2; y++) {
> -			for (x = 0; x < fmt->fmt.pix.width / 2; x++)
> -				avg_lum += *buf++;
> -			buf += fmt->fmt.pix.bytesperline - fmt->fmt.pix.width / 2;
> -		}
> -		avg_lum /= fmt->fmt.pix.height * fmt->fmt.pix.width / 4;
> +		avg_lum = get_luminosity_bayer8(buf, fmt);
>  		break;
>  
>  	case V4L2_PIX_FMT_RGB24:
> diff --git a/lib/libv4lconvert/processing/libv4lprocessing.c b/lib/libv4lconvert/processing/libv4lprocessing.c
> index b061f50..b98d024 100644
> --- a/lib/libv4lconvert/processing/libv4lprocessing.c
> +++ b/lib/libv4lconvert/processing/libv4lprocessing.c
> @@ -164,6 +165,10 @@ void v4lprocessing_processing(struct v4lprocessing_data *data,
>  	case V4L2_PIX_FMT_SGRBG8:
>  	case V4L2_PIX_FMT_SBGGR8:
>  	case V4L2_PIX_FMT_SRGGB8:
> +	case V4L2_PIX_FMT_SGBRG10:
> +	case V4L2_PIX_FMT_SGRBG10:
> +	case V4L2_PIX_FMT_SBGGR10:
> +	case V4L2_PIX_FMT_SRGGB10:
>  	case V4L2_PIX_FMT_RGB24:
>  	case V4L2_PIX_FMT_BGR24:
>  		break;
> 
> 
> 



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170501/f729b3a9/attachment-0001.sig>

^ permalink raw reply

* [PATCH] net: phy: Allow BCM5481x PHYs to setup internal TX/RX clock delay
From: Florian Fainelli @ 2017-05-01  0:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493530461-11935-1-git-send-email-abhishek.shah@broadcom.com>



On 04/29/2017 10:34 PM, Abhishek Shah wrote:
> This patch allows users to enable/disable internal TX and/or RX
> clock delay for BCM5481x series PHYs so as to satisfy RGMII timing
> specifications.
> 
> On a particular platform, whether TX and/or RX clock delay is required
> depends on how PHY connected to the MAC IP. This requirement can be
> specified through "phy-mode" property in the platform device tree.
> 
> Signed-off-by: Abhishek Shah <abhishek.shah@broadcom.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

> ---
>  drivers/net/phy/broadcom.c | 69 ++++++++++++++++++++++------------------------
>  1 file changed, 33 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
> index 9cd8b27..a32dc5d 100644
> --- a/drivers/net/phy/broadcom.c
> +++ b/drivers/net/phy/broadcom.c
> @@ -74,27 +74,40 @@ static int bcm54612e_config_init(struct phy_device *phydev)
>  	return 0;
>  }
>  
> -static int bcm54810_config(struct phy_device *phydev)
> +static int bcm5481x_config(struct phy_device *phydev)
>  {
>  	int rc, val;
>  
> -	val = bcm_phy_read_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL);
> -	val &= ~BCM54810_EXP_BROADREACH_LRE_MISC_CTL_EN;
> -	rc = bcm_phy_write_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL,
> -			       val);
> -	if (rc < 0)
> -		return rc;
> -
> +	/* handling PHY's internal RX clock delay */
>  	val = bcm54xx_auxctl_read(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC);
> -	val &= ~MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_SKEW_EN;
>  	val |= MII_BCM54XX_AUXCTL_MISC_WREN;
> +	if (phydev->interface == PHY_INTERFACE_MODE_RGMII ||
> +	    phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> +		/* Disable RGMII RXC-RXD skew */
> +		val &= ~MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_SKEW_EN;
> +	}
> +	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> +	    phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
> +		/* Enable RGMII RXC-RXD skew */
> +		val |= MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_SKEW_EN;
> +	}
>  	rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
>  				  val);
>  	if (rc < 0)
>  		return rc;
>  
> +	/* handling PHY's internal TX clock delay */
>  	val = bcm_phy_read_shadow(phydev, BCM54810_SHD_CLK_CTL);
> -	val &= ~BCM54810_SHD_CLK_CTL_GTXCLK_EN;
> +	if (phydev->interface == PHY_INTERFACE_MODE_RGMII ||
> +	    phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
> +		/* Disable internal TX clock delay */
> +		val &= ~BCM54810_SHD_CLK_CTL_GTXCLK_EN;
> +	}
> +	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> +	    phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> +		/* Enable internal TX clock delay */
> +		val |= BCM54810_SHD_CLK_CTL_GTXCLK_EN;
> +	}
>  	rc = bcm_phy_write_shadow(phydev, BCM54810_SHD_CLK_CTL, val);
>  	if (rc < 0)
>  		return rc;
> @@ -244,7 +257,7 @@ static void bcm54xx_adjust_rxrefclk(struct phy_device *phydev)
>  
>  static int bcm54xx_config_init(struct phy_device *phydev)
>  {
> -	int reg, err;
> +	int reg, err, val;
>  
>  	reg = phy_read(phydev, MII_BCM54XX_ECR);
>  	if (reg < 0)
> @@ -283,8 +296,14 @@ static int bcm54xx_config_init(struct phy_device *phydev)
>  		if (err)
>  			return err;
>  	} else if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810) {
> -		err = bcm54810_config(phydev);
> -		if (err)
> +		/* For BCM54810, we need to disable BroadR-Reach function */
> +		val = bcm_phy_read_exp(phydev,
> +				       BCM54810_EXP_BROADREACH_LRE_MISC_CTL);
> +		val &= ~BCM54810_EXP_BROADREACH_LRE_MISC_CTL_EN;
> +		err = bcm_phy_write_exp(phydev,
> +					BCM54810_EXP_BROADREACH_LRE_MISC_CTL,
> +					val);
> +		if (err < 0)
>  			return err;
>  	}
>  
> @@ -392,29 +411,7 @@ static int bcm5481_config_aneg(struct phy_device *phydev)
>  	ret = genphy_config_aneg(phydev);
>  
>  	/* Then we can set up the delay. */
> -	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
> -		u16 reg;
> -
> -		/*
> -		 * There is no BCM5481 specification available, so down
> -		 * here is everything we know about "register 0x18". This
> -		 * at least helps BCM5481 to successfully receive packets
> -		 * on MPC8360E-RDK board. Peter Barada <peterb@logicpd.com>
> -		 * says: "This sets delay between the RXD and RXC signals
> -		 * instead of using trace lengths to achieve timing".
> -		 */
> -
> -		/* Set RDX clk delay. */
> -		reg = 0x7 | (0x7 << 12);
> -		phy_write(phydev, 0x18, reg);
> -
> -		reg = phy_read(phydev, 0x18);
> -		/* Set RDX-RXC skew. */
> -		reg |= (1 << 8);
> -		/* Write bits 14:0. */
> -		reg |= (1 << 15);
> -		phy_write(phydev, 0x18, reg);
> -	}
> +	bcm5481x_config(phydev);
>  
>  	if (of_property_read_bool(np, "enet-phy-lane-swap")) {
>  		/* Lane Swap - Undocumented register...magic! */
> 

-- 
Florian

^ permalink raw reply

* [PATCH net-next] bpf, arm64: implement jiting of BPF_XADD
From: Daniel Borkmann @ 2017-05-01  0:57 UTC (permalink / raw)
  To: linux-arm-kernel

This work adds BPF_XADD for BPF_W/BPF_DW to the arm64 JIT and therefore
completes JITing of all BPF instructions, meaning we can thus also remove
the 'notyet' label and do not need to fall back to the interpreter when
BPF_XADD is used in a program!

This now also brings arm64 JIT in line with x86_64, s390x, ppc64, sparc64,
where all current eBPF features are supported.

BPF_W example from test_bpf:

  .u.insns_int = {
    BPF_ALU32_IMM(BPF_MOV, R0, 0x12),
    BPF_ST_MEM(BPF_W, R10, -40, 0x10),
    BPF_STX_XADD(BPF_W, R10, R0, -40),
    BPF_LDX_MEM(BPF_W, R0, R10, -40),
    BPF_EXIT_INSN(),
  },

  [...]
  00000020:  52800247  mov w7, #0x12 // #18
  00000024:  928004eb  mov x11, #0xffffffffffffffd8 // #-40
  00000028:  d280020a  mov x10, #0x10 // #16
  0000002c:  b82b6b2a  str w10, [x25,x11]
  // start of xadd mapping:
  00000030:  928004ea  mov x10, #0xffffffffffffffd8 // #-40
  00000034:  8b19014a  add x10, x10, x25
  00000038:  f9800151  prfm pstl1strm, [x10]
  0000003c:  885f7d4b  ldxr w11, [x10]
  00000040:  0b07016b  add w11, w11, w7
  00000044:  880b7d4b  stxr w11, w11, [x10]
  00000048:  35ffffab  cbnz w11, 0x0000003c
  // end of xadd mapping:
  [...]

BPF_DW example from test_bpf:

  .u.insns_int = {
    BPF_ALU32_IMM(BPF_MOV, R0, 0x12),
    BPF_ST_MEM(BPF_DW, R10, -40, 0x10),
    BPF_STX_XADD(BPF_DW, R10, R0, -40),
    BPF_LDX_MEM(BPF_DW, R0, R10, -40),
    BPF_EXIT_INSN(),
  },

  [...]
  00000020:  52800247  mov w7,  #0x12 // #18
  00000024:  928004eb  mov x11, #0xffffffffffffffd8 // #-40
  00000028:  d280020a  mov x10, #0x10 // #16
  0000002c:  f82b6b2a  str x10, [x25,x11]
  // start of xadd mapping:
  00000030:  928004ea  mov x10, #0xffffffffffffffd8 // #-40
  00000034:  8b19014a  add x10, x10, x25
  00000038:  f9800151  prfm pstl1strm, [x10]
  0000003c:  c85f7d4b  ldxr x11, [x10]
  00000040:  8b07016b  add x11, x11, x7
  00000044:  c80b7d4b  stxr w11, x11, [x10]
  00000048:  35ffffab  cbnz w11, 0x0000003c
  // end of xadd mapping:
  [...]

Tested on Cavium ThunderX ARMv8, test suite results after the patch:

  No JIT:   [ 3751.855362] test_bpf: Summary: 311 PASSED, 0 FAILED, [0/303 JIT'ed]
  With JIT: [ 3573.759527] test_bpf: Summary: 311 PASSED, 0 FAILED, [303/303 JIT'ed]

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 ( Based against net-next where BPF related patches are usually
   routed, if something else is preferred please let me know. )

 arch/arm64/include/asm/insn.h |  30 ++++++++++++
 arch/arm64/kernel/insn.c      | 106 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm64/net/bpf_jit.h      |  19 ++++++++
 arch/arm64/net/bpf_jit_comp.c |  16 +++++--
 lib/test_bpf.c                | 105 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 271 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
index aecc07e..29cb2ca 100644
--- a/arch/arm64/include/asm/insn.h
+++ b/arch/arm64/include/asm/insn.h
@@ -80,6 +80,7 @@ enum aarch64_insn_register_type {
 	AARCH64_INSN_REGTYPE_RM,
 	AARCH64_INSN_REGTYPE_RD,
 	AARCH64_INSN_REGTYPE_RA,
+	AARCH64_INSN_REGTYPE_RS,
 };
 
 enum aarch64_insn_register {
@@ -188,6 +189,8 @@ enum aarch64_insn_ldst_type {
 	AARCH64_INSN_LDST_STORE_PAIR_PRE_INDEX,
 	AARCH64_INSN_LDST_LOAD_PAIR_POST_INDEX,
 	AARCH64_INSN_LDST_STORE_PAIR_POST_INDEX,
+	AARCH64_INSN_LDST_LOAD_EX,
+	AARCH64_INSN_LDST_STORE_EX,
 };
 
 enum aarch64_insn_adsb_type {
@@ -240,6 +243,23 @@ enum aarch64_insn_logic_type {
 	AARCH64_INSN_LOGIC_BIC_SETFLAGS
 };
 
+enum aarch64_insn_prfm_type {
+	AARCH64_INSN_PRFM_TYPE_PLD,
+	AARCH64_INSN_PRFM_TYPE_PLI,
+	AARCH64_INSN_PRFM_TYPE_PST,
+};
+
+enum aarch64_insn_prfm_target {
+	AARCH64_INSN_PRFM_TARGET_L1,
+	AARCH64_INSN_PRFM_TARGET_L2,
+	AARCH64_INSN_PRFM_TARGET_L3,
+};
+
+enum aarch64_insn_prfm_policy {
+	AARCH64_INSN_PRFM_POLICY_KEEP,
+	AARCH64_INSN_PRFM_POLICY_STRM,
+};
+
 #define	__AARCH64_INSN_FUNCS(abbr, mask, val)	\
 static __always_inline bool aarch64_insn_is_##abbr(u32 code) \
 { return (code & (mask)) == (val); } \
@@ -248,6 +268,7 @@ enum aarch64_insn_logic_type {
 
 __AARCH64_INSN_FUNCS(adr,	0x9F000000, 0x10000000)
 __AARCH64_INSN_FUNCS(adrp,	0x9F000000, 0x90000000)
+__AARCH64_INSN_FUNCS(prfm,	0x3FC00000, 0x39800000)
 __AARCH64_INSN_FUNCS(prfm_lit,	0xFF000000, 0xD8000000)
 __AARCH64_INSN_FUNCS(str_reg,	0x3FE0EC00, 0x38206800)
 __AARCH64_INSN_FUNCS(ldr_reg,	0x3FE0EC00, 0x38606800)
@@ -357,6 +378,11 @@ u32 aarch64_insn_gen_load_store_pair(enum aarch64_insn_register reg1,
 				     int offset,
 				     enum aarch64_insn_variant variant,
 				     enum aarch64_insn_ldst_type type);
+u32 aarch64_insn_gen_load_store_ex(enum aarch64_insn_register reg,
+				   enum aarch64_insn_register base,
+				   enum aarch64_insn_register state,
+				   enum aarch64_insn_size_type size,
+				   enum aarch64_insn_ldst_type type);
 u32 aarch64_insn_gen_add_sub_imm(enum aarch64_insn_register dst,
 				 enum aarch64_insn_register src,
 				 int imm, enum aarch64_insn_variant variant,
@@ -397,6 +423,10 @@ u32 aarch64_insn_gen_logical_shifted_reg(enum aarch64_insn_register dst,
 					 int shift,
 					 enum aarch64_insn_variant variant,
 					 enum aarch64_insn_logic_type type);
+u32 aarch64_insn_gen_prefetch(enum aarch64_insn_register base,
+			      enum aarch64_insn_prfm_type type,
+			      enum aarch64_insn_prfm_target target,
+			      enum aarch64_insn_prfm_policy policy);
 s32 aarch64_get_branch_offset(u32 insn);
 u32 aarch64_set_branch_offset(u32 insn, s32 offset);
 
diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index 3a63954..b884a92 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -474,6 +474,7 @@ static u32 aarch64_insn_encode_register(enum aarch64_insn_register_type type,
 		shift = 10;
 		break;
 	case AARCH64_INSN_REGTYPE_RM:
+	case AARCH64_INSN_REGTYPE_RS:
 		shift = 16;
 		break;
 	default:
@@ -757,6 +758,111 @@ u32 aarch64_insn_gen_load_store_pair(enum aarch64_insn_register reg1,
 					     offset >> shift);
 }
 
+u32 aarch64_insn_gen_load_store_ex(enum aarch64_insn_register reg,
+				   enum aarch64_insn_register base,
+				   enum aarch64_insn_register state,
+				   enum aarch64_insn_size_type size,
+				   enum aarch64_insn_ldst_type type)
+{
+	u32 insn;
+
+	switch (type) {
+	case AARCH64_INSN_LDST_LOAD_EX:
+		insn = aarch64_insn_get_load_ex_value();
+		break;
+	case AARCH64_INSN_LDST_STORE_EX:
+		insn = aarch64_insn_get_store_ex_value();
+		break;
+	default:
+		pr_err("%s: unknown load/store exclusive encoding %d\n", __func__, type);
+		return AARCH64_BREAK_FAULT;
+	}
+
+	insn = aarch64_insn_encode_ldst_size(size, insn);
+
+	insn = aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RT, insn,
+					    reg);
+
+	insn = aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RN, insn,
+					    base);
+
+	insn = aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RT2, insn,
+					    AARCH64_INSN_REG_ZR);
+
+	return aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RS, insn,
+					    state);
+}
+
+static u32 aarch64_insn_encode_prfm_imm(enum aarch64_insn_prfm_type type,
+					enum aarch64_insn_prfm_target target,
+					enum aarch64_insn_prfm_policy policy,
+					u32 insn)
+{
+	u32 imm_type = 0, imm_target = 0, imm_policy = 0;
+
+	switch (type) {
+	case AARCH64_INSN_PRFM_TYPE_PLD:
+		break;
+	case AARCH64_INSN_PRFM_TYPE_PLI:
+		imm_type = BIT(0);
+		break;
+	case AARCH64_INSN_PRFM_TYPE_PST:
+		imm_type = BIT(1);
+		break;
+	default:
+		pr_err("%s: unknown prfm type encoding %d\n", __func__, type);
+		return AARCH64_BREAK_FAULT;
+	}
+
+	switch (target) {
+	case AARCH64_INSN_PRFM_TARGET_L1:
+		break;
+	case AARCH64_INSN_PRFM_TARGET_L2:
+		imm_target = BIT(0);
+		break;
+	case AARCH64_INSN_PRFM_TARGET_L3:
+		imm_target = BIT(1);
+		break;
+	default:
+		pr_err("%s: unknown prfm target encoding %d\n", __func__, target);
+		return AARCH64_BREAK_FAULT;
+	}
+
+	switch (policy) {
+	case AARCH64_INSN_PRFM_POLICY_KEEP:
+		break;
+	case AARCH64_INSN_PRFM_POLICY_STRM:
+		imm_policy = BIT(0);
+		break;
+	default:
+		pr_err("%s: unknown prfm policy encoding %d\n", __func__, policy);
+		return AARCH64_BREAK_FAULT;
+	}
+
+	/* In this case, imm5 is encoded into Rt field. */
+	insn &= ~GENMASK(4, 0);
+	insn |= imm_policy | (imm_target << 1) | (imm_type << 3);
+
+	return insn;
+}
+
+u32 aarch64_insn_gen_prefetch(enum aarch64_insn_register base,
+			      enum aarch64_insn_prfm_type type,
+			      enum aarch64_insn_prfm_target target,
+			      enum aarch64_insn_prfm_policy policy)
+{
+	u32 insn = aarch64_insn_get_prfm_value();
+
+	insn = aarch64_insn_encode_ldst_size(AARCH64_INSN_SIZE_64, insn);
+
+	insn = aarch64_insn_encode_prfm_imm(type, target, policy, insn);
+
+	insn = aarch64_insn_encode_register(AARCH64_INSN_REGTYPE_RN, insn,
+					    base);
+
+	return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_12, insn, 0);
+}
+
 u32 aarch64_insn_gen_add_sub_imm(enum aarch64_insn_register dst,
 				 enum aarch64_insn_register src,
 				 int imm, enum aarch64_insn_variant variant,
diff --git a/arch/arm64/net/bpf_jit.h b/arch/arm64/net/bpf_jit.h
index 7c16e54..b02a926 100644
--- a/arch/arm64/net/bpf_jit.h
+++ b/arch/arm64/net/bpf_jit.h
@@ -83,6 +83,25 @@
 /* Rt = Rn[0]; Rt2 = Rn[8]; Rn += 16; */
 #define A64_POP(Rt, Rt2, Rn)  A64_LS_PAIR(Rt, Rt2, Rn, 16, LOAD, POST_INDEX)
 
+/* Load/store exclusive */
+#define A64_SIZE(sf) \
+	((sf) ? AARCH64_INSN_SIZE_64 : AARCH64_INSN_SIZE_32)
+#define A64_LSX(sf, Rt, Rn, Rs, type) \
+	aarch64_insn_gen_load_store_ex(Rt, Rn, Rs, A64_SIZE(sf), \
+				       AARCH64_INSN_LDST_##type)
+/* Rt = [Rn]; (atomic) */
+#define A64_LDXR(sf, Rt, Rn) \
+	A64_LSX(sf, Rt, Rn, A64_ZR, LOAD_EX)
+/* [Rn] = Rt; (atomic) Rs = [state] */
+#define A64_STXR(sf, Rt, Rn, Rs) \
+	A64_LSX(sf, Rt, Rn, Rs, STORE_EX)
+
+/* Prefetch */
+#define A64_PRFM(Rn, type, target, policy) \
+	aarch64_insn_gen_prefetch(Rn, AARCH64_INSN_PRFM_TYPE_##type, \
+				  AARCH64_INSN_PRFM_TARGET_##target, \
+				  AARCH64_INSN_PRFM_POLICY_##policy)
+
 /* Add/subtract (immediate) */
 #define A64_ADDSUB_IMM(sf, Rd, Rn, imm12, type) \
 	aarch64_insn_gen_add_sub_imm(Rd, Rn, imm12, \
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 3047368..4f2b351 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -321,6 +321,7 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx)
 	const s32 imm = insn->imm;
 	const int i = insn - ctx->prog->insnsi;
 	const bool is64 = BPF_CLASS(code) == BPF_ALU64;
+	const bool isdw = BPF_SIZE(code) == BPF_DW;
 	u8 jmp_cond;
 	s32 jmp_offset;
 
@@ -681,7 +682,16 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx)
 	case BPF_STX | BPF_XADD | BPF_W:
 	/* STX XADD: lock *(u64 *)(dst + off) += src */
 	case BPF_STX | BPF_XADD | BPF_DW:
-		goto notyet;
+		emit_a64_mov_i(1, tmp, off, ctx);
+		emit(A64_ADD(1, tmp, tmp, dst), ctx);
+		emit(A64_PRFM(tmp, PST, L1, STRM), ctx);
+		emit(A64_LDXR(isdw, tmp2, tmp), ctx);
+		emit(A64_ADD(isdw, tmp2, tmp2, src), ctx);
+		emit(A64_STXR(isdw, tmp2, tmp, tmp2), ctx);
+		jmp_offset = -3;
+		check_imm19(jmp_offset);
+		emit(A64_CBNZ(0, tmp2, jmp_offset), ctx);
+		break;
 
 	/* R0 = ntohx(*(size *)(((struct sk_buff *)R6)->data + imm)) */
 	case BPF_LD | BPF_ABS | BPF_W:
@@ -748,10 +758,6 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx)
 		}
 		break;
 	}
-notyet:
-		pr_info_once("*** NOT YET: opcode %02x ***\n", code);
-		return -EFAULT;
-
 	default:
 		pr_err_once("unknown opcode %02x\n", code);
 		return -EINVAL;
diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index 0362da0..3a7730c 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -434,6 +434,41 @@ static int bpf_fill_ld_abs_vlan_push_pop(struct bpf_test *self)
 	return 0;
 }
 
+static int __bpf_fill_stxdw(struct bpf_test *self, int size)
+{
+	unsigned int len = BPF_MAXINSNS;
+	struct bpf_insn *insn;
+	int i;
+
+	insn = kmalloc_array(len, sizeof(*insn), GFP_KERNEL);
+	if (!insn)
+		return -ENOMEM;
+
+	insn[0] = BPF_ALU32_IMM(BPF_MOV, R0, 1);
+	insn[1] = BPF_ST_MEM(size, R10, -40, 42);
+
+	for (i = 2; i < len - 2; i++)
+		insn[i] = BPF_STX_XADD(size, R10, R0, -40);
+
+	insn[len - 2] = BPF_LDX_MEM(size, R0, R10, -40);
+	insn[len - 1] = BPF_EXIT_INSN();
+
+	self->u.ptr.insns = insn;
+	self->u.ptr.len = len;
+
+	return 0;
+}
+
+static int bpf_fill_stxw(struct bpf_test *self)
+{
+	return __bpf_fill_stxdw(self, BPF_W);
+}
+
+static int bpf_fill_stxdw(struct bpf_test *self)
+{
+	return __bpf_fill_stxdw(self, BPF_DW);
+}
+
 static struct bpf_test tests[] = {
 	{
 		"TAX",
@@ -4303,6 +4338,41 @@ static int bpf_fill_ld_abs_vlan_push_pop(struct bpf_test *self)
 		{ { 0, 0x22 } },
 	},
 	{
+		"STX_XADD_W: Test side-effects, r10: 0x12 + 0x10 = 0x22",
+		.u.insns_int = {
+			BPF_ALU64_REG(BPF_MOV, R1, R10),
+			BPF_ALU32_IMM(BPF_MOV, R0, 0x12),
+			BPF_ST_MEM(BPF_W, R10, -40, 0x10),
+			BPF_STX_XADD(BPF_W, R10, R0, -40),
+			BPF_ALU64_REG(BPF_MOV, R0, R10),
+			BPF_ALU64_REG(BPF_SUB, R0, R1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 0 } },
+	},
+	{
+		"STX_XADD_W: Test side-effects, r0: 0x12 + 0x10 = 0x22",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0x12),
+			BPF_ST_MEM(BPF_W, R10, -40, 0x10),
+			BPF_STX_XADD(BPF_W, R10, R0, -40),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 0x12 } },
+	},
+	{
+		"STX_XADD_W: X + 1 + 1 + 1 + ...",
+		{ },
+		INTERNAL,
+		{ },
+		{ { 0, 4134 } },
+		.fill_helper = bpf_fill_stxw,
+	},
+	{
 		"STX_XADD_DW: Test: 0x12 + 0x10 = 0x22",
 		.u.insns_int = {
 			BPF_ALU32_IMM(BPF_MOV, R0, 0x12),
@@ -4315,6 +4385,41 @@ static int bpf_fill_ld_abs_vlan_push_pop(struct bpf_test *self)
 		{ },
 		{ { 0, 0x22 } },
 	},
+	{
+		"STX_XADD_DW: Test side-effects, r10: 0x12 + 0x10 = 0x22",
+		.u.insns_int = {
+			BPF_ALU64_REG(BPF_MOV, R1, R10),
+			BPF_ALU32_IMM(BPF_MOV, R0, 0x12),
+			BPF_ST_MEM(BPF_DW, R10, -40, 0x10),
+			BPF_STX_XADD(BPF_DW, R10, R0, -40),
+			BPF_ALU64_REG(BPF_MOV, R0, R10),
+			BPF_ALU64_REG(BPF_SUB, R0, R1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 0 } },
+	},
+	{
+		"STX_XADD_DW: Test side-effects, r0: 0x12 + 0x10 = 0x22",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0x12),
+			BPF_ST_MEM(BPF_DW, R10, -40, 0x10),
+			BPF_STX_XADD(BPF_DW, R10, R0, -40),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 0x12 } },
+	},
+	{
+		"STX_XADD_DW: X + 1 + 1 + 1 + ...",
+		{ },
+		INTERNAL,
+		{ },
+		{ { 0, 4134 } },
+		.fill_helper = bpf_fill_stxdw,
+	},
 	/* BPF_JMP | BPF_EXIT */
 	{
 		"JMP_EXIT",
-- 
1.9.3

^ permalink raw reply related

* [PATCH v3 1/2] net: dsa: b53: Add compatible strings for the Cygnus-family BCM11360.
From: David Miller @ 2017-05-01  2:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170428222204.7103-1-eric@anholt.net>

From: Eric Anholt <eric@anholt.net>
Date: Fri, 28 Apr 2017 15:22:03 -0700

> Cygnus is a small family of SoCs, of which we currently have
> devicetree for BCM11360 and BCM58300.  The 11360's B53 is mostly the
> same as 58xx, just requiring a tiny bit of setup that was previously
> missing.
> 
> v2: Reorder the entry in the docs (suggestion by Scott Branden), add
>     missing '"'
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> Acked-by: Rob Herring <robh@kernel.org>

The second patch with the DTS file update doesn't apply cleanly
at all to net-next.

So I'm dropping this series.

^ permalink raw reply

* [PATCH] net: phy: Allow BCM5481x PHYs to setup internal TX/RX clock delay
From: David Miller @ 2017-05-01  2:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1493530461-11935-1-git-send-email-abhishek.shah@broadcom.com>

From: Abhishek Shah <abhishek.shah@broadcom.com>
Date: Sun, 30 Apr 2017 11:04:21 +0530

> This patch allows users to enable/disable internal TX and/or RX
> clock delay for BCM5481x series PHYs so as to satisfy RGMII timing
> specifications.
> 
> On a particular platform, whether TX and/or RX clock delay is required
> depends on how PHY connected to the MAC IP. This requirement can be
> specified through "phy-mode" property in the platform device tree.
> 
> Signed-off-by: Abhishek Shah <abhishek.shah@broadcom.com>

Applied.

^ permalink raw reply

* [PATCH v2 0/2] ARM: sun8i: a83t: device tree cleanup
From: Chen-Yu Tsai @ 2017-05-01  3:14 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Maxime,

Here's v2 of the A83T device tree cleanup patches. I dropped the change
to the uart device node name for now.

Also added a second patch changing the underscores in device node names
I just added to hyphens. AFAIK that is the preferred naming scheme.
Please squash it into "ARM: sun8i: a83t: Rename pinmux setting names".


Regards
ChenYu

Chen-Yu Tsai (2):
  ARM: sun8i: a83t: Drop leading zeroes from device node addresses
  ARM: sun8i: a83t: Replace underscores with hyphens in pinmux node
    names

 arch/arm/boot/dts/sun8i-a83t.dtsi | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

-- 
2.11.0

^ permalink raw reply

* [PATCH v2 1/2] ARM: sun8i: a83t: Drop leading zeroes from device node addresses
From: Chen-Yu Tsai @ 2017-05-01  3:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170501031408.10469-1-wens@csie.org>

Kbuild now complains about leading zeroes in the address portion of
device node names.

Get rid of them all, except for the uart device node. U-boot currently
hard codes the device node path. We can remove the leading zero for
the uart once we teach U-boot to use the aliases or stdout-path
property.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun8i-a83t.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 5f5c10c04dd3..aecde8be53bc 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -162,7 +162,7 @@
 		#size-cells = <1>;
 		ranges;
 
-		pio: pinctrl at 01c20800 {
+		pio: pinctrl at 1c20800 {
 			compatible = "allwinner,sun8i-a83t-pinctrl";
 			interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>,
@@ -193,7 +193,7 @@
 			};
 		};
 
-		timer at 01c20c00 {
+		timer at 1c20c00 {
 			compatible = "allwinner,sun4i-a10-timer";
 			reg = <0x01c20c00 0xa0>;
 			interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
@@ -201,7 +201,7 @@
 			clocks = <&osc24M>;
 		};
 
-		watchdog at 01c20ca0 {
+		watchdog at 1c20ca0 {
 			compatible = "allwinner,sun6i-a31-wdt";
 			reg = <0x01c20ca0 0x20>;
 			interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
@@ -218,7 +218,7 @@
 			status = "disabled";
 		};
 
-		gic: interrupt-controller at 01c81000 {
+		gic: interrupt-controller at 1c81000 {
 			compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
 			reg = <0x01c81000 0x1000>,
 			      <0x01c82000 0x2000>,
-- 
2.11.0

^ permalink raw reply related

* [PATCH v2 2/2] ARM: sun8i: a83t: Replace underscores with hyphens in pinmux node names
From: Chen-Yu Tsai @ 2017-05-01  3:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170501031408.10469-1-wens@csie.org>

We should use hyphens and not underscores in device node names.

Replace the ones that were just added.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun8i-a83t.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index aecde8be53bc..c0a1e4f74b89 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -174,7 +174,7 @@
 			#interrupt-cells = <3>;
 			#gpio-cells = <3>;
 
-			mmc0_pins: mmc0_pins {
+			mmc0_pins: mmc0-pins {
 				pins = "PF0", "PF1", "PF2",
 				       "PF3", "PF4", "PF5";
 				function = "mmc0";
@@ -182,12 +182,12 @@
 				bias-pull-up;
 			};
 
-			uart0_pb_pins: uart0_pb_pins {
+			uart0_pb_pins: uart0-pb-pins {
 				pins = "PB9", "PB10";
 				function = "uart0";
 			};
 
-			uart0_pf_pins: uart0_pf_pins {
+			uart0_pf_pins: uart0-pf-pins {
 				pins = "PF2", "PF4";
 				function = "uart0";
 			};
-- 
2.11.0

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox