Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -next] clk: zx296718: use builtin_platform_driver to simplify the code
From: Wei Yongjun @ 2016-09-16 13:03 UTC (permalink / raw)
  To: linux-arm-kernel

From: Wei Yongjun <weiyongjun1@huawei.com>

Use the builtin_platform_driver() macro to make the code simpler.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/clk/zte/clk-zx296718.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/clk/zte/clk-zx296718.c b/drivers/clk/zte/clk-zx296718.c
index 025e324..b4fe8dd 100644
--- a/drivers/clk/zte/clk-zx296718.c
+++ b/drivers/clk/zte/clk-zx296718.c
@@ -917,8 +917,4 @@ static struct platform_driver zx_clk_driver = {
 	},
 };
 
-static int __init zx_clkc_init(void)
-{
-	return platform_driver_register(&zx_clk_driver);
-}
-device_initcall(zx_clkc_init);
+builtin_platform_driver(zx_clk_driver);

^ permalink raw reply related

* [PATCH v26 7/7] Documentation: dt: chosen properties for arm64 kdump
From: Rob Herring @ 2016-09-16 13:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160907043203.6309-1-takahiro.akashi@linaro.org>

On Wed, Sep 07, 2016 at 01:32:03PM +0900, AKASHI Takahiro wrote:
> From: James Morse <james.morse@arm.com>
> 
> Add documentation for
> 	linux,crashkernel-base and crashkernel-size,
> 	linux,elfcorehdr
> used by arm64 kexec/kdump to decribe the kdump reserved area, and
> the elfcorehdr's location within it.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> [takahiro.akashi at linaro.org: added "linux,crashkernel-base" and "-size" ]
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  Documentation/devicetree/bindings/chosen.txt | 30 ++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [PATCHv9 0/6] dmaengine: rcar-dmac: add iommu support for slave transfers
From: Laurent Pinchart @ 2016-09-16 13:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cea119f1-18d5-675d-ee33-c22b0c8e7693@arm.com>

Hi Robin,

On Friday 16 Sep 2016 13:49:21 Robin Murphy wrote:
> On 16/09/16 13:05, Laurent Pinchart wrote:
> [...]
> 
> >>>> One concern I have is that we might get an awkward situation if we ever
> >>>> encounter one DMA engine hardware that is used in different systems
> >>>> that all have an IOMMU, but on some of them the connection between the
> >>>> DMA master and the slave FIFO bypasses the IOMMU while on others the
> >>>> IOMMU is required.
> >>> 
> >>> Do you mean systems where some of the channels of a specific DMA engine
> >>> go through the IOMMU while others do not ? We indeed have no solution
> >>> today for such a situation.
> >>> 
> >>> The problem is a bit broader than that, we'll also have an issue with
> >>> DMA engines that have different channels served by different IOMMUs. I
> >>> recall discussing this in the past with you, and the solution you
> >>> proposed was to add a channel index to struct dma_attrs seems good to
> >>> me. To support the case where some channels don't go through an IOMMU we
> >>> would only need support for null entries in the IOMMUs list associated
> >>> with a device (for instance in the DT case null entries in the iommus
> >>> property).
> >> 
> >> I think at that point we just create the channels as child devices of
> >> the main dmaengine device so they each get their own DMA ops, and can do
> >> whatever. The Qualcomm HIDMA driver already does that for a very similar
> >> reason (so that the IOMMU can map individual channels into different
> >> guest VMs).
> > 
> > That's another option, but it seems more like a workaround to me, instead
> > of a proper solution to fix the more global problem of multiple memory
> > paths within a single device. I have other hardware devices that can act
> > as bus masters through different paths (for instance a display-related
> > device that fetches data and commands through different paths). Luckily
> > so far all those paths are served by the same IOMMU, but there's no
> > guarantee this will remain true in the future. Furthermore, even today,
> > the IOMMU connected to that device has the ability to selectively enable
> > and disable its ports. I have to keep them all enabled due to the lack of
> > channel information in the DMA mapping and IOMMU APIs, leading to
> > increased power consumption.
> 
> Indeed, I think both the Exynos and Rockchip IOMMU drivers already do
> cater for a device mastering though multiple discrete IOMMUs, not being
> the fancy multi-port multi-context ones like yours and mine.
> 
> I guess what we could really do with is a decent abstraction of
> multi-master peripherals at the device level; a "threads within the same
> process" sort of granularity, as it were. I'd envisage it more along the
> lines of how we handle NUMA, i.e. dma_map_page_attrs(...) becomes a
> wrapper for dma_map_page_attrs_multi(..., CHANNEL_ALL), and trickier
> users can call the latter with the a more specific channel(s) argument
> (maybe it's a bitmask rather than an index).

That's pretty much what I've discussed with Arnd in the past, except that we 
were planning to add the channel to struct dma_attrs. Hence my disappointment 
seeing the structure go away.

> Meanwhile, dev->archdata.dma_ops may point to a device-specific array of
> dma_map_ops, which the DMA API backend iterates over if necessary.
> 
> Strangely, that doesn't actually sound too horrible.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* [PATCH v2 1/3] ipmi: add an Aspeed BT IPMI BMC driver
From: Cédric Le Goater @ 2016-09-16 13:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160916122949.GA23381@Red>

On 09/16/2016 02:29 PM, LABBE Corentin wrote:
> Hello
> 
> I have some minor comment below:
> 
> On Fri, Sep 16, 2016 at 12:39:25PM +0200, C?dric Le Goater wrote:
>> From: Alistair Popple <alistair@popple.id.au>
>>
>> This patch adds a simple device driver to expose the iBT interface on
>> Aspeed SOCs (AST2400 and AST2500) as a character device. Such SOCs are
>> commonly used as BMCs (BaseBoard Management Controllers) and this
>> driver implements the BMC side of the BT interface.
>>
>> The BT (Block Transfer) interface is used to perform in-band IPMI
>> communication between a host and its BMC. Entire messages are buffered
>> before sending a notification to the other end, host or BMC, that
>> there is data to be read. Usually, the host emits requests and the BMC
>> responses but the specification provides a mean for the BMC to send
>> SMS Attention (BMC-to-Host attention or System Management Software
>> attention) messages.
>>
>> For this purpose, the driver introduces a specific ioctl on the
>> device: 'BT_BMC_IOCTL_SMS_ATN' that can be used by the system running
>> on the BMC to signal the host of such an event.
>>
>> The device name defaults to '/dev/ipmi-bt-host'
>>
>> Signed-off-by: Alistair Popple <alistair@popple.id.au>
>> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> [clg: - checkpatch fixes
>>       - added a devicetree binding documentation
>>       - replace 'bt_host' by 'bt_bmc' to reflect that the driver is
>>         the BMC side of the IPMI BT interface
>>       - renamed the device to 'ipmi-bt-host'
>>       - introduced a temporary buffer to copy_{to,from}_user
>>       - used platform_get_irq()
>>       - moved the driver under drivers/char/ipmi/ but kept it as a misc
>>         device
>>       - changed the compatible cell to "aspeed,ast2400-bt-bmc"
>> ]
>> Signed-off-by: C?dric Le Goater <clg@kaod.org>
>> ---
>>
>>  Changes since v1:
>>
>>  - replace 'bt_host' by 'bt_bmc' to reflect that the driver is
>>    the BMC side of the IPMI BT interface
>>  - renamed the device to 'ipmi-bt-host'
>>  - introduced a temporary buffer to copy_{to,from}_user
>>  - used platform_get_irq()
>>  - moved the driver under drivers/char/ipmi/ but kept it as a misc
>>    device
>>  - changed the compatible cell to "aspeed,ast2400-bt-bmc"
>>
>>  .../bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt   |  23 +
>>  drivers/Makefile                                   |   2 +-
>>  drivers/char/ipmi/Kconfig                          |   7 +
>>  drivers/char/ipmi/Makefile                         |   1 +
>>  drivers/char/ipmi/bt-bmc.c                         | 486 +++++++++++++++++++++
>>  include/uapi/linux/Kbuild                          |   1 +
>>  include/uapi/linux/bt-bmc.h                        |  18 +
>>  7 files changed, 537 insertions(+), 1 deletion(-)
>>  create mode 100644 Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt
>>  create mode 100644 drivers/char/ipmi/bt-bmc.c
>>  create mode 100644 include/uapi/linux/bt-bmc.h
>>
>> diff --git a/Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt b/Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt
>> new file mode 100644
>> index 000000000000..fbbacd958240
>> --- /dev/null
> 
> [..]
> 
>> +#include <linux/module.h>
>> +#include <linux/moduleparam.h>
>> +#include <linux/errno.h>
>> +#include <linux/poll.h>
>> +#include <linux/sched.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/slab.h>
>> +#include <linux/init.h>
>> +#include <linux/device.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/io.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/delay.h>
>> +#include <linux/miscdevice.h>
>> +#include <linux/timer.h>
>> +#include <linux/jiffies.h>
>> +#include <linux/bt-bmc.h>
> 
> Please sort them in alphabetical order, some of them seems not needed also (like spinlock.h)

sure. I will clean them up.

>> +
>> +/*
>> + * This is a BMC device used to communicate to the host
>> + */
>> +#define DEVICE_NAME	"ipmi-bt-host"
>> +
>> +#define BT_IO_BASE	0xe4
>> +#define BT_IRQ		10
>> +
>> +#define BT_CR0		0x0
>> +#define   BT_CR0_IO_BASE		16
>> +#define   BT_CR0_IRQ			12
>> +#define   BT_CR0_EN_CLR_SLV_RDP		0x8
>> +#define   BT_CR0_EN_CLR_SLV_WRP		0x4
>> +#define   BT_CR0_ENABLE_IBT		0x1
>> +#define BT_CR1		0x4
>> +#define   BT_CR1_IRQ_H2B	0x01
>> +#define   BT_CR1_IRQ_HBUSY	0x40
>> +#define BT_CR2		0x8
>> +#define   BT_CR2_IRQ_H2B	0x01
>> +#define   BT_CR2_IRQ_HBUSY	0x40
>> +#define BT_CR3		0xc
>> +#define BT_CTRL		0x10
>> +#define   BT_CTRL_B_BUSY		0x80
>> +#define   BT_CTRL_H_BUSY		0x40
>> +#define   BT_CTRL_OEM0			0x20
>> +#define   BT_CTRL_SMS_ATN		0x10
>> +#define   BT_CTRL_B2H_ATN		0x08
>> +#define   BT_CTRL_H2B_ATN		0x04
>> +#define   BT_CTRL_CLR_RD_PTR		0x02
>> +#define   BT_CTRL_CLR_WR_PTR		0x01
>> +#define BT_BMC2HOST	0x14
>> +#define BT_INTMASK	0x18
>> +#define   BT_INTMASK_B2H_IRQEN		0x01
>> +#define   BT_INTMASK_B2H_IRQ		0x02
>> +#define   BT_INTMASK_BMC_HWRST		0x80
> 
> Why to use 3 space after some define ?

The difference in alignment is to distinguish the register number from 
the bits signification. Is that OK ? 

> [..]
> 
>> +
>> +#define BT_BMC_BUFFER_SIZE 256
> 
> Put this define with others
> 
> [..]

ok

>> +
>> +static irqreturn_t bt_bmc_irq(int irq, void *arg)
>> +{
>> +	struct bt_bmc *bt_bmc = arg;
>> +	uint32_t reg;
> 
> u32 is prefered other uint32_t, do you have run checkpatch --strict ?

no. I just did and I see it is catching quite a few new issues. I will 
send a v3 with fixes.

>> +
>> +	reg = ioread32(bt_bmc->base + BT_CR2);
>> +	reg &= BT_CR2_IRQ_H2B | BT_CR2_IRQ_HBUSY;
>> +	if (!reg)
>> +		return IRQ_NONE;
>> +
>> +	/* ack pending IRQs */
>> +	iowrite32(reg, bt_bmc->base + BT_CR2);
>> +
>> +	wake_up(&bt_bmc->queue);
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int bt_bmc_config_irq(struct bt_bmc *bt_bmc,
>> +		struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	uint32_t reg;
>> +	int rc;
>> +
>> +	bt_bmc->irq = platform_get_irq(pdev, 0);
>> +	if (!bt_bmc->irq)
>> +		return -ENODEV;
>> +
>> +	rc = devm_request_irq(dev, bt_bmc->irq, bt_bmc_irq, IRQF_SHARED,
>> +			DEVICE_NAME, bt_bmc);
>> +	if (rc < 0) {
>> +		dev_warn(dev, "Unable to request IRQ %d\n", bt_bmc->irq);
>> +		bt_bmc->irq = 0;
>> +		return rc;
>> +	}
>> +
>> +	/* Configure IRQs on the bmc clearing the H2B and HBUSY bits;
>> +	 * H2B will be asserted when the bmc has data for us; HBUSY
>> +	 * will be cleared (along with B2H) when we can write the next
>> +	 * message to the BT buffer
>> +	 */
> 
> This comment doesnt have the style recommended for new driver.

ah yes. I missed that one. 

>> +	reg = ioread32(bt_bmc->base + BT_CR1);
>> +	reg |= BT_CR1_IRQ_H2B | BT_CR1_IRQ_HBUSY;
>> +	iowrite32(reg, bt_bmc->base + BT_CR1);
>> +
>> +	return 0;
>> +}
>> +
>> +static int bt_bmc_probe(struct platform_device *pdev)
>> +{
>> +	struct bt_bmc *bt_bmc;
>> +	struct device *dev;
>> +	struct resource *res;
>> +	int rc;
>> +
>> +	if (!pdev || !pdev->dev.of_node)
>> +		return -ENODEV;
>> +
>> +	dev = &pdev->dev;
>> +	dev_info(dev, "Found bt bmc device\n");
>> +
>> +	bt_bmc = devm_kzalloc(dev, sizeof(*bt_bmc), GFP_KERNEL);
>> +	if (!bt_bmc)
>> +		return -ENOMEM;
>> +
>> +	dev_set_drvdata(&pdev->dev, bt_bmc);
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (!res) {
>> +		dev_err(dev, "Unable to find resources\n");
>> +		rc = -ENXIO;
>> +		goto out_free;
>> +	}
>> +
>> +	bt_bmc->base = devm_ioremap_resource(&pdev->dev, res);
>> +	if (!bt_bmc->base) {
>> +		rc = -ENOMEM;
>> +		goto out_free;
>> +	}
>> +
>> +	init_waitqueue_head(&bt_bmc->queue);
>> +
>> +	bt_bmc->miscdev.minor	= MISC_DYNAMIC_MINOR,
>> +	bt_bmc->miscdev.name	= DEVICE_NAME,
>> +	bt_bmc->miscdev.fops	= &bt_bmc_fops,
>> +	bt_bmc->miscdev.parent = dev;
>> +	rc = misc_register(&bt_bmc->miscdev);
>> +	if (rc) {
>> +		dev_err(dev, "Unable to register device\n");
> 
> Be more precise by saying misc device

ok

> 
>> +		goto out_unmap;
>> +	}
>> +
>> +	bt_bmc_config_irq(bt_bmc, pdev);
>> +
>> +	if (bt_bmc->irq) {
>> +		dev_info(dev, "Using IRQ %d\n", bt_bmc->irq);
>> +	} else {
>> +		dev_info(dev, "No IRQ; using timer\n");
>> +		setup_timer(&bt_bmc->poll_timer, poll_timer,
>> +				(unsigned long)bt_bmc);
>> +		bt_bmc->poll_timer.expires = jiffies + msecs_to_jiffies(10);
>> +		add_timer(&bt_bmc->poll_timer);
>> +	}
>> +
>> +	iowrite32((BT_IO_BASE << BT_CR0_IO_BASE) |
>> +		  (BT_IRQ << BT_CR0_IRQ) |
>> +		  BT_CR0_EN_CLR_SLV_RDP |
>> +		  BT_CR0_EN_CLR_SLV_WRP |
>> +		  BT_CR0_ENABLE_IBT,
>> +		  bt_bmc->base + BT_CR0);
>> +
>> +	clr_b_busy(bt_bmc);
>> +
>> +	return 0;
>> +
>> +out_unmap:
>> +	devm_iounmap(&pdev->dev, bt_bmc->base);
> 
> Why do you use devm_iounmap/devm_kfree since the interest with devm_ functions 
> is that all cleanup is done when driver is removed.

Indeed. So I can cleanup bt_bmc_remove() also.

>> +
>> +out_free:
>> +	devm_kfree(dev, bt_bmc);
>> +	return rc;
> 
> I think you should remove the space after this return


Thanks,

C.


> Regards
> 
> Corentin Labbe
> 

^ permalink raw reply

* [PATCHv9 0/6] dmaengine: rcar-dmac: add iommu support for slave transfers
From: Laurent Pinchart @ 2016-09-16 12:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4397626.d3mSLX4nFk@wuerfel>

Hi Arnd,

On Friday 16 Sep 2016 14:22:31 Arnd Bergmann wrote:
> On Friday, September 16, 2016 3:09:29 PM CEST Laurent Pinchart wrote:
> >> I wasn't thinking quite that far, though that is also a theoretical
> >> problem. However, the simple solution would be to have a bit in the DMA
> >> specifier let the driver know whether translation is needed or not.
> >> 
> >> The simpler case I was thinking of is where the entire DMA engine
> >> either goes through an IOMMU or doesn't (depending on the integration
> >> into the SoC), so we'd have to find out through some DT property
> >> or compatible string in the DMA enginen driver.
> > 
> > Don't we already get that information from the iommus DT property ? If the
> > DMA engine goes through an IOMMU the property will be set, otherwise it
> > will not.
>
> It depends. A dmaengine typically at least has two DMA masters,
> possibly more. It's likely that some dmaengine implementations are
> connected to RAM through an IOMMU, but have direct access to an
> I/O bus for the slave FIFOs.

Sure, but I expect the DMA engine DT node to list all the relevant IOMMU(s) 
(if any) in the iommus property in a way that allows the DMA engine driver to 
know what IOMMU port is used for what purpose. It will then be up to the DMA 
engine driver to select the right port identifier to pass to the DMA mapping 
API.

I'm not sure how this would work with Robin's proposal of creating one device 
per channel though, as there would still be a single node in DT for the DMA 
engine device. Furthermore, a single channel might indeed have multiple DMA 
masters, not all of them being served by an IOMMU. We would thus still need 
memory port identifiers in the DMA mapping API.

> >>> The problem is a bit broader than that, we'll also have an issue with
> >>> DMA engines that have different channels served by different IOMMUs.
> >> 
> >> Do you mean a theoretical problem, or a chip that you already know
> >> exists?
> > 
> > That's theoretical. The problem I'm facing today is a DMA engine whose
> > channels are served by different ports of the same IOMMU. This works in a
> > suboptimal way because I have to keep all the IOMMU ports enabled
> > regardless of whether they're used or not, as the DMA engine and IOMMU
> > APIs don't carry channel information.
> 
> Ok

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* genirq: Setting trigger mode 0 for irq 11 failed (txx9_irq_set_type+0x0/0xb8)
From: Marc Zyngier @ 2016-09-16 12:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474023805.17258.10.camel@gmail.com>

+Krzystof, Kukjin,

On 16/09/16 12:03, Alban Browaeys wrote:
> Le vendredi 16 septembre 2016 ? 08:51 +0100, Marc Zyngier a ?crit :
>> Hi Alban,
>>
>> On 16/09/16 00:02, Alban Browaeys wrote:
>>> I am seeing this on arm odroid u2 devicetree :
>>> genirq: Setting trigger mode 0 for irq 16 failed
>>> (gic_set_type+0x0/0x64)
>>
>> Passing IRQ_TYPE_NONE to a cascading interrupt is risky at best...
>> Can you point me to the various DTs and their failing interrupts?
> 
> mine is:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidu3.dts
> 
> I got a report of this issue to another odroid :
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidx2.dts
> 
> 
> 
> they both get their settings from :
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412.dtsi
> 
> relevant in the chain are:
> - combiner modified:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12.dtsi#n460

How wonderful. This section is an utter pile of crap. Really.
Having 0 as the trigger is illegal, and the valid values are fully
documented in the GIC binding. No wonder things start breaking.

> - gic:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi#n576
> - gic and combiner initial settings:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4.dtsi#n134
> 
> 
> 
>> Also, can you please give the following patch a go and let me know
>> if that fixes the issue (I'm interested in the potential warning
>> here).
> 
> 1st batch of warnings is :
> 
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at kernel/irq/chip.c:833 __irq_do_set_handler+0x1c0/0x1c4
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.8.0-rc6-debug+ #30
> Hardware name: ODROID-U2/U3
> [<c010fc74>] (unwind_backtrace) from [<c010c9a0>] (show_stack+0x10/0x14)
> [<c010c9a0>] (show_stack) from [<c035cafc>] (dump_stack+0xa8/0xd4)
> [<c035cafc>] (dump_stack) from [<c01214dc>] (__warn+0xe8/0x100)
> [<c01214dc>] (__warn) from [<c01215a4>] (warn_slowpath_null+0x20/0x28)
> [<c01215a4>] (warn_slowpath_null) from [<c017d394>] (__irq_do_set_handler+0x1c0/0x1c4)
> [<c017d394>] (__irq_do_set_handler) from [<c017d450>] (irq_set_chained_handler_and_data+0x38/0x54)
> [<c017d450>] (irq_set_chained_handler_and_data) from [<c0a15878>] (combiner_of_init+0x1a0/0x1c4)
> [<c0a15878>] (combiner_of_init) from [<c0a1ead4>] (of_irq_init+0x194/0x2e8)
> [<c0a1ead4>] (of_irq_init) from [<c0a07450>] (exynos_init_irq+0x8/0x3c)
> [<c0a07450>] (exynos_init_irq) from [<c0a0190c>] (init_IRQ+0x2c/0x88)
> [<c0a0190c>] (init_IRQ) from [<c0a00b78>] (start_kernel+0x284/0x388)
> [<c0a00b78>] (start_kernel) from [<40008078>] (0x40008078)
> ---[ end trace f68728a0d3053b52 ]---

That's our above friend the combiner.

> 
> 2nd batch is :
> 
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 1 at kernel/irq/chip.c:833 __irq_do_set_handler+0x1c0/0x1c4
> Modules linked in:
> CPU: 1 PID: 1 Comm: swapper/0 Tainted: G        W       4.8.0-rc6-debug+ #30
> Hardware name: ODROID-U2/U3
> [<c010fc74>] (unwind_backtrace) from [<c010c9a0>] (show_stack+0x10/0x14)
> [<c010c9a0>] (show_stack) from [<c035cafc>] (dump_stack+0xa8/0xd4)
> [<c035cafc>] (dump_stack) from [<c01214dc>] (__warn+0xe8/0x100)
> [<c01214dc>] (__warn) from [<c01215a4>] (warn_slowpath_null+0x20/0x28)
> [<c01215a4>] (warn_slowpath_null) from [<c017d394>] (__irq_do_set_handler+0x1c0/0x1c4)
> [<c017d394>] (__irq_do_set_handler) from [<c017d450>] (irq_set_chained_handler_and_data+0x38/0x54)
> [<c017d450>] (irq_set_chained_handler_and_data) from [<c038e340>] (exynos_eint_wkup_init+0x188/0x2dc)
> [<c038e340>] (exynos_eint_wkup_init) from [<c038d668>] (samsung_pinctrl_probe+0x874/0xa18)
> [<c038d668>] (samsung_pinctrl_probe) from [<c04342c8>] (platform_drv_probe+0x4c/0xb0)
> [<c04342c8>] (platform_drv_probe) from [<c043267c>] (driver_probe_device+0x24c/0x440)
> [<c043267c>] (driver_probe_device) from [<c0430658>] (bus_for_each_drv+0x64/0x98)
> [<c0430658>] (bus_for_each_drv) from [<c04322e8>] (__device_attach+0xb4/0x144)
> [<c04322e8>] (__device_attach) from [<c04316f4>] (bus_probe_device+0x88/0x90)
> [<c04316f4>] (bus_probe_device) from [<c042f850>] (device_add+0x428/0x5c8)
> [<c042f850>] (device_add) from [<c054c3f8>] (of_platform_device_create_pdata+0x84/0xb8)
> [<c054c3f8>] (of_platform_device_create_pdata) from [<c054c59c>] (of_platform_bus_create+0x164/0x440)
> [<c054c59c>] (of_platform_bus_create) from [<c054ca20>] (of_platform_populate+0x80/0x114)
> [<c054ca20>] (of_platform_populate) from [<c0a1d458>] (of_platform_default_populate_init+0x6c/0x80)
> [<c0a1d458>] (of_platform_default_populate_init) from [<c01018d4>] (do_one_initcall+0x50/0x198)
> [<c01018d4>] (do_one_initcall) from [<c0a00ecc>] (kernel_init_freeable+0x250/0x2f0)
> [<c0a00ecc>] (kernel_init_freeable) from [<c06c1064>] (kernel_init+0x8/0x114)
> [<c06c1064>] (kernel_init) from [<c0108710>] (ret_from_fork+0x14/0x24)
> ---[ end trace f68728a0d3053b66 ]---

And that's from the following stuff:

	&pinctrl_0 {
        	compatible = "samsung,exynos4x12-pinctrl";
	        reg = <0x11400000 0x1000>;
	        interrupts = <0 47 0>;
	};

	&pinctrl_1 {
	        compatible = "samsung,exynos4x12-pinctrl";
	        reg = <0x11000000 0x1000>;
	        interrupts = <0 46 0>;

	        wakup_eint: wakeup-interrupt-controller {
	                compatible = "samsung,exynos4210-wakeup-eint";
	                interrupt-parent = <&gic>;
	                interrupts = <0 32 0>;
	        };
	};

	[...]

	&pinctrl_3 {
	        compatible = "samsung,exynos4x12-pinctrl";
	        reg = <0x106E0000 0x1000>;
	        interrupts = <0 72 0>;
	};

which perpetuates this fine tradition...

At that stage, I'm not sure I should care. Does the workaround make your
platform usable? The Samsung maintainers should really try and fix their
DT, because it is a miracle this has made it that far.

I'm also interested in hearing from Geert, whose platform doesn't seem
to be DT driven.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH v3 0/2] KVM: ARM: Enable vtimers with user space gic
From: Paolo Bonzini @ 2016-09-16 12:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <10C5B047-6A7F-4255-BE56-5358AF88DA5E@suse.de>



On 16/09/2016 14:44, Alexander Graf wrote:
> 
>> On 16 Sep 2016, at 14:40, Paolo Bonzini <pbonzini@redhat.com>
>> wrote:
>> 
>> 
>> 
>> On 16/09/2016 14:29, Christoffer Dall wrote:
>>>> It may be useful for migrating a gicv2 VM to a gicv3 host
>>>> without gicv2 emulation as well.
>>> 
>>> I don't see why you'd do this; the VGIC hardware can perfectly
>>> well be used for nesting as well, and this works rather well.
>> 
>> Can GICv3 emulate GICv2 in a guest?
> 
> It depends on the gicv3 configuration. As an SOC vendor you can
> either enable gicv2 compatibility or disable it. ThunderX for example
> is gicv3 only.

And QEMU complains on startup and exits, I hope.

I am not too optimistic about having migration from kernel GIC to
userspace GIC, honestly.  But GICv2 emulation on GICv3-only hosts is a
very reasonable thing to want, if only for testing/debugging purposes.

Paolo

^ permalink raw reply

* [PATCHv9 0/6] dmaengine: rcar-dmac: add iommu support for slave transfers
From: Robin Murphy @ 2016-09-16 12:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1757972.L4qX7sAfEW@avalon>

On 16/09/16 13:05, Laurent Pinchart wrote:
[...]
>>>> One concern I have is that we might get an awkward situation if we ever
>>>> encounter one DMA engine hardware that is used in different systems that
>>>> all have an IOMMU, but on some of them the connection between the DMA
>>>> master and the slave FIFO bypasses the IOMMU while on others the IOMMU
>>>> is required.
>>>
>>> Do you mean systems where some of the channels of a specific DMA engine go
>>> through the IOMMU while others do not ? We indeed have no solution today
>>> for such a situation.
>>>
>>> The problem is a bit broader than that, we'll also have an issue with DMA
>>> engines that have different channels served by different IOMMUs. I recall
>>> discussing this in the past with you, and the solution you proposed was to
>>> add a channel index to struct dma_attrs seems good to me. To support the
>>> case where some channels don't go through an IOMMU we would only need
>>> support for null entries in the IOMMUs list associated with a device (for
>>> instance in the DT case null entries in the iommus property).
>>
>> I think at that point we just create the channels as child devices of
>> the main dmaengine device so they each get their own DMA ops, and can do
>> whatever. The Qualcomm HIDMA driver already does that for a very similar
>> reason (so that the IOMMU can map individual channels into different
>> guest VMs).
> 
> That's another option, but it seems more like a workaround to me, instead of a 
> proper solution to fix the more global problem of multiple memory paths within 
> a single device. I have other hardware devices that can act as bus masters 
> through different paths (for instance a display-related device that fetches 
> data and commands through different paths). Luckily so far all those paths are 
> served by the same IOMMU, but there's no guarantee this will remain true in 
> the future. Furthermore, even today, the IOMMU connected to that device has 
> the ability to selectively enable and disable its ports. I have to keep them 
> all enabled due to the lack of channel information in the DMA mapping and 
> IOMMU APIs, leading to increased power consumption.

Indeed, I think both the Exynos and Rockchip IOMMU drivers already do
cater for a device mastering though multiple discrete IOMMUs, not being
the fancy multi-port multi-context ones like yours and mine.

I guess what we could really do with is a decent abstraction of
multi-master peripherals at the device level; a "threads within the same
process" sort of granularity, as it were. I'd envisage it more along the
lines of how we handle NUMA, i.e. dma_map_page_attrs(...) becomes a
wrapper for dma_map_page_attrs_multi(..., CHANNEL_ALL), and trickier
users can call the latter with the a more specific channel(s) argument
(maybe it's a bitmask rather than an index). Meanwhile,
dev->archdata.dma_ops may point to a device-specific array of
dma_map_ops, which the DMA API backend iterates over if necessary.

Strangely, that doesn't actually sound too horrible.

Robin.

> 
>>> Now I see that struct dma_attrs has been replaced by unsigned long in
>>>
>>> commit 00085f1efa387a8ce100e3734920f7639c80caa3
>>> Author: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>> Date:   Wed Aug 3 13:46:00 2016 -0700
>>>
>>>     dma-mapping: use unsigned long for dma_attrs
>>>
>>> We still have enough bits to reserve some of them for a channel number,
>>> but I'm not very happy with that patch as I can see how a future proposal
>>> to handle the channel number through the DMA attributes will get rejected
>>> on the grounds of bits starvation then :-(
>>>
>>>> I don't have any idea for how this could be handled in a generic way, so
>>>> my best answer here is to hope we never get there, and if we do, handle
>>>> it using some local hack in the driver.
> 

^ permalink raw reply

* [PATCH V6 0/3] Adding support for address filters
From: Adrian Hunter @ 2016-09-16 12:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473957428-28311-1-git-send-email-mathieu.poirier@linaro.org>

On 15/09/16 19:37, Mathieu Poirier wrote:
> This patch set makes it possible to use the current filter
> framework with address filters.  That way address filters for
> HW tracers such as CoreSight and IntelPT can be communicated
> to the kernel drivers.

FYI, I am working on patches to add support for entering address filters
using symbol names.  I will probably send them next week.

> 
> In this revision precursor work is done to make function
> perf_evsel__append_filter() generic, along with changes to
> current customers.  From there the work on address filters is
> introduced.
> 
> Thanks,
> Mathieu
> 
> ---
> Changes for V6:
>  - Split work in 3 (small) patches.
>  - Adding tracepoint and address filter append() functions
> 
> Changes for V5:
>  - Modified perf_evsel__append_filter() to take a string format
>    rather than an operation.
> 
> Changes for V4:
>  - Added support for address filters over more than one
>    nibble.
>  - Removed Jiri's ack, this version is too different from
>    what was reviewed.
> 
> Changes for V3:
>  - Added Jiri's ack.
>  - Rebased to v4.8-rc5.
> 
> Changes for V2:
>  - Rebased to v4.8-rc4.
>  - Revisited error path.
> 
> Mathieu Poirier (3):
>   perf tools: making perf_evsel__append_filter() generic
>   perf tools: new tracepoint specific function
>   perf tools: adding support for address filters
> 
>  tools/perf/builtin-trace.c     |  8 ++++++--
>  tools/perf/util/evsel.c        | 16 +++++++++++++---
>  tools/perf/util/evsel.h        |  5 +++--
>  tools/perf/util/parse-events.c | 41 +++++++++++++++++++++++++++++++++++------
>  4 files changed, 57 insertions(+), 13 deletions(-)
> 

^ permalink raw reply

* [PATCH v3 0/2] KVM: ARM: Enable vtimers with user space gic
From: Alexander Graf @ 2016-09-16 12:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <6b92c411-8b27-a4c0-6969-38e32bbf18ab@redhat.com>


> On 16 Sep 2016, at 14:40, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> 
> 
> On 16/09/2016 14:29, Christoffer Dall wrote:
>>> It may be useful for migrating a gicv2 VM to a gicv3 host without gicv2 emulation as well.
>> 
>> I don't see why you'd do this; the VGIC hardware can perfectly well be
>> used for nesting as well, and this works rather well.
> 
> Can GICv3 emulate GICv2 in a guest?

It depends on the gicv3 configuration. As an SOC vendor you can either enable gicv2 compatibility or disable it. ThunderX for example is gicv3 only. LS2085 can handle gicv2 in the guest with gicv3 on the host.


Alex

^ permalink raw reply

* [PATCH V6 3/3] perf tools: adding support for address filters
From: Adrian Hunter @ 2016-09-16 12:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473957428-28311-4-git-send-email-mathieu.poirier@linaro.org>

On 15/09/16 19:37, Mathieu Poirier wrote:
> This patch makes it possible to use the current filter
> framework with address filters.  That way address filters for
> HW tracers such as CoreSight and IntelPT can be communicated

IntelPT -> Intel PT

> to the kernel drivers.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Otherwise:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  tools/perf/util/evsel.c        |  5 +++++
>  tools/perf/util/evsel.h        |  2 ++
>  tools/perf/util/parse-events.c | 39 ++++++++++++++++++++++++++++++++++-----
>  3 files changed, 41 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index a99e82d97df2..e0bb399dcdd4 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1067,6 +1067,11 @@ int perf_evsel__append_tp_filter(struct perf_evsel *evsel, const char *filter)
>  	return perf_evsel__append_filter(evsel, "(%s) && (%s)", filter);
>  }
>  
> +int perf_evsel__append_addr_filter(struct perf_evsel *evsel, const char *filter)
> +{
> +	return perf_evsel__append_filter(evsel, "%s,%s", filter);
> +}
> +
>  int perf_evsel__enable(struct perf_evsel *evsel)
>  {
>  	int nthreads = thread_map__nr(evsel->threads);
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 36ed0997e65b..49c51fb3d05c 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -234,6 +234,8 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
>  
>  int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
>  int perf_evsel__append_tp_filter(struct perf_evsel *evsel, const char *filter);
> +int perf_evsel__append_addr_filter(struct perf_evsel *evsel,
> +				   const char *filter);
>  int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
>  			     const char *filter);
>  int perf_evsel__enable(struct perf_evsel *evsel);
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 751b48fc641c..c23f2d5fc134 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1755,20 +1755,49 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
>  static int set_filter(struct perf_evsel *evsel, const void *arg)
>  {
>  	const char *str = arg;
> +	bool found = false;
> +	int nr_addr_filters = 0;
> +	struct perf_pmu *pmu = NULL;
>  
> -	if (evsel == NULL || evsel->attr.type != PERF_TYPE_TRACEPOINT) {
> -		fprintf(stderr,
> -			"--filter option should follow a -e tracepoint option\n");
> -		return -1;
> +	if (evsel == NULL)
> +		goto err;
> +
> +	if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
> +		if (perf_evsel__append_tp_filter(evsel, str) < 0) {
> +			fprintf(stderr,
> +				"not enough memory to hold filter string\n");
> +			return -1;
> +		}
> +
> +		return 0;
>  	}
>  
> -	if (perf_evsel__append_tp_filter(evsel, str) < 0) {
> +	while ((pmu = perf_pmu__scan(pmu)) != NULL)
> +		if (pmu->type == evsel->attr.type) {
> +			found = true;
> +			break;
> +		}
> +
> +	if (found)
> +		perf_pmu__scan_file(pmu, "nr_addr_filters",
> +				    "%d", &nr_addr_filters);
> +
> +	if (!nr_addr_filters)
> +		goto err;
> +
> +	if (perf_evsel__append_addr_filter(evsel, str) < 0) {
>  		fprintf(stderr,
>  			"not enough memory to hold filter string\n");
>  		return -1;
>  	}
>  
>  	return 0;
> +
> +err:
> +	fprintf(stderr,
> +		"--filter option should follow a -e tracepoint or HW tracer option\n");
> +
> +	return -1;
>  }
>  
>  int parse_filter(const struct option *opt, const char *str,
> 

^ permalink raw reply

* [PATCH V6 2/3] perf tools: new tracepoint specific function
From: Adrian Hunter @ 2016-09-16 12:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473957428-28311-3-git-send-email-mathieu.poirier@linaro.org>

On 15/09/16 19:37, Mathieu Poirier wrote:
> Making function perf_evsel__append_filter() static and
> introducing a new tracepoint specific function to append
> filters.  That way we eliminate redundant code and avoid
> formatting mistake.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  tools/perf/builtin-trace.c     | 7 +++----
>  tools/perf/util/evsel.c        | 9 +++++++--
>  tools/perf/util/evsel.h        | 3 +--
>  tools/perf/util/parse-events.c | 4 ++--
>  4 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 4c2704332c14..34effab09f39 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -2148,11 +2148,10 @@ static int trace__set_ev_qualifier_filter(struct trace *trace)
>  	if (filter == NULL)
>  		goto out_enomem;
>  
> -	if (!perf_evsel__append_filter(trace->syscalls.events.sys_enter,
> -				       "(%s) && (%s)", filter)) {
> +	if (!perf_evsel__append_tp_filter(trace->syscalls.events.sys_enter,
> +					  filter)) {
>  		sys_exit = trace->syscalls.events.sys_exit;
> -		err = perf_evsel__append_filter(sys_exit,
> -						"(%s) && (%s)", filter);
> +		err = perf_evsel__append_tp_filter(sys_exit, filter);
>  	}
>  
>  	free(filter);
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index b93369745e08..a99e82d97df2 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1045,8 +1045,8 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
>  	return -1;
>  }
>  
> -int perf_evsel__append_filter(struct perf_evsel *evsel,
> -			      const char *fmt, const char *filter)
> +static int perf_evsel__append_filter(struct perf_evsel *evsel,
> +				     const char *fmt, const char *filter)
>  {
>  	char *new_filter;
>  
> @@ -1062,6 +1062,11 @@ int perf_evsel__append_filter(struct perf_evsel *evsel,
>  	return -1;
>  }
>  
> +int perf_evsel__append_tp_filter(struct perf_evsel *evsel, const char *filter)
> +{
> +	return perf_evsel__append_filter(evsel, "(%s) && (%s)", filter);
> +}
> +
>  int perf_evsel__enable(struct perf_evsel *evsel)
>  {
>  	int nthreads = thread_map__nr(evsel->threads);
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 7ab59f15892f..36ed0997e65b 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -233,8 +233,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
>  			       bool use_sample_identifier);
>  
>  int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
> -int perf_evsel__append_filter(struct perf_evsel *evsel,
> -			      const char *fmt, const char *filter);
> +int perf_evsel__append_tp_filter(struct perf_evsel *evsel, const char *filter);
>  int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
>  			     const char *filter);
>  int perf_evsel__enable(struct perf_evsel *evsel);
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 9692300585f6..751b48fc641c 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1762,7 +1762,7 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
>  		return -1;
>  	}
>  
> -	if (perf_evsel__append_filter(evsel, "(%s) && (%s)", str) < 0) {
> +	if (perf_evsel__append_tp_filter(evsel, str) < 0) {
>  		fprintf(stderr,
>  			"not enough memory to hold filter string\n");
>  		return -1;
> @@ -1793,7 +1793,7 @@ static int add_exclude_perf_filter(struct perf_evsel *evsel,
>  
>  	snprintf(new_filter, sizeof(new_filter), "common_pid != %d", getpid());
>  
> -	if (perf_evsel__append_filter(evsel, "(%s) && (%s)", new_filter) < 0) {
> +	if (perf_evsel__append_tp_filter(evsel, new_filter) < 0) {
>  		fprintf(stderr,
>  			"not enough memory to hold filter string\n");
>  		return -1;
> 

^ permalink raw reply

* [PATCH V6 1/3] perf tools: making perf_evsel__append_filter() generic
From: Adrian Hunter @ 2016-09-16 12:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473957428-28311-2-git-send-email-mathieu.poirier@linaro.org>

On 15/09/16 19:37, Mathieu Poirier wrote:
> By making function perf_evsel__append_filter() take a format rather
> than an operator it is possible to reuse the code for other purposes
> (ex. intelPT and CoreSight) than tracepoints.

intelPT -> Intel PT

> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Otherwise:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  tools/perf/builtin-trace.c     | 9 +++++++--
>  tools/perf/util/evsel.c        | 4 ++--
>  tools/perf/util/evsel.h        | 2 +-
>  tools/perf/util/parse-events.c | 4 ++--
>  4 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index b8c6766301db..4c2704332c14 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -2140,6 +2140,7 @@ out_delete_sys_enter:
>  static int trace__set_ev_qualifier_filter(struct trace *trace)
>  {
>  	int err = -1;
> +	struct perf_evsel *sys_exit;
>  	char *filter = asprintf_expr_inout_ints("id", !trace->not_ev_qualifier,
>  						trace->ev_qualifier_ids.nr,
>  						trace->ev_qualifier_ids.entries);
> @@ -2147,8 +2148,12 @@ static int trace__set_ev_qualifier_filter(struct trace *trace)
>  	if (filter == NULL)
>  		goto out_enomem;
>  
> -	if (!perf_evsel__append_filter(trace->syscalls.events.sys_enter, "&&", filter))
> -		err = perf_evsel__append_filter(trace->syscalls.events.sys_exit, "&&", filter);
> +	if (!perf_evsel__append_filter(trace->syscalls.events.sys_enter,
> +				       "(%s) && (%s)", filter)) {
> +		sys_exit = trace->syscalls.events.sys_exit;
> +		err = perf_evsel__append_filter(sys_exit,
> +						"(%s) && (%s)", filter);
> +	}
>  
>  	free(filter);
>  out:
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 21fd573106ed..b93369745e08 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1046,14 +1046,14 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter)
>  }
>  
>  int perf_evsel__append_filter(struct perf_evsel *evsel,
> -			      const char *op, const char *filter)
> +			      const char *fmt, const char *filter)
>  {
>  	char *new_filter;
>  
>  	if (evsel->filter == NULL)
>  		return perf_evsel__set_filter(evsel, filter);
>  
> -	if (asprintf(&new_filter,"(%s) %s (%s)", evsel->filter, op, filter) > 0) {
> +	if (asprintf(&new_filter, fmt, evsel->filter, filter) > 0) {
>  		free(evsel->filter);
>  		evsel->filter = new_filter;
>  		return 0;
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 4d44129e050b..7ab59f15892f 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -234,7 +234,7 @@ void perf_evsel__set_sample_id(struct perf_evsel *evsel,
>  
>  int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
>  int perf_evsel__append_filter(struct perf_evsel *evsel,
> -			      const char *op, const char *filter);
> +			      const char *fmt, const char *filter);
>  int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
>  			     const char *filter);
>  int perf_evsel__enable(struct perf_evsel *evsel);
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 6c913c3914fb..9692300585f6 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1762,7 +1762,7 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
>  		return -1;
>  	}
>  
> -	if (perf_evsel__append_filter(evsel, "&&", str) < 0) {
> +	if (perf_evsel__append_filter(evsel, "(%s) && (%s)", str) < 0) {
>  		fprintf(stderr,
>  			"not enough memory to hold filter string\n");
>  		return -1;
> @@ -1793,7 +1793,7 @@ static int add_exclude_perf_filter(struct perf_evsel *evsel,
>  
>  	snprintf(new_filter, sizeof(new_filter), "common_pid != %d", getpid());
>  
> -	if (perf_evsel__append_filter(evsel, "&&", new_filter) < 0) {
> +	if (perf_evsel__append_filter(evsel, "(%s) && (%s)", new_filter) < 0) {
>  		fprintf(stderr,
>  			"not enough memory to hold filter string\n");
>  		return -1;
> 

^ permalink raw reply

* [PATCH v3 0/2] KVM: ARM: Enable vtimers with user space gic
From: Alexander Graf @ 2016-09-16 12:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160916122937.GA14140@cbox>


> On 16 Sep 2016, at 14:29, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> 
> On Fri, Sep 16, 2016 at 02:25:01PM +0200, Alexander Graf wrote:
>> 
>>> On 16 Sep 2016, at 12:20, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> 
>>> Hi Alex,
>>> 
>>> On 16/09/16 07:26, Alexander Graf wrote:
>>>> Some systems out there (well, one type in particular - the Raspberry Pi series)
>>>> do have virtualization capabilities in the core, but no ARM GIC interrupt
>>>> controller.
>>>> 
>>>> To run on these systems, the cleanest route is to just handle all
>>>> interrupt delivery in user space and only deal with IRQ pins in the core
>>>> side in KVM.
>>>> 
>>>> This works pretty well already, but breaks when the guest starts to use
>>>> architected timers, as these are handled straight inside kernel space today.
>>>> 
>>>> This patch set allows user space to receive vtimer events as well as mask
>>>> them, so that we can handle all vtimer related interrupt injection from user
>>>> space, enabling us to use architected timer with user space gic emulation.
>>> 
>>> I have already voiced my concerns in the past, including face to face,
>>> and I'm going to repeat it: I not keen at all on adding a new userspace
>>> interface that is going to bitrot extremely quickly.
>>> 
>>> Let's face it, this new ABI will have a single user, with a limited
>>> shelf life. I understand that the RPi is a popular product, but it looks
>>> fairly obvious that this kind of sub-standard HW will eventually
>>> disappear. We'll then be left with a userspace ABI that will break at
>> 
>> I?m not 100% convinced that this is the case. Emulating the GIC in user space can have other interesting use cases. For example, it might come in handy for nesting. It may be useful for migrating a gicv2 VM to a gicv3 host without gicv2 emulation as well.
>> 
> 
> I don't see why you'd do this; the VGIC hardware can perfectly well be
> used for nesting as well, and this works rather well.

Mostly because it?s more. I like my problems self-contained, and simulating only nesting on the CPU level is less to worry about than simulating vgic switchover as well. Of course eventually with nesting you?d want a nested vgic ;).

> 
>>> every single release, given that nobody in the RPi community actually
>>> uses a mainline kernel.
>> 
>> I actually verified all of this patch on 4.8-rc5 upstream, which is the only 64bit kernel you can find for the RPi. So I?d expect the situation to change with 64bit.
>> 
>>> And breaking this ABI will introduce userspace exploitable bugs, like
>>> the one you've already shown. If anything, I would have loved to
>>> completely kill the whole userspace GIC, because nobody cares. Yes, I
>>> understand it is fun to have KVM running on the RPi. But the maintenance
>>> costs far outweigh the fun aspect already.
>> 
>> Having CPU pins accessible is very useful for use cases of KVM that are beyond your traditional VM.
>> 
>>> You could still run KVM with an external emulated timer (not the arch
>>> timer). No need for a new ABI for that.
>> 
>> That?s what I thought too, but turns out that it?s not quite as simple as I hoped ;).
> 
> Why not?  I thought a few people had this running recently.  What were
> the issues?  Perhaps I can convince someone to submit the patches they
> used if useful.

Just give it a try - I didn?t get any timer events and couldn?t quite figure out why. Patch is attached below.


Alex

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a193b5a..f118ab8 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -91,6 +91,7 @@ typedef struct {
     bool secure;
     bool highmem;
     int32_t gic_version;
+    bool archtimer;
 } VirtMachineState;

 #define TYPE_VIRT_MACHINE   MACHINE_TYPE_NAME("virt")
@@ -177,6 +178,7 @@ static const MemMapEntry a15memmap[] = {
     [VIRT_FW_CFG] =             { 0x09020000, 0x00000018 },
     [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
     [VIRT_SECURE_UART] =        { 0x09040000, 0x00001000 },
+    [VIRT_SP804] =              { 0x09050000, 0x00001000 },
     [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
     /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
     [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
@@ -195,6 +197,7 @@ static const int a15irqmap[] = {
     [VIRT_PCIE] = 3, /* ... to 6 */
     [VIRT_GPIO] = 7,
     [VIRT_SECURE_UART] = 8,
+    [VIRT_SP804] = 9,
     [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
     [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
     [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
@@ -352,11 +355,13 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi, int gictype)
                                 "arm,armv7-timer");
     }
     qemu_fdt_setprop(vbi->fdt, "/timer", "always-on", NULL, 0);
+#if 0
     qemu_fdt_setprop_cells(vbi->fdt, "/timer", "interrupts",
                        GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_S_EL1_IRQ, irqflags,
                        GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_NS_EL1_IRQ, irqflags,
                        GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_VIRT_IRQ, irqflags,
                        GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_NS_EL2_IRQ, irqflags);
+#endif
 }

 static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
@@ -655,6 +660,29 @@ static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
     g_free(nodename);
 }

+static void create_sp804(const VirtBoardInfo *vbi, qemu_irq *pic)
+{
+    char *nodename;
+    hwaddr base = vbi->memmap[VIRT_SP804].base;
+    hwaddr size = vbi->memmap[VIRT_SP804].size;
+    int irq = vbi->irqmap[VIRT_SP804];
+    const char compat[] = "arm,sp804\0arm,primecell";
+
+    sysbus_create_simple("sp804", base, pic[irq]);
+
+    nodename = g_strdup_printf("/sp804@%" PRIx64, base);
+    qemu_fdt_add_subnode(vbi->fdt, nodename);
+    qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
+                                 2, base, 2, size);
+    qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupts",
+                           GIC_FDT_IRQ_TYPE_SPI, irq,
+                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+    qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle);
+    qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk");
+    g_free(nodename);
+}
+
 static DeviceState *gpio_key_dev;
 static void virt_powerdown_req(Notifier *n, void *opaque)
 {
@@ -1354,6 +1385,10 @@ static void machvirt_init(MachineState *machine)

     create_rtc(vbi, pic);

+    if (!vms->archtimer) {
+        create_sp804(vbi, pic);
+    }
+
     create_pcie(vbi, pic, vms->highmem);

     create_gpio(vbi, pic);
@@ -1448,6 +1483,20 @@ static void virt_set_gic_version(Object *obj, const char *value, Error **errp)
     }
 }

+static bool virt_get_archtimer(Object *obj, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    return vms->archtimer;
+}
+
+static void virt_set_archtimer(Object *obj, bool value, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    vms->archtimer = value;
+}
+
 static void virt_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -1510,6 +1559,15 @@ static void virt_2_7_instance_init(Object *obj)
     object_property_set_description(obj, "gic-version",
                                     "Set GIC version. "
                                     "Valid values are 2, 3 and host", NULL);
+
+    /* Architected Timers are available by default */
+    vms->archtimer = true;
+    object_property_add_bool(obj, "archtimer", virt_get_archtimer,
+                             virt_set_archtimer, NULL);
+    object_property_set_description(obj, "archtimer",
+                                    "Set on/off to enable/disable exposure "
+                                    "of architected timers to the guest",
+                                    NULL);
 }

 static void virt_machine_2_7_options(MachineClass *mc)
diff --git a/hw/timer/arm_timer.c b/hw/timer/arm_timer.c
index 111a16d..b71db7e 100644
--- a/hw/timer/arm_timer.c
+++ b/hw/timer/arm_timer.c
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 9650193..510cdc0 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -67,6 +67,7 @@ enum {
     VIRT_GPIO,
     VIRT_SECURE_UART,
     VIRT_SECURE_MEM,
+    VIRT_SP804,
 };

 typedef struct MemMapEntry {

^ permalink raw reply related

* [PATCH v3 0/2] KVM: ARM: Enable vtimers with user space gic
From: Paolo Bonzini @ 2016-09-16 12:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160916122937.GA14140@cbox>



On 16/09/2016 14:29, Christoffer Dall wrote:
> > It may be useful for migrating a gicv2 VM to a gicv3 host without gicv2 emulation as well.
> 
> I don't see why you'd do this; the VGIC hardware can perfectly well be
> used for nesting as well, and this works rather well.

Can GICv3 emulate GICv2 in a guest?

Paolo

^ permalink raw reply

* [PATCH v3 0/2] KVM: ARM: Enable vtimers with user space gic
From: Paolo Bonzini @ 2016-09-16 12:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160916123039.GB14140@cbox>



On 16/09/2016 14:30, Christoffer Dall wrote:
> > > > This patch set allows user space to receive vtimer events as well as mask
> > > > them, so that we can handle all vtimer related interrupt injection from user
> > > > space, enabling us to use architected timer with user space gic emulation.
> > >
> > > I have already voiced my concerns in the past, including face to face,
> > > and I'm going to repeat it: I not keen at all on adding a new userspace
> > > interface that is going to bitrot extremely quickly.
> > 
> > You don't have automated tests set up?  It's not going to bitrot if you
> > test it, either with kvm-unit-tests or just by smoke-testing Linux.
> > It's _for_ the raspi, but it's not limited to it.
> 
> Our automated testing situation is not great, no.  Something we're
> looking at, but have resource problems with.

But it's not a good reason to hold back a feature...

Paolo

^ permalink raw reply

* [PATCH v3 0/2] KVM: ARM: Enable vtimers with user space gic
From: Christoffer Dall @ 2016-09-16 12:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <7beb406b-6194-8451-2d92-9a4394ef91dd@redhat.com>

On Fri, Sep 16, 2016 at 02:18:45PM +0200, Paolo Bonzini wrote:
> 
> 
> On 16/09/2016 12:20, Marc Zyngier wrote:
> > > This patch set allows user space to receive vtimer events as well as mask
> > > them, so that we can handle all vtimer related interrupt injection from user
> > > space, enabling us to use architected timer with user space gic emulation.
> >
> > I have already voiced my concerns in the past, including face to face,
> > and I'm going to repeat it: I not keen at all on adding a new userspace
> > interface that is going to bitrot extremely quickly.
> 
> You don't have automated tests set up?  It's not going to bitrot if you
> test it, either with kvm-unit-tests or just by smoke-testing Linux.
> It's _for_ the raspi, but it's not limited to it.
> 

Our automated testing situation is not great, no.  Something we're
looking at, but have resource problems with.

-Christoffer

^ permalink raw reply

* [PATCH v2 1/3] ipmi: add an Aspeed BT IPMI BMC driver
From: LABBE Corentin @ 2016-09-16 12:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474022367-21029-2-git-send-email-clg@kaod.org>

Hello

I have some minor comment below:

On Fri, Sep 16, 2016 at 12:39:25PM +0200, C?dric Le Goater wrote:
> From: Alistair Popple <alistair@popple.id.au>
> 
> This patch adds a simple device driver to expose the iBT interface on
> Aspeed SOCs (AST2400 and AST2500) as a character device. Such SOCs are
> commonly used as BMCs (BaseBoard Management Controllers) and this
> driver implements the BMC side of the BT interface.
> 
> The BT (Block Transfer) interface is used to perform in-band IPMI
> communication between a host and its BMC. Entire messages are buffered
> before sending a notification to the other end, host or BMC, that
> there is data to be read. Usually, the host emits requests and the BMC
> responses but the specification provides a mean for the BMC to send
> SMS Attention (BMC-to-Host attention or System Management Software
> attention) messages.
> 
> For this purpose, the driver introduces a specific ioctl on the
> device: 'BT_BMC_IOCTL_SMS_ATN' that can be used by the system running
> on the BMC to signal the host of such an event.
> 
> The device name defaults to '/dev/ipmi-bt-host'
> 
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> [clg: - checkpatch fixes
>       - added a devicetree binding documentation
>       - replace 'bt_host' by 'bt_bmc' to reflect that the driver is
>         the BMC side of the IPMI BT interface
>       - renamed the device to 'ipmi-bt-host'
>       - introduced a temporary buffer to copy_{to,from}_user
>       - used platform_get_irq()
>       - moved the driver under drivers/char/ipmi/ but kept it as a misc
>         device
>       - changed the compatible cell to "aspeed,ast2400-bt-bmc"
> ]
> Signed-off-by: C?dric Le Goater <clg@kaod.org>
> ---
> 
>  Changes since v1:
> 
>  - replace 'bt_host' by 'bt_bmc' to reflect that the driver is
>    the BMC side of the IPMI BT interface
>  - renamed the device to 'ipmi-bt-host'
>  - introduced a temporary buffer to copy_{to,from}_user
>  - used platform_get_irq()
>  - moved the driver under drivers/char/ipmi/ but kept it as a misc
>    device
>  - changed the compatible cell to "aspeed,ast2400-bt-bmc"
> 
>  .../bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt   |  23 +
>  drivers/Makefile                                   |   2 +-
>  drivers/char/ipmi/Kconfig                          |   7 +
>  drivers/char/ipmi/Makefile                         |   1 +
>  drivers/char/ipmi/bt-bmc.c                         | 486 +++++++++++++++++++++
>  include/uapi/linux/Kbuild                          |   1 +
>  include/uapi/linux/bt-bmc.h                        |  18 +
>  7 files changed, 537 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt
>  create mode 100644 drivers/char/ipmi/bt-bmc.c
>  create mode 100644 include/uapi/linux/bt-bmc.h
> 
> diff --git a/Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt b/Documentation/devicetree/bindings/char/ipmi/aspeed,ast2400-bt-bmc.txt
> new file mode 100644
> index 000000000000..fbbacd958240
> --- /dev/null

[..]

> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/errno.h>
> +#include <linux/poll.h>
> +#include <linux/sched.h>
> +#include <linux/spinlock.h>
> +#include <linux/slab.h>
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/miscdevice.h>
> +#include <linux/timer.h>
> +#include <linux/jiffies.h>
> +#include <linux/bt-bmc.h>

Please sort them in alphabetical order, some of them seems not needed also (like spinlock.h)

> +
> +/*
> + * This is a BMC device used to communicate to the host
> + */
> +#define DEVICE_NAME	"ipmi-bt-host"
> +
> +#define BT_IO_BASE	0xe4
> +#define BT_IRQ		10
> +
> +#define BT_CR0		0x0
> +#define   BT_CR0_IO_BASE		16
> +#define   BT_CR0_IRQ			12
> +#define   BT_CR0_EN_CLR_SLV_RDP		0x8
> +#define   BT_CR0_EN_CLR_SLV_WRP		0x4
> +#define   BT_CR0_ENABLE_IBT		0x1
> +#define BT_CR1		0x4
> +#define   BT_CR1_IRQ_H2B	0x01
> +#define   BT_CR1_IRQ_HBUSY	0x40
> +#define BT_CR2		0x8
> +#define   BT_CR2_IRQ_H2B	0x01
> +#define   BT_CR2_IRQ_HBUSY	0x40
> +#define BT_CR3		0xc
> +#define BT_CTRL		0x10
> +#define   BT_CTRL_B_BUSY		0x80
> +#define   BT_CTRL_H_BUSY		0x40
> +#define   BT_CTRL_OEM0			0x20
> +#define   BT_CTRL_SMS_ATN		0x10
> +#define   BT_CTRL_B2H_ATN		0x08
> +#define   BT_CTRL_H2B_ATN		0x04
> +#define   BT_CTRL_CLR_RD_PTR		0x02
> +#define   BT_CTRL_CLR_WR_PTR		0x01
> +#define BT_BMC2HOST	0x14
> +#define BT_INTMASK	0x18
> +#define   BT_INTMASK_B2H_IRQEN		0x01
> +#define   BT_INTMASK_B2H_IRQ		0x02
> +#define   BT_INTMASK_BMC_HWRST		0x80

Why to use 3 space after some define ?

[..]

> +
> +#define BT_BMC_BUFFER_SIZE 256

Put this define with others

[..]

> +
> +static irqreturn_t bt_bmc_irq(int irq, void *arg)
> +{
> +	struct bt_bmc *bt_bmc = arg;
> +	uint32_t reg;

u32 is prefered other uint32_t, do you have run checkpatch --strict ?

> +
> +	reg = ioread32(bt_bmc->base + BT_CR2);
> +	reg &= BT_CR2_IRQ_H2B | BT_CR2_IRQ_HBUSY;
> +	if (!reg)
> +		return IRQ_NONE;
> +
> +	/* ack pending IRQs */
> +	iowrite32(reg, bt_bmc->base + BT_CR2);
> +
> +	wake_up(&bt_bmc->queue);
> +	return IRQ_HANDLED;
> +}
> +
> +static int bt_bmc_config_irq(struct bt_bmc *bt_bmc,
> +		struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	uint32_t reg;
> +	int rc;
> +
> +	bt_bmc->irq = platform_get_irq(pdev, 0);
> +	if (!bt_bmc->irq)
> +		return -ENODEV;
> +
> +	rc = devm_request_irq(dev, bt_bmc->irq, bt_bmc_irq, IRQF_SHARED,
> +			DEVICE_NAME, bt_bmc);
> +	if (rc < 0) {
> +		dev_warn(dev, "Unable to request IRQ %d\n", bt_bmc->irq);
> +		bt_bmc->irq = 0;
> +		return rc;
> +	}
> +
> +	/* Configure IRQs on the bmc clearing the H2B and HBUSY bits;
> +	 * H2B will be asserted when the bmc has data for us; HBUSY
> +	 * will be cleared (along with B2H) when we can write the next
> +	 * message to the BT buffer
> +	 */

This comment doesnt have the style recommended for new driver.

> +	reg = ioread32(bt_bmc->base + BT_CR1);
> +	reg |= BT_CR1_IRQ_H2B | BT_CR1_IRQ_HBUSY;
> +	iowrite32(reg, bt_bmc->base + BT_CR1);
> +
> +	return 0;
> +}
> +
> +static int bt_bmc_probe(struct platform_device *pdev)
> +{
> +	struct bt_bmc *bt_bmc;
> +	struct device *dev;
> +	struct resource *res;
> +	int rc;
> +
> +	if (!pdev || !pdev->dev.of_node)
> +		return -ENODEV;
> +
> +	dev = &pdev->dev;
> +	dev_info(dev, "Found bt bmc device\n");
> +
> +	bt_bmc = devm_kzalloc(dev, sizeof(*bt_bmc), GFP_KERNEL);
> +	if (!bt_bmc)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(&pdev->dev, bt_bmc);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(dev, "Unable to find resources\n");
> +		rc = -ENXIO;
> +		goto out_free;
> +	}
> +
> +	bt_bmc->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (!bt_bmc->base) {
> +		rc = -ENOMEM;
> +		goto out_free;
> +	}
> +
> +	init_waitqueue_head(&bt_bmc->queue);
> +
> +	bt_bmc->miscdev.minor	= MISC_DYNAMIC_MINOR,
> +	bt_bmc->miscdev.name	= DEVICE_NAME,
> +	bt_bmc->miscdev.fops	= &bt_bmc_fops,
> +	bt_bmc->miscdev.parent = dev;
> +	rc = misc_register(&bt_bmc->miscdev);
> +	if (rc) {
> +		dev_err(dev, "Unable to register device\n");

Be more precise by saying misc device

> +		goto out_unmap;
> +	}
> +
> +	bt_bmc_config_irq(bt_bmc, pdev);
> +
> +	if (bt_bmc->irq) {
> +		dev_info(dev, "Using IRQ %d\n", bt_bmc->irq);
> +	} else {
> +		dev_info(dev, "No IRQ; using timer\n");
> +		setup_timer(&bt_bmc->poll_timer, poll_timer,
> +				(unsigned long)bt_bmc);
> +		bt_bmc->poll_timer.expires = jiffies + msecs_to_jiffies(10);
> +		add_timer(&bt_bmc->poll_timer);
> +	}
> +
> +	iowrite32((BT_IO_BASE << BT_CR0_IO_BASE) |
> +		  (BT_IRQ << BT_CR0_IRQ) |
> +		  BT_CR0_EN_CLR_SLV_RDP |
> +		  BT_CR0_EN_CLR_SLV_WRP |
> +		  BT_CR0_ENABLE_IBT,
> +		  bt_bmc->base + BT_CR0);
> +
> +	clr_b_busy(bt_bmc);
> +
> +	return 0;
> +
> +out_unmap:
> +	devm_iounmap(&pdev->dev, bt_bmc->base);

Why do you use devm_iounmap/devm_kfree since the interest with devm_ functions is that all cleanup is done when driver is removed.

> +
> +out_free:
> +	devm_kfree(dev, bt_bmc);
> +	return rc;

I think you should remove the space after this return

Regards

Corentin Labbe

^ permalink raw reply

* [PATCH v3 0/2] KVM: ARM: Enable vtimers with user space gic
From: Christoffer Dall @ 2016-09-16 12:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <060ADA14-786E-400E-9B11-61C34B7081B5@suse.de>

On Fri, Sep 16, 2016 at 02:25:01PM +0200, Alexander Graf wrote:
> 
> > On 16 Sep 2016, at 12:20, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > 
> > Hi Alex,
> > 
> > On 16/09/16 07:26, Alexander Graf wrote:
> >> Some systems out there (well, one type in particular - the Raspberry Pi series)
> >> do have virtualization capabilities in the core, but no ARM GIC interrupt
> >> controller.
> >> 
> >> To run on these systems, the cleanest route is to just handle all
> >> interrupt delivery in user space and only deal with IRQ pins in the core
> >> side in KVM.
> >> 
> >> This works pretty well already, but breaks when the guest starts to use
> >> architected timers, as these are handled straight inside kernel space today.
> >> 
> >> This patch set allows user space to receive vtimer events as well as mask
> >> them, so that we can handle all vtimer related interrupt injection from user
> >> space, enabling us to use architected timer with user space gic emulation.
> > 
> > I have already voiced my concerns in the past, including face to face,
> > and I'm going to repeat it: I not keen at all on adding a new userspace
> > interface that is going to bitrot extremely quickly.
> > 
> > Let's face it, this new ABI will have a single user, with a limited
> > shelf life. I understand that the RPi is a popular product, but it looks
> > fairly obvious that this kind of sub-standard HW will eventually
> > disappear. We'll then be left with a userspace ABI that will break at
> 
> I?m not 100% convinced that this is the case. Emulating the GIC in user space can have other interesting use cases. For example, it might come in handy for nesting. It may be useful for migrating a gicv2 VM to a gicv3 host without gicv2 emulation as well.
> 

I don't see why you'd do this; the VGIC hardware can perfectly well be
used for nesting as well, and this works rather well.

> > every single release, given that nobody in the RPi community actually
> > uses a mainline kernel.
> 
> I actually verified all of this patch on 4.8-rc5 upstream, which is the only 64bit kernel you can find for the RPi. So I?d expect the situation to change with 64bit.
> 
> > And breaking this ABI will introduce userspace exploitable bugs, like
> > the one you've already shown. If anything, I would have loved to
> > completely kill the whole userspace GIC, because nobody cares. Yes, I
> > understand it is fun to have KVM running on the RPi. But the maintenance
> > costs far outweigh the fun aspect already.
> 
> Having CPU pins accessible is very useful for use cases of KVM that are beyond your traditional VM.
> 
> > You could still run KVM with an external emulated timer (not the arch
> > timer). No need for a new ABI for that.
> 
> That?s what I thought too, but turns out that it?s not quite as simple as I hoped ;).

Why not?  I thought a few people had this running recently.  What were
the issues?  Perhaps I can convince someone to submit the patches they
used if useful.

Thanks,
-Christoffer

^ permalink raw reply

* [PATCH v2] KVM: arm/arm64: Route vtimer events to user space
From: Alexander Graf @ 2016-09-16 12:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160916094433.GA4794@cbox>


> On 16 Sep 2016, at 11:44, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> 
> Alex,
> 
> On Fri, Sep 16, 2016 at 07:09:13AM +0200, Alexander Graf wrote:
>> We have 2 modes for dealing with interrupts in the ARM world. We can either
>> handle them all using hardware acceleration through the vgic or we can emulate
>> a gic in user space and only drive CPU IRQ pins from there.
>> 
>> Unfortunately, when driving IRQs from user space, we never tell user space
>> about timer events that may result in interrupt line state changes, so we
>> lose out on timer events if we run with user space gic emulation.
>> 
>> This patch set fixes that by routing vtimer expiration events to user space.
>> With this patch I can successfully run edk2 and Linux with user space gic
>> emulation.
> 
> I have two versions of v2.  Are there any differences or did it just go
> out twice or got duplicated somehow on my end?

Just ignore v2. I shouldn?t send emails that early in the morning. v3 is much better ;)


Alex

^ permalink raw reply

* [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks
From: Christopher Covington @ 2016-09-16 12:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <EE11001F9E5DDD47B7634E2F8A612F2E1F8730EA@lhreml507-mbx>

On 09/16/2016 05:02 AM, Gabriele Paoloni wrote:
> Hi Lorenzo and Tomasz
> 
> Many Thanks for looking at this
> 
>> -----Original Message-----
>> From: Lorenzo Pieralisi [mailto:lorenzo.pieralisi at arm.com]
>> Sent: 15 September 2016 11:59
>> To: liudongdong (C)
>> Cc: Tomasz Nowicki; helgaas at kernel.org; will.deacon at arm.com;
>> catalin.marinas at arm.com; rafael at kernel.org; arnd at arndb.de;
>> hanjun.guo at linaro.org; okaya at codeaurora.org; jchandra at broadcom.com;
>> cov at codeaurora.org; dhdang at apm.com; ard.biesheuvel at linaro.org;
>> robert.richter at caviumnetworks.com; mw at semihalf.com;
>> Liviu.Dudau at arm.com; ddaney at caviumnetworks.com; Wangyijing;
>> msalter at redhat.com; linux-pci at vger.kernel.org; linux-arm-
>> kernel at lists.infradead.org; linaro-acpi at lists.linaro.org;
>> jcm at redhat.com; andrea.gallo at linaro.org; jeremy.linton at arm.com;
>> Gabriele Paoloni; jhugo at codeaurora.org; linux-acpi at vger.kernel.org;
>> linux-kernel at vger.kernel.org
>> Subject: Re: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM
>> quirks
>>
>> On Tue, Sep 13, 2016 at 07:38:39PM +0800, Dongdong Liu wrote:
>>
>> [...]
>>
>>> Our host bridge is non ECAM only for the RC bus config space;
>>> for any other bus underneath the root bus we support ECAM access.
>>>
>>> RC config resource with hardcode as DEFINE_RES_MEM(0xb0070000,
>> SZ_4K),
>>> EP config resource we get it from MCFG table.
>>> So we need to override ops, but config resource we only need to
>> hardcode with RC config resource.
>>>
>>> Our host controller ACPI support patch can be found:
>>> https://lkml.org/lkml/2016/8/31/340
>>
>> Sorry I misread your code. IIUC you create an array of resources that
>> represent non-ECAM config space (and incidentally contain debug
>> registers to check the link status - that you need to check for every
>> given config access (?)), but you still need to have an MCFG entry that
> 
> The link status check is inherited from the designware framework (see
> http://lxr.free-electrons.com/source/drivers/pci/host/pcie-designware.c#L678)
> 
> However I think that in this case we can just check the link status
> in our init function and return an error if the link is down
> 
>> covers the bus number subject to quirk to make sure this mechanism
>> works. Correct ?
> 
> Well we need the quirks for the root bus numbers but if read this v6
> quirk mechanism correctly even if we do not specify an mcfg entry for
> bus 0 oci_mcfg_match_quirks() is called anyway and we can set our
> special configuration space addresses for the root buses (i.e. I think
> we can have a clean MCFG table with entries only for those bus ranges
> that are really ECAM)  
> 
>>
>> This also means that, with the MCFG tables you have and current
>> mainline kernel you are able to probe a root bridge (because the MCFG
>> table covers the bus number that is not ECAM), with enumeration
>> going haywire because it is trying to carry out ECAM accesses on
>> non-ECAM space.
> 
> Yes correct, we cannot access the host controller configuration space
> with our current MCFG table and current Linux mainline
> 
>>
>> Is my reading correct ?
>>
>> Anyway, that's not stricly related to this discussion, it is time we
>> converge on this patchset, we can add a domain range if that
>> simplifies things.
> 
> IMO it would be better to have the domain range to avoid
> a very large and repetitive static quirk array

The v6 framework requires what, 21 additional lines of quirk data? What
if you were to define some preprocessor macros to slim it down? I think
something like the following would bring it down to roughly 7 additional
lines.

#define PCI_ACPI_QUIRK_QUAD_DOM(rev, dom, ops) \
    { "HISI ", rev, 0, dom+0, MCFG_BUS_ANY, ops,
      MCFG_RES_EMPTY}, \
    { "HISI ", rev, 0, dom+1, MCFG_BUS_ANY, ops,
      MCFG_RES_EMPTY}, \
    { "HISI ", rev, 0, dom+2, MCFG_BUS_ANY, ops,
      MCFG_RES_EMPTY}, \
    { "HISI ", rev, 0, dom+3, MCFG_BUS_ANY, ops,
      MCFG_RES_EMPTY},

PCI_ACPI_QUIRK_QUAD_DOM("HIP05 ", 0, &hisi_pcie_hip05_ops)
PCI_ACPI_QUIRK_QUAD_DOM("HIP06 ", 0, &hisi_pcie_hip05_ops)
PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 0, &hisi_pcie_hip06_ops)
PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 4, &hisi_pcie_hip07_ops)
PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 8, &hisi_pcie_hip07_ops)
PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 12, &hisi_pcie_hip07_ops)

Cov
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply

* [PATCH v3 0/2] KVM: ARM: Enable vtimers with user space gic
From: Alexander Graf @ 2016-09-16 12:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <57DBC782.7080305@arm.com>


> On 16 Sep 2016, at 12:20, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
> Hi Alex,
> 
> On 16/09/16 07:26, Alexander Graf wrote:
>> Some systems out there (well, one type in particular - the Raspberry Pi series)
>> do have virtualization capabilities in the core, but no ARM GIC interrupt
>> controller.
>> 
>> To run on these systems, the cleanest route is to just handle all
>> interrupt delivery in user space and only deal with IRQ pins in the core
>> side in KVM.
>> 
>> This works pretty well already, but breaks when the guest starts to use
>> architected timers, as these are handled straight inside kernel space today.
>> 
>> This patch set allows user space to receive vtimer events as well as mask
>> them, so that we can handle all vtimer related interrupt injection from user
>> space, enabling us to use architected timer with user space gic emulation.
> 
> I have already voiced my concerns in the past, including face to face,
> and I'm going to repeat it: I not keen at all on adding a new userspace
> interface that is going to bitrot extremely quickly.
> 
> Let's face it, this new ABI will have a single user, with a limited
> shelf life. I understand that the RPi is a popular product, but it looks
> fairly obvious that this kind of sub-standard HW will eventually
> disappear. We'll then be left with a userspace ABI that will break at

I?m not 100% convinced that this is the case. Emulating the GIC in user space can have other interesting use cases. For example, it might come in handy for nesting. It may be useful for migrating a gicv2 VM to a gicv3 host without gicv2 emulation as well.

> every single release, given that nobody in the RPi community actually
> uses a mainline kernel.

I actually verified all of this patch on 4.8-rc5 upstream, which is the only 64bit kernel you can find for the RPi. So I?d expect the situation to change with 64bit.

> And breaking this ABI will introduce userspace exploitable bugs, like
> the one you've already shown. If anything, I would have loved to
> completely kill the whole userspace GIC, because nobody cares. Yes, I
> understand it is fun to have KVM running on the RPi. But the maintenance
> costs far outweigh the fun aspect already.

Having CPU pins accessible is very useful for use cases of KVM that are beyond your traditional VM.

> You could still run KVM with an external emulated timer (not the arch
> timer). No need for a new ABI for that.

That?s what I thought too, but turns out that it?s not quite as simple as I hoped ;). Also, I much rather at least have a common notion of ?arch timers are always available on arm64? than ?kvm always uses the vgic?. The former has much more impact and much more reach.


Alex

^ permalink raw reply

* [PATCHv9 0/6] dmaengine: rcar-dmac: add iommu support for slave transfers
From: Arnd Bergmann @ 2016-09-16 12:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5489283.7nVYvaYEH8@avalon>

On Friday, September 16, 2016 3:09:29 PM CEST Laurent Pinchart wrote:
> > I wasn't thinking quite that far, though that is also a theoretical
> > problem. However, the simple solution would be to have a bit in the DMA
> > specifier let the driver know whether translation is needed or not.
> > 
> > The simpler case I was thinking of is where the entire DMA engine
> > either goes through an IOMMU or doesn't (depending on the integration
> > into the SoC), so we'd have to find out through some DT property
> > or compatible string in the DMA enginen driver.
> 
> Don't we already get that information from the iommus DT property ? If the DMA 
> engine goes through an IOMMU the property will be set, otherwise it will not.

It depends. A dmaengine typically at least has two DMA masters,
possibly more. It's likely that some dmaengine implementations are
connected to RAM through an IOMMU, but have direct access to an
I/O bus for the slave FIFOs.

> > > The problem is a bit broader than that, we'll also have an issue with DMA
> > > engines that have different channels served by different IOMMUs.
> > 
> > Do you mean a theoretical problem, or a chip that you already know exists?
> 
> That's theoretical. The problem I'm facing today is a DMA engine whose 
> channels are served by different ports of the same IOMMU. This works in a 
> suboptimal way because I have to keep all the IOMMU ports enabled regardless 
> of whether they're used or not, as the DMA engine and IOMMU APIs don't carry 
> channel information.

Ok

	Arnd

^ permalink raw reply

* [PATCH v5 6/6] arm/arm64: vgic-new: Implement KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO ioctl
From: vijay.kilari at gmail.com @ 2016-09-16 12:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474028453-29132-1-git-send-email-vijay.kilari@gmail.com>

From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>

Userspace requires to store and restore of line_level for
level triggered interrupts using ioctl KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO.

Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
---
 arch/arm64/include/uapi/asm/kvm.h   |  6 +++++
 virt/kvm/arm/vgic/vgic-kvm-device.c | 50 ++++++++++++++++++++++++++++++++++++-
 virt/kvm/arm/vgic/vgic-mmio-v3.c    | 11 ++++++++
 virt/kvm/arm/vgic/vgic-mmio.c       | 33 ++++++++++++++++++++++++
 virt/kvm/arm/vgic/vgic-mmio.h       |  5 ++++
 virt/kvm/arm/vgic/vgic.h            |  3 +++
 6 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 91c7137..4100f8c 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -211,6 +211,12 @@ struct kvm_arch_memory_slot {
 #define KVM_DEV_ARM_VGIC_GRP_CTRL	4
 #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
 #define KVM_DEV_ARM_VGIC_CPU_SYSREGS    6
+#define KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO 7
+#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT	10
+#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK \
+			(0x3fffffULL << KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT)
+#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK	0x3ff
+#define VGIC_LEVEL_INFO_LINE_LEVEL	0
 
 #define   KVM_DEV_ARM_VGIC_CTRL_INIT	0
 
diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
index a48101f..de5d836 100644
--- a/virt/kvm/arm/vgic/vgic-kvm-device.c
+++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
@@ -514,6 +514,25 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
 						  regid, reg);
 		break;
 	}
+	case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
+		unsigned int info, intid;
+
+		info = (attr->attr & KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK) >>
+			KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT;
+		if (info == VGIC_LEVEL_INFO_LINE_LEVEL) {
+			if (is_write)
+				tmp32 = *reg;
+			intid = attr->attr &
+				KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK;
+			ret = vgic_v3_line_level_info_uaccess(vcpu, is_write,
+							      intid, &tmp32);
+			if (!is_write)
+				*reg = tmp32;
+		} else {
+			ret = -EINVAL;
+		}
+		break;
+	}
 	default:
 		ret = -EINVAL;
 		break;
@@ -556,6 +575,17 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
 
 		return vgic_v3_attr_regs_access(dev, attr, &reg, true);
 	}
+	case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
+		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
+		u64 reg;
+		u32 tmp32;
+
+		if (get_user(tmp32, uaddr))
+			return -EFAULT;
+
+		reg = tmp32;
+		return vgic_v3_attr_regs_access(dev, attr, &reg, true);
+	}
 	}
 	return -ENXIO;
 }
@@ -591,8 +621,18 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
 			return ret;
 		return put_user(reg, uaddr);
 	}
-	}
+	case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
+		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
+		u64 reg;
+		u32 tmp32;
 
+		ret = vgic_v3_attr_regs_access(dev, attr, &reg, false);
+		if (ret)
+			return ret;
+		tmp32 = reg;
+		return put_user(tmp32, uaddr);
+	}
+	}
 	return -ENXIO;
 }
 
@@ -613,11 +653,19 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
 		return vgic_v3_has_attr_regs(dev, attr);
 	case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
 		return 0;
+	case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
+		if (((attr->attr & KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK) >>
+		      KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT) ==
+		      VGIC_LEVEL_INFO_LINE_LEVEL)
+			return 0;
+		break;
+	}
 	case KVM_DEV_ARM_VGIC_GRP_CTRL:
 		switch (attr->attr) {
 		case KVM_DEV_ARM_VGIC_CTRL_INIT:
 			return 0;
 		}
+		break;
 	}
 	return -ENXIO;
 }
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index af748d7..9070412 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -806,3 +806,14 @@ int vgic_v3_redist_uaccess(struct kvm_vcpu *vcpu, bool is_write,
 		return vgic_uaccess(vcpu, &rd_dev, is_write,
 				    offset, val);
 }
+
+int vgic_v3_line_level_info_uaccess(struct kvm_vcpu *vcpu, bool is_write,
+				    u32 intid, u32 *val)
+{
+	if (is_write)
+		vgic_write_irq_line_level_info(vcpu, intid, *val);
+	else
+		*val = vgic_read_irq_line_level_info(vcpu, intid);
+
+	return 0;
+}
diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
index 173d6f0..fb018eb 100644
--- a/virt/kvm/arm/vgic/vgic-mmio.c
+++ b/virt/kvm/arm/vgic/vgic-mmio.c
@@ -371,6 +371,39 @@ void vgic_mmio_write_config(struct kvm_vcpu *vcpu,
 	}
 }
 
+unsigned long vgic_read_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid)
+{
+	int i;
+	unsigned long val = 0;
+
+	for (i = 0; i < 32; i++) {
+		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
+
+		if (irq->line_level)
+			val |= (1U << i);
+
+		vgic_put_irq(vcpu->kvm, irq);
+	}
+
+	return val;
+}
+
+void vgic_write_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid,
+				    const unsigned long val)
+{
+	int i;
+
+	for_each_set_bit(i, &val, 32) {
+		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
+
+		spin_lock(&irq->irq_lock);
+		irq->line_level = true;
+		spin_unlock(&irq->irq_lock);
+
+		vgic_put_irq(vcpu->kvm, irq);
+	}
+}
+
 static int match_region(const void *key, const void *elt)
 {
 	const unsigned int offset = (unsigned long)key;
diff --git a/virt/kvm/arm/vgic/vgic-mmio.h b/virt/kvm/arm/vgic/vgic-mmio.h
index acbf99e..938702c 100644
--- a/virt/kvm/arm/vgic/vgic-mmio.h
+++ b/virt/kvm/arm/vgic/vgic-mmio.h
@@ -181,6 +181,11 @@ int vgic_validate_mmio_region_addr(struct kvm_device *dev,
 				   const struct vgic_register_region *regions,
 				   int nr_regions, gpa_t addr);
 
+unsigned long vgic_read_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid);
+
+void vgic_write_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid,
+				    const unsigned long val);
+
 unsigned int vgic_v2_init_dist_iodev(struct vgic_io_device *dev);
 
 unsigned int vgic_v3_init_dist_iodev(struct vgic_io_device *dev);
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index 4c0076c..baf2edd 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -108,6 +108,9 @@ int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write,
 			 u64 id, u64 *val);
 int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
 				u64 *reg);
+int vgic_v3_line_level_info_uaccess(struct kvm_vcpu *vcpu, bool is_write,
+				    u32 intid, u32 *val);
+
 #else
 static inline int vgic_register_its_iodevs(struct kvm *kvm)
 {
-- 
1.9.1

^ permalink raw reply related

* [PATCH 5/6] arm/arm64: vgic-new: Implement VGICv3 CPU interface access
From: vijay.kilari at gmail.com @ 2016-09-16 12:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474028453-29132-1-git-send-email-vijay.kilari@gmail.com>

From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>

VGICv3 CPU interface registers are accessed using
KVM_DEV_ARM_VGIC_CPU_SYSREGS ioctl. These registers are accessed
as 64-bit. The cpu MPIDR value is passed along with register id.
is used to identify the cpu for registers access.

The version of VGIC v3 specification is define here
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/445611.html

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
---
 arch/arm64/include/uapi/asm/kvm.h   |   3 +
 arch/arm64/kvm/Makefile             |   1 +
 include/linux/irqchip/arm-gic-v3.h  |  30 ++++
 virt/kvm/arm/vgic/vgic-kvm-device.c |  27 ++++
 virt/kvm/arm/vgic/vgic-mmio-v3.c    |  18 +++
 virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 296 ++++++++++++++++++++++++++++++++++++
 virt/kvm/arm/vgic/vgic.h            |  10 ++
 7 files changed, 385 insertions(+)

diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 56dc08d..91c7137 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -206,9 +206,12 @@ struct kvm_arch_memory_slot {
 			(0xffffffffULL << KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT)
 #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
 #define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
+#define   KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK (0xffff)
 #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
 #define KVM_DEV_ARM_VGIC_GRP_CTRL	4
 #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
+#define KVM_DEV_ARM_VGIC_CPU_SYSREGS    6
+
 #define   KVM_DEV_ARM_VGIC_CTRL_INIT	0
 
 /* Device Control API on vcpu fd */
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index d50a82a..1a14e29 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -32,5 +32,6 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-kvm-device.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-its.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o
+kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-sys-reg-v3.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o
 kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 88d83d3..d4e9c7d 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -355,6 +355,27 @@
 #define ICC_CTLR_EL1_EOImode_SHIFT	(1)
 #define ICC_CTLR_EL1_EOImode_drop_dir	(0U << ICC_CTLR_EL1_EOImode_SHIFT)
 #define ICC_CTLR_EL1_EOImode_drop	(1U << ICC_CTLR_EL1_EOImode_SHIFT)
+#define ICC_CTLR_EL1_EOImode_MASK	(1 << ICC_CTLR_EL1_EOImode_SHIFT)
+#define ICC_CTLR_EL1_CBPR_SHIFT		(0)
+#define ICC_CTLR_EL1_CBPR_MASK		(1 << ICC_CTLR_EL1_CBPR_SHIFT)
+#define ICC_CTLR_EL1_PRI_BITS_SHIFT	(8)
+#define ICC_CTLR_EL1_PRI_BITS_MASK	(0x7 << ICC_CTLR_EL1_PRI_BITS_SHIFT)
+#define ICC_CTLR_EL1_ID_BITS_SHIFT	(11)
+#define ICC_CTLR_EL1_ID_BITS_MASK	(0x7 << ICC_CTLR_EL1_ID_BITS_SHIFT)
+#define ICC_CTLR_EL1_SEIS_SHIFT		(14)
+#define ICC_CTLR_EL1_SEIS_MASK		(0x1 << ICC_CTLR_EL1_SEIS_SHIFT)
+#define ICC_CTLR_EL1_A3V_SHIFT		(15)
+#define ICC_CTLR_EL1_A3V_MASK		(0x1 << ICC_CTLR_EL1_A3V_SHIFT)
+#define ICC_PMR_EL1_SHIFT		(0)
+#define ICC_PMR_EL1_MASK		(0xff << ICC_PMR_EL1_SHIFT)
+#define ICC_BPR0_EL1_SHIFT		(0)
+#define ICC_BPR0_EL1_MASK		(0x7 << ICC_BPR0_EL1_SHIFT)
+#define ICC_BPR1_EL1_SHIFT		(0)
+#define ICC_BPR1_EL1_MASK		(0x7 << ICC_BPR1_EL1_SHIFT)
+#define ICC_IGRPEN0_EL1_SHIFT		(0)
+#define ICC_IGRPEN0_EL1_MASK		(1 << ICC_IGRPEN0_EL1_SHIFT)
+#define ICC_IGRPEN1_EL1_SHIFT		(0)
+#define ICC_IGRPEN1_EL1_MASK		(1 << ICC_IGRPEN1_EL1_SHIFT)
 #define ICC_SRE_EL1_SRE			(1U << 0)
 
 /*
@@ -398,6 +419,15 @@
 #define ICH_VMCR_ENG1_SHIFT		1
 #define ICH_VMCR_ENG1_MASK		(1 << ICH_VMCR_ENG1_SHIFT)
 
+#define ICH_VTR_PRI_BITS_SHIFT		29
+#define ICH_VTR_PRI_BITS_MASK		(7 << ICH_VTR_PRI_BITS_SHIFT)
+#define ICH_VTR_ID_BITS_SHIFT		23
+#define ICH_VTR_ID_BITS_MASK		(7 << ICH_VTR_ID_BITS_SHIFT)
+#define ICH_VTR_SEIS_SHIFT		22
+#define ICH_VTR_SEIS_MASK		(1 << ICH_VTR_SEIS_SHIFT)
+#define ICH_VTR_A3V_SHIFT		21
+#define ICH_VTR_A3V_MASK		(1 << ICH_VTR_A3V_SHIFT)
+
 #define ICC_IAR1_EL1_SPURIOUS		0x3ff
 
 #define ICC_SRE_EL2_SRE			(1 << 0)
diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
index a4656fc..a48101f 100644
--- a/virt/kvm/arm/vgic/vgic-kvm-device.c
+++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
@@ -506,6 +506,14 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
 		if (!is_write)
 			*reg = tmp32;
 		break;
+	case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
+		u64 regid;
+
+		regid = (attr->attr & KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK);
+		ret = vgic_v3_cpu_sysregs_uaccess(vcpu, is_write,
+						  regid, reg);
+		break;
+	}
 	default:
 		ret = -EINVAL;
 		break;
@@ -539,6 +547,15 @@ 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_CPU_SYSREGS: {
+		u64 __user *uaddr = (u64 __user *)(long)attr->addr;
+		u64 reg;
+
+		if (get_user(reg, uaddr))
+			return -EFAULT;
+
+		return vgic_v3_attr_regs_access(dev, attr, &reg, true);
+	}
 	}
 	return -ENXIO;
 }
@@ -565,6 +582,15 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
 		tmp32 = reg;
 		return put_user(tmp32, uaddr);
 	}
+	case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
+		u64 __user *uaddr = (u64 __user *)(long)attr->addr;
+		u64 reg;
+
+		ret = vgic_v3_attr_regs_access(dev, attr, &reg, false);
+		if (ret)
+			return ret;
+		return put_user(reg, uaddr);
+	}
 	}
 
 	return -ENXIO;
@@ -583,6 +609,7 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
 		break;
 	case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
 	case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
+	case KVM_DEV_ARM_VGIC_CPU_SYSREGS:
 		return vgic_v3_has_attr_regs(dev, attr);
 	case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
 		return 0;
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index 83dece8..af748d7 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -23,6 +23,7 @@
 
 #include "vgic.h"
 #include "vgic-mmio.h"
+#include "sys_regs.h"
 
 /* extract @num bytes at @offset bytes offset in data */
 unsigned long extract_bytes(u64 data, unsigned int offset,
@@ -639,6 +640,23 @@ int vgic_v3_has_attr_regs(struct kvm_device *dev, struct kvm_device_attr *attr)
 		nr_regions = ARRAY_SIZE(vgic_v3_rdbase_registers);
 		break;
 	}
+	case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
+		u64 reg, id;
+		unsigned long mpidr;
+		struct kvm_vcpu *vcpu;
+
+		mpidr = (attr->attr & KVM_DEV_ARM_VGIC_V3_MPIDR_MASK) >>
+			 KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT;
+
+		vcpu = kvm_mpidr_to_vcpu(dev->kvm, mpidr);
+		if (!vcpu)
+			return -EINVAL;
+		if (vcpu->vcpu_id >= atomic_read(&dev->kvm->online_vcpus))
+			return -EINVAL;
+
+		id = (attr->attr & KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK);
+		return vgic_v3_has_cpu_sysregs_attr(vcpu, 0, id, &reg);
+	}
 	default:
 		return -ENXIO;
 	}
diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-v3.c b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
new file mode 100644
index 0000000..8e4f403
--- /dev/null
+++ b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
@@ -0,0 +1,296 @@
+#include <linux/irqchip/arm-gic-v3.h>
+#include <linux/kvm.h>
+#include <linux/kvm_host.h>
+#include <kvm/iodev.h>
+#include <kvm/arm_vgic.h>
+#include <asm/kvm_emulate.h>
+#include <asm/kvm_arm.h>
+#include <asm/kvm_mmu.h>
+
+#include "vgic.h"
+#include "vgic-mmio.h"
+#include "sys_regs.h"
+
+static bool access_gic_ctlr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
+			    const struct sys_reg_desc *r)
+{
+	struct vgic_vmcr vmcr;
+	u64 val;
+	u32 ich_vtr;
+
+	vgic_get_vmcr(vcpu, &vmcr);
+	if (p->is_write) {
+		val = p->regval;
+		vmcr.ctlr &= ~(ICH_VMCR_CBPR_MASK | ICH_VMCR_EOIM_MASK);
+		vmcr.ctlr |= ((val & ICC_CTLR_EL1_CBPR_MASK) >>
+			      ICC_CTLR_EL1_CBPR_SHIFT) << ICH_VMCR_CBPR_SHIFT;
+		vmcr.ctlr |= ((val & ICC_CTLR_EL1_EOImode_MASK) >>
+			     ICC_CTLR_EL1_EOImode_SHIFT) << ICH_VMCR_EOIM_SHIFT;
+		vgic_set_vmcr(vcpu, &vmcr);
+	} else {
+		ich_vtr = kvm_call_hyp(__vgic_v3_get_ich_vtr_el2);
+
+		val = 0;
+		val |= ((ich_vtr & ICH_VTR_PRI_BITS_MASK) >>
+			ICH_VTR_PRI_BITS_SHIFT) << ICC_CTLR_EL1_PRI_BITS_SHIFT;
+		val |= ((ich_vtr & ICH_VTR_ID_BITS_MASK) >>
+			ICH_VTR_ID_BITS_SHIFT) << ICC_CTLR_EL1_ID_BITS_SHIFT;
+		val |= ((ich_vtr & ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT)
+			<< ICC_CTLR_EL1_SEIS_SHIFT;
+		val |= ((ich_vtr & ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT)
+			<< ICC_CTLR_EL1_A3V_SHIFT;
+		val |= ((vmcr.ctlr & ICH_VMCR_CBPR_MASK) >>
+			ICH_VMCR_CBPR_SHIFT) << ICC_CTLR_EL1_CBPR_SHIFT;
+		val |= ((vmcr.ctlr & ICH_VMCR_EOIM_MASK) >>
+			ICH_VMCR_EOIM_SHIFT) << ICC_CTLR_EL1_EOImode_SHIFT;
+
+		p->regval = val;
+	}
+
+	return true;
+}
+
+static bool access_gic_pmr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
+			   const struct sys_reg_desc *r)
+{
+	struct vgic_vmcr vmcr;
+
+	vgic_get_vmcr(vcpu, &vmcr);
+	if (p->is_write) {
+		vmcr.pmr = (p->regval & ICC_PMR_EL1_MASK) >> ICC_PMR_EL1_SHIFT;
+		vgic_set_vmcr(vcpu, &vmcr);
+	} else {
+		p->regval = (vmcr.pmr << ICC_PMR_EL1_SHIFT) & ICC_PMR_EL1_MASK;
+	}
+
+	return true;
+}
+
+static bool access_gic_bpr0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
+			    const struct sys_reg_desc *r)
+{
+	struct vgic_vmcr vmcr;
+
+	vgic_get_vmcr(vcpu, &vmcr);
+	if (p->is_write) {
+		vmcr.bpr = (p->regval & ICC_BPR0_EL1_MASK) >>
+			    ICC_BPR0_EL1_SHIFT;
+		vgic_set_vmcr(vcpu, &vmcr);
+	} else {
+		p->regval = (vmcr.bpr << ICC_BPR0_EL1_SHIFT) &
+			     ICC_BPR0_EL1_MASK;
+	}
+
+	return true;
+}
+
+static bool access_gic_bpr1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
+			    const struct sys_reg_desc *r)
+{
+	struct vgic_vmcr vmcr;
+
+	vgic_get_vmcr(vcpu, &vmcr);
+	if (p->is_write) {
+		vmcr.abpr = (p->regval & ICC_BPR1_EL1_MASK) >>
+			     ICC_BPR1_EL1_SHIFT;
+		vgic_set_vmcr(vcpu, &vmcr);
+	} else {
+		p->regval = (vmcr.abpr << ICC_BPR1_EL1_SHIFT) &
+			     ICC_BPR1_EL1_MASK;
+	}
+
+	return true;
+}
+
+static bool access_gic_grpen0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
+			      const struct sys_reg_desc *r)
+{
+	struct vgic_vmcr vmcr;
+
+	vgic_get_vmcr(vcpu, &vmcr);
+	if (p->is_write) {
+		vmcr.grpen0 = (p->regval & ICC_IGRPEN0_EL1_MASK) >>
+				      ICC_IGRPEN0_EL1_SHIFT;
+		vgic_set_vmcr(vcpu, &vmcr);
+	} else {
+		p->regval = (vmcr.grpen0 << ICC_IGRPEN0_EL1_SHIFT) &
+			     ICC_IGRPEN0_EL1_MASK;
+	}
+
+	return true;
+}
+
+static bool access_gic_grpen1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
+			      const struct sys_reg_desc *r)
+{
+	struct vgic_vmcr vmcr;
+
+	vgic_get_vmcr(vcpu, &vmcr);
+	if (p->is_write) {
+		vmcr.grpen1 = (p->regval & ICC_IGRPEN1_EL1_MASK) >>
+				      ICC_IGRPEN1_EL1_SHIFT;
+		vgic_set_vmcr(vcpu, &vmcr);
+	} else {
+		p->regval = (vmcr.grpen1 << ICC_IGRPEN1_EL1_SHIFT) &
+			     ICC_IGRPEN1_EL1_MASK;
+	}
+
+	return true;
+}
+
+static void vgic_v3_access_apr_reg(struct kvm_vcpu *vcpu,
+				   struct sys_reg_params *p, u8 apr, u8 idx)
+{
+	struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
+	uint32_t *ap_reg;
+
+	if (apr)
+		ap_reg = &vgicv3->vgic_ap1r[idx];
+	else
+		ap_reg = &vgicv3->vgic_ap0r[idx];
+
+	if (p->is_write)
+		*ap_reg = p->regval;
+	else
+		p->regval = *ap_reg;
+}
+
+static void access_gic_aprn(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
+			    const struct sys_reg_desc *r, u8 apr)
+{
+	u8 num_pri_bits, idx;
+	u32 ich_vtr = kvm_call_hyp(__vgic_v3_get_ich_vtr_el2);
+
+
+	num_pri_bits = ((ich_vtr & ICH_VTR_PRI_BITS_MASK) >>
+			ICH_VTR_PRI_BITS_SHIFT) + 1;
+	idx = r->Op2 & 3;
+
+	switch (num_pri_bits) {
+	case 7:
+		if (idx > 3)
+			goto err;
+		vgic_v3_access_apr_reg(vcpu, p, apr, idx);
+		break;
+	case 6:
+		if (idx > 1)
+			goto err;
+		vgic_v3_access_apr_reg(vcpu, p, apr, idx);
+		break;
+	default:
+		if (idx > 0)
+			goto err;
+		vgic_v3_access_apr_reg(vcpu, p, apr, idx);
+	}
+
+	return;
+err:
+	if (!p->is_write)
+		p->regval = 0;
+}
+
+static bool access_gic_ap0r(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
+			    const struct sys_reg_desc *r)
+{
+	access_gic_aprn(vcpu, p, r, 0);
+
+	return true;
+}
+
+static bool access_gic_ap1r(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
+			    const struct sys_reg_desc *r)
+{
+	access_gic_aprn(vcpu, p, r, 1);
+
+	return true;
+}
+
+static bool access_gic_sre(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
+			   const struct sys_reg_desc *r)
+{
+	struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
+
+	/* Read only. Write ignore */
+	if (!p->is_write)
+		p->regval = vgicv3->vgic_sre;
+
+	return true;
+}
+
+static const struct sys_reg_desc gic_v3_icc_reg_descs[] = {
+	/* ICC_PMR_EL1 */
+	{ Op0(3), Op1(0), CRn(4), CRm(6), Op2(0), access_gic_pmr },
+	/* ICC_BPR0_EL1 */
+	{ Op0(3), Op1(0), CRn(12), CRm(8), Op2(3), access_gic_bpr0 },
+	/* ICC_AP0R0_EL1 */
+	{ Op0(3), Op1(0), CRn(12), CRm(8), Op2(4), access_gic_ap0r },
+	/* ICC_AP0R1_EL1 */
+	{ Op0(3), Op1(0), CRn(12), CRm(8), Op2(5), access_gic_ap0r },
+	/* ICC_AP0R2_EL1 */
+	{ Op0(3), Op1(0), CRn(12), CRm(8), Op2(6), access_gic_ap0r },
+	/* ICC_AP0R3_EL1 */
+	{ Op0(3), Op1(0), CRn(12), CRm(8), Op2(7), access_gic_ap0r },
+	/* ICC_AP1R0_EL1 */
+	{ Op0(3), Op1(0), CRn(12), CRm(9), Op2(0), access_gic_ap1r },
+	/* ICC_AP1R1_EL1 */
+	{ Op0(3), Op1(0), CRn(12), CRm(9), Op2(1), access_gic_ap1r },
+	/* ICC_AP1R2_EL1 */
+	{ Op0(3), Op1(0), CRn(12), CRm(9), Op2(2), access_gic_ap1r },
+	/* ICC_AP1R3_EL1 */
+	{ Op0(3), Op1(0), CRn(12), CRm(9), Op2(3), access_gic_ap1r },
+	/* ICC_BPR1_EL1 */
+	{ Op0(3), Op1(0), CRn(12), CRm(12), Op2(3), access_gic_bpr1 },
+	/* ICC_CTLR_EL1 */
+	{ Op0(3), Op1(0), CRn(12), CRm(12), Op2(4), access_gic_ctlr },
+	/* ICC_SRE_EL1 */
+	{ Op0(3), Op1(0), CRn(12), CRm(12), Op2(5), access_gic_sre },
+	/* ICC_IGRPEN0_EL1 */
+	{ Op0(3), Op1(0), CRn(12), CRm(12), Op2(6), access_gic_grpen0 },
+	/* ICC_GRPEN1_EL1 */
+	{ Op0(3), Op1(0), CRn(12), CRm(12), Op2(7), access_gic_grpen1 },
+};
+
+int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
+				u64 *reg)
+{
+	struct sys_reg_params params;
+	u64 sysreg = (id & KVM_DEV_ARM_VGIC_SYSREG_MASK) | KVM_REG_SIZE_U64;
+
+	params.regval = *reg;
+	params.is_write = is_write;
+	params.is_aarch32 = false;
+	params.is_32bit = false;
+
+	if (find_reg_by_id(sysreg, &params, gic_v3_icc_reg_descs,
+			      ARRAY_SIZE(gic_v3_icc_reg_descs)))
+		return 0;
+
+	return -ENXIO;
+}
+
+int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write, u64 id,
+				u64 *reg)
+{
+	struct sys_reg_params params;
+	const struct sys_reg_desc *r;
+	u64 sysreg = (id & KVM_DEV_ARM_VGIC_SYSREG_MASK) | KVM_REG_SIZE_U64;
+
+	if (is_write)
+		params.regval = *reg;
+	params.is_write = is_write;
+	params.is_aarch32 = false;
+	params.is_32bit = false;
+
+	r = find_reg_by_id(sysreg, &params, gic_v3_icc_reg_descs,
+			   ARRAY_SIZE(gic_v3_icc_reg_descs));
+	if (!r)
+		return -ENXIO;
+
+	if (!r->access(vcpu, &params, r))
+		return -EINVAL;
+
+	if (!is_write)
+		*reg = params.regval;
+
+	return 0;
+}
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index 9ee54e3..4c0076c 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -30,6 +30,12 @@
 
 #define vgic_irq_is_sgi(intid) ((intid) < VGIC_NR_SGIS)
 
+#define   KVM_DEV_ARM_VGIC_SYSREG_MASK (KVM_REG_ARM64_SYSREG_OP0_MASK | \
+					KVM_REG_ARM64_SYSREG_OP1_MASK | \
+					KVM_REG_ARM64_SYSREG_CRN_MASK | \
+					KVM_REG_ARM64_SYSREG_CRM_MASK | \
+					KVM_REG_ARM64_SYSREG_OP2_MASK)
+
 struct vgic_vmcr {
 	u32	ctlr;
 	u32	abpr;
@@ -98,6 +104,10 @@ int vgic_v3_dist_uaccess(struct kvm_vcpu *vcpu, bool is_write,
 			 int offset, u32 *val);
 int vgic_v3_redist_uaccess(struct kvm_vcpu *vcpu, bool is_write,
 			 int offset, u32 *val);
+int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write,
+			 u64 id, u64 *val);
+int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
+				u64 *reg);
 #else
 static inline int vgic_register_its_iodevs(struct kvm *kvm)
 {
-- 
1.9.1

^ 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