* [PATCH] ARM: dts: rockchip: Reserve unusable memory region on rk3066
From: Huang, Tao @ 2016-10-05 2:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161001140939.GA31220@vaio-ubuntu>
Hi, Pawe?:
On 2016?10?01? 22:09, =?UTF-8?q?Pawe=C5=82=20Jarosz?= wrote:
> For some reason accessing memory region above 0xfe000000 freezes
> system on rk3066. There is similiar bug on later rockchip soc (rk3288)
RK3066 only support 2GB memory from 0x60000000 to 0xE0000000, can not access
above 0xfe000000. I think you mean 0x9F000000?
> solved same way.
>
> Signed-off-by: Pawe? Jarosz <paweljarosz3691@gmail.com>
> ---
> arch/arm/boot/dts/rk3066a.dtsi | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3066a.dtsi b/arch/arm/boot/dts/rk3066a.dtsi
> index 0d0dae3..44c8956 100644
> --- a/arch/arm/boot/dts/rk3066a.dtsi
> +++ b/arch/arm/boot/dts/rk3066a.dtsi
> @@ -93,6 +93,19 @@
> };
> };
>
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + /*
> + * The rk3066 cannot use the memory area above 0x9F000000
> + * for some unknown reason.
> + */
I don't remember RK3066 has such limit. I will double check with our IC
design team.
Do you know which master can not access this memory area
[0x9F000000~0xA0000000)?
> + unusable at 9F000000 {
> + reg = <0x9F000000 0x1000000>;
> + };
> + };
> +
> i2s0: i2s at 10118000 {
> compatible = "rockchip,rk3066-i2s";
> reg = <0x10118000 0x2000>;
^ permalink raw reply
* [PATCH] ARM: dts: lpc32xx: set pwm1 & pwm2 default clock rate
From: Vladimir Zapolskiy @ 2016-10-05 2:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474916056-11336-1-git-send-email-slemieux.tyco@gmail.com>
Hi Sylvain,
On 26.09.2016 21:54, Sylvain Lemieux wrote:
> From: Sylvain Lemieux <slemieux@tycoint.com>
>
> Probably most of NXP LPC32xx boards have 13MHz main oscillator
> and therefore for HCLK PLL and ARM core clock rate default
> hardware setting of 16 * 13MHz = 208MHz and the AHB bus clock
> rate of 208MHz / 2 = 104MHz.
>
> The change explicitly defines the peripheral PWM1/PWM2 default
> clock output rate of 104MHz. If needed it can be redefined
> in a board DTS file.
>
> Signed-off-by: Sylvain Lemieux <slemieux.tyco@gmail.com>
> ---
> Note:
> * There is a dependency on the following patch:
> "ARM: dts: lpc32xx: set default parent clock for pwm1 & pwm2"
> http://www.spinics.net/lists/arm-kernel/msg530277.html
> * This patch should be apply after
> "ARM: dts: lpc32xx: add pwm-cells to base dts file"
> http://www.spinics.net/lists/arm-kernel/msg534050.html
> - There is no dependency between the patches.
>
> arch/arm/boot/dts/lpc32xx.dtsi | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/boot/dts/lpc32xx.dtsi b/arch/arm/boot/dts/lpc32xx.dtsi
> index c031c94..d669200 100644
> --- a/arch/arm/boot/dts/lpc32xx.dtsi
> +++ b/arch/arm/boot/dts/lpc32xx.dtsi
> @@ -471,6 +471,7 @@
> clocks = <&clk LPC32XX_CLK_PWM1>;
> assigned-clocks = <&clk LPC32XX_CLK_PWM1>;
> assigned-clock-parents = <&clk LPC32XX_CLK_PERIPH>;
> + assigned-clock-rates = <104000000>;
PWM controller clock source can be 32KHz or CLK_PERIPH, the latter is
equal either to SYSCLK or HCLK PLL divided by HCLKDIV_CTRL[6:2].
Do you set the divider to 1? If yes, then I would say
1) this is very specific to your board, generally CLK_PERIPH
is set to be about 10-13MHz,
2) HCLKDIV or PERIPH clock configuration shall not be done in pwm device node.
104MHz is too high value to be set by default for PWM clock.
> status = "disabled";
> #pwm-cells = <2>;
> };
> @@ -481,6 +482,7 @@
> clocks = <&clk LPC32XX_CLK_PWM2>;
> assigned-clocks = <&clk LPC32XX_CLK_PWM2>;
> assigned-clock-parents = <&clk LPC32XX_CLK_PERIPH>;
> + assigned-clock-rates = <104000000>;
> status = "disabled";
> #pwm-cells = <2>;
> };
>
--
With best wishes,
Vladimir
^ permalink raw reply
* [PATCH v2 2/2] arm64: dts: rockchip: arch counter doesn't tick in system suspend
From: Doug Anderson @ 2016-10-04 22:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475604730-140264-2-git-send-email-briannorris@chromium.org>
Hi,
On Tue, Oct 4, 2016 at 11:12 AM, Brian Norris <briannorris@chromium.org> wrote:
> The "arm,no-tick-in-suspend" property was introduced to note
> implementations where the system counter does not quite follow the ARM
> specification that it "must be implemented in an always-on power
> domain".
>
> Particularly, RK3399's counter stops ticking when we switch from the
> 24MHz clock to the 32KHz clock in low-power suspend, so let's mark it as
> such.
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> v2: new in v2
>
> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
> 1 file changed, 1 insertion(+)
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply
* [PATCH v2 1/2] clocksource: arm_arch_timer: Don't assume clock runs in suspend
From: Doug Anderson @ 2016-10-04 22:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475604730-140264-1-git-send-email-briannorris@chromium.org>
Hi,
On Tue, Oct 4, 2016 at 11:12 AM, Brian Norris <briannorris@chromium.org> wrote:
> The ARM ARM specifies that the system counter "must be implemented in an
> always-on power domain," and so we try to use the counter as a source of
> timekeeping across suspend/resume. Unfortunately, some SoCs (e.g.,
> Rockchip's RK3399) do not keep the counter ticking properly when
> switched from their high-power clock to the lower-power clock used in
> system suspend. Support this quirk by adding a new device tree property.
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> v2:
> * add new device tree property, instead of re-using the "always-on"
> property (which has different meaning)
>
> Documentation/devicetree/bindings/arm/arch_timer.txt | 5 +++++
> drivers/clocksource/arm_arch_timer.c | 9 ++++++++-
> 2 files changed, 13 insertions(+), 1 deletion(-)
FWIW:
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply
* ks_dw_pcie_initiate_link_train() question
From: Murali Karicheri @ 2016-10-04 21:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004174051.GE3449@localhost>
On 10/04/2016 01:40 PM, Bjorn Helgaas wrote:
> Hi Murali,
>
> This code looks suspicious. Can you comment?
>
> void ks_dw_pcie_initiate_link_train(struct keystone_pcie *ks_pcie)
> {
> u32 val;
>
> /* Disable Link training */
> val = readl(ks_pcie->va_app_base + CMD_STATUS);
> val &= ~LTSSM_EN_VAL;
> writel(LTSSM_EN_VAL | val, ks_pcie->va_app_base + CMD_STATUS);
>
> Here we cleared the LTSSM_EN_VAL bit in "val", but then we add it
> right back in before writing it back to CMD_STATUS.
>
> That looks like a cut and paste error to me, but of course I don't
> know the hardware.
>
> /* Initiate Link Training */
> val = readl(ks_pcie->va_app_base + CMD_STATUS);
> writel(LTSSM_EN_VAL | val, ks_pcie->va_app_base + CMD_STATUS);
> }
>
>
Bjorn,
Good catch! That is a cut-n-paste error. Here is the description from
device manual
================
Disable link training by de-asserting the LTSSM_EN bit in the PCIESS
Command Status Register (CMD_STATUS[LTSSM_EN]=0). Upon reset, the
LTSSM_EN is de-asserted automatically by hardware.
Initiate link training can be initiated by asserting LTSSM_EN bit in the
CMD_STATUS register (CMD_STATUS[LTSSM_EN]=1).
================================================
Probably it works because it is de-asserted automatically upon
reset by hardware. Let me test this and send you a patch?
--
Murali Karicheri
Linux Kernel, Keystone
^ permalink raw reply
* [PATCH] efi/arm: fix absolute relocation detection for older toolchains
From: Matt Fleming @ 2016-10-04 21:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu94ZEuGyrcGTKMvTPKpvaR+R3AAXNY=diX28RDhc=Cv7Q@mail.gmail.com>
On Tue, 04 Oct, at 11:34:31AM, Ard Biesheuvel wrote:
>
> These relocations are harmless, since the debug ones are only
> interpreted by the debugger, and the ones generated by
> EXPORT_SYMBOL(sort) will never be referenced, since the symbols they
> contain are either renamed to __efistub_xxx (arm64), or they are not
> part of the kernel proper (arm)
>
> So both cases are false positives, but the diagnostic is important,
> and so breaking the build is appropriate for any other absolute
> relocation that may appear.
>
> The effect of the patch is not that the diagnostic is ignored, but
> that these relocations are not generated in the first place (-g0) or
> removed explicitly (ksymtab/krcrctab+sort) rather than via a wildcard.
> So other than not breaking the build, this patch should have no user
> observeable differences.
Thanks Ard, sounds reasonable. Feel free to take this through
whichever tree you think is best.
Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>
^ permalink raw reply
* [PATCH V2 3/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function"
From: Sinan Kaya @ 2016-10-04 21:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475615720-31047-1-git-send-email-okaya@codeaurora.org>
The SCI function was removed in two steps (first refactor and then remove).
This patch does the revert in one step.
The commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements")
refactored the original code so that SCI penalty is calculated dynamically
by the time get_penalty function is called. This patch does a partial
revert for the SCI functionality only.
The commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") is
for the removal of the function. SCI penalty API was replaced by the
runtime penalty calculation based on the value of
acpi_gbl_FADT.sci_interrupt.
The IRQ type does not get updated at the right time for some platforms and
results in incorrect penalty assignment for PCI IRQs as
irq_get_trigger_type returns the wrong type.
The register_gsi function delivers the IRQ found in the ACPI table to
the interrupt controller driver. Penalties are calculated before a
link object is enabled to find out which interrupt has the least number
of users. By the time penalties are calculated, the IRQ is not registered
yet and the API returns the wrong type.
Tested-by: Jonathan Liu <net147@gmail.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Link: https://lkml.org/lkml/2016/10/4/283
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
arch/x86/kernel/acpi/boot.c | 1 +
drivers/acpi/pci_link.c | 34 ++++++++++++++--------------------
include/linux/acpi.h | 1 +
3 files changed, 16 insertions(+), 20 deletions(-)
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 90d84c3..0ffd26e 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -453,6 +453,7 @@ static void __init acpi_sci_ioapic_setup(u8 bus_irq, u16 polarity, u16 trigger,
polarity = acpi_sci_flags & ACPI_MADT_POLARITY_MASK;
mp_override_legacy_irq(bus_irq, polarity, trigger, gsi);
+ acpi_penalize_sci_irq(bus_irq, trigger, polarity);
/*
* stash over-ride to indicate we've been here
diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index 06c2a11..6a2af19 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -495,27 +495,10 @@ static int acpi_irq_pci_sharing_penalty(int irq)
static int acpi_irq_get_penalty(int irq)
{
- int penalty = 0;
-
- /*
- * Penalize IRQ used by ACPI SCI. If ACPI SCI pin attributes conflict
- * with PCI IRQ attributes, mark ACPI SCI as ISA_ALWAYS so it won't be
- * use for PCI IRQs.
- */
- if (irq == acpi_gbl_FADT.sci_interrupt) {
- u32 type = irq_get_trigger_type(irq) & IRQ_TYPE_SENSE_MASK;
-
- if (type != IRQ_TYPE_LEVEL_LOW)
- penalty += PIRQ_PENALTY_ISA_ALWAYS;
- else
- penalty += PIRQ_PENALTY_PCI_USING;
- }
-
- if (irq < ACPI_MAX_ISA_IRQS)
- return penalty + acpi_irq_penalty[irq];
+ if (irq < ACPI_MAX_IRQS)
+ return acpi_irq_penalty[irq];
- penalty += acpi_irq_pci_sharing_penalty(irq);
- return penalty;
+ return acpi_irq_pci_sharing_penalty(irq);
}
int __init acpi_irq_penalty_init(void)
@@ -886,6 +869,17 @@ bool acpi_isa_irq_available(int irq)
acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS);
}
+void acpi_penalize_sci_irq(int irq, int trigger, int polarity)
+{
+ if (irq >= 0 && irq < ARRAY_SIZE(acpi_irq_penalty)) {
+ if (trigger != ACPI_MADT_TRIGGER_LEVEL ||
+ polarity != ACPI_MADT_POLARITY_ACTIVE_LOW)
+ acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_ALWAYS;
+ else
+ acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING;
+ }
+}
+
/*
* Over-ride default table to reserve additional IRQs for use by ISA
* e.g. acpi_irq_isa=5
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 4d8452c..85ac7d5 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -318,6 +318,7 @@ struct pci_dev;
int acpi_pci_irq_enable (struct pci_dev *dev);
void acpi_penalize_isa_irq(int irq, int active);
bool acpi_isa_irq_available(int irq);
+void acpi_penalize_sci_irq(int irq, int trigger, int polarity);
void acpi_pci_irq_disable (struct pci_dev *dev);
extern int ec_read(u8 addr, u8 *val);
--
1.9.1
^ permalink raw reply related
* [PATCH V2 2/3] ACPI, PCI IRQ: add PCI_USING penalty for ISA interrupts
From: Sinan Kaya @ 2016-10-04 21:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475615720-31047-1-git-send-email-okaya@codeaurora.org>
The change introduced in commit 103544d86976 ("ACPI,PCI,IRQ: reduce
resource requirements") removed PCI_USING penalty from
acpi_pci_link_allocate function as there is no longer a fixed size penalty
array for both PCI and IRQ interrupts.
We need to add the PCI_USING penalty for ISA interrupts too if the link is
in use and matches our ISA IRQ number.
Tested-by: Jonathan Liu <net147@gmail.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/acpi/pci_link.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index f3792f4..06c2a11 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -620,6 +620,10 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link)
acpi_device_bid(link->device));
return -ENODEV;
} else {
+ if (link->irq.active < ACPI_MAX_IRQS)
+ acpi_irq_penalty[link->irq.active] +=
+ PIRQ_PENALTY_PCI_USING;
+
printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n",
acpi_device_name(link->device),
acpi_device_bid(link->device), link->irq.active);
--
1.9.1
^ permalink raw reply related
* [PATCH V2 1/3] Revert "ACPI, PCI, IRQ: reduce static IRQ array size to 16"
From: Sinan Kaya @ 2016-10-04 21:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475615720-31047-1-git-send-email-okaya@codeaurora.org>
This reverts commit 5c5087a55390 ("ACPI,PCI,IRQ: reduce static IRQ array
size to 16").
The code maintains a fixed size array for IRQ penalties. The array
gets updated by external calls such as acpi_penalize_sci_irq,
acpi_penalize_isa_irq to reflect the actual interrupt usage of the
system. Since the IRQ distribution is platform specific, this is
not known ahead of time. The IRQs get updated based on the SCI
interrupt number BIOS has chosen or the ISA IRQs that were assigned
to existing peripherals.
By the time ACPI gets initialized, this code tries to determine an
IRQ number based on penalty values in this array. It will try to locate
the IRQ with the least penalty assignment so that interrupt sharing is
avoided if possible.
A couple of notes about the external APIs:
1. These API can be called before the ACPI is started. Therefore, one
cannot assume that the PCI link objects are initialized for calculating
penalties.
2. The polarity and trigger information passed via the
acpi_penalize_sci_irq from the BIOS may not match what the IRQ subsystem
is reporting as the call might have been placed before the IRQ is
registered by the interrupt subsystem.
The previous change was in the direction to remove these external API and
try to calculate the penalties at runtime for the ISA path as well. This
didn't work out well with the existing platforms.
Restoring the old behavior for IRQ < 256 and the new behavior will remain
effective for IRQ >= 256.
Tested-by: Jonathan Liu <net147@gmail.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Link: http://www.gossamer-threads.com/lists/linux/kernel/2537016#2537016
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/acpi/pci_link.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index c983bf7..f3792f4 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -438,6 +438,7 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)
* enabled system.
*/
+#define ACPI_MAX_IRQS 256
#define ACPI_MAX_ISA_IRQS 16
#define PIRQ_PENALTY_PCI_POSSIBLE (16*16)
@@ -446,7 +447,7 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)
#define PIRQ_PENALTY_ISA_USED (16*16*16*16*16)
#define PIRQ_PENALTY_ISA_ALWAYS (16*16*16*16*16*16)
-static int acpi_isa_irq_penalty[ACPI_MAX_ISA_IRQS] = {
+static int acpi_irq_penalty[ACPI_MAX_IRQS] = {
PIRQ_PENALTY_ISA_ALWAYS, /* IRQ0 timer */
PIRQ_PENALTY_ISA_ALWAYS, /* IRQ1 keyboard */
PIRQ_PENALTY_ISA_ALWAYS, /* IRQ2 cascade */
@@ -511,7 +512,7 @@ static int acpi_irq_get_penalty(int irq)
}
if (irq < ACPI_MAX_ISA_IRQS)
- return penalty + acpi_isa_irq_penalty[irq];
+ return penalty + acpi_irq_penalty[irq];
penalty += acpi_irq_pci_sharing_penalty(irq);
return penalty;
@@ -538,14 +539,14 @@ int __init acpi_irq_penalty_init(void)
for (i = 0; i < link->irq.possible_count; i++) {
if (link->irq.possible[i] < ACPI_MAX_ISA_IRQS)
- acpi_isa_irq_penalty[link->irq.
+ acpi_irq_penalty[link->irq.
possible[i]] +=
penalty;
}
} else if (link->irq.active &&
- (link->irq.active < ACPI_MAX_ISA_IRQS)) {
- acpi_isa_irq_penalty[link->irq.active] +=
+ (link->irq.active < ACPI_MAX_IRQS)) {
+ acpi_irq_penalty[link->irq.active] +=
PIRQ_PENALTY_PCI_POSSIBLE;
}
}
@@ -828,7 +829,7 @@ static void acpi_pci_link_remove(struct acpi_device *device)
}
/*
- * modify acpi_isa_irq_penalty[] from cmdline
+ * modify acpi_irq_penalty[] from cmdline
*/
static int __init acpi_irq_penalty_update(char *str, int used)
{
@@ -837,24 +838,24 @@ static int __init acpi_irq_penalty_update(char *str, int used)
for (i = 0; i < 16; i++) {
int retval;
int irq;
- int new_penalty;
retval = get_option(&str, &irq);
if (!retval)
break; /* no number found */
- /* see if this is a ISA IRQ */
- if ((irq < 0) || (irq >= ACPI_MAX_ISA_IRQS))
+ if (irq < 0)
+ continue;
+
+ if (irq >= ARRAY_SIZE(acpi_irq_penalty))
continue;
if (used)
- new_penalty = acpi_irq_get_penalty(irq) +
- PIRQ_PENALTY_ISA_USED;
+ acpi_irq_penalty[irq] = acpi_irq_get_penalty(irq) +
+ PIRQ_PENALTY_ISA_USED;
else
- new_penalty = 0;
+ acpi_irq_penalty[irq] = 0;
- acpi_isa_irq_penalty[irq] = new_penalty;
if (retval != 2) /* no next number */
break;
}
@@ -870,14 +871,14 @@ static int __init acpi_irq_penalty_update(char *str, int used)
*/
void acpi_penalize_isa_irq(int irq, int active)
{
- if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty)))
- acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) +
- (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING);
+ if (irq >= 0 && irq < ARRAY_SIZE(acpi_irq_penalty))
+ acpi_irq_penalty[irq] = acpi_irq_get_penalty(irq) +
+ (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING);
}
bool acpi_isa_irq_available(int irq)
{
- return irq >= 0 && (irq >= ARRAY_SIZE(acpi_isa_irq_penalty) ||
+ return irq >= 0 && (irq >= ARRAY_SIZE(acpi_irq_penalty) ||
acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS);
}
--
1.9.1
^ permalink raw reply related
* [PATCH V2 0/3] ACPI,PCI,IRQ: revert penalty calculation for SCI
From: Sinan Kaya @ 2016-10-04 21:15 UTC (permalink / raw)
To: linux-arm-kernel
Restoring the old behavior for IRQ < 256 and the dynamic penalty behavior
will remain effective for IRQ >= 256.
By the time ACPI gets initialized, this code tries to determine an
IRQ number based on penalty values in this array. It will try to locate
the IRQ with the least penalty assignment so that interrupt sharing is
avoided if possible.
A couple of notes about the external APIs:
1. These API can be called before the ACPI is started. Therefore, one
cannot assume that the PCI link objects are initialized for calculating
penalties.
2. The polarity and trigger information passed via the
acpi_penalize_sci_irq from the BIOS may not match what the IRQ subsystem
is reporting as the call might have been placed before the IRQ is
registered by the interrupt subsystem.
The reverted changes were in the direction to remove these external API and
try to calculate the penalties at runtime for the ISA, SCI as well as PCI
IRQS. This didn't work out well with the existing platforms.
Changes from V1 (https://lkml.org/lkml/2016/10/1/106):
* Commit message updates
Sinan Kaya (3):
Revert "ACPI,PCI,IRQ: reduce static IRQ array size to 16"
ACPI, PCI IRQ: add PCI_USING penalty for ISA interrupts
Revert "ACPI,PCI,IRQ: remove SCI penalize function"
arch/x86/kernel/acpi/boot.c | 1 +
drivers/acpi/pci_link.c | 71 ++++++++++++++++++++++-----------------------
include/linux/acpi.h | 1 +
3 files changed, 37 insertions(+), 36 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH 07/15] clk: sunxi: mod0: improve function-level documentation
From: Maxime Ripard @ 2016-10-04 21:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475351192-27079-8-git-send-email-Julia.Lawall@lip6.fr>
On Sat, Oct 01, 2016 at 09:46:24PM +0200, Julia Lawall wrote:
> Use the actual function name in the function documentation.
>
> Issue detected using Coccinelle (http://coccinelle.lip6.fr/)
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Applied, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161004/6642e966/attachment.sig>
^ permalink raw reply
* [PATCH v3 2/7] i2c: bcm2835: Protect against unexpected TXW/RXR interrupts
From: Noralf Trønnes @ 2016-10-04 19:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8760p9xlqy.fsf@eliezer.anholt.net>
Den 03.10.2016 21:42, skrev Eric Anholt:
> Noralf Tr?nnes <noralf@tronnes.org> writes:
>
>> Den 29.09.2016 00:00, skrev Eric Anholt:
>>> Noralf Tr?nnes <noralf@tronnes.org> writes:
>>>
>>>> If an unexpected TXW or RXR interrupt occurs (msg_buf_remaining == 0),
>>>> the driver has no way to fill/drain the FIFO to stop the interrupts.
>>>> In this case the controller has to be disabled and the transfer
>>>> completed to avoid hang.
>>>>
>>>> (CLKT | ERR) and DONE interrupts are completed in their own paths, and
>>>> the controller is disabled in the transfer function after completion.
>>>> Unite the code paths and do disabling inside the interrupt routine.
>>>>
>>>> Clear interrupt status bits in the united completion path instead of
>>>> trying to do it on every interrupt which isn't necessary.
>>>> Only CLKT, ERR and DONE can be cleared that way.
>>>>
>>>> Add the status value to the error value in case of TXW/RXR errors to
>>>> distinguish them from the other S_LEN error.
>>> I was surprised that not writing the TXW/RXR bits on handling their
>>> interrupts was OK, given that we were doing so before, but it's a level
>>> interrupt and those bits are basically ignored on write.
>>>
>>> This patch and 3, 4, and 6 are:
>>>
>>> Reviewed-by: Eric Anholt <eric@anholt.net>
>>>
>>> Patch 5 is:
>>>
>>> Acked-by: Eric Anholt <eric@anholt.net>
>>>
>>> Note for future debug: The I2C_C_CLEAR on errors will take some time to
>>> resolve -- if you were in non-idle state and I2C_C_READ, it sets an
>>> abort_rx flag and runs through the state machine to send a NACK and a
>>> STOP, I think. Since we're setting CLEAR without I2CEN, that NACK will
>>> be hanging around queued up for next time we start the engine.
>> Maybe you're able to explain the issues I had with reset:
>> https://github.com/raspberrypi/linux/issues/1653
> One of the questions I think you might have is "what state does the
> controller end up in after the various interrupts?"
>
> ERR:
> - produced if we get a nack that's not at the end of a read.
>
> - Proceeds to repeated start if BCM2835_I2C_C_ST is queued, otherwise
> stop.
>
> CLKT:
> - Triggered by a counter outside of the state machine when stretching
> happens and then times out.
>
> - Sets cs_override, which causes proceeding through the state machine as
> if the clock wasn't getting stretched, until the end of the next byte
> sent/received.
>
> - According to Wolfram we shouldn't be timing out on clock stretching
> for i2c, just on the transfer as a whole
> (https://patchwork.kernel.org/patch/9148431/), so I wrote
> https://github.com/anholt/linux/commit/894200276239d2e4c60b378bdc52164fcb13af8d.
> However, I don't see an obvious way to get back to IDLE while the
> slave is still stretching, without triggering the clock stretching
> timeout path.
If the transfer times out, whatever the reason, we clear the fifo
(and disable). Doesn't that get us back to IDLE?
Code with my patches:
static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
int num)
{
[...]
bcm2835_i2c_start_transfer(i2c_dev);
time_left = wait_for_completion_timeout(&i2c_dev->completion,
adap->timeout);
if (!time_left) {
bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C,
BCM2835_I2C_C_CLEAR);
dev_err(i2c_dev->dev, "i2c transfer timed out\n");
return -ETIMEDOUT;
}
> DONE:
> - Signaled at STOP, and just moves to IDLE state which keeps scl/sda
> high and waits for a BCM2835_I2C_C_ST while we're not clearing the
> FIFOs (if you do signal start while the fifos are clearing, the start
> will hang around until the fifo clear is done). This is the only way
> to get to IDLE.
>
> I'm don't think I have an answer to the "what should I do?" question you
> had, but hopefully this helps.
^ permalink raw reply
* [RFC] arm64: Enforce observed order for spinlock and data
From: Mark Rutland @ 2016-10-04 19:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <884bd5d3a9a1bcf2a276130ffc17412a@codeaurora.org>
Hi Brent,
Could you *please* clarify if you are trying to solve:
(a) a correctness issue (e.g. data corruption) seen in practice.
(b) a correctness issue (e.g. data corruption) found by inspection.
(c) A performance issue, seen in practice.
(d) A performance issue, found by inspection.
Any one of these is fine; we just need to know in order to be able to
help effectively, and so far it hasn't been clear.
On Tue, Oct 04, 2016 at 01:53:35PM -0400, bdegraaf at codeaurora.org wrote:
> After looking at this, the problem is not with the lockref code per
> se: it is a problem with arch_spin_value_unlocked(). In the
> out-of-order case, arch_spin_value_unlocked() can return TRUE for a
> spinlock that is in fact locked but the lock is not observable yet via
> an ordinary load.
Given arch_spin_value_unlocked() doesn't perform any load itself, I
assume the ordinary load that you are referring to is the READ_ONCE()
early in CMPXCHG_LOOP().
It's worth noting that even if we ignore ordering and assume a
sequentially-consistent machine, READ_ONCE() can give us a stale value.
We could perform the read, then another agent can acquire the lock, then
we can move onto the cmpxchg(), i.e.
CPU0 CPU1
old = READ_ONCE(x.lock_val)
spin_lock(x.lock)
cmpxchg(x.lock_val, old, new)
spin_unlock(x.lock)
If the 'old' value is stale, the cmpxchg *must* fail, and the cmpxchg
should return an up-to-date value which we will then retry with.
> Other than ensuring order on the locking side (as the prior patch
> did), there is a way to make arch_spin_value_unlock's TRUE return
> value deterministic,
In general, this cannot be made deterministic. As above, there is a race
that cannot be avoided.
> but it requires that it does a write-back to the lock to ensure we
> didn't observe the unlocked value while another agent was in process
> of writing back a locked value.
The cmpxchg gives us this guarantee. If it successfully stores, then the
value it observed was the same as READ_ONCE() saw, and the update was
atomic.
There *could* have been an intervening sequence between the READ_ONCE
and cmpxchg (e.g. put(); get()) but that's not problematic for lockref.
Until you've taken your reference it was possible that things changed
underneath you.
Thanks,
Mark.
^ permalink raw reply
* [PATCH] ARM: dts: rockchip: Reserve unusable memory region on rk3066
From: Heiko Stübner @ 2016-10-04 18:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <81d1fb8b-ee0e-35a7-77db-5e15c3f46449@gmail.com>
Hi Pawe?,
Am Dienstag, 4. Oktober 2016, 13:56:07 schrieb Pawe? Jarosz:
> >>>> I don't think this is a sane workaround, but it is at best difficult to
> >>>> tell, given there's no reason given for why this memory is unusable.
> >>>>
> >>>> For instance, if bus accesses to this address hang, then this patch
> >>>> only
> >>>> makes the hand less likely, since the kernel will still map the region
> >>>> (and
> >>>> therefore the CPU can perform speculative accesses).
> >>>>
> >>>> Are issues with this memory consistently seen in practice?
> >>>>
> >>>> Can you enable CONFIG_MEMTEST and pass 'memtest' to the kernel, to
> >>>> determine if the memory is returning erroneous values?
> >>>
> >>> just for the sake of completeness, on the rk3288 the issue was the dma
> >>> not
> >>> being able to access the specific memory region (interestingly also the
> >>> last 16MB but of the 4GB area supported on the rk3288). So memory itself
> >>> was ok, just dma access to it failed.
> >>
> >> How odd.
> >>
> >>> We didn't find any other sane solution to limit the dma access in a
> >>> general way at the time, so opted for just blocking the memory region
> >>> (as
> >>> it was similarly only
> >>
> >> I was under the impression that dma-ranges could describe this kind of
> >> DMA addressing limitation. Was there some problem with that? Perhaps the
> >> driver is not acquiring/configuring its mask correctly?
> >
> > I remember looking at (and trying) different options back then.
> >
> > dma-mask wanted power-of-2 values (so it's either 4GB or 2GB (or lower)),
> > zone-dma was a 32bit (and non-dt) thing and dma-ranges seem to simply also
> > calculate a dma-mask from the value, so you're down to 2GB again.
> >
> > So just blocking of those 16MB at the end for 4GB devices somehow sounded
> > nicer than limiting dma access to only half the memory.
> >
> > I may be overlooking something but that was what I came up with last year.
> >
> >
> > Heiko
>
> Is there a chance to accept this patch?
>
> I know it's not the best solution to this problem, but i don't know
> a better one.
there is always a "chance". But with changes like these, we always try to find
a real cause first, before resorting to solutions like this. So it's definitly
not off the table, but I'd like to investigate further first, so that we don't
accumulate unnecessary hacks over time.
Especially that your region seems to be in the middle of the designated ram
area is strange.
Could you please tell which board you're using (and how much memory it has)
Thanks
Heiko
^ permalink raw reply
* [RFC] arm64: Enforce observed order for spinlock and data
From: bdegraaf at codeaurora.org @ 2016-10-04 18:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <884bd5d3a9a1bcf2a276130ffc17412a@codeaurora.org>
On 2016-10-04 13:53, bdegraaf at codeaurora.org wrote:
> On 2016-10-04 06:12, Mark Rutland wrote:
>> On Mon, Oct 03, 2016 at 03:20:57PM -0400, bdegraaf at codeaurora.org
>> wrote:
>>> On 2016-10-01 14:11, Mark Rutland wrote:
>>> >Hi Brent,
>>> >
>>> >Evidently my questions weren't sufficiently clear; even with your
>>> >answers it's not clear to me what precise issue you're attempting to
>>> >solve. I've tried to be more specific this time.
>>> >
>>> >At a high-level, can you clarify whether you're attempting to solve is:
>>> >
>>> >(a) a functional correctness issue (e.g. data corruption)
>>> >(b) a performance issue
>>> >
>>> >And whether this was seen in practice, or found through code
>>> >inspection?
>>
>>> Thinking about this, as the reader/writer code has no known "abuse"
>>> case, I'll remove it from the patchset, then provide a v2 patchset
>>> with a detailed explanation for the lockref problem using the commits
>>> you provided as an example, as well as performance consideration.
>>
>> If there's a functional problem, let's consider that in isolation
>> first.
>> Once we understand that, then we can consider doing what is optimal.
>>
>> As should be obvious from the above, I'm confused because this patch
>> conflates functional details with performance optimisations which (to
>> me) sound architecturally dubious.
>>
>> I completely agree with Peter that if the problem lies with lockref,
>> it
>> should be solved in the lockref code.
>>
>> Thanks,
>> Mark.
>
> After looking at this, the problem is not with the lockref code per se:
> it is
> a problem with arch_spin_value_unlocked(). In the out-of-order case,
> arch_spin_value_unlocked() can return TRUE for a spinlock that is in
> fact
> locked but the lock is not observable yet via an ordinary load. Other
> than
> ensuring order on the locking side (as the prior patch did), there is a
> way
> to make arch_spin_value_unlock's TRUE return value deterministic, but
> it
> requires that it does a write-back to the lock to ensure we didn't
> observe
> the unlocked value while another agent was in process of writing back a
> locked value.
>
> Brent
Scratch that--things get complicated as the lock itself gets "cloned,"
which
could happen during the out-of-order window. I'll post back later after
I've
analyzed it fully.
^ permalink raw reply
* [PATCH v2 2/2] arm64: dts: rockchip: arch counter doesn't tick in system suspend
From: Brian Norris @ 2016-10-04 18:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475604730-140264-1-git-send-email-briannorris@chromium.org>
The "arm,no-tick-in-suspend" property was introduced to note
implementations where the system counter does not quite follow the ARM
specification that it "must be implemented in an always-on power
domain".
Particularly, RK3399's counter stops ticking when we switch from the
24MHz clock to the 32KHz clock in low-power suspend, so let's mark it as
such.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: new in v2
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 76b28649f0b0..401c8be8c8ac 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -174,6 +174,7 @@
<GIC_PPI 14 IRQ_TYPE_LEVEL_LOW 0>,
<GIC_PPI 11 IRQ_TYPE_LEVEL_LOW 0>,
<GIC_PPI 10 IRQ_TYPE_LEVEL_LOW 0>;
+ arm,no-tick-in-suspend;
};
xin24m: xin24m {
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v2 1/2] clocksource: arm_arch_timer: Don't assume clock runs in suspend
From: Brian Norris @ 2016-10-04 18:12 UTC (permalink / raw)
To: linux-arm-kernel
The ARM ARM specifies that the system counter "must be implemented in an
always-on power domain," and so we try to use the counter as a source of
timekeeping across suspend/resume. Unfortunately, some SoCs (e.g.,
Rockchip's RK3399) do not keep the counter ticking properly when
switched from their high-power clock to the lower-power clock used in
system suspend. Support this quirk by adding a new device tree property.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2:
* add new device tree property, instead of re-using the "always-on"
property (which has different meaning)
Documentation/devicetree/bindings/arm/arch_timer.txt | 5 +++++
drivers/clocksource/arm_arch_timer.c | 9 ++++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
index ef5fbe9a77c7..ad440a2b8051 100644
--- a/Documentation/devicetree/bindings/arm/arch_timer.txt
+++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
@@ -38,6 +38,11 @@ to deliver its interrupts via SPIs.
architecturally-defined reset values. Only supported for 32-bit
systems which follow the ARMv7 architected reset values.
+- arm,no-tick-in-suspend : The main counter does not tick when the system is in
+ low-power system suspend on some SoCs. This behavior does not match the
+ Architecture Reference Manual's specification that the system counter "must
+ be implemented in an always-on power domain."
+
Example:
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 73c487da6d2a..a2503db7e533 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -81,6 +81,7 @@ static struct clock_event_device __percpu *arch_timer_evt;
static enum ppi_nr arch_timer_uses_ppi = VIRT_PPI;
static bool arch_timer_c3stop;
static bool arch_timer_mem_use_virtual;
+static bool arch_counter_suspend_stop;
static bool evtstrm_enable = IS_ENABLED(CONFIG_ARM_ARCH_TIMER_EVTSTREAM);
@@ -576,7 +577,7 @@ static struct clocksource clocksource_counter = {
.rating = 400,
.read = arch_counter_read,
.mask = CLOCKSOURCE_MASK(56),
- .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
+ .flags = CLOCK_SOURCE_IS_CONTINUOUS,
};
static struct cyclecounter cyclecounter = {
@@ -616,6 +617,8 @@ static void __init arch_counter_register(unsigned type)
arch_timer_read_counter = arch_counter_get_cntvct_mem;
}
+ if (!arch_counter_suspend_stop)
+ clocksource_counter.flags |= CLOCK_SOURCE_SUSPEND_NONSTOP;
start_count = arch_timer_read_counter();
clocksource_register_hz(&clocksource_counter, arch_timer_rate);
cyclecounter.mult = clocksource_counter.mult;
@@ -907,6 +910,10 @@ static int __init arch_timer_of_init(struct device_node *np)
of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
arch_timer_uses_ppi = PHYS_SECURE_PPI;
+ /* On some systems, the counter stops ticking when in suspend. */
+ arch_counter_suspend_stop = of_property_read_bool(np,
+ "arm,no-tick-in-suspend");
+
return arch_timer_init();
}
CLOCKSOURCE_OF_DECLARE(armv7_arch_timer, "arm,armv7-timer", arch_timer_of_init);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [RFC] arm64: Enforce observed order for spinlock and data
From: bdegraaf at codeaurora.org @ 2016-10-04 17:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004101208.GA18083@leverpostej>
On 2016-10-04 06:12, Mark Rutland wrote:
> On Mon, Oct 03, 2016 at 03:20:57PM -0400, bdegraaf at codeaurora.org
> wrote:
>> On 2016-10-01 14:11, Mark Rutland wrote:
>> >Hi Brent,
>> >
>> >Evidently my questions weren't sufficiently clear; even with your
>> >answers it's not clear to me what precise issue you're attempting to
>> >solve. I've tried to be more specific this time.
>> >
>> >At a high-level, can you clarify whether you're attempting to solve is:
>> >
>> >(a) a functional correctness issue (e.g. data corruption)
>> >(b) a performance issue
>> >
>> >And whether this was seen in practice, or found through code
>> >inspection?
>
>> Thinking about this, as the reader/writer code has no known "abuse"
>> case, I'll remove it from the patchset, then provide a v2 patchset
>> with a detailed explanation for the lockref problem using the commits
>> you provided as an example, as well as performance consideration.
>
> If there's a functional problem, let's consider that in isolation
> first.
> Once we understand that, then we can consider doing what is optimal.
>
> As should be obvious from the above, I'm confused because this patch
> conflates functional details with performance optimisations which (to
> me) sound architecturally dubious.
>
> I completely agree with Peter that if the problem lies with lockref, it
> should be solved in the lockref code.
>
> Thanks,
> Mark.
After looking at this, the problem is not with the lockref code per se:
it is
a problem with arch_spin_value_unlocked(). In the out-of-order case,
arch_spin_value_unlocked() can return TRUE for a spinlock that is in
fact
locked but the lock is not observable yet via an ordinary load. Other
than
ensuring order on the locking side (as the prior patch did), there is a
way
to make arch_spin_value_unlock's TRUE return value deterministic, but it
requires that it does a write-back to the lock to ensure we didn't
observe
the unlocked value while another agent was in process of writing back a
locked value.
Brent
^ permalink raw reply
* [PATCH] clocksource: arm_arch_timer: Don't assume clock runs in suspend
From: Brian Norris @ 2016-10-04 17:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160929170847.1227a312@arm.com>
Hi Marc,
On Thu, Sep 29, 2016 at 05:08:47PM +0100, Marc Zyngier wrote:
> On Tue, 27 Sep 2016 18:23:11 -0700
> Brian Norris <briannorris@chromium.org> wrote:
> > On Tue, Sep 20, 2016 at 08:47:07AM +0100, Marc Zyngier wrote:
> > <Begin side note>
> > rk3288 (ARMv7 system widely used for our Chromebooks) has the same
> > issue, except the kernel we're using for production (based on v3.14)
> > doesn't have the following commit, which stopped utilizing the RTC:
> >
> > commit 0fa88cb4b82b5cf7429bc1cef9db006ca035754e
> > Author: Xunlei Pang <pang.xunlei@linaro.org>
> > Date: Wed Apr 1 20:34:38 2015 -0700
> >
> > time, drivers/rtc: Don't bother with rtc_resume() for the nonstop clocksource
> >
> > And any mainline testing on rk3288 doesn't see the problem, because
> > mainline doesn't support its lowest-power sleep modes well enough (see
> > ROCKCHIP_ARM_OFF_LOGIC_DEEP in arch/arm/mach-rockchip/pm.c).
>
> Arghh... So even my favourite Chromebook (from which I'm typing this
> email) is affected? Not very nice...
Yep. But if you're running mainline, you just get to have high S3 power
consumption instead!
> > <End side note>
> As for the 64bit kernel, it would be interesting to verify that on
> resume, the VDSO does return the right (corrected) value, and not
> something stale.
It would be interesting, except all my current user spaces are built for
32-bit, so it's not too easy for me to test. Perhaps I could pull in
this [1]. (On the bright side, this means that VDSO can't possibly be
breaking on my systems!)
Brian
[1] http://www.spinics.net/lists/arm-kernel/msg530185.html
^ permalink raw reply
* ks_dw_pcie_initiate_link_train() question
From: Bjorn Helgaas @ 2016-10-04 17:40 UTC (permalink / raw)
To: linux-arm-kernel
Hi Murali,
This code looks suspicious. Can you comment?
void ks_dw_pcie_initiate_link_train(struct keystone_pcie *ks_pcie)
{
u32 val;
/* Disable Link training */
val = readl(ks_pcie->va_app_base + CMD_STATUS);
val &= ~LTSSM_EN_VAL;
writel(LTSSM_EN_VAL | val, ks_pcie->va_app_base + CMD_STATUS);
Here we cleared the LTSSM_EN_VAL bit in "val", but then we add it
right back in before writing it back to CMD_STATUS.
That looks like a cut and paste error to me, but of course I don't
know the hardware.
/* Initiate Link Training */
val = readl(ks_pcie->va_app_base + CMD_STATUS);
writel(LTSSM_EN_VAL | val, ks_pcie->va_app_base + CMD_STATUS);
}
^ permalink raw reply
* [RFC 05/11] iommu/dma: iommu_dma_(un)map_mixed
From: Auger Eric @ 2016-10-04 17:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7474f131-9e44-bfde-6937-7cdbd6b2c8a5@arm.com>
Hi Robin,
On 04/10/2016 19:18, Robin Murphy wrote:
> On 02/10/16 10:56, Christoffer Dall wrote:
>> On Fri, Sep 30, 2016 at 02:24:40PM +0100, Robin Murphy wrote:
>>> Hi Eric,
>>>
>>> On 27/09/16 21:48, Eric Auger wrote:
>>>> iommu_dma_map_mixed and iommu_dma_unmap_mixed operate on
>>>> IOMMU_DOMAIN_MIXED typed domains. On top of standard iommu_map/unmap
>>>> they reserve the IOVA window to prevent the iova allocator to
>>>> allocate in those areas.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>> ---
>>>> drivers/iommu/dma-iommu.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>>>> include/linux/dma-iommu.h | 18 ++++++++++++++++++
>>>> 2 files changed, 66 insertions(+)
>>>>
>>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>>> index 04bbc85..db21143 100644
>>>> --- a/drivers/iommu/dma-iommu.c
>>>> +++ b/drivers/iommu/dma-iommu.c
>>>> @@ -759,3 +759,51 @@ int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
>>>> return 0;
>>>> }
>>>> EXPORT_SYMBOL(iommu_get_dma_msi_region_cookie);
>>>> +
>>>> +int iommu_dma_map_mixed(struct iommu_domain *domain, unsigned long iova,
>>>> + phys_addr_t paddr, size_t size, int prot)
>>>> +{
>>>> + struct iova_domain *iovad;
>>>> + unsigned long lo, hi;
>>>> + int ret;
>>>> +
>>>> + if (domain->type != IOMMU_DOMAIN_MIXED)
>>>> + return -EINVAL;
>>>> +
>>>> + if (!domain->iova_cookie)
>>>> + return -EINVAL;
>>>> +
>>>> + iovad = cookie_iovad(domain);
>>>> +
>>>> + lo = iova_pfn(iovad, iova);
>>>> + hi = iova_pfn(iovad, iova + size - 1);
>>>> + reserve_iova(iovad, lo, hi);
>>>
>>> This can't work reliably - reserve_iova() will (for good reason) merge
>>> any adjacent or overlapping entries, so any unmap is liable to free more
>>> IOVA space than actually gets unmapped, and things will get subtly out
>>> of sync and go wrong later.
>>>
>>> The more general issue with this whole approach, though, is that it
>>> effectively rules out userspace doing guest memory hotplug or similar,
>>> and I'm not we want to paint ourselves into that corner. Basically, as
>>> soon as a device is attached to a guest, the entirety of the unallocated
>>> IPA space becomes reserved, and userspace can never add anything further
>>> to it, because any given address *might* be in use for an MSI mapping.
>>
>> Ah, we didn't think of that when discussing this design at KVM Forum,
>> because the idea was that the IOVA allocator was in charge of that
>> resource, and the IOVA was a separate concept from the IPA space.
>>
>> I think what tripped us up, is that while the above is true for the MSI
>> configuration where we trap the bar and do the allocation at VFIO init
>> time, the guest device driver can program DMA to any address without
>> trapping, and therefore there's an inherent relationship between the
>> IOVA and the IPA space. Is that right?
>
> Yes, for anything the guest knows about and/or can touch directly, IOVA
> must equal IPA, or DMA is going to go horribly wrong. It's only direct
> interactions between device and host behind the guest's back where we
> (may) have some freedom with IOVA assignment.
>
>>> I think it still makes most sense to stick with the original approach of
>>> cooperating with userspace to reserve a bounded area - it's just that we
>>> can then let automatic mapping take care of itself within that area.
>>
>> I was thinking that it's also possible to do it the other way around: To
>> let userspace say wherever memory may be hotplugged and do the
>> allocation within the remaining area, but I suppose that's pretty much
>> the same thing, and it should just depend on what's easiest to implement
>> and what userspace can best predict.
>
> Indeed, if userspace *is* able to pre-emptively claim everything it
> might ever want, that does kind of implicitly solve the "tell me where I
> can put this" problem (assuming it doesn't simply claim the whole
> address space, of course), but I'm not so sure it works well if there
> are any specific restrictions (e.g. if some device is going to require
> the MSI range to be 32-bit addressable). It also fails to address the
> issue below...
>
>>> Speaking of which, I've realised the same fundamental reservation
>>> problem already applies to PCI without ACS, regardless of MSIs. I just
>>> tried on my Juno with guest memory placed at 0x4000000000, (i.e.
>>> matching the host PA of the 64-bit PCI window), and sure enough when the
>>> guest kicks off some DMA on the passed-through NIC, the root complex
>>> interprets the guest IPA as (unsupported) peer-to-peer DMA to a BAR
>>> claimed by the video card, and it fails. I guess this doesn't get hit in
>>> practice on x86 because the guest memory map is unlikely to be much
>>> different from the host's.
>>>
>>> It seems like we basically need a general way of communicating fixed and
>>> movable host reservations to userspace :/
>>>
>>
>> Yes, this makes sense to me. Do we have any existing way of
>> discovering this from userspace or can we think of something?
>
> I know virtually nothing about the userspace interface, but I was under
> the impression it would require something new. I wasn't even aware you
> could do the VFIO-under-QEMU-TCG thing which Eric points out,
I meant running a non x86 VM on an x86 host. Quoting Alex:
"x86 isn't problem-free in this space. An x86 VM is going to know that
the 0xfee00000 address range is special, it won't be backed by RAM and
won't be a DMA target, thus we'll never attempt to map it for an iova
address. However, if we run a non-x86 VM or a userspace driver, it
doesn't necessarily know that there's anything special about that range
of iovas. I intend to resolve this with an extension to the iommu info
ioctl that describes the available iova space for the iommu. The
interrupt region would simply be excluded."
In my v12 I added such VFIO IOMMU info ioctl to retrieve the MSI
topology. Now for the issue you pointed out (PCI without ACS) I
understand this is a generalisation of the same issue and the VFIO IOMMU
info capability chain API could be used as well. I can submit something
separately. But anyway at QEMU level, due to the static mapping in
mach-virt, at the moment, we just can reject the assignment I am afraid.
Thanks
Eric
so it
> seems like the general "tell userspace about addresses it can't use"
> issue is perhaps the more pressing one. On investigation, QEMU's static
> memory map with RAM at 0x4000000 is already busted for VFIO on Juno, as
> that results in attempting DMA to config space, which goes about as well
> as one might expect.
>
> Robin.
>
>>
>> Thanks,
>> -Christoffer
>>
>
^ permalink raw reply
* [RFC 05/11] iommu/dma: iommu_dma_(un)map_mixed
From: Robin Murphy @ 2016-10-04 17:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161002095614.GA23218@cbox>
On 02/10/16 10:56, Christoffer Dall wrote:
> On Fri, Sep 30, 2016 at 02:24:40PM +0100, Robin Murphy wrote:
>> Hi Eric,
>>
>> On 27/09/16 21:48, Eric Auger wrote:
>>> iommu_dma_map_mixed and iommu_dma_unmap_mixed operate on
>>> IOMMU_DOMAIN_MIXED typed domains. On top of standard iommu_map/unmap
>>> they reserve the IOVA window to prevent the iova allocator to
>>> allocate in those areas.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> ---
>>> drivers/iommu/dma-iommu.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/dma-iommu.h | 18 ++++++++++++++++++
>>> 2 files changed, 66 insertions(+)
>>>
>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>> index 04bbc85..db21143 100644
>>> --- a/drivers/iommu/dma-iommu.c
>>> +++ b/drivers/iommu/dma-iommu.c
>>> @@ -759,3 +759,51 @@ int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
>>> return 0;
>>> }
>>> EXPORT_SYMBOL(iommu_get_dma_msi_region_cookie);
>>> +
>>> +int iommu_dma_map_mixed(struct iommu_domain *domain, unsigned long iova,
>>> + phys_addr_t paddr, size_t size, int prot)
>>> +{
>>> + struct iova_domain *iovad;
>>> + unsigned long lo, hi;
>>> + int ret;
>>> +
>>> + if (domain->type != IOMMU_DOMAIN_MIXED)
>>> + return -EINVAL;
>>> +
>>> + if (!domain->iova_cookie)
>>> + return -EINVAL;
>>> +
>>> + iovad = cookie_iovad(domain);
>>> +
>>> + lo = iova_pfn(iovad, iova);
>>> + hi = iova_pfn(iovad, iova + size - 1);
>>> + reserve_iova(iovad, lo, hi);
>>
>> This can't work reliably - reserve_iova() will (for good reason) merge
>> any adjacent or overlapping entries, so any unmap is liable to free more
>> IOVA space than actually gets unmapped, and things will get subtly out
>> of sync and go wrong later.
>>
>> The more general issue with this whole approach, though, is that it
>> effectively rules out userspace doing guest memory hotplug or similar,
>> and I'm not we want to paint ourselves into that corner. Basically, as
>> soon as a device is attached to a guest, the entirety of the unallocated
>> IPA space becomes reserved, and userspace can never add anything further
>> to it, because any given address *might* be in use for an MSI mapping.
>
> Ah, we didn't think of that when discussing this design at KVM Forum,
> because the idea was that the IOVA allocator was in charge of that
> resource, and the IOVA was a separate concept from the IPA space.
>
> I think what tripped us up, is that while the above is true for the MSI
> configuration where we trap the bar and do the allocation at VFIO init
> time, the guest device driver can program DMA to any address without
> trapping, and therefore there's an inherent relationship between the
> IOVA and the IPA space. Is that right?
Yes, for anything the guest knows about and/or can touch directly, IOVA
must equal IPA, or DMA is going to go horribly wrong. It's only direct
interactions between device and host behind the guest's back where we
(may) have some freedom with IOVA assignment.
>> I think it still makes most sense to stick with the original approach of
>> cooperating with userspace to reserve a bounded area - it's just that we
>> can then let automatic mapping take care of itself within that area.
>
> I was thinking that it's also possible to do it the other way around: To
> let userspace say wherever memory may be hotplugged and do the
> allocation within the remaining area, but I suppose that's pretty much
> the same thing, and it should just depend on what's easiest to implement
> and what userspace can best predict.
Indeed, if userspace *is* able to pre-emptively claim everything it
might ever want, that does kind of implicitly solve the "tell me where I
can put this" problem (assuming it doesn't simply claim the whole
address space, of course), but I'm not so sure it works well if there
are any specific restrictions (e.g. if some device is going to require
the MSI range to be 32-bit addressable). It also fails to address the
issue below...
>> Speaking of which, I've realised the same fundamental reservation
>> problem already applies to PCI without ACS, regardless of MSIs. I just
>> tried on my Juno with guest memory placed at 0x4000000000, (i.e.
>> matching the host PA of the 64-bit PCI window), and sure enough when the
>> guest kicks off some DMA on the passed-through NIC, the root complex
>> interprets the guest IPA as (unsupported) peer-to-peer DMA to a BAR
>> claimed by the video card, and it fails. I guess this doesn't get hit in
>> practice on x86 because the guest memory map is unlikely to be much
>> different from the host's.
>>
>> It seems like we basically need a general way of communicating fixed and
>> movable host reservations to userspace :/
>>
>
> Yes, this makes sense to me. Do we have any existing way of
> discovering this from userspace or can we think of something?
I know virtually nothing about the userspace interface, but I was under
the impression it would require something new. I wasn't even aware you
could do the VFIO-under-QEMU-TCG thing which Eric points out, so it
seems like the general "tell userspace about addresses it can't use"
issue is perhaps the more pressing one. On investigation, QEMU's static
memory map with RAM at 0x4000000 is already busted for VFIO on Juno, as
that results in attempting DMA to config space, which goes about as well
as one might expect.
Robin.
>
> Thanks,
> -Christoffer
>
^ permalink raw reply
* [PATCH v2] arm: Added support for getcpu() vDSO using TPIDRURW
From: Mark Rutland @ 2016-10-04 17:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475595363-4272-1-git-send-email-fredrik.markstrom@gmail.com>
On Tue, Oct 04, 2016 at 05:35:33PM +0200, Fredrik Markstrom wrote:
> This makes getcpu() ~1000 times faster, this is very useful when
> implementing per-cpu buffers in userspace (to avoid cache line
> bouncing). As an example lttng ust becomes ~30% faster.
>
> The patch will break applications using TPIDRURW (which is context switched
> since commit 4780adeefd042482f624f5e0d577bf9cdcbb760 ("ARM: 7735/2:
It looks like you dropped the leading 'a' from the commit ID. For
everyone else's benefit, the full ID is:
a4780adeefd042482f624f5e0d577bf9cdcbb760
Please note that arm64 has done similar for compat tasks since commit:
d00a3810c16207d2 ("arm64: context-switch user tls register tpidr_el0 for
compat tasks")
> Preserve the user r/w register TPIDRURW on context switch and fork")) and
> is therefore made configurable.
As you note above, this is an ABI break and *will* break some existing
applications. That's generally a no-go.
This also leaves arm64's compat with the existing behaviour, differing
from arm.
I was under the impression that other mechanisms were being considered
for fast userspace access to per-cpu data structures, e.g. restartable
sequences. What is the state of those? Why is this better?
If getcpu() specifically is necessary, is there no other way to
implement it?
> +notrace int __vdso_getcpu(unsigned int *cpup, unsigned int *nodep,
> + struct getcpu_cache *tcache)
> +{
> + unsigned long node_and_cpu;
> +
> + asm("mrc p15, 0, %0, c13, c0, 2\n" : "=r"(node_and_cpu));
> +
> + if (nodep)
> + *nodep = cpu_to_node(node_and_cpu >> 16);
> + if (cpup)
> + *cpup = node_and_cpu & 0xffffUL;
Given this is directly user-accessible, this format is a de-facto ABI,
even if it's not documented as such. Is this definitely the format you
want long-term?
Thanks,
Mark.
^ permalink raw reply
* [PATCH V3 8/8] arm64: dma-mapping: Remove the notifier trick to handle early setting of dma_ops
From: Sricharan R @ 2016-10-04 17:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475600632-21289-1-git-send-email-sricharan@codeaurora.org>
With arch_setup_dma_ops now being called late during device's probe after the
device's iommu is probed, the notifier trick required to handle the early
setup of dma_ops before the iommu group gets created is not required.
So removing the notifier's here.
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
arch/arm64/mm/dma-mapping.c | 100 ++------------------------------------------
1 file changed, 3 insertions(+), 97 deletions(-)
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index faf4b92..eb593af 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -799,24 +799,6 @@ static struct dma_map_ops iommu_dma_ops = {
.mapping_error = iommu_dma_mapping_error,
};
-/*
- * TODO: Right now __iommu_setup_dma_ops() gets called too early to do
- * everything it needs to - the device is only partially created and the
- * IOMMU driver hasn't seen it yet, so it can't have a group. Thus we
- * need this delayed attachment dance. Once IOMMU probe ordering is sorted
- * to move the arch_setup_dma_ops() call later, all the notifier bits below
- * become unnecessary, and will go away.
- */
-struct iommu_dma_notifier_data {
- struct list_head list;
- struct device *dev;
- const struct iommu_ops *ops;
- u64 dma_base;
- u64 size;
-};
-static LIST_HEAD(iommu_dma_masters);
-static DEFINE_MUTEX(iommu_dma_notifier_lock);
-
static bool do_iommu_attach(struct device *dev, const struct iommu_ops *ops,
u64 dma_base, u64 size)
{
@@ -837,79 +819,9 @@ static bool do_iommu_attach(struct device *dev, const struct iommu_ops *ops,
return true;
}
-static void queue_iommu_attach(struct device *dev, const struct iommu_ops *ops,
- u64 dma_base, u64 size)
-{
- struct iommu_dma_notifier_data *iommudata;
-
- iommudata = kzalloc(sizeof(*iommudata), GFP_KERNEL);
- if (!iommudata)
- return;
-
- iommudata->dev = dev;
- iommudata->ops = ops;
- iommudata->dma_base = dma_base;
- iommudata->size = size;
-
- mutex_lock(&iommu_dma_notifier_lock);
- list_add(&iommudata->list, &iommu_dma_masters);
- mutex_unlock(&iommu_dma_notifier_lock);
-}
-
-static int __iommu_attach_notifier(struct notifier_block *nb,
- unsigned long action, void *data)
-{
- struct iommu_dma_notifier_data *master, *tmp;
-
- if (action != BUS_NOTIFY_BIND_DRIVER)
- return 0;
-
- mutex_lock(&iommu_dma_notifier_lock);
- list_for_each_entry_safe(master, tmp, &iommu_dma_masters, list) {
- if (data == master->dev && do_iommu_attach(master->dev,
- master->ops, master->dma_base, master->size)) {
- list_del(&master->list);
- kfree(master);
- break;
- }
- }
- mutex_unlock(&iommu_dma_notifier_lock);
- return 0;
-}
-
-static int __init register_iommu_dma_ops_notifier(struct bus_type *bus)
-{
- struct notifier_block *nb = kzalloc(sizeof(*nb), GFP_KERNEL);
- int ret;
-
- if (!nb)
- return -ENOMEM;
-
- nb->notifier_call = __iommu_attach_notifier;
-
- ret = bus_register_notifier(bus, nb);
- if (ret) {
- pr_warn("Failed to register DMA domain notifier; IOMMU DMA ops unavailable on bus '%s'\n",
- bus->name);
- kfree(nb);
- }
- return ret;
-}
-
static int __init __iommu_dma_init(void)
{
- int ret;
-
- ret = iommu_dma_init();
- if (!ret)
- ret = register_iommu_dma_ops_notifier(&platform_bus_type);
- if (!ret)
- ret = register_iommu_dma_ops_notifier(&amba_bustype);
-#ifdef CONFIG_PCI
- if (!ret)
- ret = register_iommu_dma_ops_notifier(&pci_bus_type);
-#endif
- return ret;
+ return iommu_dma_init();
}
arch_initcall(__iommu_dma_init);
@@ -920,18 +832,12 @@ static void __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
if (!ops)
return;
- /*
- * TODO: As a concession to the future, we're ready to handle being
- * called both early and late (i.e. after bus_add_device). Once all
- * the platform bus code is reworked to call us late and the notifier
- * junk above goes away, move the body of do_iommu_attach here.
- */
+
group = iommu_group_get(dev);
+
if (group) {
do_iommu_attach(dev, ops, dma_base, size);
iommu_group_put(group);
- } else {
- queue_iommu_attach(dev, ops, dma_base, size);
}
}
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related
* [PATCH V3 7/8] arm/arm64: dma-mapping: Call iommu's remove_device callback during device detach
From: Sricharan R @ 2016-10-04 17:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475600632-21289-1-git-send-email-sricharan@codeaurora.org>
dma_deconfigure calls arch_teardown_dma_ops in the device_detach path,
which is called when the device gets detached from the driver.
When the device was added, iommu's add_device callback was used to
add the device in to its iommu_group and setup the device to be ready
to use its iommu. Similarly, call remove_device callback to remove the
device from the group and reset any other device's iommu configurations.
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
arch/arm/mm/dma-mapping.c | 8 ++++++++
arch/arm64/mm/dma-mapping.c | 7 +++++++
2 files changed, 15 insertions(+)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index b9191f0..cbe22de 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -2289,11 +2289,19 @@ static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size,
static void arm_teardown_iommu_dma_ops(struct device *dev)
{
struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
+ const struct iommu_ops *ops;
if (!mapping)
return;
__arm_iommu_detach_device(dev);
+
+ if (dev->iommu_fwspec) {
+ ops = dev->iommu_fwspec->ops;
+ if (ops->remove_device)
+ ops->remove_device(dev);
+ }
+
arm_iommu_release_mapping(mapping);
set_dma_ops(dev, NULL);
}
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 610d8e5..faf4b92 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -938,6 +938,13 @@ static void __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
void arch_teardown_dma_ops(struct device *dev)
{
struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
+ const struct iommu_ops *ops;
+
+ if (dev->iommu_fwspec) {
+ ops = dev->iommu_fwspec->ops;
+ if (ops->remove_device)
+ ops->remove_device(dev);
+ }
if (WARN_ON(domain))
iommu_detach_device(domain, dev);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ 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