* [PATCH v2 1/5] arm64: dts: renesas: r8a77980: add FCPVD support
From: Laurent Pinchart @ 2018-06-08 14:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <73f61c34-d0ec-75cd-b6fd-a41cfb39078f@cogentembedded.com>
Hi Sergei,
Thank you for the patch.
On Thursday, 7 June 2018 23:19:31 EEST Sergei Shtylyov wrote:
> Describe FCPVD0 in the R8A77980 device tree; it will be used by VSPD0 in
> the next patch...
>
> Based on the original (and large) patch by Vladimir Barinov.
>
> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> arch/arm64/boot/dts/renesas/r8a77980.dtsi | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> ===================================================================
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> @@ -653,6 +653,14 @@
> resets = <&cpg 408>;
> };
>
> + fcpvd0: fcp at fea27000 {
> + compatible = "renesas,fcpv";
> + reg = <0 0xfea27000 0 0x200>;
> + clocks = <&cpg CPG_MOD 603>;
> + power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
> + resets = <&cpg 603>;
> + };
> +
> prr: chipid at fff00044 {
> compatible = "renesas,prr";
> reg = <0 0xfff00044 0 4>;
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH 1/2] reset: imx7: Fix always writing bits as 0
From: Lucas Stach @ 2018-06-08 14:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c441872d7dd355ae09e945447441389e93e3b888.1527621510.git.leonard.crestez@nxp.com>
Am Dienstag, den 29.05.2018, 22:39 +0300 schrieb Leonard Crestez:
> Right now the only user of reset-imx7 is pci-imx6 and the
> reset_control_assert and deassert calls on pciephy_reset don't toggle
> the PCIEPHY_BTN and PCIEPHY_G_RST bits as expected. Fix this by writing
> 1 or 0 respectively.
>
> The reference manual is not very clear regarding SRC_PCIEPHY_RCR but for
> other registers like MIPIPHY and HSICPHY the bits are explicitly
> documented as "1 means assert, 0 means deassert".
>
> The values are still reversed for IMX7_RESET_PCIE_CTRL_APPS_EN.
>
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> ?drivers/reset/reset-imx7.c | 2 +-
> ?1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> index 4db177bc89bc..fdeac1946429 100644
> --- a/drivers/reset/reset-imx7.c
> +++ b/drivers/reset/reset-imx7.c
> @@ -78,11 +78,11 @@ static struct imx7_src *to_imx7_src(struct reset_controller_dev *rcdev)
> ?static int imx7_reset_set(struct reset_controller_dev *rcdev,
> > ? ??unsigned long id, bool assert)
> ?{
> > ? struct imx7_src *imx7src = to_imx7_src(rcdev);
> > ? const struct imx7_src_signal *signal = &imx7_src_signals[id];
> > - unsigned int value = 0;
> > + unsigned int value = assert ? signal->bit : 0;
> ?
> > ? switch (id) {
> > ? case IMX7_RESET_PCIEPHY:
> > ? /*
> > ? ?* wait for more than 10us to release phy g_rst and
^ permalink raw reply
* [PATCH v2 4/5] arm64: dts: renesas: r8a77980: add LVDS support
From: Laurent Pinchart @ 2018-06-08 14:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3e46c70c-c6be-d019-95a9-bf09c888c9c0@cogentembedded.com>
Hi Sergei,
Thank you for the patch.
On Thursday, 7 June 2018 23:23:06 EEST Sergei Shtylyov wrote:
> Define the generic R8A77980 part of the LVDS device node.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> arch/arm64/boot/dts/renesas/r8a77980.dtsi | 29 ++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> ===================================================================
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> @@ -696,6 +696,35 @@
> port at 1 {
> reg = <1>;
> du_out_lvds0: endpoint {
> + remote-endpoint = <&lvds0_in>;
> + };
> + };
> + };
> + };
> +
> + lvds0: lvds-encoder at feb90000 {
> + compatible = "renesas,r8a77980-lvds";
> + reg = <0 0xfeb90000 0 0x14>;
> + clocks = <&cpg CPG_MOD 727>;
> + power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
> + resets = <&cpg 727>;
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port at 0 {
> + reg = <0>;
> + lvds0_in: endpoint {
> + remote-endpoint =
> + <&du_out_lvds0>;
> + };
> + };
> +
> + port at 1 {
> + reg = <1>;
> + lvds0_out: endpoint {
> };
> };
> };
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH v2 0/5] Add R8A77980/Condor/V3HSK LVDS/HDMI support
From: Laurent Pinchart @ 2018-06-08 14:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <390fabf0-4caf-1600-3780-1893207d94f7@cogentembedded.com>
Hi Sergei,
On Thursday, 7 June 2018 23:17:03 EEST Sergei Shtylyov wrote:
> Hello!
>
> Here's the set of 5 patches against Simon Horman's 'renesas.git' repo's
> 'renesas-devel-20180604-v4.17' tag. We're adding the R8A77980 FCPVD/VSPD/
> DU/LVDS device nodes and then describing the LVDS decoder and HDMI encoder
> connected to the LVDS output. These patches depend on the Thine THC63LVD1024
> driver and the R8A77980 LVDS support patch in order to work, and R8A77980
> GPIO DT patches in order to apply/compile...
>
> [1/5] arm64: dts: renesas: r8a77980: add FCPVD support
> [2/5] arm64: dts: renesas: r8a77980: add VSPD support
> [3/5] arm64: dts: renesas: r8a77980: add DU support
> [4/5] arm64: dts: renesas: r8a77980: add LVDS support
Based on the recent request of the ARM SoC maintainers to avoid a plethora of
small patches, I think you can squash 1/5 to 4/5 all together.
> [5/5] arm64: dts: renesas: condor/v3hsk: add DU/LVDS/HDMI support
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH 2/2] PCI: imx: Initial imx7d pm support
From: Lucas Stach @ 2018-06-08 14:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a6176c8bcfbca08ce4e93e304bdcd48c4600f8e2.1527621510.git.leonard.crestez@nxp.com>
Am Dienstag, den 29.05.2018, 22:39 +0300 schrieb Leonard Crestez:
> On imx7d the phy is turned off in suspend and must be reset on resume.
> Right now lspci -v fails after a suspend/resume cycle, fix this by
> adding minimal suspend/resume code from the nxp vendor tree.
>
> This is currently only enabled for imx7 but the same sequence can be
> applied to other imx pcie variants.
>
> Tested on imx7d-sabresd with an Intel 5622ANHMW wireless pcie adapter.
>
> > The original author is mostly Richard Zhu <hongxing.zhu@nxp.com>, this
> patch adjusts the code to the upstream imx7d implemention using reset
> controls and power domains.
>
> > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
> ?drivers/pci/dwc/pci-imx6.c | 94 ++++++++++++++++++++++++++++++++++++--
> ?1 file changed, 89 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
> index 4818ef875f8a..ff6077eeb387 100644
> --- a/drivers/pci/dwc/pci-imx6.c
> +++ b/drivers/pci/dwc/pci-imx6.c
> @@ -540,10 +540,27 @@ static int imx6_pcie_wait_for_speed_change(struct imx6_pcie *imx6_pcie)
> ?
> > ? dev_err(dev, "Speed change timeout\n");
> > ? return -EINVAL;
> ?}
> ?
> +static void imx6_pcie_ltssm_enable(struct device *dev)
> +{
> > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> +
> > + switch (imx6_pcie->variant) {
> > + case IMX6Q:
> > + case IMX6SX:
> > + case IMX6QP:
> > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > + ???IMX6Q_GPR12_PCIE_CTL_2,
> > + ???IMX6Q_GPR12_PCIE_CTL_2);
> > + break;
> > + case IMX7D:
> > + reset_control_deassert(imx6_pcie->apps_reset);
> > + }
> +}
> +
> ?static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
> ?{
> > ? struct dw_pcie *pci = imx6_pcie->pci;
> > ? struct device *dev = pci->dev;
> > ? u32 tmp;
> @@ -558,15 +575,11 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
> > ? tmp &= ~PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK;
> > ? tmp |= PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1;
> > ? dw_pcie_writel_dbi(pci, PCIE_RC_LCR, tmp);
> ?
> > ? /* Start LTSSM. */
> > - if (imx6_pcie->variant == IMX7D)
> > - reset_control_deassert(imx6_pcie->apps_reset);
> > - else
> > - regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > - ???IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
> > + imx6_pcie_ltssm_enable(dev);
> ?
> > ? ret = imx6_pcie_wait_for_link(imx6_pcie);
> > ? if (ret)
> > ? goto err_reset_phy;
> ?
> @@ -681,10 +694,80 @@ static int imx6_add_pcie_port(struct imx6_pcie *imx6_pcie,
> ?
> ?static const struct dw_pcie_ops dw_pcie_ops = {
> > ? .link_up = imx6_pcie_link_up,
> ?};
> ?
> +#ifdef CONFIG_PM_SLEEP
> +static int imx6_pcie_suspend_noirq(struct device *dev)
> +{
> > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> +
> + if (imx6_pcie->variant == IMX7D) {
Instead of this large indented block, please just have an early return
at the start of this function, like:
if (imx6_pcie->variant != IMX7D)
return 0;
Same for the resume function.
> + /* Disable clks */
> > + clk_disable_unprepare(imx6_pcie->pcie);
> > + clk_disable_unprepare(imx6_pcie->pcie_phy);
> > + clk_disable_unprepare(imx6_pcie->pcie_bus);
> > + /* turn off external osc input */
> > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > + ???IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> > + ???IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> > + }
> +
> > + return 0;
> +}
> +
> +static void imx6_pcie_ltssm_disable(struct device *dev)
> +{
> > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> +
> > + switch (imx6_pcie->variant) {
> > + case IMX6Q:
> > + case IMX6SX:
> > + case IMX6QP:
> > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > + ???IMX6Q_GPR12_PCIE_CTL_2, 0);
> > + break;
> > + case IMX7D:
> > + reset_control_assert(imx6_pcie->apps_reset);
> > + break;
> > + }
> +}
> +
> +static int imx6_pcie_resume_noirq(struct device *dev)
> +{
> > + int ret = 0;
> > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> > + struct pcie_port *pp = &imx6_pcie->pci->pp;
> +
> > + if (imx6_pcie->variant == IMX7D) {
> > + imx6_pcie_ltssm_disable(dev);
The LTSSM disable seems misplaced here. I would have expected it to be
in the suspend function.
> + imx6_pcie_assert_core_reset(imx6_pcie);
> > + imx6_pcie_init_phy(imx6_pcie);
> > + imx6_pcie_deassert_core_reset(imx6_pcie);
> +
> > + /*
> > + ?* controller maybe turn off, re-configure again
> > + ?*/
> > + dw_pcie_setup_rc(pp);
> +
> > + imx6_pcie_ltssm_enable(dev);
> +
> > + ret = imx6_pcie_wait_for_link(imx6_pcie);
> > + if (ret < 0)
> + pr_info("pcie link is down after resume.\n");
If the PHY has been powered down and LTSSM stopped I guess we need to
do the full imx6_pcie_establish_link() dance again here to reliably re-
establish the link. The above seems unsafe.
> + }
> +
> > + return 0;
> +}
> +
> +static const struct dev_pm_ops imx6_pcie_pm_ops = {
> > + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx6_pcie_suspend_noirq,
> > + ??????imx6_pcie_resume_noirq)
> +};
> +#endif
This structure must be outside of the ifdef, or you'll break the build
on !CONFIG_PM_SLEEP.
> ?static int imx6_pcie_probe(struct platform_device *pdev)
> ?{
> > ? struct device *dev = &pdev->dev;
> > ? struct dw_pcie *pci;
> > ? struct imx6_pcie *imx6_pcie;
> @@ -847,10 +930,11 @@ static const struct of_device_id imx6_pcie_of_match[] = {
> ?static struct platform_driver imx6_pcie_driver = {
> > ? .driver = {
> > > ? .name = "imx6q-pcie",
> > ? .of_match_table = imx6_pcie_of_match,
> > ? .suppress_bind_attrs = true,
> > + .pm = &imx6_pcie_pm_ops,
> > ? },
> > ? .probe????= imx6_pcie_probe,
> > ? .shutdown = imx6_pcie_shutdown,
> ?};
> ?
Regards,
Lucas
^ permalink raw reply
* [PATCH v2 13/16] dt-bindings/interrupt-controller: add documentation for Marvell SEI controller
From: Miquel Raynal @ 2018-06-08 14:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180605205121.GA19249@rob-hp-laptop>
Hi Rob, Marc,
On Tue, 5 Jun 2018 14:51:21 -0600, Rob Herring <robh@kernel.org> wrote:
> On Tue, May 22, 2018 at 11:40:39AM +0200, Miquel Raynal wrote:
> > Describe the System Error Interrupt (SEI) controller. It aggregates two
> > types of interrupts, wired and MSIs from respectively the AP and the
> > CPs, into a single SPI interrupt.
> >
> > Suggested-by: Haim Boot <hayim@marvell.com>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > .../bindings/interrupt-controller/marvell,sei.txt | 50 ++++++++++++++++++++++
> > 1 file changed, 50 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt b/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> > new file mode 100644
> > index 000000000000..689981036c30
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> > @@ -0,0 +1,50 @@
> > +Marvell SEI (System Error Interrupt) Controller
> > +-----------------------------------------------
> > +
> > +Marvell SEI (System Error Interrupt) controller is an interrupt
> > +aggregator. It receives interrupts from several sources and aggregates
> > +them to a single interrupt line (an SPI) on the parent interrupt
> > +controller.
> > +
> > +This interrupt controller can handle up to 64 SEIs, a set comes from the
> > +AP and is wired while a second set comes from the CPs by the mean of
> > +MSIs. Each 'domain' is represented as a subnode.
> > +
> > +Required properties:
> > +
> > +- compatible: should be "marvell,armada-8k-sei".
> > +- reg: SEI registers location and length.
> > +- interrupts: identifies the parent IRQ that will be triggered.
> > +
> > +Child node 'sei-wired-controller' required properties:
> > +
> > +- marvell,sei-ranges: ranges of wired interrupts.
> > +- #interrupt-cells: number of cells to define an SEI wired interrupt
> > + coming from the AP, should be 1. The cell is the IRQ
> > + number.
> > +- interrupt-controller: identifies the node as an interrupt controller.
> > +
> > +Child node 'sei-msi-controller' required properties:
> > +
> > +- marvell,sei-ranges: ranges of non-wired interrupts triggered by way of
> > + MSIs.
> > +- msi-controller: identifies the node as an MSI controller.
> > +
> > +Example:
> > +
> > + sei: sei at 3f0200 {
> > + compatible = "marvell,armada-8k-sei";
> > + reg = <0x3f0200 0x40>;
> > + interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > + sei_wired_controller: sei-wired-controller at 0 {
> > + marvell,sei-ranges = <0 21>;
> > + #interrupt-cells = <1>;
> > + interrupt-controller;
> > + };
> > +
> > + sei_msi_controller: sei-msi-controller at 21 {
> > + marvell,sei-ranges = <21 43>;
> > + msi-controller;
> > + };
>
> I still think this should just be all one node. There's several examples
> in the tree of nodes which are both interrupt-controller and
> msi-controller. Marvell MPIC is one example.
I checked Marvell MPIC example (armada 370 XP), it does not use
hierarchy domains, so I totally understand your point but I'm not sure
how I could get inspired by this driver (I'm looking for others).
Here I'm stuck. I know from a pure DT point of view the following is
not a valid argument. But from Linux, there is no easy way to handle
this situation without two different device nodes due to the internals
of the irqchip subsystem. There is simply no easy solution and having
only one node would require consequent changes in the core.
Maybe Marc will have an idea, but I think we already gave up on this
topic :/
Regards,
Miqu?l
^ permalink raw reply
* [PATCH v2 5/5] arm64: perf: Add support for chaining event counters
From: Suzuki K Poulose @ 2018-06-08 14:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180606180119.4ofhges6codarbmk@lakrids.cambridge.arm.com>
Hi Mark,
On 06/06/2018 07:01 PM, Mark Rutland wrote:
> On Tue, May 29, 2018 at 11:55:56AM +0100, Suzuki K Poulose wrote:
>> Add support for 64bit event by using chained event counters
>> and 64bit cycle counters.
>>
>> Arm v8 PMUv3 allows chaining a pair of adjacent PMU counters
>> (with the lower counter number being always "even"). The low
>> counter is programmed to count the event of interest and the
>> high counter(odd numbered) is programmed with a special event
>> code (0x1e - Chain).
>
> I found this a little difficult to read. How about:
>
> PMUv3 allows chaining a pair of adjacent 32-bit counters, effectively
> forming a 64-bit counter. The low/even counter is programmed to count
> the event of interest, and the high/odd counter is programmed to count
> the CHAIN event, taken when the low/even counter overflows.
>
Sure, that looks better.
>> Thus we need special allocation schemes
>> to make the full use of available counters. So, we allocate the
>> counters from either ends. i.e, chained counters are allocated
>> from the lower end in pairs of two and the normal counters are
>> allocated from the higher number. Also makes necessary changes to
>> handle the chained events as a single event with 2 counters.
>
> Why do we need to allocate in this way?
>
> I can see this might make allocating a pair of counters more likely in
> some cases, but there are still others where it wouldn't be possible
> (and we'd rely on the rotation logic to repack the counters for us).
It makes the efficient use of the counters in all cases and allows
counting maximum number of events with any given set, keeping the
precedence on the order of their "inserts".
e.g, if the number of counters happened to be "odd" (not sure if it is
even possible).
>
> [...]
>
>> +static inline u32 armv8pmu_read_evcntr(int idx)
>> +{
>> + return (armv8pmu_select_counter(idx) == idx) ?
>> + read_sysreg(pmxevcntr_el0) : 0;
>> +}
>
> Given armv8pmu_select_counter() always returns the idx passed to it, I
> don't think we need to check anything here. We can clean that up to be
> void, and remove the existing checks.
>
> [...]
OK.
>
>> +static inline u64 armv8pmu_read_chain_counter(int idx)
>> +{
>> + u64 prev_hi, hi, lo;
>> +
>> + hi = armv8pmu_read_evcntr(idx);
>> + do {
>> + prev_hi = hi;
>> + isb();
>> + lo = armv8pmu_read_evcntr(idx - 1);
>> + isb();
>> + hi = armv8pmu_read_evcntr(idx);
>> + } while (prev_hi != hi);
>> +
>> + return (hi << 32) | lo;
>> +}
>
>> +static inline void armv8pmu_write_chain_counter(int idx, u64 value)
>> +{
>> + armv8pmu_write_evcntr(idx, value >> 32);
>> + isb();
>> + armv8pmu_write_evcntr(idx - 1, value);
>> +}
>
> Can we use upper_32_bits() and lower_32_bits() here?
>
> As a more general thing, I think we need to clean up the way we
> read/write counters, because I don't think that it's right that we poke
> them while they're running -- that means you get some arbitrary skew on
> counter groups.
>
> It looks like the only case we do that is the IRQ handler, so we should
> be able to stop/start the PMU there.
Since we don't stop the "counting" of events usually when an IRQ is
triggered, the skew will be finally balanced when the events are stopped
in a the group. So, I think, stopping the PMU may not be really a good
thing after all. Just my thought.
>
> With that, we can get rid of the ISB here, and likewise in
> read_chain_counter, which wouldn't need to be a loop.
>
>> +
>> +static inline void armv8pmu_write_hw_counter(struct perf_event *event,
>> + u64 value)
>> +{
>> + int idx = event->hw.idx;
>> +
>> + if (armv8pmu_event_is_chained(event))
>> + armv8pmu_write_chain_counter(idx, value);
>> + else
>> + armv8pmu_write_evcntr(idx, value);
>> +}
>> +
>> static inline void armv8pmu_write_counter(struct perf_event *event, u64 value)
>> {
>> struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
>> @@ -541,14 +612,14 @@ static inline void armv8pmu_write_counter(struct perf_event *event, u64 value)
>> smp_processor_id(), idx);
>> else if (idx == ARMV8_IDX_CYCLE_COUNTER) {
>> /*
>> - * Set the upper 32bits as this is a 64bit counter but we only
>> - * count using the lower 32bits and we want an interrupt when
>> - * it overflows.
>> + * Set the upper 32bits if we are counting this in
>> + * 32bit mode, as this is a 64bit counter.
>> */
>
> It would be good to keep the explaination as to why.
>
Sure
>> - value |= 0xffffffff00000000ULL;
>> + if (!armv8pmu_event_is_64bit(event))
>> + value |= 0xffffffff00000000ULL;
>> write_sysreg(value, pmccntr_el0);
>> - } else if (armv8pmu_select_counter(idx) == idx)
>> - write_sysreg(value, pmxevcntr_el0);
>> + } else
>> + armv8pmu_write_hw_counter(event, value);
>> }
>
>> +static inline void armv8pmu_write_event_type(struct perf_event *event)
>> +{
>> + struct hw_perf_event *hwc = &event->hw;
>> + int idx = hwc->idx;
>> +
>> + /*
>> + * For chained events, write the the low counter event type
>> + * followed by the high counter. The high counter is programmed
>> + * with CHAIN event code with filters set to count at all ELs.
>> + */
>> + if (armv8pmu_event_is_chained(event)) {
>> + u32 chain_evt = ARMV8_PMUV3_PERFCTR_CHAIN |
>> + ARMV8_PMU_INCLUDE_EL2;
>> +
>> + armv8pmu_write_evtype(idx - 1, hwc->config_base);
>> + isb();
>> + armv8pmu_write_evtype(idx, chain_evt);
>
> The ISB isn't necessary here, AFAICT. We only do this while the PMU is
> disabled; no?
You're right. I was just following the ARM ARM.
>
>> + } else
>> + armv8pmu_write_evtype(idx, hwc->config_base);
>> +}
>
> [...]
>
>> @@ -679,6 +679,12 @@ static void cpu_pm_pmu_setup(struct arm_pmu *armpmu, unsigned long cmd)
>> continue;
>>
>> event = hw_events->events[idx];
>> + /*
>> + * If there is no event at this idx (e.g, an idx used
>> + * by a chained event in Arm v8 PMUv3), skip it.
>> + */
>> + if (!event)
>> + continue;
>
> We may as well lose the used_mask test if we're looking at the event
> regardless.
Ok
Suzuki
^ permalink raw reply
* [PATCH v2 5/5] arm64: perf: Add support for chaining event counters
From: Mark Rutland @ 2018-06-08 15:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4a5b5e7f-fc0b-84e3-fc65-b9f860029207@arm.com>
On Fri, Jun 08, 2018 at 03:46:57PM +0100, Suzuki K Poulose wrote:
> On 06/06/2018 07:01 PM, Mark Rutland wrote:
> > > Thus we need special allocation schemes
> > > to make the full use of available counters. So, we allocate the
> > > counters from either ends. i.e, chained counters are allocated
> > > from the lower end in pairs of two and the normal counters are
> > > allocated from the higher number. Also makes necessary changes to
> > > handle the chained events as a single event with 2 counters.
> >
> > Why do we need to allocate in this way?
> >
> > I can see this might make allocating a pair of counters more likely in
> > some cases, but there are still others where it wouldn't be possible
> > (and we'd rely on the rotation logic to repack the counters for us).
>
> It makes the efficient use of the counters in all cases and allows
> counting maximum number of events with any given set, keeping the precedence
> on the order of their "inserts".
> e.g, if the number of counters happened to be "odd" (not sure if it is even
> possible).
Unfortunately, that doesn't always work.
Say you have a system with 8 counters, and you open 8 (32-bit) events.
Then you close the events in counters 0, 2, 4, and 6. The live events
aren't moved, and stay where they are, in counters 1, 3, 5, and 7.
Now, say you open a 64-bit event. When we try to add it, we try to
allocate an index for two consecutive counters, and find that we can't,
despite 4 counters being free.
We return -EAGAIN to the core perf code, whereupon it removes any other
events in that group (none in this case).
Then we wait for the rotation logic to pull all of the events out and
schedule them back in, re-packing them, which should (eventually) work
regardless of how we allocate counters.
... we might need to re-pack events to solve that. :/
[...]
> > > +static inline void armv8pmu_write_chain_counter(int idx, u64 value)
> > > +{
> > > + armv8pmu_write_evcntr(idx, value >> 32);
> > > + isb();
> > > + armv8pmu_write_evcntr(idx - 1, value);
> > > +}
> >
> > Can we use upper_32_bits() and lower_32_bits() here?
> >
> > As a more general thing, I think we need to clean up the way we
> > read/write counters, because I don't think that it's right that we poke
> > them while they're running -- that means you get some arbitrary skew on
> > counter groups.
> >
> > It looks like the only case we do that is the IRQ handler, so we should
> > be able to stop/start the PMU there.
>
> Since we don't stop the "counting" of events usually when an IRQ is
> triggered, the skew will be finally balanced when the events are stopped
> in a the group. So, I think, stopping the PMU may not be really a good
> thing after all. Just my thought.
That's not quite right -- if one event in a group overflows, we'll
reprogram it *while* other events are counting, losing some events in
the process.
Stopping the PMU for the duration of the IRQ handler ensures that events
in a group are always in-sync with one another.
Thanks,
Mark.
^ permalink raw reply
* arm: mach-mvebu: dts: enable-method is always overwritten
From: Yves Lefloch @ 2018-06-08 15:43 UTC (permalink / raw)
To: linux-arm-kernel
Hello everybody,
I'm facing an issue that I believe to be a conflict between device-tree and
machine_desc.
My platform is arm/mach-mvebu. I have a DT based on "armada-xp-db-dxbc2.dts" (I
just included it and added a few okays), my CPU is a Marvell Bobcat2 switching
chip. My kernel is a vanilla 4.16.
Everything works fine except that my second core won't boot: `CPU1: failed to
come online'.
I tracked down the problem to arch/arm/mach-mvebu/platsmp.c: in this file is
defined a machine_desc that hardcodes the SMP ops to `marvell,armada-xp-smp'
whereas my device tree (by including armada-xp-98dx3236.dtsi) attempts to set
the ops to `marvell,98dx3236-smp' through enable-method. In setup_arch() the
machine_desc's ops overwrites the enable-method's ops, causing the wrong
smp_boot_secondary() call to be issued.
Now there is a note from 2014 saying that this machine_desc's `smp' field is
hardcoded like that because of "old Device Trees that were not specifying the
cpus enable-method property". As far as I can tell, this is still the case, for
instance "armada-370-db.dts" doesn't have any enable-method property.
I have worked around this by commenting out `armada_xp_smp_ops.smp' but
obviously I would prefer to keep a vanilla kernel.
So I propose to:
- Add `enable-method = "marvell,armada-xp-smp"' to armada-370-xp.dtsi, because
it seems that all Armada 370/XP include it;
- Remove the `smp' field of `armada_xp_smp_ops'.
If you agree with the diagnosis and the proposed fix I will write a patch.
Regards,
Yves Lefloch.
^ permalink raw reply
* [PATCH v4 1/7] interconnect: Add generic on-chip interconnect API
From: Alexandre Bailon @ 2018-06-08 15:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180309210958.16672-2-georgi.djakov@linaro.org>
On 03/09/2018 10:09 PM, Georgi Djakov wrote:
> This patch introduce a new API to get requirements and configure the
> interconnect buses across the entire chipset to fit with the current
> demand.
>
> The API is using a consumer/provider-based model, where the providers are
> the interconnect buses and the consumers could be various drivers.
> The consumers request interconnect resources (path) between endpoints and
> set the desired constraints on this data flow path. The providers receive
> requests from consumers and aggregate these requests for all master-slave
> pairs on that path. Then the providers configure each participating in the
> topology node according to the requested data flow path, physical links and
> constraints. The topology could be complicated and multi-tiered and is SoC
> specific.
>
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---
> Documentation/interconnect/interconnect.rst | 96 ++++++
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/interconnect/Kconfig | 10 +
> drivers/interconnect/Makefile | 1 +
> drivers/interconnect/core.c | 489 ++++++++++++++++++++++++++++
> include/linux/interconnect-provider.h | 109 +++++++
> include/linux/interconnect.h | 40 +++
> 8 files changed, 748 insertions(+)
> create mode 100644 Documentation/interconnect/interconnect.rst
> create mode 100644 drivers/interconnect/Kconfig
> create mode 100644 drivers/interconnect/Makefile
> create mode 100644 drivers/interconnect/core.c
> create mode 100644 include/linux/interconnect-provider.h
> create mode 100644 include/linux/interconnect.h
>
> diff --git a/Documentation/interconnect/interconnect.rst b/Documentation/interconnect/interconnect.rst
> new file mode 100644
> index 000000000000..23eba68e8424
> --- /dev/null
> +++ b/Documentation/interconnect/interconnect.rst
> @@ -0,0 +1,96 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=====================================
> +GENERIC SYSTEM INTERCONNECT SUBSYSTEM
> +=====================================
> +
> +Introduction
> +------------
> +
> +This framework is designed to provide a standard kernel interface to control
> +the settings of the interconnects on a SoC. These settings can be throughput,
> +latency and priority between multiple interconnected devices or functional
> +blocks. This can be controlled dynamically in order to save power or provide
> +maximum performance.
> +
> +The interconnect bus is a hardware with configurable parameters, which can be
> +set on a data path according to the requests received from various drivers.
> +An example of interconnect buses are the interconnects between various
> +components or functional blocks in chipsets. There can be multiple interconnects
> +on a SoC that can be multi-tiered.
> +
> +Below is a simplified diagram of a real-world SoC interconnect bus topology.
> +
> +::
> +
> + +----------------+ +----------------+
> + | HW Accelerator |--->| M NoC |<---------------+
> + +----------------+ +----------------+ |
> + | | +------------+
> + +-----+ +-------------+ V +------+ | |
> + | DDR | | +--------+ | PCIe | | |
> + +-----+ | | Slaves | +------+ | |
> + ^ ^ | +--------+ | | C NoC |
> + | | V V | |
> + +------------------+ +------------------------+ | | +-----+
> + | |-->| |-->| |-->| CPU |
> + | |-->| |<--| | +-----+
> + | Mem NoC | | S NoC | +------------+
> + | |<--| |---------+ |
> + | |<--| |<------+ | | +--------+
> + +------------------+ +------------------------+ | | +-->| Slaves |
> + ^ ^ ^ ^ ^ | | +--------+
> + | | | | | | V
> + +------+ | +-----+ +-----+ +---------+ +----------------+ +--------+
> + | CPUs | | | GPU | | DSP | | Masters |-->| P NoC |-->| Slaves |
> + +------+ | +-----+ +-----+ +---------+ +----------------+ +--------+
> + |
> + +-------+
> + | Modem |
> + +-------+
> +
> +Terminology
> +-----------
> +
> +Interconnect provider is the software definition of the interconnect hardware.
> +The interconnect providers on the above diagram are M NoC, S NoC, C NoC and Mem
> +NoC.
> +
> +Interconnect node is the software definition of the interconnect hardware
> +port. Each interconnect provider consists of multiple interconnect nodes,
> +which are connected to other SoC components including other interconnect
> +providers. The point on the diagram where the CPUs connects to the memory is
> +called an interconnect node, which belongs to the Mem NoC interconnect provider.
> +
> +Interconnect endpoints are the first or the last element of the path. Every
> +endpoint is a node, but not every node is an endpoint.
> +
> +Interconnect path is everything between two endpoints including all the nodes
> +that have to be traversed to reach from a source to destination node. It may
> +include multiple master-slave pairs across several interconnect providers.
> +
> +Interconnect consumers are the entities which make use of the data paths exposed
> +by the providers. The consumers send requests to providers requesting various
> +throughput, latency and priority. Usually the consumers are device drivers, that
> +send request based on their needs. An example for a consumer is a video decoder
> +that supports various formats and image sizes.
> +
> +Interconnect providers
> +----------------------
> +
> +Interconnect provider is an entity that implements methods to initialize and
> +configure a interconnect bus hardware. The interconnect provider drivers should
> +be registered with the interconnect provider core.
> +
> +The interconnect framework provider API functions are documented in
> +.. kernel-doc:: include/linux/interconnect-provider.h
> +
> +Interconnect consumers
> +----------------------
> +
> +Interconnect consumers are the clients which use the interconnect APIs to
> +get paths between endpoints and set their bandwidth/latency/QoS requirements
> +for these interconnect paths.
> +
> +The interconnect framework consumer API functions are documented in
> +.. kernel-doc:: include/linux/interconnect.h
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 879dc0604cba..96a1db022cee 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -219,4 +219,6 @@ source "drivers/siox/Kconfig"
>
> source "drivers/slimbus/Kconfig"
>
> +source "drivers/interconnect/Kconfig"
> +
> endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 24cd47014657..0cca95740d9b 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -185,3 +185,4 @@ obj-$(CONFIG_TEE) += tee/
> obj-$(CONFIG_MULTIPLEXER) += mux/
> obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/
> obj-$(CONFIG_SIOX) += siox/
> +obj-$(CONFIG_INTERCONNECT) += interconnect/
> diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
> new file mode 100644
> index 000000000000..a261c7d41deb
> --- /dev/null
> +++ b/drivers/interconnect/Kconfig
> @@ -0,0 +1,10 @@
> +menuconfig INTERCONNECT
> + tristate "On-Chip Interconnect management support"
> + help
> + Support for management of the on-chip interconnects.
> +
> + This framework is designed to provide a generic interface for
> + managing the interconnects in a SoC.
> +
> + If unsure, say no.
> +
> diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
> new file mode 100644
> index 000000000000..5edf0ae80818
> --- /dev/null
> +++ b/drivers/interconnect/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_INTERCONNECT) += core.o
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> new file mode 100644
> index 000000000000..6306e258b9b9
> --- /dev/null
> +++ b/drivers/interconnect/core.c
> @@ -0,0 +1,489 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Interconnect framework core driver
> + *
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/idr.h>
> +#include <linux/init.h>
> +#include <linux/interconnect.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +
> +static DEFINE_IDR(icc_idr);
> +static LIST_HEAD(icc_provider_list);
> +static DEFINE_MUTEX(icc_provider_list_mutex);
> +static DEFINE_MUTEX(icc_path_mutex);
> +
> +/**
> + * struct icc_req - constraints that are attached to each node
> + *
> + * @req_node: entry in list of requests for the particular @node
> + * @node: the interconnect node to which this constraint applies
> + * @avg_bw: an integer describing the average bandwidth in kbps
> + * @peak_bw: an integer describing the peak bandwidth in kbps
> + */
> +struct icc_req {
> + struct hlist_node req_node;
> + struct icc_node *node;
> + u32 avg_bw;
> + u32 peak_bw;
> +};
> +
> +/**
> + * struct icc_path - interconnect path structure
> + * @num_nodes: number of hops (nodes)
> + * @reqs: array of the requests applicable to this path of nodes
> + */
> +struct icc_path {
> + size_t num_nodes;
> + struct icc_req reqs[0];
> +};
> +
> +static struct icc_node *node_find(const int id)
> +{
> + struct icc_node *node;
> +
> + node = idr_find(&icc_idr, id);
> +
> + return node;
> +}
> +
> +static struct icc_path *path_allocate(struct icc_node *node, ssize_t num_nodes)
> +{
> + struct icc_path *path;
> + size_t i;
> +
> + path = kzalloc(sizeof(*path) + num_nodes * sizeof(*path->reqs),
> + GFP_KERNEL);
> + if (!path)
> + return ERR_PTR(-ENOMEM);
> +
> + path->num_nodes = num_nodes;
> +
> + for (i = 0; i < num_nodes; i++) {
> + hlist_add_head(&path->reqs[i].req_node, &node->req_list);
> +
> + path->reqs[i].node = node;
> + /* reference to previous node was saved during path traversal */
> + node = node->reverse;
> + }
> +
> + return path;
> +}
> +
> +static struct icc_path *path_find(struct icc_node *src, struct icc_node *dst)
> +{
> + struct icc_node *node = NULL;
> + struct list_head traverse_list;
> + struct list_head edge_list;
> + struct list_head tmp_list;
> + size_t i, number = 0;
> + bool found = false;
> +
> + INIT_LIST_HEAD(&traverse_list);
> + INIT_LIST_HEAD(&edge_list);
> + INIT_LIST_HEAD(&tmp_list);
> +
> + list_add_tail(&src->search_list, &traverse_list);
> +
> + do {
> + list_for_each_entry(node, &traverse_list, search_list) {
> + if (node == dst) {
> + found = true;
> + list_add(&node->search_list, &tmp_list);
> + break;
> + }
> + for (i = 0; i < node->num_links; i++) {
> + struct icc_node *tmp = node->links[i];
> +
> + if (!tmp)
> + return ERR_PTR(-ENOENT);
> +
> + if (tmp->is_traversed)
> + continue;
> +
> + tmp->is_traversed = true;
> + tmp->reverse = node;
> + list_add_tail(&tmp->search_list, &edge_list);
> + }
> + }
> + if (found)
> + break;
> +
> + list_splice_init(&traverse_list, &tmp_list);
> + list_splice_init(&edge_list, &traverse_list);
> +
> + /* count the number of nodes */
> + number++;
> +
> + } while (!list_empty(&traverse_list));
> +
> + /* reset the traversed state */
> + list_for_each_entry(node, &tmp_list, search_list)
> + node->is_traversed = false;
> +
> + if (found)
> + return path_allocate(dst, number);
> +
> + return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +static int path_init(struct icc_path *path)
> +{
> + struct icc_node *node;
> + size_t i;
> +
> + for (i = 0; i < path->num_nodes; i++) {
> + node = path->reqs[i].node;
> +
> + mutex_lock(&node->provider->lock);
> + node->provider->users++;
> + mutex_unlock(&node->provider->lock);
> + }
> +
> + return 0;
> +}
> +
> +static void node_aggregate(struct icc_node *node)
> +{
> + struct icc_req *r;
> + u32 agg_avg = 0;
> + u32 agg_peak = 0;
> +
> + hlist_for_each_entry(r, &node->req_list, req_node) {
> + /* sum(averages) and max(peaks) */
> + agg_avg += r->avg_bw;
> + agg_peak = max(agg_peak, r->peak_bw);
> + }
> +
> + node->avg_bw = agg_avg;
> + node->peak_bw = agg_peak;
> +}
> +
> +static void provider_aggregate(struct icc_provider *provider, u32 *avg_bw,
> + u32 *peak_bw)
> +{
> + struct icc_node *n;
> + u32 agg_avg = 0;
> + u32 agg_peak = 0;
> +
> + /* aggregate for the interconnect provider */
> + list_for_each_entry(n, &provider->nodes, node_list) {
> + /* sum the average and max the peak */
> + agg_avg += n->avg_bw;
> + agg_peak = max(agg_peak, n->peak_bw);
> + }
> +
> + *avg_bw = agg_avg;
> + *peak_bw = agg_peak;
> +}
> +
> +static int constraints_apply(struct icc_path *path)
> +{
> + struct icc_node *next, *prev = NULL;
> + int i;
> +
> + for (i = 0; i < path->num_nodes; i++, prev = next) {
> + struct icc_provider *provider;
> + u32 avg_bw = 0;
> + u32 peak_bw = 0;
> + int ret;
> +
> + next = path->reqs[i].node;
> + /*
> + * Both endpoints should be valid master-slave pairs of the
> + * same interconnect provider that will be configured.
> + */
> + if (!next || !prev)
> + continue;
> +
> + if (next->provider != prev->provider)
> + continue;
> +
> + provider = next->provider;
> + mutex_lock(&provider->lock);
> +
> + /* aggregate requests for the provider */
> + provider_aggregate(provider, &avg_bw, &peak_bw);
> +
> + if (provider->set) {
> + /* set the constraints */
> + ret = provider->set(prev, next, avg_bw, peak_bw);
> + }
> +
> + mutex_unlock(&provider->lock);
> +
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * icc_set() - set constraints on an interconnect path between two endpoints
> + * @path: reference to the path returned by icc_get()
> + * @avg_bw: average bandwidth in kbps
> + * @peak_bw: peak bandwidth in kbps
> + *
> + * This function is used by an interconnect consumer to express its own needs
> + * in term of bandwidth and QoS for a previously requested path between two
> + * endpoints. The requests are aggregated and each node is updated accordingly.
> + *
> + * Returns 0 on success, or an approproate error code otherwise.
> + */
> +int icc_set(struct icc_path *path, u32 avg_bw, u32 peak_bw)
> +{
> + struct icc_node *node;
> + size_t i;
> + int ret;
> +
> + if (!path)
> + return 0;
> +
> + for (i = 0; i < path->num_nodes; i++) {
> + node = path->reqs[i].node;
> +
> + mutex_lock(&icc_path_mutex);
> +
> + /* update the consumer request for this path */
> + path->reqs[i].avg_bw = avg_bw;
> + path->reqs[i].peak_bw = peak_bw;
> +
> + /* aggregate requests for this node */
> + node_aggregate(node);
> +
> + mutex_unlock(&icc_path_mutex);
> + }
> +
> + ret = constraints_apply(path);
> + if (ret)
> + pr_err("interconnect: error applying constraints (%d)", ret);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(icc_set);
> +
> +/**
> + * icc_get() - return a handle for path between two endpoints
> + * @src_id: source device port id
> + * @dst_id: destination device port id
> + *
> + * This function will search for a path between two endpoints and return an
> + * icc_path handle on success. Use icc_put() to release
> + * constraints when the they are not needed anymore.
> + *
> + * Return: icc_path pointer on success, or ERR_PTR() on error
> + */
> +struct icc_path *icc_get(const int src_id, const int dst_id)
> +{
> + struct icc_node *src, *dst;
> + struct icc_path *path = ERR_PTR(-EPROBE_DEFER);
> +
> + src = node_find(src_id);
> + if (!src)
> + goto out;
> +
> + dst = node_find(dst_id);
> + if (!dst)
> + goto out;
> +
> + mutex_lock(&icc_path_mutex);
> + path = path_find(src, dst);
> + mutex_unlock(&icc_path_mutex);
> + if (IS_ERR(path))
> + goto out;
> +
> + path_init(path);
> +
> +out:
> + return path;
> +}
> +EXPORT_SYMBOL_GPL(icc_get);
> +
> +/**
> + * icc_put() - release the reference to the icc_path
> + * @path: interconnect path
> + *
> + * Use this function to release the constraints on a path when the path is
> + * no longer needed. The constraints will be re-aggregated.
> + */
> +void icc_put(struct icc_path *path)
> +{
> + struct icc_node *node;
> + size_t i;
> + int ret;
> +
> + if (!path || WARN_ON_ONCE(IS_ERR(path)))
> + return;
> +
> + ret = icc_set(path, 0, 0);
> + if (ret)
> + pr_err("%s: error (%d)\n", __func__, ret);
> +
> + for (i = 0; i < path->num_nodes; i++) {
> + node = path->reqs[i].node;
> + hlist_del(&path->reqs[i].req_node);
> +
> + mutex_lock(&node->provider->lock);
> + node->provider->users--;
> + mutex_unlock(&node->provider->lock);
> + }
> +
> + kfree(path);
> +}
> +EXPORT_SYMBOL_GPL(icc_put);
> +
> +/**
> + * icc_node_create() - create a node
> + * @id: node id
> + *
> + * Return: icc_node pointer on success, or ERR_PTR() on error
> + */
> +struct icc_node *icc_node_create(int id)
> +{
> + struct icc_node *node;
> +
> + /* check if node already exists */
> + node = node_find(id);
> + if (node)
> + return node;
> +
> + node = kzalloc(sizeof(*node), GFP_KERNEL);
> + if (!node)
> + return ERR_PTR(-ENOMEM);
> +
> + id = idr_alloc(&icc_idr, node, id, id + 1, GFP_KERNEL);
> + if (WARN(id < 0, "couldn't get idr"))
> + return ERR_PTR(id);
> +
> + node->id = id;
> +
> + return node;
> +}
> +EXPORT_SYMBOL_GPL(icc_node_create);
> +
> +/**
> + * icc_link_create() - create a link between two nodes
> + * @src_id: source node id
I guess src_id has become node and is not an id anymore,
so it should be updated.
> + * @dst_id: destination node id
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_link_create(struct icc_node *node, const int dst_id)
> +{
> + struct icc_node *dst;
> + struct icc_node **new;
> + int ret = 0;
> +
> + if (IS_ERR_OR_NULL(node))
> + return PTR_ERR(node);
> +
> + mutex_lock(&node->provider->lock);
> +
> + dst = node_find(dst_id);
> + if (!dst)
> + dst = icc_node_create(dst_id);
> +
> + new = krealloc(node->links,
> + (node->num_links + 1) * sizeof(*node->links),
> + GFP_KERNEL);
> + if (!new) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + node->links = new;
> + node->links[node->num_links++] = dst;
> +
> +out:
> + mutex_unlock(&node->provider->lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(icc_link_create);
> +
> +/**
> + * icc_add_node() - add an interconnect node to interconnect provider
> + * @node: pointer to the interconnect node
> + * @provider: pointer to the interconnect provider
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_node_add(struct icc_node *node, struct icc_provider *provider)
> +{
> + if (WARN_ON(!node))
> + return -EINVAL;
> +
> + if (WARN_ON(!provider))
> + return -EINVAL;
> +
> + node->provider = provider;
> +
> + mutex_lock(&provider->lock);
> + list_add_tail(&node->node_list, &provider->nodes);
> + mutex_unlock(&provider->lock);
> +
> + return 0;
> +}
> +
> +/**
> + * icc_add_provider() - add a new interconnect provider
> + * @icc_provider: the interconnect provider that will be added into topology
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_add_provider(struct icc_provider *provider)
> +{
> + if (WARN_ON(!provider))
> + return -EINVAL;
> +
> + if (WARN_ON(!provider->set))
> + return -EINVAL;
> +
> + mutex_init(&provider->lock);
> + INIT_LIST_HEAD(&provider->nodes);
> +
> + mutex_lock(&icc_provider_list_mutex);
> + list_add(&provider->provider_list, &icc_provider_list);
> + mutex_unlock(&icc_provider_list_mutex);
> +
> + dev_dbg(provider->dev, "interconnect provider added to topology\n");
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(icc_add_provider);
> +
> +/**
> + * icc_del_provider() - delete previously added interconnect provider
> + * @icc_provider: the interconnect provider that will be removed from topology
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_del_provider(struct icc_provider *provider)
> +{
> + mutex_lock(&provider->lock);
> + if (provider->users) {
> + pr_warn("interconnect provider still has %d users\n",
> + provider->users);
> + }
> + mutex_unlock(&provider->lock);
> +
> + mutex_lock(&icc_provider_list_mutex);
> + list_del(&provider->provider_list);
> + mutex_unlock(&icc_provider_list_mutex);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(icc_del_provider);
> +
> +MODULE_AUTHOR("Georgi Djakov <georgi.djakov@linaro.org");
> +MODULE_DESCRIPTION("Interconnect Driver Core");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/interconnect-provider.h b/include/linux/interconnect-provider.h
> new file mode 100644
> index 000000000000..779b5b5b1306
> --- /dev/null
> +++ b/include/linux/interconnect-provider.h
> @@ -0,0 +1,109 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#ifndef _LINUX_INTERCONNECT_PROVIDER_H
> +#define _LINUX_INTERCONNECT_PROVIDER_H
> +
> +#include <linux/interconnect.h>
> +
> +struct icc_node;
> +
> +/**
> + * struct icc_provider - interconnect provider (controller) entity that might
> + * provide multiple interconnect controls
> + *
> + * @provider_list: list of the registered interconnect providers
> + * @nodes: internal list of the interconnect provider nodes
> + * @set: pointer to device specific set operation function
> + * @dev: the device this interconnect provider belongs to
> + * @lock: lock to provide consistency during aggregation/update of constraints
> + * @users: count of active users
> + * @data: pointer to private data
> + */
> +struct icc_provider {
> + struct list_head provider_list;
> + struct list_head nodes;
> + int (*set)(struct icc_node *src, struct icc_node *dst,
> + u32 avg_bw, u32 peak_bw);
> + struct device *dev;
> + struct mutex lock;
> + int users;
> + void *data;
> +};
> +
> +/**
> + * struct icc_node - entity that is part of the interconnect topology
> + *
> + * @id: platform specific node id
> + * @name: node name used in debugfs
> + * @links: a list of targets where we can go next when traversing
> + * @num_links: number of links to other interconnect nodes
> + * @provider: points to the interconnect provider of this node
> + * @node_list: list of interconnect nodes associated with @provider
> + * @search_list: list used when walking the nodes graph
> + * @reverse: pointer to previous node when walking the nodes graph
> + * @is_traversed: flag that is used when walking the nodes graph
> + * @req_list: a list of QoS constraint requests associated with this node
> + * @avg_bw: aggregated value of average bandwidth
> + * @peak_bw: aggregated value of peak bandwidth
> + * @data: pointer to private data
> + */
> +struct icc_node {
> + int id;
> + const char *name;
> + struct icc_node **links;
> + size_t num_links;
> +
> + struct icc_provider *provider;
> + struct list_head node_list;
> + struct list_head orphan_list;
> + struct list_head search_list;
> + struct icc_node *reverse;
> + bool is_traversed;
> + struct hlist_head req_list;
> + u32 avg_bw;
> + u32 peak_bw;
> + void *data;
> +};
> +
> +#if IS_ENABLED(CONFIG_INTERCONNECT)
> +
> +struct icc_node *icc_node_create(int id);
> +int icc_node_add(struct icc_node *node, struct icc_provider *provider);
> +int icc_link_create(struct icc_node *node, const int dst_id);
> +int icc_add_provider(struct icc_provider *provider);
> +int icc_del_provider(struct icc_provider *provider);
> +
> +#else
> +
> +static inline struct icc_node *icc_node_create(int id)
> +{
> + return ERR_PTR(-ENOTSUPP);
> +}
> +
> +int icc_node_add(struct icc_node *node, struct icc_provider *provider)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline int icc_link_create(struct icc_node *node, const int dst_id)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline int icc_add_provider(struct icc_provider *provider)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline int icc_del_provider(struct icc_provider *provider)
> +{
> + return -ENOTSUPP;
> +}
> +
> +#endif /* CONFIG_INTERCONNECT */
> +
> +#endif /* _LINUX_INTERCONNECT_PROVIDER_H */
> diff --git a/include/linux/interconnect.h b/include/linux/interconnect.h
> new file mode 100644
> index 000000000000..5a7cf72b76a5
> --- /dev/null
> +++ b/include/linux/interconnect.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#ifndef _LINUX_INTERCONNECT_H
> +#define _LINUX_INTERCONNECT_H
> +
> +#include <linux/types.h>
> +#include <linux/mutex.h>
> +
> +struct icc_path;
> +struct device;
> +
> +#if IS_ENABLED(CONFIG_INTERCONNECT)
> +
> +struct icc_path *icc_get(const int src_id, const int dst_id);
> +void icc_put(struct icc_path *path);
> +int icc_set(struct icc_path *path, u32 avg_bw, u32 peak_bw);
> +
> +#else
> +
> +static inline struct icc_path *icc_get(const int src_id, const int dst_id)
> +{
> + return NULL;
> +}
> +
> +static inline void icc_put(struct icc_path *path)
> +{
> +}
> +
> +static inline int icc_set(struct icc_path *path, u32 avg_bw, u32 peak_bw)
> +{
> + return 0;
> +}
> +
> +#endif /* CONFIG_INTERCONNECT */
> +
> +#endif /* _LINUX_INTERCONNECT_H */
>
^ permalink raw reply
* arm: mach-mvebu: dts: enable-method is always overwritten
From: Andrew Lunn @ 2018-06-08 15:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a2d4701e-28b3-07ee-6b2f-588b2e9a75c4@online.net>
On Fri, Jun 08, 2018 at 05:43:11PM +0200, Yves Lefloch wrote:
> Hello everybody,
Adding Chris Packham. He did some work in this area.
Andrew
>
> I'm facing an issue that I believe to be a conflict between device-tree and
> machine_desc.
>
> My platform is arm/mach-mvebu. I have a DT based on "armada-xp-db-dxbc2.dts"
> (I just included it and added a few okays), my CPU is a Marvell Bobcat2
> switching chip. My kernel is a vanilla 4.16.
>
> Everything works fine except that my second core won't boot: `CPU1: failed
> to come online'.
> I tracked down the problem to arch/arm/mach-mvebu/platsmp.c: in this file is
> defined a machine_desc that hardcodes the SMP ops to `marvell,armada-xp-smp'
> whereas my device tree (by including armada-xp-98dx3236.dtsi) attempts to
> set the ops to `marvell,98dx3236-smp' through enable-method. In setup_arch()
> the machine_desc's ops overwrites the enable-method's ops, causing the wrong
> smp_boot_secondary() call to be issued.
>
> Now there is a note from 2014 saying that this machine_desc's `smp' field is
> hardcoded like that because of "old Device Trees that were not specifying
> the cpus enable-method property". As far as I can tell, this is still the
> case, for instance "armada-370-db.dts" doesn't have any enable-method
> property.
>
> I have worked around this by commenting out `armada_xp_smp_ops.smp' but
> obviously I would prefer to keep a vanilla kernel.
>
> So I propose to:
> - Add `enable-method = "marvell,armada-xp-smp"' to armada-370-xp.dtsi,
> because it seems that all Armada 370/XP include it;
> - Remove the `smp' field of `armada_xp_smp_ops'.
>
> If you agree with the diagnosis and the proposed fix I will write a patch.
>
> Regards,
> Yves Lefloch.
^ permalink raw reply
* [PATCH v2 5/5] arm64: perf: Add support for chaining event counters
From: Suzuki K Poulose @ 2018-06-08 16:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180608152419.kamu7ptmgsoah5m3@lakrids.cambridge.arm.com>
On 08/06/18 16:24, Mark Rutland wrote:
> On Fri, Jun 08, 2018 at 03:46:57PM +0100, Suzuki K Poulose wrote:
>> On 06/06/2018 07:01 PM, Mark Rutland wrote:
>>>> Thus we need special allocation schemes
>>>> to make the full use of available counters. So, we allocate the
>>>> counters from either ends. i.e, chained counters are allocated
>>>> from the lower end in pairs of two and the normal counters are
>>>> allocated from the higher number. Also makes necessary changes to
>>>> handle the chained events as a single event with 2 counters.
>>>
>>> Why do we need to allocate in this way?
>>>
>>> I can see this might make allocating a pair of counters more likely in
>>> some cases, but there are still others where it wouldn't be possible
>>> (and we'd rely on the rotation logic to repack the counters for us).
>>
>> It makes the efficient use of the counters in all cases and allows
>> counting maximum number of events with any given set, keeping the precedence
>> on the order of their "inserts".
>> e.g, if the number of counters happened to be "odd" (not sure if it is even
>> possible).
>
> Unfortunately, that doesn't always work.
>
> Say you have a system with 8 counters, and you open 8 (32-bit) events.
I was talking about the following (imaginary) case :
We have 7 counters, and you have 5 32bit counters and 1 64bit counter.
Without the above scheme, you would place first 5 events on the first
5 counters and then you can't place the 64bit counter, as you are now
left with a low/odd counter and a high/even counter.
>
> Then you close the events in counters 0, 2, 4, and 6. The live events
> aren't moved, and stay where they are, in counters 1, 3, 5, and 7.
>
> Now, say you open a 64-bit event. When we try to add it, we try to
> allocate an index for two consecutive counters, and find that we can't,
> despite 4 counters being free.
>
> We return -EAGAIN to the core perf code, whereupon it removes any other
> events in that group (none in this case).
>
> Then we wait for the rotation logic to pull all of the events out and
> schedule them back in, re-packing them, which should (eventually) work
> regardless of how we allocate counters.
>
> ... we might need to re-pack events to solve that. :/
I agree, removing and putting them back in is not going to work unless
we re-pack or delay allocating the counters until we start the PMU.
>
> [...]
>
>>>> +static inline void armv8pmu_write_chain_counter(int idx, u64 value)
>>>> +{
>>>> + armv8pmu_write_evcntr(idx, value >> 32);
>>>> + isb();
>>>> + armv8pmu_write_evcntr(idx - 1, value);
>>>> +}
>>>
>>> Can we use upper_32_bits() and lower_32_bits() here?
>>>
>>> As a more general thing, I think we need to clean up the way we
>>> read/write counters, because I don't think that it's right that we poke
>>> them while they're running -- that means you get some arbitrary skew on
>>> counter groups.
>>>
>>> It looks like the only case we do that is the IRQ handler, so we should
>>> be able to stop/start the PMU there.
>>
>> Since we don't stop the "counting" of events usually when an IRQ is
>> triggered, the skew will be finally balanced when the events are stopped
>> in a the group. So, I think, stopping the PMU may not be really a good
>> thing after all. Just my thought.
>
> That's not quite right -- if one event in a group overflows, we'll
> reprogram it *while* other events are counting, losing some events in
> the process.
Oh yes, you're right. I can fix it.
>
> Stopping the PMU for the duration of the IRQ handler ensures that events
> in a group are always in-sync with one another.
Suzuki
^ permalink raw reply
* [GIT PULL] arm64 patches for 4.18
From: Catalin Marinas @ 2018-06-08 16:40 UTC (permalink / raw)
To: linux-arm-kernel
Hi Linus,
Please pull the arm64 updates for 4.18 below. Apart from the core arm64
and perf changes, the Spectre v4 mitigation touches the arm KVM code and
the ACPI PPTT support touches drivers/ (acpi and cacheinfo). I should
have the maintainers' acks in place.
Thanks.
The following changes since commit 75bc37fefc4471e718ba8e651aa74673d4e0a9eb:
Linux 4.17-rc4 (2018-05-06 16:57:38 -1000)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux tags/arm64-upstream
for you to fetch changes up to 0fe42512b2f03f9e5a20b9f55ef1013a68b4cd48:
arm64: Fix syscall restarting around signal suppressed by tracer (2018-06-08 13:21:39 +0100)
----------------------------------------------------------------
arm64 updates for 4.18:
- Spectre v4 mitigation (Speculative Store Bypass Disable) support for
arm64 using SMC firmware call to set a hardware chicken bit
- ACPI PPTT (Processor Properties Topology Table) parsing support and
enable the feature for arm64
- Report signal frame size to user via auxv (AT_MINSIGSTKSZ). The
primary motivation is Scalable Vector Extensions which requires more
space on the signal frame than the currently defined MINSIGSTKSZ
- ARM perf patches: allow building arm-cci as module, demote dev_warn()
to dev_dbg() in arm-ccn event_init(), miscellaneous cleanups
- cmpwait() WFE optimisation to avoid some spurious wakeups
- L1_CACHE_BYTES reverted back to 64 (for performance reasons that have
to do with some network allocations) while keeping ARCH_DMA_MINALIGN
to 128. cache_line_size() returns the actual hardware Cache Writeback
Granule
- Turn LSE atomics on by default in Kconfig
- Kernel fault reporting tidying
- Some #include and miscellaneous cleanups
----------------------------------------------------------------
Arnd Bergmann (3):
drivers/bus: arm-cci: fix build warnings
ARM: mcpm, perf/arm-cci: export mcpm_is_available
arm64: cpu_errata: include required headers
Catalin Marinas (4):
Revert "arm64: Increase the max granular size"
arm64: Increase ARCH_DMA_MINALIGN to 128
Merge branch 'for-next/perf' of git://git.kernel.org/.../will/linux
arm64: KVM: Move VCPU_WORKAROUND_2_FLAG macros to the top of the file
Dave Martin (4):
arm64/sve: Write ZCR_EL1 on context switch only if changed
arm64/sve: Thin out initialisation sanity-checks for sve_max_vl
arm64: signal: Report signal frame size to userspace via auxv
arm64: Fix syscall restarting around signal suppressed by tracer
Jeremy Linton (13):
drivers: base: cacheinfo: move cache_setup_of_node()
drivers: base: cacheinfo: setup DT cache properties early
cacheinfo: rename of_node to fw_token
arm64/acpi: Create arch specific cpu to acpi id helper
ACPI/PPTT: Add Processor Properties Topology Table parsing
ACPI: Enable PPTT support on ARM64
drivers: base cacheinfo: Add support for ACPI based firmware tables
arm64: Add support for ACPI based firmware tables
arm64: topology: rename cluster_id
arm64: topology: enable ACPI/PPTT based CPU topology
ACPI: Add PPTT to injectable table list
arm64: topology: divorce MC scheduling domain from core_siblings
arm64: topology: Avoid checking numa mask for scheduler MC selection
John Garry (1):
drivers/perf: Remove ARM_SPE_PMU explicit PERF_EVENTS dependency
Marc Zyngier (14):
arm/arm64: smccc: Add SMCCC-specific return codes
arm64: Call ARCH_WORKAROUND_2 on transitions between EL0 and EL1
arm64: Add per-cpu infrastructure to call ARCH_WORKAROUND_2
arm64: Add ARCH_WORKAROUND_2 probing
arm64: Add 'ssbd' command-line option
arm64: ssbd: Add global mitigation state accessor
arm64: ssbd: Skip apply_ssbd if not using dynamic mitigation
arm64: ssbd: Restore mitigation status on CPU resume
arm64: ssbd: Introduce thread flag to control userspace mitigation
arm64: ssbd: Add prctl interface for per-thread mitigation
arm64: KVM: Add HYP per-cpu accessors
arm64: KVM: Add ARCH_WORKAROUND_2 support for guests
arm64: KVM: Handle guest's ARCH_WORKAROUND_2 requests
arm64: KVM: Add ARCH_WORKAROUND_2 discovery through ARCH_FEATURES_FUNC_ID
Mark Rutland (4):
arm_pmu: simplify arm_pmu::handle_irq
drivers/perf: arm-ccn: don't log to dmesg in event_init
arm64: make is_permission_fault() name clearer
arm64: Unify kernel fault reporting
Masahiro Yamada (1):
arm64: remove no-op macro VMLINUX_SYMBOL()
Robin Murphy (5):
arm64: Select ARCH_HAS_FAST_MULTIPLIER
perf/arm-cci: Remove unnecessary period adjustment
perf/arm-cc*: Fix MODULE_LICENSE() tags
perf/arm-cci: Remove pointless PMU disabling
perf/arm-cci: Allow building as a module
Sudeep Holla (1):
ACPI / PPTT: fix build when CONFIG_ACPI_PPTT is not enabled
Vincenzo Frascino (1):
arm64: Remove duplicate include
Will Deacon (2):
arm64: cmpwait: Clear event register before arming exclusive monitor
arm64: Kconfig: Enable LSE atomics by default
Wolfram Sang (1):
perf: simplify getting .drvdata
Documentation/admin-guide/kernel-parameters.txt | 17 +
arch/arm/common/mcpm_entry.c | 2 +
arch/arm/include/asm/kvm_host.h | 12 +
arch/arm/include/asm/kvm_mmu.h | 5 +
arch/arm/kernel/perf_event_v6.c | 4 +-
arch/arm/kernel/perf_event_v7.c | 3 +-
arch/arm/kernel/perf_event_xscale.c | 6 +-
arch/arm64/Kconfig | 15 +-
arch/arm64/include/asm/acpi.h | 4 +
arch/arm64/include/asm/cache.h | 6 +-
arch/arm64/include/asm/cmpxchg.h | 4 +-
arch/arm64/include/asm/cpucaps.h | 3 +-
arch/arm64/include/asm/cpufeature.h | 22 +
arch/arm64/include/asm/elf.h | 13 +
arch/arm64/include/asm/fpsimdmacros.h | 12 +-
arch/arm64/include/asm/kvm_asm.h | 30 +-
arch/arm64/include/asm/kvm_host.h | 26 +
arch/arm64/include/asm/kvm_mmu.h | 25 +-
arch/arm64/include/asm/processor.h | 5 +
arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/include/asm/topology.h | 6 +-
arch/arm64/include/uapi/asm/auxvec.h | 3 +-
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/armv8_deprecated.c | 3 +-
arch/arm64/kernel/asm-offsets.c | 1 +
arch/arm64/kernel/cacheinfo.c | 15 +-
arch/arm64/kernel/cpu_errata.c | 182 +++++++
arch/arm64/kernel/cpufeature.c | 10 +-
arch/arm64/kernel/entry-fpsimd.S | 2 +-
arch/arm64/kernel/entry.S | 30 ++
arch/arm64/kernel/fpsimd.c | 18 +-
arch/arm64/kernel/hibernate.c | 11 +
arch/arm64/kernel/perf_event.c | 3 +-
arch/arm64/kernel/ptrace.c | 5 -
arch/arm64/kernel/signal.c | 57 ++-
arch/arm64/kernel/ssbd.c | 110 ++++
arch/arm64/kernel/suspend.c | 8 +
arch/arm64/kernel/topology.c | 104 +++-
arch/arm64/kernel/vmlinux.lds.S | 20 +-
arch/arm64/kvm/hyp/hyp-entry.S | 38 +-
arch/arm64/kvm/hyp/switch.c | 42 ++
arch/arm64/kvm/reset.c | 4 +
arch/arm64/mm/dma-mapping.c | 5 +
arch/arm64/mm/fault.c | 46 +-
arch/riscv/kernel/cacheinfo.c | 1 -
drivers/acpi/Kconfig | 3 +
drivers/acpi/Makefile | 1 +
drivers/acpi/pptt.c | 655 ++++++++++++++++++++++++
drivers/acpi/tables.c | 2 +-
drivers/base/cacheinfo.c | 157 +++---
drivers/perf/Kconfig | 36 +-
drivers/perf/arm-cci.c | 47 +-
drivers/perf/arm-ccn.c | 22 +-
drivers/perf/arm_pmu.c | 2 +-
drivers/perf/arm_spe_pmu.c | 6 +-
include/linux/acpi.h | 19 +
include/linux/arm-smccc.h | 10 +
include/linux/cacheinfo.h | 25 +-
include/linux/perf/arm_pmu.h | 2 +-
virt/kvm/arm/arm.c | 4 +
virt/kvm/arm/psci.c | 18 +-
61 files changed, 1689 insertions(+), 260 deletions(-)
create mode 100644 arch/arm64/kernel/ssbd.c
create mode 100644 drivers/acpi/pptt.c
--
Catalin
^ permalink raw reply
* [PATCH 2/2] imx_v6_v7_defconfig: Enable imx6qdl-sabreauto sensors
From: Fabio Estevam @ 2018-06-08 17:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <70b90919f98a6941425b39065dc8d37c95f0fd9f.1528390418.git.leonard.crestez@nxp.com>
On Thu, Jun 7, 2018 at 2:00 PM, Leonard Crestez <leonard.crestez@nxp.com> wrote:
> CONFIG_SENSORS_ISL29018 supports isil,il29023 light sensor
> CONFIG_MMA8452 supports fsl,mma8451 accelerometer
>
> CONFIG_MAG3110 for fsl,mag3110 is already enabled
>
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Just a minor nit in the Subject. The "ARM" prefix is missing.
Maybe Shawn can adjust it when applying the patch.
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
^ permalink raw reply
* [PATCH 04/24] 32-bit userspace ABI: introduce ARCH_32BIT_OFF_T config option
From: Catalin Marinas @ 2018-06-08 17:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180516081910.10067-5-ynorov@caviumnetworks.com>
On Wed, May 16, 2018 at 11:18:49AM +0300, Yury Norov wrote:
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 76c0b54443b1..ee079244dc3c 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -264,6 +264,21 @@ config ARCH_THREAD_STACK_ALLOCATOR
> config ARCH_WANTS_DYNAMIC_TASK_STRUCT
> bool
>
> +config ARCH_32BIT_OFF_T
> + bool
> + depends on !64BIT
> + help
> + All new 32-bit architectures should have 64-bit off_t type on
> + userspace side which corresponds to the loff_t kernel type. This
> + is the requirement for modern ABIs. Some existing architectures
> + already have 32-bit off_t. This option is enabled for all such
> + architectures explicitly. Namely: arc, arm, blackfin, cris, frv,
> + h8300, hexagon, m32r, m68k, metag, microblaze, mips32, mn10300,
> + nios2, openrisc, parisc32, powerpc32, score, sh, sparc, tile32,
> + unicore32, x86_32 and xtensa. This is the complete list. Any
> + new 32-bit architecture should declare 64-bit off_t type on user
> + side and so should not enable this option.
Do you know if this is the case for riscv and nds32, merged in the
meantime? If not, I suggest you drop this patch altogether and just
define force_o_largefile() for arm64/ilp32 as we don't seem to stick to
"all new 32-bit architectures should have 64-bit off_t".
--
Catalin
^ permalink raw reply
* [PATCH v4 1/3] arm64: mm: Support Common Not Private translations
From: James Morse @ 2018-06-08 17:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526638022-4137-2-git-send-email-vladimir.murzin@arm.com>
Hi Vladimir,
On 18/05/18 11:07, Vladimir Murzin wrote:
> Common Not Private (CNP) is a feature of ARMv8.2 extension which
> allows translation table entries to be shared between different PEs in
> the same inner shareable domain, so the hardware can use this fact to
> optimise the caching of such entries in the TLB.
>
> CNP occupies one bit in TTBRx_ELy and VTTBR_EL2, which advertises to
> the hardware that the translation table entries pointed to by this
> TTBR are the same as every PE in the same inner shareable domain for
> which the equivalent TTBR also has CNP bit set. In case CNP bit is set
> but TTBR does not point at the same translation table entries for a
> given ASID and VMID, then the system is mis-configured, so the results
> of translations are UNPREDICTABLE.
>
> For EL1 we postpone setting CNP till all cpus are up and rely on
> cpufeature framework to 1) patch the code which is sensitive to CNP
> and 2) update TTBR1_EL1 with CNP bit set. TTBR1_EL1 can be
> reprogrammed as result of hibernation or cpuidle (via __enable_mmu).
> cpuidle's path has been changed to restore CnP and for hibernation the
> code has been changed to save raw TTBR1_EL1 and blindly restore it on
> resume.
>
> For EL0 there are a few cases we need to care of changes in
> TTBR0_EL1:
> - a switch to idmap
> - software emulated PAN
(Nit: I don't think this has anything to do with EL0, its just the low half of
the VA space in TTBR0...)
> we rule out latter via Kconfig options and for the former we make
> sure that CNP is set for non-zero ASIDs only.
Reviewed-by: James Morse <james.morse@arm.com>
> diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
> index 39ec0b8..4f9e3ea 100644
> --- a/arch/arm64/include/asm/mmu_context.h
> +++ b/arch/arm64/include/asm/mmu_context.h
> @@ -149,6 +149,18 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp)
>
> phys_addr_t pgd_phys = virt_to_phys(pgdp);
>
> + if (system_supports_cnp() && !WARN_ON(pgdp != lm_alias(swapper_pg_dir))) {
> + /*
> + * cpu_replace_ttbr1() is used when there's a boot CPU
> + * up (i.e. cpufeature framework is not up yet) and
> + * latter only when we enable CNP via cpufeature's
> + * enable() callback.
> + * Also we rely on the cpu_hwcap bit being set before
(Nit: stray whitespace)
> + * calling the enable() function.
> + */
> + pgd_phys |= TTBR_CNP_BIT;
> + }
> +
> replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1);
>
> cpu_install_idmap();
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 9d1b06d..199e9dd 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -858,6 +860,16 @@ static bool has_cache_dic(const struct arm64_cpu_capabilities *entry,
> return read_sanitised_ftr_reg(SYS_CTR_EL0) & BIT(CTR_DIC_SHIFT);
> }
>
> +static bool __maybe_unused
> +has_useable_cnp(const struct arm64_cpu_capabilities *entry, int scope)
> +{
> +#ifdef CONFIG_CRASH_DUMP
> + if (elfcorehdr_size)
> + return false;
> +#endif
It might be worth a comment why kdump is relevant, (here or in the commit
message). Kdump isn't guaranteed to power-off all secondary CPUs, CNP may share
TLB entries with a CPU stuck in the crashed kernel.
I don't think we can't trust it even if we bring all CPUs into the new kernel as
the DT may not reflect all the CPUs the crashed-kernel had. (kexec doesn't have
this problem as it always powers-off secondaries before kexec-ing)
> + return has_cpuid_feature(entry, scope);
> +}
> +
> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> static int __kpti_forced; /* 0: not forced, >0: forced on, <0: forced off */
>
> @@ -1642,6 +1667,11 @@ cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused)
> return (cpus_have_const_cap(ARM64_HAS_PAN) && !cpus_have_const_cap(ARM64_HAS_UAO));
> }
>
> +static void __maybe_unused cpu_enable_cnp (struct arm64_cpu_capabilities const *cap)
Nit: stray space between cpu_enable_cnp and the parameters.
> +{
> + cpu_replace_ttbr1(lm_alias(swapper_pg_dir));
> +}
> +
> /*
> * We emulate only the following system register space.
> * Op0 = 0x3, CRn = 0x0, Op1 = 0x0, CRm = [0, 4 - 7]
> diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
> index 1ec5f28..ea27121 100644
> --- a/arch/arm64/kernel/hibernate.c
> +++ b/arch/arm64/kernel/hibernate.c
> @@ -125,7 +125,7 @@ int arch_hibernation_header_save(void *addr, unsigned int max_size)
> return -EOVERFLOW;
>
> arch_hdr_invariants(&hdr->invariants);
> - hdr->ttbr1_el1 = __pa_symbol(swapper_pg_dir);
> + hdr->ttbr1_el1 = read_sysreg(ttbr1_el1);
> hdr->reenter_kernel = _cpu_resume;
>
> /* We can't use __hyp_get_vectors() because kvm may still be loaded */
We also run__cpu_suspend_exit() from hibernate's restore, which will restore CNP
so this isn't strictly necessary.
There is some future-cleanup we can do here, if hibernate set the idmap when
calling __cpu_suspend_exit(), that function could do the replace_ttbr1() without
an extra install/uninstall of the idmap.
Thanks,
James
^ permalink raw reply
* [PATCH v4 2/3] arm64: KVM: Enable Common Not Private translations
From: James Morse @ 2018-06-08 17:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526638022-4137-3-git-send-email-vladimir.murzin@arm.com>
Hi Vladimir,
On 18/05/18 11:07, Vladimir Murzin wrote:
> We rely on cpufeature framework to detect and enable CNP so for KVM we
> need to patch hyp to set CNP bit just before TTBR0_EL2 gets written.
>
> For the guest we encode CNP bit while building vttbr, so we don't need
> to bother with that in a world switch.
With the bare-constant fix suggested by Catalin,
Reviewed-by: James Morse <james.morse@arm.com>
Thanks,
James
^ permalink raw reply
* [PATCH v4 3/3] arm64: Introduce command line parameter to disable CNP
From: James Morse @ 2018-06-08 17:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b4771d84-2403-3bd4-9566-4d9b1403d112@arm.com>
Hi Vladimir, Catalin,
On 24/05/18 09:20, Vladimir Murzin wrote:
> On 23/05/18 18:17, Catalin Marinas wrote:
>> On Fri, May 18, 2018 at 11:07:02AM +0100, Vladimir Murzin wrote:
>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>>> index 11fc28e..8f59d47 100644
>>> --- a/Documentation/admin-guide/kernel-parameters.txt
>>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>>> @@ -2636,6 +2636,10 @@
>>> + nocnp [ARM64]
>>> + Disable CNP (Common not Private translations)
>>> + even if it is supported by processor.
>> Do we actually have a use-case for this command line option? I'm not
>> considering hardware errata as these are handled separately in the
>> kernel.
> Well, I cannot count all cases, yet we might see CnP support advertised
> by CPU via ID register (where CPU meant to be part of bL) but not really
> doing optimisations in hardware.
>
> Probably, some userspace (benchmarks) might not benefit of CnP; otoh maybe
> better way for such case would be user-space asking kernel to {dis,en}able
> CnP...
>
> I have no strong opinion on patch, so I'm fine to drop it and come back
> when/if we get results from real hardware.
We may want to disable CNP without rebuilding the kernel, which would also have
the affect of code-layout changes...
Thanks,
James
^ permalink raw reply
* [PATCH v6 2/2] regulator: add QCOM RPMh regulator driver
From: David Collins @ 2018-06-08 18:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180608002653.GD88063@google.com>
Hello Matthias,
On 06/07/2018 05:26 PM, Matthias Kaehlcke wrote:
> On Mon, Jun 04, 2018 at 12:15:12PM -0700, David Collins wrote:
>> static int rpmh_regulator_send_request(struct rpmh_vreg *vreg,
>> + struct tcs_cmd *cmd, int count, bool wait_for_ack)
>>
>
> nit: as of now this is only called with a single command. If you
> anticipate that this is unlikely to change consider removing 'count',
> not having it in the calls slightly improves readability.
The count parameter was needed in the original version of the patch. That
need is no longer present after removing features in subsequent versions.
I'll remove this parameter.
>> +static int _rpmh_regulator_vrm_set_voltage_sel(struct regulator_dev *rdev,
>> + unsigned int selector, bool wait_for_ack)
>> +{
>> + struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
>> + struct tcs_cmd cmd = {
>> + .addr = vreg->addr + RPMH_REGULATOR_REG_VRM_VOLTAGE,
>> + };
>> + int ret;
>> +
>> + /* VRM voltage control register is set with voltage in millivolts. */
>> + cmd.data = DIV_ROUND_UP(regulator_list_voltage_linear_range(rdev,
>> + selector), 1000);
>> +
>> + ret = rpmh_regulator_send_request(vreg, &cmd, 1, wait_for_ack);
>> + if (!ret)
>> + vreg->voltage_selector = selector;
>> +
>> + return 0;
>
> Shouldn't this return 'ret'?
Yes; good catch. I'll fix it.
>> +static int rpmh_regulator_enable(struct regulator_dev *rdev)
>> +{
>> + struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
>> + struct tcs_cmd cmd = {
>> + .addr = vreg->addr + RPMH_REGULATOR_REG_ENABLE,
>> + .data = 1,
>> + };
>> + int ret;
>> +
>> + if (vreg->enabled == -EINVAL &&
>> + vreg->voltage_selector != -ENOTRECOVERABLE) {
>> + ret = _rpmh_regulator_vrm_set_voltage_sel(rdev,
>> + vreg->voltage_selector, true);
>> + if (ret < 0)
>> + return ret;
>> + }
>> +
>> + ret = rpmh_regulator_send_request(vreg, &cmd, 1, true);
>> + if (!ret)
>> + vreg->enabled = true;
>> +
>> + return ret;
>> +}
>> +
>> +static int rpmh_regulator_disable(struct regulator_dev *rdev)
>> +{
>> + struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
>> + struct tcs_cmd cmd = {
>> + .addr = vreg->addr + RPMH_REGULATOR_REG_ENABLE,
>> + .data = 0,
>> + };
>> + int ret;
>> +
>> + if (vreg->enabled == -EINVAL &&
>> + vreg->voltage_selector != -ENOTRECOVERABLE) {
>> + ret = _rpmh_regulator_vrm_set_voltage_sel(rdev,
>> + vreg->voltage_selector, true);
>> + if (ret < 0)
>> + return ret;
>> + }
>> +
>> + ret = rpmh_regulator_send_request(vreg, &cmd, 1, false);
>> + if (!ret)
>> + vreg->enabled = false;
>> +
>> + return ret;
>> +}
>
> nit: rpmh_regulator_enable() and rpmh_regulator_disable() are
> essentially the same code, consider introducing a helper like
> _rpmh_regulator_enable(struct regulator_dev *rdev, bool enable).
Sure, I'll add a helper function.
>> +static int rpmh_regulator_init_vreg(struct rpmh_vreg *vreg, struct device *dev,
>> + struct device_node *node, const char *pmic_id,
>> + const struct rpmh_vreg_init_data *rpmh_data)
>> +{
>> + struct regulator_config reg_config = {};
>> + char rpmh_resource_name[20] = "";
>> + struct regulator_dev *rdev;
>> + struct regulator_init_data *init_data;
>> + int ret;
>> +
>> + vreg->dev = dev;
>> +
>> + for (; rpmh_data->name; rpmh_data++)
>> + if (!strcmp(rpmh_data->name, node->name))
>> + break;
>
> nit: it's a bit odd to use the parameter itself for iteration, but I
> guess it's a matter of preferences.
I'll change this to add a specific iterator so that it is less surprising.
Thanks,
David
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH 04/20] coresight: Cleanup platform description data
From: Mathieu Poirier @ 2018-06-08 19:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528235011-30691-5-git-send-email-suzuki.poulose@arm.com>
On Tue, Jun 05, 2018 at 10:43:15PM +0100, Suzuki K Poulose wrote:
> Nobody uses the "clk" field in struct coresight_platform_data.
> Remove it.
>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> include/linux/coresight.h | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index e5421b8..69a5c9f 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -87,7 +87,6 @@ struct coresight_dev_subtype {
> * @child_ports:child component port number the current component is
> connected to.
> * @nr_outport: number of output ports for this component.
> - * @clk: The clock this component is associated to.
> */
> struct coresight_platform_data {
> int cpu;
> @@ -97,7 +96,6 @@ struct coresight_platform_data {
> const char **child_names;
> int *child_ports;
> int nr_outport;
> - struct clk *clk;
> };
I'm going to queue this up for the next rc. No need to send it as part of
another revision.
Thanks,
Mathieu
>
> /**
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH RESEND v4 0/2] support exception state migration and set VSESR_EL2 by user space
From: Dongjiu Geng @ 2018-06-08 19:48 UTC (permalink / raw)
To: linux-arm-kernel
This series patch is separated from https://www.spinics.net/lists/kvm/msg168917.html
1. Detect whether KVM can set set guest SError syndrome
2. Support to Set VSESR_EL2 and inject SError by user space.
3. Support live migration to keep SError pending state and VSESR_EL2 value
The user space patch is here: https://www.mail-archive.com/qemu-devel at nongnu.org/msg539534.html
Dongjiu Geng (2):
arm64: KVM: export the capability to set guest SError syndrome
arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
Documentation/virtual/kvm/api.txt | 42 +++++++++++++++++++++++++++++++++---
arch/arm/include/asm/kvm_host.h | 6 ++++++
arch/arm/include/uapi/asm/kvm.h | 12 +++++++++++
arch/arm/kvm/guest.c | 12 +++++++++++
arch/arm64/include/asm/kvm_emulate.h | 5 +++++
arch/arm64/include/asm/kvm_host.h | 7 ++++++
arch/arm64/include/uapi/asm/kvm.h | 13 +++++++++++
arch/arm64/kvm/guest.c | 36 +++++++++++++++++++++++++++++++
arch/arm64/kvm/inject_fault.c | 6 +++---
arch/arm64/kvm/reset.c | 4 ++++
include/uapi/linux/kvm.h | 1 +
virt/kvm/arm/arm.c | 19 ++++++++++++++++
12 files changed, 157 insertions(+), 6 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH RESEND v4 1/2] arm64: KVM: export the capability to set guest SError syndrome
From: Dongjiu Geng @ 2018-06-08 19:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528487320-2873-1-git-send-email-gengdongjiu@huawei.com>
For the arm64 RAS Extension, user space can inject a virtual-SError
with specified ESR. So user space needs to know whether KVM support
to inject such SError, this interface adds this query for this capability.
KVM will check whether system support RAS Extension, if supported, KVM
returns true to user space, otherwise returns false.
Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
Reviewed-by: James Morse <james.morse@arm.com>
---
Documentation/virtual/kvm/api.txt | 11 +++++++++++
arch/arm64/kvm/reset.c | 3 +++
include/uapi/linux/kvm.h | 1 +
3 files changed, 15 insertions(+)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 758bf40..fdac969 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -4603,3 +4603,14 @@ Architectures: s390
This capability indicates that kvm will implement the interfaces to handle
reset, migration and nested KVM for branch prediction blocking. The stfle
facility 82 should not be provided to the guest without this capability.
+
+8.14 KVM_CAP_ARM_SET_SERROR_ESR
+
+Architectures: arm, arm64
+
+This capability indicates that userspace can specify the syndrome value reported
+to the guest OS when guest takes a virtual SError interrupt exception.
+If KVM has this capability, userspace can only specify the ISS field for the ESR
+syndrome, it can not specify the EC field which is not under control by KVM.
+If this virtual SError is taken to EL1 using AArch64, this value will be reported
+in ISS filed of ESR_EL1.
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index 3256b92..38c8a64 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -77,6 +77,9 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_ARM_PMU_V3:
r = kvm_arm_support_pmu_v3();
break;
+ case KVM_CAP_ARM_INJECT_SERROR_ESR:
+ r = cpus_have_const_cap(ARM64_HAS_RAS_EXTN);
+ break;
case KVM_CAP_SET_GUEST_DEBUG:
case KVM_CAP_VCPU_ATTRIBUTES:
r = 1;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index b02c41e..e88f976 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -948,6 +948,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_S390_BPB 152
#define KVM_CAP_GET_MSR_FEATURES 153
#define KVM_CAP_HYPERV_EVENTFD 154
+#define KVM_CAP_ARM_INJECT_SERROR_ESR 155
#ifdef KVM_CAP_IRQ_ROUTING
--
2.7.4
^ permalink raw reply related
* [PATCH RESEND v4 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
From: Dongjiu Geng @ 2018-06-08 19:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528487320-2873-1-git-send-email-gengdongjiu@huawei.com>
For the migrating VMs, user space may need to know the exception
state. For example, in the machine A, KVM make an SError pending,
when migrate to B, KVM also needs to pend an SError.
This new IOCTL exports user-invisible states related to SError.
Together with appropriate user space changes, user space can get/set
the SError exception state to do migrate/snapshot/suspend.
Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
---
change since v3:
1. Fix the memset() issue in the kvm_arm_vcpu_get_events()
change since v2:
1. Add kvm_vcpu_events structure definition for arm platform to avoid the build errors.
change since v1:
Address Marc's comments, thanks Marc's review
1. serror_has_esr always true when ARM64_HAS_RAS_EXTN is set
2. remove Spurious blank line in kvm_arm_vcpu_set_events()
3. rename pend_guest_serror() to kvm_set_sei_esr()
4. Make kvm_arm_vcpu_get_events() did all the work rather than having this split responsibility.
5. using sizeof(events) instead of sizeof(struct kvm_vcpu_events)
this series patch is separated from https://www.spinics.net/lists/kvm/msg168917.html
The user space patch is here: https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06965.html
change since V12:
1. change (vcpu->arch.hcr_el2 & HCR_VSE) to !!(vcpu->arch.hcr_el2 & HCR_VSE) in kvm_arm_vcpu_get_events()
Change since V11:
Address James's comments, thanks James
1. Align the struct of kvm_vcpu_events to 64 bytes
2. Avoid exposing the stale ESR value in the kvm_arm_vcpu_get_events()
3. Change variables 'injected' name to 'serror_pending' in the kvm_arm_vcpu_set_events()
4. Change to sizeof(events) from sizeof(struct kvm_vcpu_events) in kvm_arch_vcpu_ioctl()
Change since V10:
Address James's comments, thanks James
1. Merge the helper function with the user.
2. Move the ISS_MASK into pend_guest_serror() to clear top bits
3. Make kvm_vcpu_events struct align to 4 bytes
4. Add something check in the kvm_arm_vcpu_set_events()
5. Check kvm_arm_vcpu_get/set_events()'s return value.
6. Initialise kvm_vcpu_events to 0 so that padding transferred to user-space doesn't
contain kernel stack.
---
Documentation/virtual/kvm/api.txt | 31 ++++++++++++++++++++++++++++---
arch/arm/include/asm/kvm_host.h | 6 ++++++
arch/arm/include/uapi/asm/kvm.h | 12 ++++++++++++
arch/arm/kvm/guest.c | 12 ++++++++++++
arch/arm64/include/asm/kvm_emulate.h | 5 +++++
arch/arm64/include/asm/kvm_host.h | 7 +++++++
arch/arm64/include/uapi/asm/kvm.h | 13 +++++++++++++
arch/arm64/kvm/guest.c | 36 ++++++++++++++++++++++++++++++++++++
arch/arm64/kvm/inject_fault.c | 6 +++---
arch/arm64/kvm/reset.c | 1 +
virt/kvm/arm/arm.c | 19 +++++++++++++++++++
11 files changed, 142 insertions(+), 6 deletions(-)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index fdac969..8896737 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -835,11 +835,13 @@ struct kvm_clock_data {
Capability: KVM_CAP_VCPU_EVENTS
Extended by: KVM_CAP_INTR_SHADOW
-Architectures: x86
+Architectures: x86, arm, arm64
Type: vm ioctl
Parameters: struct kvm_vcpu_event (out)
Returns: 0 on success, -1 on error
+X86:
+
Gets currently pending exceptions, interrupts, and NMIs as well as related
states of the vcpu.
@@ -881,15 +883,32 @@ Only two fields are defined in the flags field:
- KVM_VCPUEVENT_VALID_SMM may be set in the flags field to signal that
smi contains a valid state.
+ARM, ARM64:
+
+Gets currently pending SError exceptions as well as related states of the vcpu.
+
+struct kvm_vcpu_events {
+ struct {
+ __u8 serror_pending;
+ __u8 serror_has_esr;
+ /* Align it to 8 bytes */
+ __u8 pad[6];
+ __u64 serror_esr;
+ } exception;
+ __u32 reserved[12];
+};
+
4.32 KVM_SET_VCPU_EVENTS
-Capability: KVM_CAP_VCPU_EVENTS
+Capebility: KVM_CAP_VCPU_EVENTS
Extended by: KVM_CAP_INTR_SHADOW
-Architectures: x86
+Architectures: x86, arm, arm64
Type: vm ioctl
Parameters: struct kvm_vcpu_event (in)
Returns: 0 on success, -1 on error
+X86:
+
Set pending exceptions, interrupts, and NMIs as well as related states of the
vcpu.
@@ -910,6 +929,12 @@ shall be written into the VCPU.
KVM_VCPUEVENT_VALID_SMM can only be set if KVM_CAP_X86_SMM is available.
+ARM, ARM64:
+
+Set pending SError exceptions as well as related states of the vcpu.
+
+See KVM_GET_VCPU_EVENTS for the data structure.
+
4.33 KVM_GET_DEBUGREGS
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index c7c28c8..39f9901 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -213,6 +213,12 @@ unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
+int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
+ struct kvm_vcpu_events *events);
+
+int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
+ struct kvm_vcpu_events *events);
+
unsigned long kvm_call_hyp(void *hypfn, ...);
void force_vm_exit(const cpumask_t *mask);
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index caae484..c3e6975 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -124,6 +124,18 @@ struct kvm_sync_regs {
struct kvm_arch_memory_slot {
};
+/* for KVM_GET/SET_VCPU_EVENTS */
+struct kvm_vcpu_events {
+ struct {
+ __u8 serror_pending;
+ __u8 serror_has_esr;
+ /* Align it to 8 bytes */
+ __u8 pad[6];
+ __u64 serror_esr;
+ } exception;
+ __u32 reserved[12];
+};
+
/* If you need to interpret the index values, here is the key: */
#define KVM_REG_ARM_COPROC_MASK 0x000000000FFF0000
#define KVM_REG_ARM_COPROC_SHIFT 16
diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
index a18f33e..c685f0e 100644
--- a/arch/arm/kvm/guest.c
+++ b/arch/arm/kvm/guest.c
@@ -261,6 +261,18 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
return -EINVAL;
}
+int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
+ struct kvm_vcpu_events *events)
+{
+ return -EINVAL;
+}
+
+int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
+ struct kvm_vcpu_events *events)
+{
+ return -EINVAL;
+}
+
int __attribute_const__ kvm_target_cpu(void)
{
switch (read_cpuid_part()) {
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 1dab3a9..18f61ff 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -81,6 +81,11 @@ static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
return (unsigned long *)&vcpu->arch.hcr_el2;
}
+static inline unsigned long vcpu_get_vsesr(struct kvm_vcpu *vcpu)
+{
+ return vcpu->arch.vsesr_el2;
+}
+
static inline void vcpu_set_vsesr(struct kvm_vcpu *vcpu, u64 vsesr)
{
vcpu->arch.vsesr_el2 = vsesr;
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 469de8a..357304a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -335,6 +335,11 @@ unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
+int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
+ struct kvm_vcpu_events *events);
+
+int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
+ struct kvm_vcpu_events *events);
#define KVM_ARCH_WANT_MMU_NOTIFIER
int kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
@@ -363,6 +368,8 @@ void handle_exit_early(struct kvm_vcpu *vcpu, struct kvm_run *run,
int kvm_perf_init(void);
int kvm_perf_teardown(void);
+void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
+
struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
void __kvm_set_tpidr_el2(u64 tpidr_el2);
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 04b3256..df4faee 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -39,6 +39,7 @@
#define __KVM_HAVE_GUEST_DEBUG
#define __KVM_HAVE_IRQ_LINE
#define __KVM_HAVE_READONLY_MEM
+#define __KVM_HAVE_VCPU_EVENTS
#define KVM_COALESCED_MMIO_PAGE_OFFSET 1
@@ -153,6 +154,18 @@ struct kvm_sync_regs {
struct kvm_arch_memory_slot {
};
+/* for KVM_GET/SET_VCPU_EVENTS */
+struct kvm_vcpu_events {
+ struct {
+ __u8 serror_pending;
+ __u8 serror_has_esr;
+ /* Align it to 8 bytes */
+ __u8 pad[6];
+ __u64 serror_esr;
+ } exception;
+ __u32 reserved[12];
+};
+
/* If you need to interpret the index values, here is the key: */
#define KVM_REG_ARM_COPROC_MASK 0x000000000FFF0000
#define KVM_REG_ARM_COPROC_SHIFT 16
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 56a0260..4426915 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -289,6 +289,42 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
return -EINVAL;
}
+int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
+ struct kvm_vcpu_events *events)
+{
+ memset(events, 0, sizeof(*events));
+
+ events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
+ events->exception.serror_has_esr =
+ cpus_have_const_cap(ARM64_HAS_RAS_EXTN);
+
+ if (events->exception.serror_pending &&
+ events->exception.serror_has_esr)
+ events->exception.serror_esr = vcpu_get_vsesr(vcpu);
+ else
+ events->exception.serror_esr = 0;
+
+ return 0;
+}
+
+int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
+ struct kvm_vcpu_events *events)
+{
+ bool serror_pending = events->exception.serror_pending;
+ bool has_esr = events->exception.serror_has_esr;
+
+ if (serror_pending && has_esr) {
+ if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
+ return -EINVAL;
+
+ kvm_set_sei_esr(vcpu, events->exception.serror_esr);
+ } else if (serror_pending) {
+ kvm_inject_vabt(vcpu);
+ }
+
+ return 0;
+}
+
int __attribute_const__ kvm_target_cpu(void)
{
unsigned long implementor = read_cpuid_implementor();
diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
index d8e7165..a55e91d 100644
--- a/arch/arm64/kvm/inject_fault.c
+++ b/arch/arm64/kvm/inject_fault.c
@@ -164,9 +164,9 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
inject_undef64(vcpu);
}
-static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)
+void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 esr)
{
- vcpu_set_vsesr(vcpu, esr);
+ vcpu_set_vsesr(vcpu, esr & ESR_ELx_ISS_MASK);
*vcpu_hcr(vcpu) |= HCR_VSE;
}
@@ -184,5 +184,5 @@ static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)
*/
void kvm_inject_vabt(struct kvm_vcpu *vcpu)
{
- pend_guest_serror(vcpu, ESR_ELx_ISV);
+ kvm_set_sei_esr(vcpu, ESR_ELx_ISV);
}
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index 38c8a64..20e919a 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -82,6 +82,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)
break;
case KVM_CAP_SET_GUEST_DEBUG:
case KVM_CAP_VCPU_ATTRIBUTES:
+ case KVM_CAP_VCPU_EVENTS:
r = 1;
break;
default:
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index a4c1b76..79ecba9 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1107,6 +1107,25 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
r = kvm_arm_vcpu_has_attr(vcpu, &attr);
break;
}
+ case KVM_GET_VCPU_EVENTS: {
+ struct kvm_vcpu_events events;
+
+ if (kvm_arm_vcpu_get_events(vcpu, &events))
+ return -EINVAL;
+
+ if (copy_to_user(argp, &events, sizeof(events)))
+ return -EFAULT;
+
+ return 0;
+ }
+ case KVM_SET_VCPU_EVENTS: {
+ struct kvm_vcpu_events events;
+
+ if (copy_from_user(&events, argp, sizeof(events)))
+ return -EFAULT;
+
+ return kvm_arm_vcpu_set_events(vcpu, &events);
+ }
default:
r = -EINVAL;
}
--
2.7.4
^ permalink raw reply related
* [PATCH 02/20] coresight: of: Fix refcounting for graph nodes
From: Mathieu Poirier @ 2018-06-08 19:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528235011-30691-3-git-send-email-suzuki.poulose@arm.com>
On Tue, Jun 05, 2018 at 10:43:13PM +0100, Suzuki K Poulose wrote:
> The coresight driver doesn't drop the references on the
> remote endpoint/port nodes. Add the missing of_node_put()
> calls. To make it easier to handle different corner cases
> cleanly, move the parsing of an endpoint to separate
> function.
Please split this as those are two different things.
>
> Reported-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> drivers/hwtracing/coresight/of_coresight.c | 139 +++++++++++++++++------------
> 1 file changed, 84 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
> index a33a92e..8a23c63 100644
> --- a/drivers/hwtracing/coresight/of_coresight.c
> +++ b/drivers/hwtracing/coresight/of_coresight.c
> @@ -111,17 +111,80 @@ int of_coresight_get_cpu(const struct device_node *node)
> }
> EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
>
> +/*
> + * of_coresight_parse_endpoint : Parse the given output endpoint @ep
> + * and fill the connection information in @pdata[*@i].
> + *
> + * Parses the local port, remote device name and the remote port. Also
> + * updates *@i to point to the next index, when an entry is added.
> + *
> + * Returns :
> + * 0 - If the parsing completed without any fatal errors.
> + * -Errno - Fatal error, abort the scanning.
> + */
> +static int of_coresight_parse_endpoint(struct device_node *ep,
> + struct coresight_platform_data *pdata,
> + int *i)
> +{
> + int ret = 0;
> + struct of_endpoint endpoint, rendpoint;
> + struct device_node *rparent = NULL;
> + struct device_node *rport = NULL;
> + struct device *rdev = NULL;
> +
> + do {
> + /*
> + * No need to deal with input ports, processing for as
> + * processing for output ports will deal with them.
> + */
> + if (of_find_property(ep, "slave-mode", NULL))
> + break;
> +
> + /* Parse the local port details */
> + if (of_graph_parse_endpoint(ep, &endpoint))
> + break;
> + /*
> + * Get a handle on the remote port and parent
> + * attached to it.
> + */
> + rparent = of_graph_get_remote_port_parent(ep);
> + if (!rparent)
> + break;
> + rport = of_graph_get_remote_port(ep);
> + if (!rport)
> + break;
> + if (of_graph_parse_endpoint(rport, &rendpoint))
> + break;
> +
> + /* If the remote device is not available, defer probing */
> + rdev = of_coresight_get_endpoint_device(rparent);
> + if (!rdev) {
> + ret = -EPROBE_DEFER;
> + break;
> + }
> +
> + pdata->outports[*i] = endpoint.port;
> + pdata->child_names[*i] = dev_name(rdev);
> + pdata->child_ports[*i] = rendpoint.id;
> + /* Move the index */
> + (*i)++;
> + } while (0);
That's a clever way of coding a classic 'goto' block.
> +
> + if (rparent)
> + of_node_put(rparent);
> + if (rport)
> + of_node_put(rport);
Perfect - thank you for that.
> +
> + return ret;
> +}
> +
> struct coresight_platform_data *
> of_get_coresight_platform_data(struct device *dev,
> const struct device_node *node)
> {
> int i = 0, ret = 0;
> struct coresight_platform_data *pdata;
> - struct of_endpoint endpoint, rendpoint;
> - struct device *rdev;
> struct device_node *ep = NULL;
> - struct device_node *rparent = NULL;
> - struct device_node *rport = NULL;
>
> pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> if (!pdata)
> @@ -129,63 +192,29 @@ of_get_coresight_platform_data(struct device *dev,
>
> /* Use device name as sysfs handle */
> pdata->name = dev_name(dev);
> + pdata->cpu = of_coresight_get_cpu(node);
>
> /* Get the number of input and output port for this component */
> of_coresight_get_ports(node, &pdata->nr_inport, &pdata->nr_outport);
>
> - if (pdata->nr_outport) {
> - ret = of_coresight_alloc_memory(dev, pdata);
> + /* If there are not output connections, we are done */
/not/no
> + if (!pdata->nr_outport)
> + return pdata;
> +
> + ret = of_coresight_alloc_memory(dev, pdata);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + /* Iterate through each port to discover topology */
> + do {
> + /* Get a handle on a port */
> + ep = of_graph_get_next_endpoint(node, ep);
> + if (!ep)
> + break;
> + ret = of_coresight_parse_endpoint(ep, pdata, &i);
> if (ret)
> return ERR_PTR(ret);
> -
> - /* Iterate through each port to discover topology */
> - do {
> - /* Get a handle on a port */
> - ep = of_graph_get_next_endpoint(node, ep);
> - if (!ep)
> - break;
> -
> - /*
> - * No need to deal with input ports, processing for as
> - * processing for output ports will deal with them.
> - */
> - if (of_find_property(ep, "slave-mode", NULL))
> - continue;
> -
> - /* Get a handle on the local endpoint */
> - ret = of_graph_parse_endpoint(ep, &endpoint);
> -
> - if (ret)
> - continue;
> -
> - /* The local out port number */
> - pdata->outports[i] = endpoint.port;
> -
> - /*
> - * Get a handle on the remote port and parent
> - * attached to it.
> - */
> - rparent = of_graph_get_remote_port_parent(ep);
> - rport = of_graph_get_remote_port(ep);
> -
> - if (!rparent || !rport)
> - continue;
> -
> - if (of_graph_parse_endpoint(rport, &rendpoint))
> - continue;
> -
> - rdev = of_coresight_get_endpoint_device(rparent);
> - if (!rdev)
> - return ERR_PTR(-EPROBE_DEFER);
> -
> - pdata->child_names[i] = dev_name(rdev);
> - pdata->child_ports[i] = rendpoint.id;
> -
> - i++;
> - } while (ep);
> - }
> -
> - pdata->cpu = of_coresight_get_cpu(node);
> + } while (ep);
>
> return pdata;
> }
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
From: Steven Rostedt @ 2018-06-08 20:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607091816.GT13775@localhost>
On Thu, 7 Jun 2018 11:18:16 +0200
Johan Hovold <johan@kernel.org> wrote:
> If you want to work around the warning and think you can do it in some
> non-contrived way, then go for it.
>
> Clearing the request buffer, checking for termination using strnlen, and
> then using memcpy might not be too bad.
>
> But after all, it is a false positive, so leaving things as they stand
> is fine too.
Not sure how contrived you think this is, but it solves the warning
without adding extra work in the normal case.
-- Steve
diff --git a/drivers/staging/greybus/fw-management.c b/drivers/staging/greybus/fw-management.c
index 71aec14f8181..4fb9f1dff47d 100644
--- a/drivers/staging/greybus/fw-management.c
+++ b/drivers/staging/greybus/fw-management.c
@@ -150,15 +150,18 @@ static int fw_mgmt_load_and_validate_operation(struct fw_mgmt *fw_mgmt,
}
request.load_method = load_method;
- strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
+ strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE - 1);
/*
* The firmware-tag should be NULL terminated, otherwise throw error and
* fail.
*/
- if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
- dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n");
- return -EINVAL;
+ if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 2] != '\0') {
+ if (tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
+ dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n");
+ return -EINVAL;
+ }
+ request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] = '\0';
}
/* Allocate ids from 1 to 255 (u8-max), 0 is an invalid id */
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox