* ARM DMA: Fix in dma_cache_maint_page
From: Subhash Jadavani @ 2013-01-16 12:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F69F72.4030807@codeaurora.org>
Hi Russell,
Is it possible to pick up James patch below? Thread here:
http://comments.gmane.org/gmane.linux.kernel.mmc/18670, have the details
on the motivation behind this fix.
Regards,
Subhash
-------- Original Message --------
Subject: Re: [PATCH v2 1/1] block: blk-merge: don't merge the pages
with non-contiguous descriptors
Date: Wed, 16 Jan 2013 18:09:14 +0530
From: Subhash Jadavani <subhashj@codeaurora.org>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
CC: linux-kernel at vger.kernel.org, linux-scsi at vger.kernel.org,
linux-mmc at vger.kernel.org, linux-arm-msm at vger.kernel.org,
martin.petersen at oracle.com, asias at redhat.com, tj at kernel.org,
linux-arm-kernel at lists.infradead.org, Russell King <linux@arm.linux.org.uk>
On 1/16/2013 4:02 PM, James Bottomley wrote:
> On Wed, 2013-01-16 at 12:07 +0530, Subhash Jadavani wrote:
>
>> Now consider this call stack from MMC block driver (this is on the ARmv7
>> based board):
>> [ 98.918174] [<c001b50c>] (v7_dma_inv_range+0x30/0x48) from
>> [<c0017b8c>] (dma_cache_maint_page+0x1c4/0x24c)
>> [ 98.927819] [<c0017b8c>] (dma_cache_maint_page+0x1c4/0x24c) from
>> [<c0017c28>] (___dma_page_cpu_to_dev+0x14/0x1c)
>> [ 98.937982] [<c0017c28>] (___dma_page_cpu_to_dev+0x14/0x1c) from
>> [<c0017ff8>] (dma_map_sg+0x3c/0x114)
> OK, so this is showing that ARM itself is making the assumption that the
> pages are contiguous in the page offset map.
>
> Fix this by doing the increment via the pfn, which will do the right
> thing whatever the memory model.
>
> Signed-off-by: James Bottomley <JBottomley@Parallels.com>
Thanks James. Yes, it make sense to fix the ARM code itself if it is the
only one giving this trouble.
I have tried your change below and it also fixes this issue (without
having my blk-merge patch). I will forward your change to Russel King to
see what he thinks about it.
Regards,
Subhash
>
> ---
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 6b2fb87..ab88c5b 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -809,7 +809,7 @@ static void dma_cache_maint_page(struct page *page, unsigned long offset,
> op(vaddr, len, dir);
> }
> offset = 0;
> - page++;
> + page = pfn_to_page(page_to_pfn(page) + 1);
> left -= len;
> } while (left);
> }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130116/bcc1625a/attachment-0001.html>
^ permalink raw reply
* [PATCH 3/4] pinctrl: add abx500 pinctrl driver core
From: Lee Jones @ 2013-01-16 12:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F5A1B7.20504@wwwdotorg.org>
> > +static int abx500_gpio_irq_init(struct abx500_pinctrl *pct)
> ...
> > +#ifdef CONFIG_ARM
> > + set_irq_flags(irq, IRQF_VALID);
> > +#else
> > + irq_set_noprobe(irq);
> > +#endif
>
> I assume that ifdef is always set one particular way?
>
> > +static void abx500_gpio_irq_remove(struct abx500_pinctrl *pct)
> ...
> > +#ifdef CONFIG_ARM
> > + set_irq_flags(irq, 0);
> > +#endif
>
> Same there.
You can safely ignore anything to do with IRQs in this patch-set, as
I have a follow-up one (originally written for the old gpio-ab8500
driver - which this is the successor to), which pulls it all out and
applies to the ab8500-core driver instead.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH v2 1/1] block: blk-merge: don't merge the pages with non-contiguous descriptors
From: Subhash Jadavani @ 2013-01-16 12:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1358332355.2384.11.camel@dabdike.int.hansenpartnership.com>
On 1/16/2013 4:02 PM, James Bottomley wrote:
> On Wed, 2013-01-16 at 12:07 +0530, Subhash Jadavani wrote:
>
>> Now consider this call stack from MMC block driver (this is on the ARmv7
>> based board):
>> [ 98.918174] [<c001b50c>] (v7_dma_inv_range+0x30/0x48) from
>> [<c0017b8c>] (dma_cache_maint_page+0x1c4/0x24c)
>> [ 98.927819] [<c0017b8c>] (dma_cache_maint_page+0x1c4/0x24c) from
>> [<c0017c28>] (___dma_page_cpu_to_dev+0x14/0x1c)
>> [ 98.937982] [<c0017c28>] (___dma_page_cpu_to_dev+0x14/0x1c) from
>> [<c0017ff8>] (dma_map_sg+0x3c/0x114)
> OK, so this is showing that ARM itself is making the assumption that the
> pages are contiguous in the page offset map.
>
> Fix this by doing the increment via the pfn, which will do the right
> thing whatever the memory model.
>
> Signed-off-by: James Bottomley <JBottomley@Parallels.com>
Thanks James. Yes, it make sense to fix the ARM code itself if it is the
only one giving this trouble.
I have tried your change below and it also fixes this issue (without
having my blk-merge patch). I will forward your change to Russel King to
see what he thinks about it.
Regards,
Subhash
>
> ---
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 6b2fb87..ab88c5b 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -809,7 +809,7 @@ static void dma_cache_maint_page(struct page *page, unsigned long offset,
> op(vaddr, len, dir);
> }
> offset = 0;
> - page++;
> + page = pfn_to_page(page_to_pfn(page) + 1);
> left -= len;
> } while (left);
> }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v4 2/9] clk: tegra: Add tegra specific clocks
From: Hiroshi Doyu @ 2013-01-16 12:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357890387-23245-3-git-send-email-pgaikwad@nvidia.com>
Prashant Gaikwad <pgaikwad@nvidia.com> wrote @ Fri, 11 Jan 2013 08:46:20 +0100:
...
> +const struct clk_ops tegra_clk_periph_ops = {
> + .get_parent = clk_periph_get_parent,
> + .set_parent = clk_periph_set_parent,
> + .recalc_rate = clk_periph_recalc_rate,
> + .round_rate = clk_periph_round_rate,
> + .set_rate = clk_periph_set_rate,
> + .is_enabled = clk_periph_is_enabled,
> + .enable = clk_periph_enable,
> + .disable = clk_periph_disable,
> +};
> +
> +const struct clk_ops tegra_clk_periph_nodiv_ops = {
> + .get_parent = clk_periph_get_parent,
> + .set_parent = clk_periph_set_parent,
> + .is_enabled = clk_periph_is_enabled,
> + .enable = clk_periph_enable,
> + .disable = clk_periph_disable,
> +};
> +
> +struct clk *tegra_clk_periph(const char *name, const char **parent_names,
> + int num_parents, struct tegra_clk_periph *periph,
> + void __iomem *clk_base, u32 offset)
> +{
> + struct clk *clk;
> + struct clk_init_data init;
> +
> + init.name = name;
> + init.ops = &tegra_clk_periph_ops;
> + init.flags = 0;
> + init.parent_names = parent_names;
> + init.num_parents = num_parents;
> +
> + periph->hw.init = &init;
> + periph->magic = TEGRA_CLK_PERIPH_MAGIC;
> + periph->mux.reg = clk_base + offset;
> + periph->divider.reg = clk_base + offset;
> + periph->gate.clk_base = clk_base;
> +
> + clk = clk_register(NULL, &periph->hw);
> + if (IS_ERR(clk))
> + return clk;
> +
> + periph->mux.hw.clk = clk;
> + periph->divider.hw.clk = clk;
> + periph->gate.hw.clk = clk;
> +
> + return clk;
> +}
> +
> +struct clk *tegra_clk_periph_nodiv(const char *name, const char **parent_names,
> + int num_parents, struct tegra_clk_periph *periph,
> + void __iomem *clk_base, u32 offset)
> +{
> + struct clk *clk;
> + struct clk_init_data init;
> +
> + init.name = name;
> + init.ops = &tegra_clk_periph_nodiv_ops;
> + init.flags = CLK_SET_RATE_PARENT;
> + init.parent_names = parent_names;
> + init.num_parents = num_parents;
> +
> + periph->hw.init = &init;
> + periph->magic = TEGRA_CLK_PERIPH_MAGIC;
> + periph->mux.reg = clk_base + offset;
> + periph->gate.clk_base = clk_base;
> +
> + clk = clk_register(NULL, &periph->hw);
> + if (IS_ERR(clk))
> + return clk;
> +
> + periph->mux.hw.clk = clk;
> + periph->gate.hw.clk = clk;
> +
> + return clk;
> +}
The above two functions are almost duplicate, can we take the common part from them?
const struct clk_ops tegra_clk_periph_nodiv_ops = {
.get_parent = clk_periph_get_parent,
.set_parent = clk_periph_set_parent,
.is_enabled = clk_periph_is_enabled,
.enable = clk_periph_enable,
.disable = clk_periph_disable,
};
struct clk *__tegra_clk_periph(const char *name, const char **parent_names,
int num_parents, struct tegra_clk_periph *periph,
void __iomem *clk_base, u32 offset, int div)
{
struct clk *clk;
struct clk_init_data init;
init.name = name;
init.parent_names = parent_names;
init.num_parents = num_parents;
init.ops = div ? &tegra_clk_periph_ops : &tegra_clk_periph_nodiv_ops
init.flags = div ? 0 :?CLK_SET_RATE_PARENT;
periph->hw.init = &init;
periph->magic = TEGRA_CLK_PERIPH_MAGIC;
periph->mux.reg = clk_base + offset;
periph->divider.reg = clk_base + offset;
periph->gate.clk_base = clk_base;
clk = clk_register(NULL, &periph->hw);
if (IS_ERR(clk))
return clk;
periph->mux.hw.clk = clk;
periph->divider.hw.clk = div ? NULL : clk;
periph->gate.hw.clk = clk;
return clk;
}
static inline struct clk *tegra_clk_periph(const char *name, const char **parent_names,
int num_parents, struct tegra_clk_periph *periph,
void __iomem *clk_base, u32 offset)
{
return __tegra_clk_periph(name, parent_names, num_parents, periph, clk_base, offset, 1);
}
static inline struct clk *tegra_clk_periph_nodiv(const char *name, const char **parent_names,
int num_parents, struct tegra_clk_periph *periph,
void __iomem *clk_base, u32 offset, )
{
return __tegra_clk_periph(name, parent_names, num_parents, periph, clk_base, offset, 0);
}
^ permalink raw reply
* [PATCH v4 0/3] mtd: nand: OMAP: ELM error correction support for BCH ecc
From: Philip, Avinash @ 2013-01-16 12:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357286211-5012-1-git-send-email-avinashphilip@ti.com>
Hi Artem,
On Fri, Jan 04, 2013 at 13:26:48, Philip, Avinash wrote:
> Support to use ELM as BCH 4 & 8 bit error correction module. Also performance
> enhancement by adding single shot read_page and write_page functions for the
> nand flashes with page size less than 4 KB.
>
> ELM module can be used to correct errors reported by BCH 4, 8 & 16 bit
> ECC scheme. For now only 4 & 8 bit support is added.
>
> BCH 4 & 8 bit error detection support is already available in mainline
> kernel and works with software error correction.
>
> This series is based on linux 3.8-rc2 and tested with [1].
> Also this patch series depend on [1] for NAND flash device
> tree data and gpmc nand device tree binding documentation updates.
>
> 1. [PATCH v7 0/5] OMAP GPMC DT bindings
> http://www.spinics.net/lists/linux-omap/msg83505.html
>
> Tested on am335x-evm for BCH 4 and 8 bit error correction.
Can you apply this patch series on l2-mtd tree as it will help RBL compatibility
ecc layout for NAND flash in am335x-platforms and hardware based BCH error
correction.
Thanks
Avinash
>
> Changes since v1:
> - Erased page is identified by checking byte [13/7] in read
> ecc. To filter out bit flips in OOB area, check 0 bits in
> the byte greater than 4.
> - GPMC ecc engine configuration moves to omap2.c NAND driver.
>
> Changes since v2:
> - Added runtime detection of elm module, instead of depending
> on platform data.
> - Added bit flip correction in OOB ecc data if bit flip happen
> OOB data.
>
> Changes since v3:
> - Availability and usability of ELM module is detected from device
> tree nodes by checking availability of ELM node in device tree.
>
> Philip Avinash (3):
> mtd: nand: omap2: Update nerrors using ecc.strength
> mtd: devices: elm: Add support for ELM error correction
> mtd: nand: omap2: Support for hardware BCH error correction.
>
> Documentation/devicetree/bindings/mtd/elm.txt | 16 +
> .../devicetree/bindings/mtd/gpmc-nand.txt | 4 +
> drivers/mtd/devices/Makefile | 4 +-
> drivers/mtd/devices/elm.c | 405 ++++++++++++++
> drivers/mtd/nand/omap2.c | 583 ++++++++++++++++++--
> include/linux/platform_data/elm.h | 54 ++
> 6 files changed, 1023 insertions(+), 43 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mtd/elm.txt
> create mode 100644 drivers/mtd/devices/elm.c
> create mode 100644 include/linux/platform_data/elm.h
>
> --
> 1.7.9.5
>
>
^ permalink raw reply
* [PATCH v5 07/14] KVM: ARM: World-switch implementation
From: Gleb Natapov @ 2013-01-16 12:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CANM98q+qScaXniyYjrd9QEZzWKzj3PkCE0YNFUp2ijnCfS_v4w@mail.gmail.com>
On Tue, Jan 15, 2013 at 09:08:11PM -0500, Christoffer Dall wrote:
> On Tue, Jan 15, 2013 at 4:43 AM, Gleb Natapov <gleb@redhat.com> wrote:
> > On Tue, Jan 08, 2013 at 01:39:24PM -0500, Christoffer Dall wrote:
> >> Provides complete world-switch implementation to switch to other guests
> >> running in non-secure modes. Includes Hyp exception handlers that
> >> capture necessary exception information and stores the information on
> >> the VCPU and KVM structures.
> >>
> >> The following Hyp-ABI is also documented in the code:
> >>
> >> Hyp-ABI: Calling HYP-mode functions from host (in SVC mode):
> >> Switching to Hyp mode is done through a simple HVC #0 instruction. The
> >> exception vector code will check that the HVC comes from VMID==0 and if
> >> so will push the necessary state (SPSR, lr_usr) on the Hyp stack.
> >> - r0 contains a pointer to a HYP function
> >> - r1, r2, and r3 contain arguments to the above function.
> >> - The HYP function will be called with its arguments in r0, r1 and r2.
> >> On HYP function return, we return directly to SVC.
> >>
> >> A call to a function executing in Hyp mode is performed like the following:
> >>
> >> <svc code>
> >> ldr r0, =BSYM(my_hyp_fn)
> >> ldr r1, =my_param
> >> hvc #0 ; Call my_hyp_fn(my_param) from HYP mode
> >> <svc code>
> >>
> >> Otherwise, the world-switch is pretty straight-forward. All state that
> >> can be modified by the guest is first backed up on the Hyp stack and the
> >> VCPU values is loaded onto the hardware. State, which is not loaded, but
> >> theoretically modifiable by the guest is protected through the
> >> virtualiation features to generate a trap and cause software emulation.
> >> Upon guest returns, all state is restored from hardware onto the VCPU
> >> struct and the original state is restored from the Hyp-stack onto the
> >> hardware.
> >>
> >> SMP support using the VMPIDR calculated on the basis of the host MPIDR
> >> and overriding the low bits with KVM vcpu_id contributed by Marc Zyngier.
> >>
> >> Reuse of VMIDs has been implemented by Antonios Motakis and adapated from
> >> a separate patch into the appropriate patches introducing the
> >> functionality. Note that the VMIDs are stored per VM as required by the ARM
> >> architecture reference manual.
> >>
> >> To support VFP/NEON we trap those instructions using the HPCTR. When
> >> we trap, we switch the FPU. After a guest exit, the VFP state is
> >> returned to the host. When disabling access to floating point
> >> instructions, we also mask FPEXC_EN in order to avoid the guest
> >> receiving Undefined instruction exceptions before we have a chance to
> >> switch back the floating point state. We are reusing vfp_hard_struct,
> >> so we depend on VFPv3 being enabled in the host kernel, if not, we still
> >> trap cp10 and cp11 in order to inject an undefined instruction exception
> >> whenever the guest tries to use VFP/NEON. VFP/NEON developed by
> >> Antionios Motakis and Rusty Russell.
> >>
> >> Aborts that are permission faults, and not stage-1 page table walk, do
> >> not report the faulting address in the HPFAR. We have to resolve the
> >> IPA, and store it just like the HPFAR register on the VCPU struct. If
> >> the IPA cannot be resolved, it means another CPU is playing with the
> >> page tables, and we simply restart the guest. This quirk was fixed by
> >> Marc Zyngier.
> >>
> >> Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
> >> Signed-off-by: Rusty Russell <rusty.russell@linaro.org>
> >> Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
> >> ---
> >> arch/arm/include/asm/kvm_arm.h | 51 ++++
> >> arch/arm/include/asm/kvm_host.h | 10 +
> >> arch/arm/kernel/asm-offsets.c | 25 ++
> >> arch/arm/kvm/arm.c | 187 ++++++++++++++++
> >> arch/arm/kvm/interrupts.S | 396 +++++++++++++++++++++++++++++++++++
> >> arch/arm/kvm/interrupts_head.S | 443 +++++++++++++++++++++++++++++++++++++++
> >> 6 files changed, 1108 insertions(+), 4 deletions(-)
> >> create mode 100644 arch/arm/kvm/interrupts_head.S
> >>
> >> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
> >> index fb22ee8..a3262a2 100644
> >> --- a/arch/arm/include/asm/kvm_arm.h
> >> +++ b/arch/arm/include/asm/kvm_arm.h
> >> @@ -98,6 +98,18 @@
> >> #define TTBCR_T0SZ 3
> >> #define HTCR_MASK (TTBCR_T0SZ | TTBCR_IRGN0 | TTBCR_ORGN0 | TTBCR_SH0)
> >>
> >> +/* Hyp System Trap Register */
> >> +#define HSTR_T(x) (1 << x)
> >> +#define HSTR_TTEE (1 << 16)
> >> +#define HSTR_TJDBX (1 << 17)
> >> +
> >> +/* Hyp Coprocessor Trap Register */
> >> +#define HCPTR_TCP(x) (1 << x)
> >> +#define HCPTR_TCP_MASK (0x3fff)
> >> +#define HCPTR_TASE (1 << 15)
> >> +#define HCPTR_TTA (1 << 20)
> >> +#define HCPTR_TCPAC (1 << 31)
> >> +
> >> /* Hyp Debug Configuration Register bits */
> >> #define HDCR_TDRA (1 << 11)
> >> #define HDCR_TDOSA (1 << 10)
> >> @@ -144,6 +156,45 @@
> >> #else
> >> #define VTTBR_X (5 - KVM_T0SZ)
> >> #endif
> >> +#define VTTBR_BADDR_SHIFT (VTTBR_X - 1)
> >> +#define VTTBR_BADDR_MASK (((1LLU << (40 - VTTBR_X)) - 1) << VTTBR_BADDR_SHIFT)
> >> +#define VTTBR_VMID_SHIFT (48LLU)
> >> +#define VTTBR_VMID_MASK (0xffLLU << VTTBR_VMID_SHIFT)
> >> +
> >> +/* Hyp Syndrome Register (HSR) bits */
> >> +#define HSR_EC_SHIFT (26)
> >> +#define HSR_EC (0x3fU << HSR_EC_SHIFT)
> >> +#define HSR_IL (1U << 25)
> >> +#define HSR_ISS (HSR_IL - 1)
> >> +#define HSR_ISV_SHIFT (24)
> >> +#define HSR_ISV (1U << HSR_ISV_SHIFT)
> >> +#define HSR_FSC (0x3f)
> >> +#define HSR_FSC_TYPE (0x3c)
> >> +#define HSR_WNR (1 << 6)
> >> +
> >> +#define FSC_FAULT (0x04)
> >> +#define FSC_PERM (0x0c)
> >> +
> >> +/* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
> >> +#define HPFAR_MASK (~0xf)
> >>
> >> +#define HSR_EC_UNKNOWN (0x00)
> >> +#define HSR_EC_WFI (0x01)
> >> +#define HSR_EC_CP15_32 (0x03)
> >> +#define HSR_EC_CP15_64 (0x04)
> >> +#define HSR_EC_CP14_MR (0x05)
> >> +#define HSR_EC_CP14_LS (0x06)
> >> +#define HSR_EC_CP_0_13 (0x07)
> >> +#define HSR_EC_CP10_ID (0x08)
> >> +#define HSR_EC_JAZELLE (0x09)
> >> +#define HSR_EC_BXJ (0x0A)
> >> +#define HSR_EC_CP14_64 (0x0C)
> >> +#define HSR_EC_SVC_HYP (0x11)
> >> +#define HSR_EC_HVC (0x12)
> >> +#define HSR_EC_SMC (0x13)
> >> +#define HSR_EC_IABT (0x20)
> >> +#define HSR_EC_IABT_HYP (0x21)
> >> +#define HSR_EC_DABT (0x24)
> >> +#define HSR_EC_DABT_HYP (0x25)
> >>
> >> #endif /* __ARM_KVM_ARM_H__ */
> >> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> >> index 1de6f0d..ddb09da 100644
> >> --- a/arch/arm/include/asm/kvm_host.h
> >> +++ b/arch/arm/include/asm/kvm_host.h
> >> @@ -21,6 +21,7 @@
> >>
> >> #include <asm/kvm.h>
> >> #include <asm/kvm_asm.h>
> >> +#include <asm/fpstate.h>
> >>
> >> #define KVM_MAX_VCPUS CONFIG_KVM_ARM_MAX_VCPUS
> >> #define KVM_USER_MEM_SLOTS 32
> >> @@ -85,6 +86,14 @@ struct kvm_vcpu_arch {
> >> u32 hxfar; /* Hyp Data/Inst Fault Address Register */
> >> u32 hpfar; /* Hyp IPA Fault Address Register */
> >>
> >> + /* Floating point registers (VFP and Advanced SIMD/NEON) */
> >> + struct vfp_hard_struct vfp_guest;
> >> + struct vfp_hard_struct *vfp_host;
> >> +
> >> + /*
> >> + * Anything that is not used directly from assembly code goes
> >> + * here.
> >> + */
> >> /* Interrupt related fields */
> >> u32 irq_lines; /* IRQ and FIQ levels */
> >>
> >> @@ -112,6 +121,7 @@ struct kvm_one_reg;
> >> 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);
> >> u64 kvm_call_hyp(void *hypfn, ...);
> >> +void force_vm_exit(const cpumask_t *mask);
> >>
> >> #define KVM_ARCH_WANT_MMU_NOTIFIER
> >> struct kvm;
> >> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
> >> index c985b48..c8b3272 100644
> >> --- a/arch/arm/kernel/asm-offsets.c
> >> +++ b/arch/arm/kernel/asm-offsets.c
> >> @@ -13,6 +13,9 @@
> >> #include <linux/sched.h>
> >> #include <linux/mm.h>
> >> #include <linux/dma-mapping.h>
> >> +#ifdef CONFIG_KVM_ARM_HOST
> >> +#include <linux/kvm_host.h>
> >> +#endif
> >> #include <asm/cacheflush.h>
> >> #include <asm/glue-df.h>
> >> #include <asm/glue-pf.h>
> >> @@ -146,5 +149,27 @@ int main(void)
> >> DEFINE(DMA_BIDIRECTIONAL, DMA_BIDIRECTIONAL);
> >> DEFINE(DMA_TO_DEVICE, DMA_TO_DEVICE);
> >> DEFINE(DMA_FROM_DEVICE, DMA_FROM_DEVICE);
> >> +#ifdef CONFIG_KVM_ARM_HOST
> >> + DEFINE(VCPU_KVM, offsetof(struct kvm_vcpu, kvm));
> >> + DEFINE(VCPU_MIDR, offsetof(struct kvm_vcpu, arch.midr));
> >> + DEFINE(VCPU_CP15, offsetof(struct kvm_vcpu, arch.cp15));
> >> + DEFINE(VCPU_VFP_GUEST, offsetof(struct kvm_vcpu, arch.vfp_guest));
> >> + DEFINE(VCPU_VFP_HOST, offsetof(struct kvm_vcpu, arch.vfp_host));
> >> + DEFINE(VCPU_REGS, offsetof(struct kvm_vcpu, arch.regs));
> >> + DEFINE(VCPU_USR_REGS, offsetof(struct kvm_vcpu, arch.regs.usr_regs));
> >> + DEFINE(VCPU_SVC_REGS, offsetof(struct kvm_vcpu, arch.regs.svc_regs));
> >> + DEFINE(VCPU_ABT_REGS, offsetof(struct kvm_vcpu, arch.regs.abt_regs));
> >> + DEFINE(VCPU_UND_REGS, offsetof(struct kvm_vcpu, arch.regs.und_regs));
> >> + DEFINE(VCPU_IRQ_REGS, offsetof(struct kvm_vcpu, arch.regs.irq_regs));
> >> + DEFINE(VCPU_FIQ_REGS, offsetof(struct kvm_vcpu, arch.regs.fiq_regs));
> >> + DEFINE(VCPU_PC, offsetof(struct kvm_vcpu, arch.regs.usr_regs.ARM_pc));
> >> + DEFINE(VCPU_CPSR, offsetof(struct kvm_vcpu, arch.regs.usr_regs.ARM_cpsr));
> >> + DEFINE(VCPU_IRQ_LINES, offsetof(struct kvm_vcpu, arch.irq_lines));
> >> + DEFINE(VCPU_HSR, offsetof(struct kvm_vcpu, arch.hsr));
> >> + DEFINE(VCPU_HxFAR, offsetof(struct kvm_vcpu, arch.hxfar));
> >> + DEFINE(VCPU_HPFAR, offsetof(struct kvm_vcpu, arch.hpfar));
> >> + DEFINE(VCPU_HYP_PC, offsetof(struct kvm_vcpu, arch.hyp_pc));
> >> + DEFINE(KVM_VTTBR, offsetof(struct kvm, arch.vttbr));
> >> +#endif
> >> return 0;
> >> }
> >> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> >> index 9b4566e..c94d278 100644
> >> --- a/arch/arm/kvm/arm.c
> >> +++ b/arch/arm/kvm/arm.c
> >> @@ -40,6 +40,7 @@
> >> #include <asm/kvm_arm.h>
> >> #include <asm/kvm_asm.h>
> >> #include <asm/kvm_mmu.h>
> >> +#include <asm/kvm_emulate.h>
> >>
> >> #ifdef REQUIRES_VIRT
> >> __asm__(".arch_extension virt");
> >> @@ -49,6 +50,10 @@ static DEFINE_PER_CPU(unsigned long, kvm_arm_hyp_stack_page);
> >> static struct vfp_hard_struct __percpu *kvm_host_vfp_state;
> >> static unsigned long hyp_default_vectors;
> >>
> >> +/* The VMID used in the VTTBR */
> >> +static atomic64_t kvm_vmid_gen = ATOMIC64_INIT(1);
> >> +static u8 kvm_next_vmid;
> >> +static DEFINE_SPINLOCK(kvm_vmid_lock);
> >>
> >> int kvm_arch_hardware_enable(void *garbage)
> >> {
> >> @@ -276,6 +281,8 @@ int __attribute_const__ kvm_target_cpu(void)
> >>
> >> int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
> >> {
> >> + /* Force users to call KVM_ARM_VCPU_INIT */
> >> + vcpu->arch.target = -1;
> >> return 0;
> >> }
> >>
> >> @@ -286,6 +293,7 @@ void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
> >> void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> >> {
> >> vcpu->cpu = cpu;
> >> + vcpu->arch.vfp_host = this_cpu_ptr(kvm_host_vfp_state);
> >> }
> >>
> >> void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> >> @@ -318,12 +326,189 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
> >>
> >> int kvm_arch_vcpu_in_guest_mode(struct kvm_vcpu *v)
> > As far as I see the function is unused.
> >
> >> {
> >> + return v->mode == IN_GUEST_MODE;
> >> +}
> >> +
> >> +/* Just ensure a guest exit from a particular CPU */
> >> +static void exit_vm_noop(void *info)
> >> +{
> >> +}
> >> +
> >> +void force_vm_exit(const cpumask_t *mask)
> >> +{
> >> + smp_call_function_many(mask, exit_vm_noop, NULL, true);
> >> +}
> > There is make_all_cpus_request() for that. It actually sends IPIs only
> > to cpus that are running vcpus.
> >
> >> +
> >> +/**
> >> + * need_new_vmid_gen - check that the VMID is still valid
> >> + * @kvm: The VM's VMID to checkt
> >> + *
> >> + * return true if there is a new generation of VMIDs being used
> >> + *
> >> + * The hardware supports only 256 values with the value zero reserved for the
> >> + * host, so we check if an assigned value belongs to a previous generation,
> >> + * which which requires us to assign a new value. If we're the first to use a
> >> + * VMID for the new generation, we must flush necessary caches and TLBs on all
> >> + * CPUs.
> >> + */
> >> +static bool need_new_vmid_gen(struct kvm *kvm)
> >> +{
> >> + return unlikely(kvm->arch.vmid_gen != atomic64_read(&kvm_vmid_gen));
> >> +}
> >> +
> >> +/**
> >> + * update_vttbr - Update the VTTBR with a valid VMID before the guest runs
> >> + * @kvm The guest that we are about to run
> >> + *
> >> + * Called from kvm_arch_vcpu_ioctl_run before entering the guest to ensure the
> >> + * VM has a valid VMID, otherwise assigns a new one and flushes corresponding
> >> + * caches and TLBs.
> >> + */
> >> +static void update_vttbr(struct kvm *kvm)
> >> +{
> >> + phys_addr_t pgd_phys;
> >> + u64 vmid;
> >> +
> >> + if (!need_new_vmid_gen(kvm))
> >> + return;
> >> +
> >> + spin_lock(&kvm_vmid_lock);
> >> +
> >> + /*
> >> + * We need to re-check the vmid_gen here to ensure that if another vcpu
> >> + * already allocated a valid vmid for this vm, then this vcpu should
> >> + * use the same vmid.
> >> + */
> >> + if (!need_new_vmid_gen(kvm)) {
> >> + spin_unlock(&kvm_vmid_lock);
> >> + return;
> >> + }
> >> +
> >> + /* First user of a new VMID generation? */
> >> + if (unlikely(kvm_next_vmid == 0)) {
> >> + atomic64_inc(&kvm_vmid_gen);
> >> + kvm_next_vmid = 1;
> >> +
> >> + /*
> >> + * On SMP we know no other CPUs can use this CPU's or each
> >> + * other's VMID after force_vm_exit returns since the
> >> + * kvm_vmid_lock blocks them from reentry to the guest.
> >> + */
> >> + force_vm_exit(cpu_all_mask);
> >> + /*
> >> + * Now broadcast TLB + ICACHE invalidation over the inner
> >> + * shareable domain to make sure all data structures are
> >> + * clean.
> >> + */
> >> + kvm_call_hyp(__kvm_flush_vm_context);
> >> + }
> >> +
> >> + kvm->arch.vmid_gen = atomic64_read(&kvm_vmid_gen);
> >> + kvm->arch.vmid = kvm_next_vmid;
> >> + kvm_next_vmid++;
> >> +
> >> + /* update vttbr to be used with the new vmid */
> >> + pgd_phys = virt_to_phys(kvm->arch.pgd);
> >> + vmid = ((u64)(kvm->arch.vmid) << VTTBR_VMID_SHIFT) & VTTBR_VMID_MASK;
> >> + kvm->arch.vttbr = pgd_phys & VTTBR_BADDR_MASK;
> >> + kvm->arch.vttbr |= vmid;
> >> +
> >> + spin_unlock(&kvm_vmid_lock);
> >> +}
> >> +
> >> +/*
> >> + * Return > 0 to return to guest, < 0 on error, 0 (and set exit_reason) on
> >> + * proper exit to QEMU.
> >> + */
> >> +static int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
> >> + int exception_index)
> >> +{
> >> + run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
> >> return 0;
> >> }
> >>
> >> +/**
> >> + * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code
> >> + * @vcpu: The VCPU pointer
> >> + * @run: The kvm_run structure pointer used for userspace state exchange
> >> + *
> >> + * This function is called through the VCPU_RUN ioctl called from user space. It
> >> + * will execute VM code in a loop until the time slice for the process is used
> >> + * or some emulation is needed from user space in which case the function will
> >> + * return with return value 0 and with the kvm_run structure filled in with the
> >> + * required data for the requested emulation.
> >> + */
> >> int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
> >> {
> >> - return -EINVAL;
> >> + int ret;
> >> + sigset_t sigsaved;
> >> +
> >> + /* Make sure they initialize the vcpu with KVM_ARM_VCPU_INIT */
> >> + if (unlikely(vcpu->arch.target < 0))
> >> + return -ENOEXEC;
> >> +
> >> + if (vcpu->sigset_active)
> >> + sigprocmask(SIG_SETMASK, &vcpu->sigset, &sigsaved);
> >> +
> >> + ret = 1;
> >> + run->exit_reason = KVM_EXIT_UNKNOWN;
> >> + while (ret > 0) {
> >> + /*
> >> + * Check conditions before entering the guest
> >> + */
> >> + cond_resched();
> >> +
> >> + update_vttbr(vcpu->kvm);
> >> +
> >> + local_irq_disable();
> >> +
> >> + /*
> >> + * Re-check atomic conditions
> >> + */
> >> + if (signal_pending(current)) {
> >> + ret = -EINTR;
> >> + run->exit_reason = KVM_EXIT_INTR;
> >> + }
> >> +
> >> + if (ret <= 0 || need_new_vmid_gen(vcpu->kvm)) {
> >> + local_irq_enable();
> >> + continue;
> >> + }
> >> +
> >> + /**************************************************************
> >> + * Enter the guest
> >> + */
> >> + trace_kvm_entry(*vcpu_pc(vcpu));
> >> + kvm_guest_enter();
> >> + vcpu->mode = IN_GUEST_MODE;
> > You need to set mode to IN_GUEST_MODE before disabling interrupt and
> > check that mode != EXITING_GUEST_MODE after disabling interrupt but
> > before entering the guest. This way you will catch kicks that were sent
> > between setting of the mode and disabling the interrupts. Also you need
> > to check vcpu->requests and exit if it is not empty. I see that you do
> > not use vcpu->requests at all, but you should since common kvm code
> > assumes that it is used. make_all_cpus_request() uses it for instance.
> >
>
> I don't quite agree, but almost:
>
> Why would you set IN_GUEST_MODE before disabling interrupts? The only
> reason I can see for to be a requirement is to leverage an implicit
> memory barrier. Receiving the IPI in this little window does nothing
> (the smp_cross_call is a noop).
>
> Checking that mode != EXITING_GUEST_MODE is equally useless in my
> opinion, as I read the requests code the only reason for this mode is
> to avoid sending an IPI twice.
>
> Kicks sent between setting the mode and disabling the interrupts is
> not the point, the point is to check the requests field (which we
> don't use at all on ARM, and generic code also doesn't use on ARM)
> after disabling interrupts, and after setting IN_GUEST_MODE.
>
Yes, you are right. There is not race here.
> The patch below fixes your issues, and while I would push back on
> anything else than direct bug fixes at this point, the current code is
> semantically incorrect wrt. KVM vcpu requests, so it's worth a fix,
> and the patch itself is trivial.
>
> >> +
> >> + ret = kvm_call_hyp(__kvm_vcpu_run, vcpu);
> > You do not take kvm->srcu lock before entering the guest. It looks
> > wrong.
> >
>
> why would I take that before entering the guest? The only thing the
Right. No need to take it before entering a guest of course. I should
have said "after". Anyway absents of srcu handling raced my suspicion
and made me check all the code for kvm->srcu handling. I expected to see
kvm->srcu handling in vcpu_run() because x86 locks srcu at the beginning
of vcpu loop, release it before guest entry and retakes it after exit.
This way all the code called from vcpu run loop is protected. This is of
course not the only way to tackle it and you can do locking only around
memslot use.
> read side RCU protects against is the memslots data structure as far
> as I can see, so the second patch pasted below fixes this for the code
> that actually accesses this data structure.
Many memory related functions that you call access memslots under the
hood and assume that locking is done by the caller. From the quick look
I found those that you've missed:
kvm_is_visible_gfn()
kvm_read_guest()
gfn_to_hva()
gfn_to_pfn_prot()
kvm_memslots()
May be there are more. Can you enable RCU debugging in your kernel config
and check? This does not guaranty that it will catch all of the places,
but better than nothing.
>
> >> +
> >> + vcpu->mode = OUTSIDE_GUEST_MODE;
> >> + kvm_guest_exit();
> >> + trace_kvm_exit(*vcpu_pc(vcpu));
> >> + /*
> >> + * We may have taken a host interrupt in HYP mode (ie
> >> + * while executing the guest). This interrupt is still
> >> + * pending, as we haven't serviced it yet!
> >> + *
> >> + * We're now back in SVC mode, with interrupts
> >> + * disabled. Enabling the interrupts now will have
> >> + * the effect of taking the interrupt again, in SVC
> >> + * mode this time.
> >> + */
> >> + local_irq_enable();
> >> +
> >> + /*
> >> + * Back from guest
> >> + *************************************************************/
> >> +
> >> + ret = handle_exit(vcpu, run, ret);
> >> + }
> >> +
> >> + if (vcpu->sigset_active)
> >> + sigprocmask(SIG_SETMASK, &sigsaved, NULL);
> >> + return ret;
> >> }
> >>
> >> static int vcpu_interrupt_line(struct kvm_vcpu *vcpu, int number, bool level)
> >> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
> >> index a923590..08adcd5 100644
> >> --- a/arch/arm/kvm/interrupts.S
> >> +++ b/arch/arm/kvm/interrupts.S
> >> @@ -20,9 +20,12 @@
> >> #include <linux/const.h>
> >> #include <asm/unified.h>
> >> #include <asm/page.h>
> >> +#include <asm/ptrace.h>
> >> #include <asm/asm-offsets.h>
> >> #include <asm/kvm_asm.h>
> >> #include <asm/kvm_arm.h>
> >> +#include <asm/vfpmacros.h>
> >> +#include "interrupts_head.S"
> >>
> >> .text
> >>
> >> @@ -31,36 +34,423 @@ __kvm_hyp_code_start:
> >>
> >> /********************************************************************
> >> * Flush per-VMID TLBs
> >> + *
> >> + * void __kvm_tlb_flush_vmid(struct kvm *kvm);
> >> + *
> >> + * We rely on the hardware to broadcast the TLB invalidation to all CPUs
> >> + * inside the inner-shareable domain (which is the case for all v7
> >> + * implementations). If we come across a non-IS SMP implementation, we'll
> >> + * have to use an IPI based mechanism. Until then, we stick to the simple
> >> + * hardware assisted version.
> >> */
> >> ENTRY(__kvm_tlb_flush_vmid)
> >> + push {r2, r3}
> >> +
> >> + add r0, r0, #KVM_VTTBR
> >> + ldrd r2, r3, [r0]
> >> + mcrr p15, 6, r2, r3, c2 @ Write VTTBR
> >> + isb
> >> + mcr p15, 0, r0, c8, c3, 0 @ TLBIALLIS (rt ignored)
> >> + dsb
> >> + isb
> >> + mov r2, #0
> >> + mov r3, #0
> >> + mcrr p15, 6, r2, r3, c2 @ Back to VMID #0
> >> + isb @ Not necessary if followed by eret
> >> +
> >> + pop {r2, r3}
> >> bx lr
> >> ENDPROC(__kvm_tlb_flush_vmid)
> >>
> >> /********************************************************************
> >> - * Flush TLBs and instruction caches of current CPU for all VMIDs
> >> + * Flush TLBs and instruction caches of all CPUs inside the inner-shareable
> >> + * domain, for all VMIDs
> >> + *
> >> + * void __kvm_flush_vm_context(void);
> >> */
> >> ENTRY(__kvm_flush_vm_context)
> >> + mov r0, #0 @ rn parameter for c15 flushes is SBZ
> >> +
> >> + /* Invalidate NS Non-Hyp TLB Inner Shareable (TLBIALLNSNHIS) */
> >> + mcr p15, 4, r0, c8, c3, 4
> >> + /* Invalidate instruction caches Inner Shareable (ICIALLUIS) */
> >> + mcr p15, 0, r0, c7, c1, 0
> >> + dsb
> >> + isb @ Not necessary if followed by eret
> >> +
> >> bx lr
> >> ENDPROC(__kvm_flush_vm_context)
> >>
> >> +
> >> /********************************************************************
> >> * Hypervisor world-switch code
> >> + *
> >> + *
> >> + * int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
> >> */
> >> ENTRY(__kvm_vcpu_run)
> >> - bx lr
> >> + @ Save the vcpu pointer
> >> + mcr p15, 4, vcpu, c13, c0, 2 @ HTPIDR
> >> +
> >> + save_host_regs
> >> +
> >> + @ Store hardware CP15 state and load guest state
> >> + read_cp15_state store_to_vcpu = 0
> >> + write_cp15_state read_from_vcpu = 1
> >> +
> >> + @ If the host kernel has not been configured with VFPv3 support,
> >> + @ then it is safer if we deny guests from using it as well.
> >> +#ifdef CONFIG_VFPv3
> >> + @ Set FPEXC_EN so the guest doesn't trap floating point instructions
> >> + VFPFMRX r2, FPEXC @ VMRS
> >> + push {r2}
> >> + orr r2, r2, #FPEXC_EN
> >> + VFPFMXR FPEXC, r2 @ VMSR
> >> +#endif
> >> +
> >> + @ Configure Hyp-role
> >> + configure_hyp_role vmentry
> >> +
> >> + @ Trap coprocessor CRx accesses
> >> + set_hstr vmentry
> >> + set_hcptr vmentry, (HCPTR_TTA | HCPTR_TCP(10) | HCPTR_TCP(11))
> >> + set_hdcr vmentry
> >> +
> >> + @ Write configured ID register into MIDR alias
> >> + ldr r1, [vcpu, #VCPU_MIDR]
> >> + mcr p15, 4, r1, c0, c0, 0
> >> +
> >> + @ Write guest view of MPIDR into VMPIDR
> >> + ldr r1, [vcpu, #CP15_OFFSET(c0_MPIDR)]
> >> + mcr p15, 4, r1, c0, c0, 5
> >> +
> >> + @ Set up guest memory translation
> >> + ldr r1, [vcpu, #VCPU_KVM]
> >> + add r1, r1, #KVM_VTTBR
> >> + ldrd r2, r3, [r1]
> >> + mcrr p15, 6, r2, r3, c2 @ Write VTTBR
> >> +
> >> + @ We're all done, just restore the GPRs and go to the guest
> >> + restore_guest_regs
> >> + clrex @ Clear exclusive monitor
> >> + eret
> >> +
> >> +__kvm_vcpu_return:
> >> + /*
> >> + * return convention:
> >> + * guest r0, r1, r2 saved on the stack
> >> + * r0: vcpu pointer
> >> + * r1: exception code
> >> + */
> >> + save_guest_regs
> >> +
> >> + @ Set VMID == 0
> >> + mov r2, #0
> >> + mov r3, #0
> >> + mcrr p15, 6, r2, r3, c2 @ Write VTTBR
> >> +
> >> + @ Don't trap coprocessor accesses for host kernel
> >> + set_hstr vmexit
> >> + set_hdcr vmexit
> >> + set_hcptr vmexit, (HCPTR_TTA | HCPTR_TCP(10) | HCPTR_TCP(11))
> >> +
> >> +#ifdef CONFIG_VFPv3
> >> + @ Save floating point registers we if let guest use them.
> >> + tst r2, #(HCPTR_TCP(10) | HCPTR_TCP(11))
> >> + bne after_vfp_restore
> >> +
> >> + @ Switch VFP/NEON hardware state to the host's
> >> + add r7, vcpu, #VCPU_VFP_GUEST
> >> + store_vfp_state r7
> >> + add r7, vcpu, #VCPU_VFP_HOST
> >> + ldr r7, [r7]
> >> + restore_vfp_state r7
> >> +
> >> +after_vfp_restore:
> >> + @ Restore FPEXC_EN which we clobbered on entry
> >> + pop {r2}
> >> + VFPFMXR FPEXC, r2
> >> +#endif
> >> +
> >> + @ Reset Hyp-role
> >> + configure_hyp_role vmexit
> >> +
> >> + @ Let host read hardware MIDR
> >> + mrc p15, 0, r2, c0, c0, 0
> >> + mcr p15, 4, r2, c0, c0, 0
> >> +
> >> + @ Back to hardware MPIDR
> >> + mrc p15, 0, r2, c0, c0, 5
> >> + mcr p15, 4, r2, c0, c0, 5
> >> +
> >> + @ Store guest CP15 state and restore host state
> >> + read_cp15_state store_to_vcpu = 1
> >> + write_cp15_state read_from_vcpu = 0
> >> +
> >> + restore_host_regs
> >> + clrex @ Clear exclusive monitor
> >> + mov r0, r1 @ Return the return code
> >> + bx lr @ return to IOCTL
> >>
> >> ENTRY(kvm_call_hyp)
> >> + hvc #0
> >> bx lr
> >>
> >>
> >> /********************************************************************
> >> * Hypervisor exception vector and handlers
> >> + *
> >> + *
> >> + * The KVM/ARM Hypervisor ABI is defined as follows:
> >> + *
> >> + * Entry to Hyp mode from the host kernel will happen _only_ when an HVC
> >> + * instruction is issued since all traps are disabled when running the host
> >> + * kernel as per the Hyp-mode initialization at boot time.
> >> + *
> >> + * HVC instructions cause a trap to the vector page + offset 0x18 (see hyp_hvc
> >> + * below) when the HVC instruction is called from SVC mode (i.e. a guest or the
> >> + * host kernel) and they cause a trap to the vector page + offset 0xc when HVC
> >> + * instructions are called from within Hyp-mode.
> >> + *
> >> + * Hyp-ABI: Calling HYP-mode functions from host (in SVC mode):
> >> + * Switching to Hyp mode is done through a simple HVC #0 instruction. The
> >> + * exception vector code will check that the HVC comes from VMID==0 and if
> >> + * so will push the necessary state (SPSR, lr_usr) on the Hyp stack.
> >> + * - r0 contains a pointer to a HYP function
> >> + * - r1, r2, and r3 contain arguments to the above function.
> >> + * - The HYP function will be called with its arguments in r0, r1 and r2.
> >> + * On HYP function return, we return directly to SVC.
> >> + *
> >> + * Note that the above is used to execute code in Hyp-mode from a host-kernel
> >> + * point of view, and is a different concept from performing a world-switch and
> >> + * executing guest code SVC mode (with a VMID != 0).
> >> */
> >>
> >> +/* Handle undef, svc, pabt, or dabt by crashing with a user notice */
> >> +.macro bad_exception exception_code, panic_str
> >> + push {r0-r2}
> >> + mrrc p15, 6, r0, r1, c2 @ Read VTTBR
> >> + lsr r1, r1, #16
> >> + ands r1, r1, #0xff
> >> + beq 99f
> >> +
> >> + load_vcpu @ Load VCPU pointer
> >> + .if \exception_code == ARM_EXCEPTION_DATA_ABORT
> >> + mrc p15, 4, r2, c5, c2, 0 @ HSR
> >> + mrc p15, 4, r1, c6, c0, 0 @ HDFAR
> >> + str r2, [vcpu, #VCPU_HSR]
> >> + str r1, [vcpu, #VCPU_HxFAR]
> >> + .endif
> >> + .if \exception_code == ARM_EXCEPTION_PREF_ABORT
> >> + mrc p15, 4, r2, c5, c2, 0 @ HSR
> >> + mrc p15, 4, r1, c6, c0, 2 @ HIFAR
> >> + str r2, [vcpu, #VCPU_HSR]
> >> + str r1, [vcpu, #VCPU_HxFAR]
> >> + .endif
> >> + mov r1, #\exception_code
> >> + b __kvm_vcpu_return
> >> +
> >> + @ We were in the host already. Let's craft a panic-ing return to SVC.
> >> +99: mrs r2, cpsr
> >> + bic r2, r2, #MODE_MASK
> >> + orr r2, r2, #SVC_MODE
> >> +THUMB( orr r2, r2, #PSR_T_BIT )
> >> + msr spsr_cxsf, r2
> >> + mrs r1, ELR_hyp
> >> + ldr r2, =BSYM(panic)
> >> + msr ELR_hyp, r2
> >> + ldr r0, =\panic_str
> >> + eret
> >> +.endm
> >> +
> >> + .text
> >> +
> >> .align 5
> >> __kvm_hyp_vector:
> >> .globl __kvm_hyp_vector
> >> - nop
> >> +
> >> + @ Hyp-mode exception vector
> >> + W(b) hyp_reset
> >> + W(b) hyp_undef
> >> + W(b) hyp_svc
> >> + W(b) hyp_pabt
> >> + W(b) hyp_dabt
> >> + W(b) hyp_hvc
> >> + W(b) hyp_irq
> >> + W(b) hyp_fiq
> >> +
> >> + .align
> >> +hyp_reset:
> >> + b hyp_reset
> >> +
> >> + .align
> >> +hyp_undef:
> >> + bad_exception ARM_EXCEPTION_UNDEFINED, und_die_str
> >> +
> >> + .align
> >> +hyp_svc:
> >> + bad_exception ARM_EXCEPTION_HVC, svc_die_str
> >> +
> >> + .align
> >> +hyp_pabt:
> >> + bad_exception ARM_EXCEPTION_PREF_ABORT, pabt_die_str
> >> +
> >> + .align
> >> +hyp_dabt:
> >> + bad_exception ARM_EXCEPTION_DATA_ABORT, dabt_die_str
> >> +
> >> + .align
> >> +hyp_hvc:
> >> + /*
> >> + * Getting here is either becuase of a trap from a guest or from calling
> >> + * HVC from the host kernel, which means "switch to Hyp mode".
> >> + */
> >> + push {r0, r1, r2}
> >> +
> >> + @ Check syndrome register
> >> + mrc p15, 4, r1, c5, c2, 0 @ HSR
> >> + lsr r0, r1, #HSR_EC_SHIFT
> >> +#ifdef CONFIG_VFPv3
> >> + cmp r0, #HSR_EC_CP_0_13
> >> + beq switch_to_guest_vfp
> >> +#endif
> >> + cmp r0, #HSR_EC_HVC
> >> + bne guest_trap @ Not HVC instr.
> >> +
> >> + /*
> >> + * Let's check if the HVC came from VMID 0 and allow simple
> >> + * switch to Hyp mode
> >> + */
> >> + mrrc p15, 6, r0, r2, c2
> >> + lsr r2, r2, #16
> >> + and r2, r2, #0xff
> >> + cmp r2, #0
> >> + bne guest_trap @ Guest called HVC
> >> +
> >> +host_switch_to_hyp:
> >> + pop {r0, r1, r2}
> >> +
> >> + push {lr}
> >> + mrs lr, SPSR
> >> + push {lr}
> >> +
> >> + mov lr, r0
> >> + mov r0, r1
> >> + mov r1, r2
> >> + mov r2, r3
> >> +
> >> +THUMB( orr lr, #1)
> >> + blx lr @ Call the HYP function
> >> +
> >> + pop {lr}
> >> + msr SPSR_csxf, lr
> >> + pop {lr}
> >> + eret
> >> +
> >> +guest_trap:
> >> + load_vcpu @ Load VCPU pointer to r0
> >> + str r1, [vcpu, #VCPU_HSR]
> >> +
> >> + @ Check if we need the fault information
> >> + lsr r1, r1, #HSR_EC_SHIFT
> >> + cmp r1, #HSR_EC_IABT
> >> + mrceq p15, 4, r2, c6, c0, 2 @ HIFAR
> >> + beq 2f
> >> + cmp r1, #HSR_EC_DABT
> >> + bne 1f
> >> + mrc p15, 4, r2, c6, c0, 0 @ HDFAR
> >> +
> >> +2: str r2, [vcpu, #VCPU_HxFAR]
> >> +
> >> + /*
> >> + * B3.13.5 Reporting exceptions taken to the Non-secure PL2 mode:
> >> + *
> >> + * Abort on the stage 2 translation for a memory access from a
> >> + * Non-secure PL1 or PL0 mode:
> >> + *
> >> + * For any Access flag fault or Translation fault, and also for any
> >> + * Permission fault on the stage 2 translation of a memory access
> >> + * made as part of a translation table walk for a stage 1 translation,
> >> + * the HPFAR holds the IPA that caused the fault. Otherwise, the HPFAR
> >> + * is UNKNOWN.
> >> + */
> >> +
> >> + /* Check for permission fault, and S1PTW */
> >> + mrc p15, 4, r1, c5, c2, 0 @ HSR
> >> + and r0, r1, #HSR_FSC_TYPE
> >> + cmp r0, #FSC_PERM
> >> + tsteq r1, #(1 << 7) @ S1PTW
> >> + mrcne p15, 4, r2, c6, c0, 4 @ HPFAR
> >> + bne 3f
> >> +
> >> + /* Resolve IPA using the xFAR */
> >> + mcr p15, 0, r2, c7, c8, 0 @ ATS1CPR
> >> + isb
> >> + mrrc p15, 0, r0, r1, c7 @ PAR
> >> + tst r0, #1
> >> + bne 4f @ Failed translation
> >> + ubfx r2, r0, #12, #20
> >> + lsl r2, r2, #4
> >> + orr r2, r2, r1, lsl #24
> >> +
> >> +3: load_vcpu @ Load VCPU pointer to r0
> >> + str r2, [r0, #VCPU_HPFAR]
> >> +
> >> +1: mov r1, #ARM_EXCEPTION_HVC
> >> + b __kvm_vcpu_return
> >> +
> >> +4: pop {r0, r1, r2} @ Failed translation, return to guest
> >> + eret
> >> +
> >> +/*
> >> + * If VFPv3 support is not available, then we will not switch the VFP
> >> + * registers; however cp10 and cp11 accesses will still trap and fallback
> >> + * to the regular coprocessor emulation code, which currently will
> >> + * inject an undefined exception to the guest.
> >> + */
> >> +#ifdef CONFIG_VFPv3
> >> +switch_to_guest_vfp:
> >> + load_vcpu @ Load VCPU pointer to r0
> >> + push {r3-r7}
> >> +
> >> + @ NEON/VFP used. Turn on VFP access.
> >> + set_hcptr vmexit, (HCPTR_TCP(10) | HCPTR_TCP(11))
> >> +
> >> + @ Switch VFP/NEON hardware state to the guest's
> >> + add r7, r0, #VCPU_VFP_HOST
> >> + ldr r7, [r7]
> >> + store_vfp_state r7
> >> + add r7, r0, #VCPU_VFP_GUEST
> >> + restore_vfp_state r7
> >> +
> >> + pop {r3-r7}
> >> + pop {r0-r2}
> >> + eret
> >> +#endif
> >> +
> >> + .align
> >> +hyp_irq:
> >> + push {r0, r1, r2}
> >> + mov r1, #ARM_EXCEPTION_IRQ
> >> + load_vcpu @ Load VCPU pointer to r0
> >> + b __kvm_vcpu_return
> >> +
> >> + .align
> >> +hyp_fiq:
> >> + b hyp_fiq
> >> +
> >> + .ltorg
> >>
> >> __kvm_hyp_code_end:
> >> .globl __kvm_hyp_code_end
> >> +
> >> + .section ".rodata"
> >> +
> >> +und_die_str:
> >> + .ascii "unexpected undefined exception in Hyp mode at: %#08x"
> >> +pabt_die_str:
> >> + .ascii "unexpected prefetch abort in Hyp mode at: %#08x"
> >> +dabt_die_str:
> >> + .ascii "unexpected data abort in Hyp mode at: %#08x"
> >> +svc_die_str:
> >> + .ascii "unexpected HVC/SVC trap in Hyp mode at: %#08x"
> >> diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
> >> new file mode 100644
> >> index 0000000..f59a580
> >> --- /dev/null
> >> +++ b/arch/arm/kvm/interrupts_head.S
> >> @@ -0,0 +1,443 @@
> >> +#define VCPU_USR_REG(_reg_nr) (VCPU_USR_REGS + (_reg_nr * 4))
> >> +#define VCPU_USR_SP (VCPU_USR_REG(13))
> >> +#define VCPU_USR_LR (VCPU_USR_REG(14))
> >> +#define CP15_OFFSET(_cp15_reg_idx) (VCPU_CP15 + (_cp15_reg_idx * 4))
> >> +
> >> +/*
> >> + * Many of these macros need to access the VCPU structure, which is always
> >> + * held in r0. These macros should never clobber r1, as it is used to hold the
> >> + * exception code on the return path (except of course the macro that switches
> >> + * all the registers before the final jump to the VM).
> >> + */
> >> +vcpu .req r0 @ vcpu pointer always in r0
> >> +
> >> +/* Clobbers {r2-r6} */
> >> +.macro store_vfp_state vfp_base
> >> + @ The VFPFMRX and VFPFMXR macros are the VMRS and VMSR instructions
> >> + VFPFMRX r2, FPEXC
> >> + @ Make sure VFP is enabled so we can touch the registers.
> >> + orr r6, r2, #FPEXC_EN
> >> + VFPFMXR FPEXC, r6
> >> +
> >> + VFPFMRX r3, FPSCR
> >> + tst r2, #FPEXC_EX @ Check for VFP Subarchitecture
> >> + beq 1f
> >> + @ If FPEXC_EX is 0, then FPINST/FPINST2 reads are upredictable, so
> >> + @ we only need to save them if FPEXC_EX is set.
> >> + VFPFMRX r4, FPINST
> >> + tst r2, #FPEXC_FP2V
> >> + VFPFMRX r5, FPINST2, ne @ vmrsne
> >> + bic r6, r2, #FPEXC_EX @ FPEXC_EX disable
> >> + VFPFMXR FPEXC, r6
> >> +1:
> >> + VFPFSTMIA \vfp_base, r6 @ Save VFP registers
> >> + stm \vfp_base, {r2-r5} @ Save FPEXC, FPSCR, FPINST, FPINST2
> >> +.endm
> >> +
> >> +/* Assume FPEXC_EN is on and FPEXC_EX is off, clobbers {r2-r6} */
> >> +.macro restore_vfp_state vfp_base
> >> + VFPFLDMIA \vfp_base, r6 @ Load VFP registers
> >> + ldm \vfp_base, {r2-r5} @ Load FPEXC, FPSCR, FPINST, FPINST2
> >> +
> >> + VFPFMXR FPSCR, r3
> >> + tst r2, #FPEXC_EX @ Check for VFP Subarchitecture
> >> + beq 1f
> >> + VFPFMXR FPINST, r4
> >> + tst r2, #FPEXC_FP2V
> >> + VFPFMXR FPINST2, r5, ne
> >> +1:
> >> + VFPFMXR FPEXC, r2 @ FPEXC (last, in case !EN)
> >> +.endm
> >> +
> >> +/* These are simply for the macros to work - value don't have meaning */
> >> +.equ usr, 0
> >> +.equ svc, 1
> >> +.equ abt, 2
> >> +.equ und, 3
> >> +.equ irq, 4
> >> +.equ fiq, 5
> >> +
> >> +.macro push_host_regs_mode mode
> >> + mrs r2, SP_\mode
> >> + mrs r3, LR_\mode
> >> + mrs r4, SPSR_\mode
> >> + push {r2, r3, r4}
> >> +.endm
> >> +
> >> +/*
> >> + * Store all host persistent registers on the stack.
> >> + * Clobbers all registers, in all modes, except r0 and r1.
> >> + */
> >> +.macro save_host_regs
> >> + /* Hyp regs. Only ELR_hyp (SPSR_hyp already saved) */
> >> + mrs r2, ELR_hyp
> >> + push {r2}
> >> +
> >> + /* usr regs */
> >> + push {r4-r12} @ r0-r3 are always clobbered
> >> + mrs r2, SP_usr
> >> + mov r3, lr
> >> + push {r2, r3}
> >> +
> >> + push_host_regs_mode svc
> >> + push_host_regs_mode abt
> >> + push_host_regs_mode und
> >> + push_host_regs_mode irq
> >> +
> >> + /* fiq regs */
> >> + mrs r2, r8_fiq
> >> + mrs r3, r9_fiq
> >> + mrs r4, r10_fiq
> >> + mrs r5, r11_fiq
> >> + mrs r6, r12_fiq
> >> + mrs r7, SP_fiq
> >> + mrs r8, LR_fiq
> >> + mrs r9, SPSR_fiq
> >> + push {r2-r9}
> >> +.endm
> >> +
> >> +.macro pop_host_regs_mode mode
> >> + pop {r2, r3, r4}
> >> + msr SP_\mode, r2
> >> + msr LR_\mode, r3
> >> + msr SPSR_\mode, r4
> >> +.endm
> >> +
> >> +/*
> >> + * Restore all host registers from the stack.
> >> + * Clobbers all registers, in all modes, except r0 and r1.
> >> + */
> >> +.macro restore_host_regs
> >> + pop {r2-r9}
> >> + msr r8_fiq, r2
> >> + msr r9_fiq, r3
> >> + msr r10_fiq, r4
> >> + msr r11_fiq, r5
> >> + msr r12_fiq, r6
> >> + msr SP_fiq, r7
> >> + msr LR_fiq, r8
> >> + msr SPSR_fiq, r9
> >> +
> >> + pop_host_regs_mode irq
> >> + pop_host_regs_mode und
> >> + pop_host_regs_mode abt
> >> + pop_host_regs_mode svc
> >> +
> >> + pop {r2, r3}
> >> + msr SP_usr, r2
> >> + mov lr, r3
> >> + pop {r4-r12}
> >> +
> >> + pop {r2}
> >> + msr ELR_hyp, r2
> >> +.endm
> >> +
> >> +/*
> >> + * Restore SP, LR and SPSR for a given mode. offset is the offset of
> >> + * this mode's registers from the VCPU base.
> >> + *
> >> + * Assumes vcpu pointer in vcpu reg
> >> + *
> >> + * Clobbers r1, r2, r3, r4.
> >> + */
> >> +.macro restore_guest_regs_mode mode, offset
> >> + add r1, vcpu, \offset
> >> + ldm r1, {r2, r3, r4}
> >> + msr SP_\mode, r2
> >> + msr LR_\mode, r3
> >> + msr SPSR_\mode, r4
> >> +.endm
> >> +
> >> +/*
> >> + * Restore all guest registers from the vcpu struct.
> >> + *
> >> + * Assumes vcpu pointer in vcpu reg
> >> + *
> >> + * Clobbers *all* registers.
> >> + */
> >> +.macro restore_guest_regs
> >> + restore_guest_regs_mode svc, #VCPU_SVC_REGS
> >> + restore_guest_regs_mode abt, #VCPU_ABT_REGS
> >> + restore_guest_regs_mode und, #VCPU_UND_REGS
> >> + restore_guest_regs_mode irq, #VCPU_IRQ_REGS
> >> +
> >> + add r1, vcpu, #VCPU_FIQ_REGS
> >> + ldm r1, {r2-r9}
> >> + msr r8_fiq, r2
> >> + msr r9_fiq, r3
> >> + msr r10_fiq, r4
> >> + msr r11_fiq, r5
> >> + msr r12_fiq, r6
> >> + msr SP_fiq, r7
> >> + msr LR_fiq, r8
> >> + msr SPSR_fiq, r9
> >> +
> >> + @ Load return state
> >> + ldr r2, [vcpu, #VCPU_PC]
> >> + ldr r3, [vcpu, #VCPU_CPSR]
> >> + msr ELR_hyp, r2
> >> + msr SPSR_cxsf, r3
> >> +
> >> + @ Load user registers
> >> + ldr r2, [vcpu, #VCPU_USR_SP]
> >> + ldr r3, [vcpu, #VCPU_USR_LR]
> >> + msr SP_usr, r2
> >> + mov lr, r3
> >> + add vcpu, vcpu, #(VCPU_USR_REGS)
> >> + ldm vcpu, {r0-r12}
> >> +.endm
> >> +
> >> +/*
> >> + * Save SP, LR and SPSR for a given mode. offset is the offset of
> >> + * this mode's registers from the VCPU base.
> >> + *
> >> + * Assumes vcpu pointer in vcpu reg
> >> + *
> >> + * Clobbers r2, r3, r4, r5.
> >> + */
> >> +.macro save_guest_regs_mode mode, offset
> >> + add r2, vcpu, \offset
> >> + mrs r3, SP_\mode
> >> + mrs r4, LR_\mode
> >> + mrs r5, SPSR_\mode
> >> + stm r2, {r3, r4, r5}
> >> +.endm
> >> +
> >> +/*
> >> + * Save all guest registers to the vcpu struct
> >> + * Expects guest's r0, r1, r2 on the stack.
> >> + *
> >> + * Assumes vcpu pointer in vcpu reg
> >> + *
> >> + * Clobbers r2, r3, r4, r5.
> >> + */
> >> +.macro save_guest_regs
> >> + @ Store usr registers
> >> + add r2, vcpu, #VCPU_USR_REG(3)
> >> + stm r2, {r3-r12}
> >> + add r2, vcpu, #VCPU_USR_REG(0)
> >> + pop {r3, r4, r5} @ r0, r1, r2
> >> + stm r2, {r3, r4, r5}
> >> + mrs r2, SP_usr
> >> + mov r3, lr
> >> + str r2, [vcpu, #VCPU_USR_SP]
> >> + str r3, [vcpu, #VCPU_USR_LR]
> >> +
> >> + @ Store return state
> >> + mrs r2, ELR_hyp
> >> + mrs r3, spsr
> >> + str r2, [vcpu, #VCPU_PC]
> >> + str r3, [vcpu, #VCPU_CPSR]
> >> +
> >> + @ Store other guest registers
> >> + save_guest_regs_mode svc, #VCPU_SVC_REGS
> >> + save_guest_regs_mode abt, #VCPU_ABT_REGS
> >> + save_guest_regs_mode und, #VCPU_UND_REGS
> >> + save_guest_regs_mode irq, #VCPU_IRQ_REGS
> >> +.endm
> >> +
> >> +/* Reads cp15 registers from hardware and stores them in memory
> >> + * @store_to_vcpu: If 0, registers are written in-order to the stack,
> >> + * otherwise to the VCPU struct pointed to by vcpup
> >> + *
> >> + * Assumes vcpu pointer in vcpu reg
> >> + *
> >> + * Clobbers r2 - r12
> >> + */
> >> +.macro read_cp15_state store_to_vcpu
> >> + mrc p15, 0, r2, c1, c0, 0 @ SCTLR
> >> + mrc p15, 0, r3, c1, c0, 2 @ CPACR
> >> + mrc p15, 0, r4, c2, c0, 2 @ TTBCR
> >> + mrc p15, 0, r5, c3, c0, 0 @ DACR
> >> + mrrc p15, 0, r6, r7, c2 @ TTBR 0
> >> + mrrc p15, 1, r8, r9, c2 @ TTBR 1
> >> + mrc p15, 0, r10, c10, c2, 0 @ PRRR
> >> + mrc p15, 0, r11, c10, c2, 1 @ NMRR
> >> + mrc p15, 2, r12, c0, c0, 0 @ CSSELR
> >> +
> >> + .if \store_to_vcpu == 0
> >> + push {r2-r12} @ Push CP15 registers
> >> + .else
> >> + str r2, [vcpu, #CP15_OFFSET(c1_SCTLR)]
> >> + str r3, [vcpu, #CP15_OFFSET(c1_CPACR)]
> >> + str r4, [vcpu, #CP15_OFFSET(c2_TTBCR)]
> >> + str r5, [vcpu, #CP15_OFFSET(c3_DACR)]
> >> + add vcpu, vcpu, #CP15_OFFSET(c2_TTBR0)
> >> + strd r6, r7, [vcpu]
> >> + add vcpu, vcpu, #CP15_OFFSET(c2_TTBR1) - CP15_OFFSET(c2_TTBR0)
> >> + strd r8, r9, [vcpu]
> >> + sub vcpu, vcpu, #CP15_OFFSET(c2_TTBR1)
> >> + str r10, [vcpu, #CP15_OFFSET(c10_PRRR)]
> >> + str r11, [vcpu, #CP15_OFFSET(c10_NMRR)]
> >> + str r12, [vcpu, #CP15_OFFSET(c0_CSSELR)]
> >> + .endif
> >> +
> >> + mrc p15, 0, r2, c13, c0, 1 @ CID
> >> + mrc p15, 0, r3, c13, c0, 2 @ TID_URW
> >> + mrc p15, 0, r4, c13, c0, 3 @ TID_URO
> >> + mrc p15, 0, r5, c13, c0, 4 @ TID_PRIV
> >> + mrc p15, 0, r6, c5, c0, 0 @ DFSR
> >> + mrc p15, 0, r7, c5, c0, 1 @ IFSR
> >> + mrc p15, 0, r8, c5, c1, 0 @ ADFSR
> >> + mrc p15, 0, r9, c5, c1, 1 @ AIFSR
> >> + mrc p15, 0, r10, c6, c0, 0 @ DFAR
> >> + mrc p15, 0, r11, c6, c0, 2 @ IFAR
> >> + mrc p15, 0, r12, c12, c0, 0 @ VBAR
> >> +
> >> + .if \store_to_vcpu == 0
> >> + push {r2-r12} @ Push CP15 registers
> >> + .else
> >> + str r2, [vcpu, #CP15_OFFSET(c13_CID)]
> >> + str r3, [vcpu, #CP15_OFFSET(c13_TID_URW)]
> >> + str r4, [vcpu, #CP15_OFFSET(c13_TID_URO)]
> >> + str r5, [vcpu, #CP15_OFFSET(c13_TID_PRIV)]
> >> + str r6, [vcpu, #CP15_OFFSET(c5_DFSR)]
> >> + str r7, [vcpu, #CP15_OFFSET(c5_IFSR)]
> >> + str r8, [vcpu, #CP15_OFFSET(c5_ADFSR)]
> >> + str r9, [vcpu, #CP15_OFFSET(c5_AIFSR)]
> >> + str r10, [vcpu, #CP15_OFFSET(c6_DFAR)]
> >> + str r11, [vcpu, #CP15_OFFSET(c6_IFAR)]
> >> + str r12, [vcpu, #CP15_OFFSET(c12_VBAR)]
> >> + .endif
> >> +.endm
> >> +
> >> +/*
> >> + * Reads cp15 registers from memory and writes them to hardware
> >> + * @read_from_vcpu: If 0, registers are read in-order from the stack,
> >> + * otherwise from the VCPU struct pointed to by vcpup
> >> + *
> >> + * Assumes vcpu pointer in vcpu reg
> >> + */
> >> +.macro write_cp15_state read_from_vcpu
> >> + .if \read_from_vcpu == 0
> >> + pop {r2-r12}
> >> + .else
> >> + ldr r2, [vcpu, #CP15_OFFSET(c13_CID)]
> >> + ldr r3, [vcpu, #CP15_OFFSET(c13_TID_URW)]
> >> + ldr r4, [vcpu, #CP15_OFFSET(c13_TID_URO)]
> >> + ldr r5, [vcpu, #CP15_OFFSET(c13_TID_PRIV)]
> >> + ldr r6, [vcpu, #CP15_OFFSET(c5_DFSR)]
> >> + ldr r7, [vcpu, #CP15_OFFSET(c5_IFSR)]
> >> + ldr r8, [vcpu, #CP15_OFFSET(c5_ADFSR)]
> >> + ldr r9, [vcpu, #CP15_OFFSET(c5_AIFSR)]
> >> + ldr r10, [vcpu, #CP15_OFFSET(c6_DFAR)]
> >> + ldr r11, [vcpu, #CP15_OFFSET(c6_IFAR)]
> >> + ldr r12, [vcpu, #CP15_OFFSET(c12_VBAR)]
> >> + .endif
> >> +
> >> + mcr p15, 0, r2, c13, c0, 1 @ CID
> >> + mcr p15, 0, r3, c13, c0, 2 @ TID_URW
> >> + mcr p15, 0, r4, c13, c0, 3 @ TID_URO
> >> + mcr p15, 0, r5, c13, c0, 4 @ TID_PRIV
> >> + mcr p15, 0, r6, c5, c0, 0 @ DFSR
> >> + mcr p15, 0, r7, c5, c0, 1 @ IFSR
> >> + mcr p15, 0, r8, c5, c1, 0 @ ADFSR
> >> + mcr p15, 0, r9, c5, c1, 1 @ AIFSR
> >> + mcr p15, 0, r10, c6, c0, 0 @ DFAR
> >> + mcr p15, 0, r11, c6, c0, 2 @ IFAR
> >> + mcr p15, 0, r12, c12, c0, 0 @ VBAR
> >> +
> >> + .if \read_from_vcpu == 0
> >> + pop {r2-r12}
> >> + .else
> >> + ldr r2, [vcpu, #CP15_OFFSET(c1_SCTLR)]
> >> + ldr r3, [vcpu, #CP15_OFFSET(c1_CPACR)]
> >> + ldr r4, [vcpu, #CP15_OFFSET(c2_TTBCR)]
> >> + ldr r5, [vcpu, #CP15_OFFSET(c3_DACR)]
> >> + add vcpu, vcpu, #CP15_OFFSET(c2_TTBR0)
> >> + ldrd r6, r7, [vcpu]
> >> + add vcpu, vcpu, #CP15_OFFSET(c2_TTBR1) - CP15_OFFSET(c2_TTBR0)
> >> + ldrd r8, r9, [vcpu]
> >> + sub vcpu, vcpu, #CP15_OFFSET(c2_TTBR1)
> >> + ldr r10, [vcpu, #CP15_OFFSET(c10_PRRR)]
> >> + ldr r11, [vcpu, #CP15_OFFSET(c10_NMRR)]
> >> + ldr r12, [vcpu, #CP15_OFFSET(c0_CSSELR)]
> >> + .endif
> >> +
> >> + mcr p15, 0, r2, c1, c0, 0 @ SCTLR
> >> + mcr p15, 0, r3, c1, c0, 2 @ CPACR
> >> + mcr p15, 0, r4, c2, c0, 2 @ TTBCR
> >> + mcr p15, 0, r5, c3, c0, 0 @ DACR
> >> + mcrr p15, 0, r6, r7, c2 @ TTBR 0
> >> + mcrr p15, 1, r8, r9, c2 @ TTBR 1
> >> + mcr p15, 0, r10, c10, c2, 0 @ PRRR
> >> + mcr p15, 0, r11, c10, c2, 1 @ NMRR
> >> + mcr p15, 2, r12, c0, c0, 0 @ CSSELR
> >> +.endm
> >> +
> >> +/*
> >> + * Save the VGIC CPU state into memory
> >> + *
> >> + * Assumes vcpu pointer in vcpu reg
> >> + */
> >> +.macro save_vgic_state
> >> +.endm
> >> +
> >> +/*
> >> + * Restore the VGIC CPU state from memory
> >> + *
> >> + * Assumes vcpu pointer in vcpu reg
> >> + */
> >> +.macro restore_vgic_state
> >> +.endm
> >> +
> >> +.equ vmentry, 0
> >> +.equ vmexit, 1
> >> +
> >> +/* Configures the HSTR (Hyp System Trap Register) on entry/return
> >> + * (hardware reset value is 0) */
> >> +.macro set_hstr operation
> >> + mrc p15, 4, r2, c1, c1, 3
> >> + ldr r3, =HSTR_T(15)
> >> + .if \operation == vmentry
> >> + orr r2, r2, r3 @ Trap CR{15}
> >> + .else
> >> + bic r2, r2, r3 @ Don't trap any CRx accesses
> >> + .endif
> >> + mcr p15, 4, r2, c1, c1, 3
> >> +.endm
> >> +
> >> +/* Configures the HCPTR (Hyp Coprocessor Trap Register) on entry/return
> >> + * (hardware reset value is 0). Keep previous value in r2. */
> >> +.macro set_hcptr operation, mask
> >> + mrc p15, 4, r2, c1, c1, 2
> >> + ldr r3, =\mask
> >> + .if \operation == vmentry
> >> + orr r3, r2, r3 @ Trap coproc-accesses defined in mask
> >> + .else
> >> + bic r3, r2, r3 @ Don't trap defined coproc-accesses
> >> + .endif
> >> + mcr p15, 4, r3, c1, c1, 2
> >> +.endm
> >> +
> >> +/* Configures the HDCR (Hyp Debug Configuration Register) on entry/return
> >> + * (hardware reset value is 0) */
> >> +.macro set_hdcr operation
> >> + mrc p15, 4, r2, c1, c1, 1
> >> + ldr r3, =(HDCR_TPM|HDCR_TPMCR)
> >> + .if \operation == vmentry
> >> + orr r2, r2, r3 @ Trap some perfmon accesses
> >> + .else
> >> + bic r2, r2, r3 @ Don't trap any perfmon accesses
> >> + .endif
> >> + mcr p15, 4, r2, c1, c1, 1
> >> +.endm
> >> +
> >> +/* Enable/Disable: stage-2 trans., trap interrupts, trap wfi, trap smc */
> >> +.macro configure_hyp_role operation
> >> + mrc p15, 4, r2, c1, c1, 0 @ HCR
> >> + bic r2, r2, #HCR_VIRT_EXCP_MASK
> >> + ldr r3, =HCR_GUEST_MASK
> >> + .if \operation == vmentry
> >> + orr r2, r2, r3
> >> + ldr r3, [vcpu, #VCPU_IRQ_LINES]
> > irq_lines are accessed atomically from vcpu_interrupt_line(), but there
> > is no memory barriers or atomic operations here. Looks suspicious though
> > I am not familiar with ARM memory model. As far as I understand
> > different translation regimes are used to access this memory, so who
> > knows what this does to access ordering.
> >
> >
>
> there's an exception taken to switch to Hyp mode, which I'm quite sure
> implies a memory barrier.
>
> >> + orr r2, r2, r3
> >> + .else
> >> + bic r2, r2, r3
> >> + .endif
> >> + mcr p15, 4, r2, c1, c1, 0
> >> +.endm
> >> +
> >> +.macro load_vcpu
> >> + mrc p15, 4, vcpu, c13, c0, 2 @ HTPIDR
> >> +.endm
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe kvm" in
> >> the body of a message to majordomo at vger.kernel.org
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
> commit e290b507f0d31c895bd515d69c0c2b50d76b20db
> Author: Christoffer Dall <c.dall@virtualopensystems.com>
> Date: Tue Jan 15 20:53:03 2013 -0500
>
> KVM: ARM: Honor vcpu->requests in the world-switch code
>
> Honor vpuc->request by checking them accordingly and explicitly raise an
> error if unsupported requests are set (we don't support any requests on
> ARM currently).
>
> Also add some commenting to explain the synchronization in more details
> here. The commenting implied renaming a variable and changing error
> handling slightly to improve readibility.
>
> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
>
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 6ff5337..b23a709 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -620,7 +620,7 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
> */
> int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
> {
> - int ret;
> + int guest_ret, ret;
> sigset_t sigsaved;
>
> /* Make sure they initialize the vcpu with KVM_ARM_VCPU_INIT */
> @@ -640,9 +640,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu,
> struct kvm_run *run)
> if (vcpu->sigset_active)
> sigprocmask(SIG_SETMASK, &vcpu->sigset, &sigsaved);
>
> - ret = 1;
> run->exit_reason = KVM_EXIT_UNKNOWN;
> - while (ret > 0) {
> + for (;;) {
> /*
> * Check conditions before entering the guest
> */
> @@ -650,18 +649,44 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu
> *vcpu, struct kvm_run *run)
>
> update_vttbr(vcpu->kvm);
>
> + /*
> + * There is a dependency between setting IN_GUEST_MODE and
> + * sending requests. We need to ensure:
> + * 1. Setting IN_GUEST_MODE before checking vcpu->requests.
> + * 2. We need to check vcpu_request after disabling IRQs
> + * (see comment about signal_pending below).
> + */
> + vcpu->mode = IN_GUEST_MODE;
> +
> local_irq_disable();
>
> /*
> - * Re-check atomic conditions
> + * We need to be careful to check these variables after
> + * disabling interrupts. For example with signals:
> + * 1. If the signal comes before the signal_pending check,
> + * we will return to user space and everything's good.
> + * 2. If the signal comes after the signal_pending check,
> + * we rely on an IPI to exit the guest and continue the
> + * while loop, which checks for pending signals again.
> */
> if (signal_pending(current)) {
> ret = -EINTR;
> run->exit_reason = KVM_EXIT_INTR;
> + local_irq_enable();
> + vcpu->mode = OUTSIDE_GUEST_MODE;
> + break;
> }
>
> - if (ret <= 0 || need_new_vmid_gen(vcpu->kvm)) {
> + if (vcpu->requests) {
> + ret = -ENOSYS; /* requests not supported */
> local_irq_enable();
> + vcpu->mode = OUTSIDE_GUEST_MODE;
> + break;
> + }
> +
> + if (need_new_vmid_gen(vcpu->kvm)) {
> + local_irq_enable();
> + vcpu->mode = OUTSIDE_GUEST_MODE;
> continue;
> }
>
> @@ -670,17 +695,15 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu
> *vcpu, struct kvm_run *run)
> */
> trace_kvm_entry(*vcpu_pc(vcpu));
> kvm_guest_enter();
> - vcpu->mode = IN_GUEST_MODE;
>
> smp_mb(); /* set mode before reading vcpu->arch.pause */
> if (unlikely(vcpu->arch.pause)) {
> /* This means ignore, try again. */
> - ret = ARM_EXCEPTION_IRQ;
> + guest_ret = ARM_EXCEPTION_IRQ;
> } else {
> - ret = kvm_call_hyp(__kvm_vcpu_run, vcpu);
> + guest_ret = kvm_call_hyp(__kvm_vcpu_run, vcpu);
> }
>
> - vcpu->mode = OUTSIDE_GUEST_MODE;
> vcpu->arch.last_pcpu = smp_processor_id();
> kvm_guest_exit();
> trace_kvm_exit(*vcpu_pc(vcpu));
> @@ -695,12 +718,15 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu
> *vcpu, struct kvm_run *run)
> * mode this time.
> */
> local_irq_enable();
> + vcpu->mode = OUTSIDE_GUEST_MODE;
>
> /*
> * Back from guest
> *************************************************************/
>
> - ret = handle_exit(vcpu, run, ret);
> + ret = handle_exit(vcpu, run, guest_ret);
> + if (ret <= 0)
> + break;
> }
>
> if (vcpu->sigset_active)
>
> commit fc9a9c5e9dd4eba4acd6bea5c8c083a9a854d662
> Author: Christoffer Dall <c.dall@virtualopensystems.com>
> Date: Tue Jan 15 20:42:15 2013 -0500
>
> KVM: ARM: Remove unused memslot parameter
>
> diff --git a/arch/arm/include/asm/kvm_mmio.h b/arch/arm/include/asm/kvm_mmio.h
> index 31ab9f5..571ccf0 100644
> --- a/arch/arm/include/asm/kvm_mmio.h
> +++ b/arch/arm/include/asm/kvm_mmio.h
> @@ -46,6 +46,6 @@ static inline void kvm_prepare_mmio(struct kvm_run *run,
>
> int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run);
> int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
> - phys_addr_t fault_ipa, struct kvm_memory_slot *memslot);
> + phys_addr_t fault_ipa);
>
> #endif /* __ARM_KVM_MMIO_H__ */
> diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c
> index d6a4ca0..f655088 100644
> --- a/arch/arm/kvm/mmio.c
> +++ b/arch/arm/kvm/mmio.c
> @@ -117,7 +117,7 @@ static int decode_hsr(struct kvm_vcpu *vcpu,
> phys_addr_t fault_ipa,
> }
>
> int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
> - phys_addr_t fault_ipa, struct kvm_memory_slot *memslot)
> + phys_addr_t fault_ipa)
> {
> struct kvm_exit_mmio mmio;
> unsigned long rt;
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index 2a83ac9..c806080 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -588,7 +588,7 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu,
> struct kvm_run *run)
> unsigned long hsr_ec;
> unsigned long fault_status;
> phys_addr_t fault_ipa;
> - struct kvm_memory_slot *memslot = NULL;
> + struct kvm_memory_slot *memslot;
> bool is_iabt;
> gfn_t gfn;
> int ret;
> @@ -624,7 +624,7 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu,
> struct kvm_run *run)
>
> /* Adjust page offset */
> fault_ipa |= vcpu->arch.hxfar & ~PAGE_MASK;
> - return io_mem_abort(vcpu, run, fault_ipa, memslot);
> + return io_mem_abort(vcpu, run, fault_ipa);
> }
>
> memslot = gfn_to_memslot(vcpu->kvm, gfn);
>
> commit 70667a06e445e240fb5e6352ccdc4bc8a290866e
> Author: Christoffer Dall <c.dall@virtualopensystems.com>
> Date: Tue Jan 15 20:51:42 2013 -0500
>
> KVM: ARM: Grab kvm->srcu lock when handling page faults
>
> The memslots data structure is protected with an SRCU lock, so we should
> grab the read side lock before traversing this data structure.
>
> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
>
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index c806080..0b7eabf 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -591,7 +591,7 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu,
> struct kvm_run *run)
> struct kvm_memory_slot *memslot;
> bool is_iabt;
> gfn_t gfn;
> - int ret;
> + int ret, idx;
>
> hsr_ec = vcpu->arch.hsr >> HSR_EC_SHIFT;
> is_iabt = (hsr_ec == HSR_EC_IABT);
> @@ -627,13 +627,17 @@ int kvm_handle_guest_abort(struct kvm_vcpu
> *vcpu, struct kvm_run *run)
> return io_mem_abort(vcpu, run, fault_ipa);
> }
>
> + idx = srcu_read_lock(&vcpu->kvm->srcu);
> memslot = gfn_to_memslot(vcpu->kvm, gfn);
> if (!memslot->user_alloc) {
> kvm_err("non user-alloc memslots not supported\n");
> - return -EINVAL;
> + ret = -EINVAL;
> + goto out_unlock;
> }
>
> ret = user_mem_abort(vcpu, fault_ipa, gfn, memslot, fault_status);
> +out_unlock:
> + srcu_read_unlock(&vcpu->kvm->srcu, idx);
> return ret ? ret : 1;
> }
>
> --
>
> Thanks,
> -Christoffer
--
Gleb.
^ permalink raw reply
* [RFC PATCH 3/4] ARM: bL_entry: Match memory barriers to architectural requirements
From: Santosh Shilimkar @ 2013-01-16 12:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130116114912.GB1963@linaro.org>
On Wednesday 16 January 2013 05:19 PM, Dave Martin wrote:
> On Wed, Jan 16, 2013 at 12:20:47PM +0530, Santosh Shilimkar wrote:
>> + Catalin, RMK
>>
>> Dave,
>>
>> On Tuesday 15 January 2013 10:18 PM, Dave Martin wrote:
>>> For architectural correctness even Strongly-Ordered memory accesses
>>> require barriers in order to guarantee that multiple CPUs have a
>>> coherent view of the ordering of memory accesses.
>>>
>>> Virtually everything done by this early code is done via explicit
>>> memory access only, so DSBs are seldom required. Existing barriers
>>> are demoted to DMB, except where a DSB is needed to synchronise
>>> non-memory signalling (i.e., before a SEV). If a particular
>>> platform performs cache maintenance in its power_up_setup function,
>>> it should force it to complete explicitly including a DSB, instead
>>> of relying on the bL_head framework code to do it.
>>>
>>> Some additional DMBs are added to ensure all the memory ordering
>>> properties required by the race avoidance algorithm. DMBs are also
>>> moved out of loops, and for clarity some are moved so that most
>>> directly follow the memory operation which needs to be
>>> synchronised.
>>>
>>> The setting of a CPU's bL_entry_vectors[] entry is also required to
>>> act as a synchronisation point, so a DMB is added after checking
>>> that entry to ensure that other CPUs do not observe gated
>>> operations leaking across the opening of the gate.
>>>
>>> Signed-off-by: Dave Martin <dave.martin@linaro.org>
>>> ---
>>
>> Sorry to pick on this again but I am not able to understand why
>> the strongly ordered access needs barriers. At least from the
>> ARM point of view, a strongly ordered write will be more of blocking
>> write and the further interconnect also is suppose to respect that
>
> This is what I originally assumed (hence the absence of barriers in
> the initial patch).
>
>> rule. SO read writes are like adding barrier after every load store
>
> This assumption turns out to be wrong, unfortunately, although in
> a uniprocessor scenario is makes no difference. A SO memory access
> does block the CPU making the access, but explicitly does not
> block the interconnect.
>
I suspected the interconnect part when you described the barrier
need for SO memory region.
> In a typical boot scenario for example, all secondary CPUs are
> quiescent or powered down, so there's no problem. But we can't make
> the same assumptions when we're trying to coordinate between
> multiple active CPUs.
>
>> so adding explicit barriers doesn't make sense. Is this a side
>> effect of some "write early response" kind of optimizations at
>> interconnect level ?
>
> Strongly-Ordered accesses are always non-shareable, so there is
> no explicit guarantee of coherency between multiple masters.
>
This is where probably issue then. My understanding is exactly
opposite here and hence I wasn't worried about multi-master
CPU scenario since sharable attributes would be taking care of it
considering the same page tables being used in SMP system.
ARM documentation says -
------------
Shareability and the S bit, with TEX remap
The memory type of a region, as indicated in the Memory type column of
Table B3-12 on page B3-1350, provides
the first level of control of whether the region is shareable:
? If the memory type is Strongly-ordered then the region is Shareable
------------------------------------------------------------
> If there is only one master, it makes no difference, but if there
> are multiple masters, there is no guarantee that they are conntected
> to a slave device (DRAM controller in this case) via a single
> slave port.
>
See above. We are talking about multiple CPUs here and not
the DSP or other co-processors. In either case we are discussing
code which is getting execute on ARM CPUs so we can safely limit
it to the multi-master ARM CPU.
> The architecture only guarantees global serialisation when there is a
> single slave device, but provides no way to know whether two accesses
> from different masters will reach the same slave port. This is in the
> realms of "implementation defined."
>
> Unfortunately, a high-performance component like a DRAM controller
> is exactly the kind of component which may implement multiple
> master ports, so you can't guarantee that accesses are serialised
> in the same order from the perspective of all masters. There may
> be some pipelining and caching between each master port and the actual
> memory, for example. This is allowed, because there is no requirement
> for the DMC to look like a single slave device from the perspective
> of multiple masters.
>
> A multi-ported slave might provide transparent coherency between master
> ports, but it is only required to guarantee this when the accesses
> are shareable (SO is always non-shared), or when explicit barriers
> are used to force synchronisation between the device's master ports.
>
> Of course, a given platform may have a DMC with only one slave
> port, in which case the barriers should not be needed. But I wanted
> this code to be generic enough to be reusable -- hence the
> addition of the barriers. The CPU does not need to wait for a DMB
> to "complete" in any sense, so this does not necessarily have a
> meaningful impact on performance.
>
> This is my understanding anyway.
>
>> Will you be able to point to specs or documents which puts
>> this requirement ?
>
> Unfortunately, this is one of this things which we require not because
> there is a statement in the ARM ARM to say that we need it -- rather,
> there is no statement in the ARM ARM to say that we don't.
>
Thanks a lot for elaborate answer. It helps to understand the rationale
at least.
Regards
Santosh
^ permalink raw reply
* [PATCH v7 1/3] ARM: make cr_alignment read-only #ifndef CONFIG_CPU_CP15
From: Uwe Kleine-König @ 2013-01-16 12:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F554C4.3020903@arm.com>
Hello Jonny,
On Tue, Jan 15, 2013 at 01:08:20PM +0000, Jonathan Austin wrote:
> On 17/10/12 09:34, Uwe Kleine-K?nig wrote:
> >This makes cr_alignment a constant 0 to break code that tries to modify
> >the value as it's likely that it's built on wrong assumption when
> >CONFIG_CPU_CP15 isn't defined. For code that is only reading the value 0
> >is more or less a fine value to report.
> >
>
> Without the context of some of the discussion that was had on the
> list about how/why to do this, this description is a bit
> confusing...
>
> I found myself asking "Why do we not #ifdef out uses of cr_alignment
> based on CONFIG_CPU_CP15" - a question which it seems is answered by
> you and Nicolas here:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-March/089968.html
>
> So, perhaps it would help to have a little bit more context in this
> commit message?
>
> I know that the cr_alignment stuff is currently tied up with
> CONFIG_CPU_15, but I wonder if you looked at using the
> CCR.UNALIGN_TRP bit and wiring that in to alignment trapping code?
> Is it something you think would make sense for the work you're
> doing?
I didn't, because I focus on getting the machine up and add bells and
whistles later.
> >+#ifdef CONFIG_CPU_CP15
> > .long cr_alignment @ r7
> >+#else
> >+ .long 0
>
> This value still gets loaded in to r7, so it might be worth keeping
> the comments going... They certainly help when reading the code.
ok
> >--- a/arch/arm/mm/mmu.c
> >+++ b/arch/arm/mm/mmu.c
> >@@ -97,6 +97,7 @@ static struct cachepolicy cache_policies[] __initdata = {
> > }
> > };
> >
> >+#ifdef CONFIG_CPU_CP15
> > /*
> > * These are useful for identifying cache coherency
> > * problems by allowing the cache or the cache and
> >@@ -195,6 +196,22 @@ void adjust_cr(unsigned long mask, unsigned long set)
> > }
> > #endif
> >
> >+#else
>
> When you read this file in its complete form there's a lot of code
> (including more preprocessor stuff) between the #ifdef and the
> #else.
>
> Could you add
>
> #else /* ifdef CONFIG_CPU_CP15 */
>
> like you have in the other cases?
also ok.
Will fix up after lunch.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH v2 3/9] ARM: PRIMA2: initialize l2x0 according to mach from DT
From: Mark Rutland @ 2013-01-16 12:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1358315610-25001-3-git-send-email-Barry.Song@csr.com>
On Wed, Jan 16, 2013 at 05:53:29AM +0000, Barry Song wrote:
> From: Barry Song <Baohua.Song@csr.com>
>
> prima2 and marco have diffetent l2 cache configuration, so
> we initialize l2x0 cache based on dtb given to kernel.
>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
> arch/arm/mach-prima2/l2x0.c | 29 ++++++++++++++++++++++++-----
> 1 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-prima2/l2x0.c b/arch/arm/mach-prima2/l2x0.c
> index c998377..e41ecd2 100644
> --- a/arch/arm/mach-prima2/l2x0.c
> +++ b/arch/arm/mach-prima2/l2x0.c
> @@ -11,19 +11,38 @@
> #include <linux/of.h>
> #include <asm/hardware/cache-l2x0.h>
>
> -static struct of_device_id prima2_l2x0_ids[] = {
> - { .compatible = "sirf,prima2-pl310-cache" },
> +struct l2x0_aux
> +{
> + u32 val;
> + u32 mask;
> +};
> +
> +static struct l2x0_aux prima2_l2x0_aux __initconst = {
> + 0x40000,
> + 0,
> +};
That 0x40000 is a bit opaque. Now would be a good time to make it a bit more
legible. Am I right in saying that's (2 << L2X0_AUX_CTRL_WAY_SIZE_SHIFT) ?
It'd also be nice if you used designated initializers:
static struct l2x0_aux prima2_l2x0_aux __initconst = {
.val = (2 << L2X0_AUX_CTRL_WAY_SIZE_SHIFT),
.mask = 0,
};
> +
> +static struct l2x0_aux marco_l2x0_aux __initconst = {
> + (2 << L2X0_AUX_CTRL_WAY_SIZE_SHIFT) |
> + (1 << L2X0_AUX_CTRL_ASSOCIATIVITY_SHIFT),
> + L2X0_AUX_CTRL_MASK,
> +};
And here too:
static struct l2x0_aux marco_l2x0_aux __initconst = {
.val = (2 << L2X0_AUX_CTRL_WAY_SIZE_SHIFT) |
(1 << L2X0_AUX_CTRL_ASSOCIATIVITY_SHIFT),
.mask = L2X0_AUX_CTRL_MASK,
};
> +
> +static struct of_device_id sirf_l2x0_ids[] __initconst = {
> + { .compatible = "sirf,prima2-pl310-cache", .data = &prima2_l2x0_aux, },
> + { .compatible = "sirf,marco-pl310-cache", .data = &marco_l2x0_aux, },
> {},
> };
I took a look at of_match_node, and it seems that the first match found in an
of_match_table will be returned first, rather than finding the match earliest
in a device node's compatible list. This is somewhat counter-intuitive.
Therefore, the marco variant should be listed first, or it will get initialised
with the prima2 configuration values.
>
> static int __init sirfsoc_l2x0_init(void)
> {
> struct device_node *np;
> + const struct l2x0_aux *aux;
>
> - np = of_find_matching_node(NULL, prima2_l2x0_ids);
> + np = of_find_matching_node(NULL, sirf_l2x0_ids);
> if (np) {
> - pr_info("Initializing prima2 L2 cache\n");
> - return l2x0_of_init(0x40000, 0);
> + aux = of_match_node(sirf_l2x0_ids, np)->data;
> + return l2x0_of_init(aux->val, aux->mask);
> }
>
> return 0;
> --
> 1.7.5.4
>
>
With those changes:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
^ permalink raw reply
* [PATCH 05/14] lib: Add I/O map cache implementation
From: Thierry Reding @ 2013-01-16 11:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130116112556.GR23505@n2100.arm.linux.org.uk>
On Wed, Jan 16, 2013 at 11:25:56AM +0000, Russell King - ARM Linux wrote:
> On Wed, Jan 16, 2013 at 11:18:22AM +0100, Thierry Reding wrote:
> > err = ioremap_page_range(virt, virt + SZ_64K - 1, phys,
>
> Why -1 here?
Right, I forgot that end in these functions always means one byte after
the end. Removing the -1 seems to get past the remapping at least.
Reading the configuration space through the mapping doesn't though. I'll
investigate some more.
Thanks for pointing this out Russell.
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130116/c42c59ae/attachment.sig>
^ permalink raw reply
* [PATCH v5 06/14] ARM: EXYNOS: add System MMU definition to DT
From: Will Deacon @ 2013-01-16 11:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <003601cdd790$082a1310$187e3930$%cho@samsung.com>
On Tue, Dec 11, 2012 at 11:09:47AM +0000, Cho KyongHo wrote:
> This commit adds System MMU nodes to DT of Exynos SoCs.
[Adding devicetree-discuss and some other IOMMU/DT people to CC]
> Signed-off-by: KyongHo Cho <pullip.cho@samsung.com>
> ---
> .../devicetree/bindings/arm/exynos/system-mmu.txt | 86 ++++++++++++
> arch/arm/boot/dts/exynos4210.dtsi | 96 ++++++++++++++
> arch/arm/boot/dts/exynos4x12.dtsi | 124 +++++++++++++++++
> arch/arm/boot/dts/exynos5250.dtsi | 147 ++++++++++++++++++++-
> 4 files changed, 451 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/arm/exynos/system-mmu.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/exynos/system-mmu.txt
> b/Documentation/devicetree/bindings/arm/exynos/system-mmu.txt
> new file mode 100644
> index 0000000..9c30a36
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/exynos/system-mmu.txt
> @@ -0,0 +1,86 @@
> +* Samsung Exynos System MMU
> +
> +Samsung's Exynos architecture includes System MMU that enables scattered
> +physical chunks to be visible as a contiguous region to DMA-capabile peripheral
> +devices like MFC, FIMC, FIMD, GScaler, FIMC-IS and so forth.
> +
> +System MMU is a sort of IOMMU and support identical translation table format to
> +ARMv7 translation tables with minimum set of page properties including access
> +permissions, shareability and security protection. In addition System MMU has
> +another capabilities like L2 TLB or block-fetch buffers to minimize translation
> +latency
ARMv7 has two translation table formats -- please specify which ones you
support. Stage-2 descriptors also have a few differences, so you should
describe what you actually support.
> +Each System MMU is included in the H/W block of a peripheral device. Thus, it is
> +important to specify that a System MMU is dedicated to which peripheral device
> +before using System MMU. System initialization must specify the relationships
> +between a System MMU and a peripheral device that owns the System MMU.
> +
> +Some device drivers may control several peripheral devices with a single device
> +descriptor like MFC. Since handling a System MMU with IOMMU API requires a
> +device descriptor that needs the System MMU, it is best to combine the System
> +MMUs of the peripheral devices and control them with a single System MMU device
> +descriptor. If it is unable to combine them into a single device descriptor,
> +they can be linked with each other by the means of device.parent relationship.
I agree with the sentiment here, but I don't think your solution really
caters for the generic case (outside of exynos). For example, I've recently
started looking at writing an IOMMU driver for ARM's System MMU
Architecture, where a likely use-case is for an SoC where there are a bunch
of MMIO devices, with subsets of these devices assigned to different IOMMUs.
This is similar to your scenario, except that the System MMU is not
necessarily tied to a specific hardware block:
Bus
=========================================
|| ||
+----------+ +----------+
| IOMMU0 | | IOMMU1 |
+----------+ +----------+
| | | |
| | | |
Device0 Device1 Device2 Device3
>From Linux's perspective, all of these devices sit on the platform bus
and are probed using device tree. That is where the fun starts: because
the platform bus has only one set of iommu_ops associated with it, the
driver needs to handle each IOMMU instance explicitly, choosing the
appropriate one for the device when setting up or tearing down an I/O
mapping. To complicate this further, each device has a unique StreamID
assigned statically in the hardware which is required when programming
the IOMMU in order to describe translations for a particular bus master.
In this case, your mmu-master property doesn't work because there are a
multiple devices sharing an IOMMU. Furthermore, the StreamIDs for each
client device need to encoded somewhere as well.
Given that this information is not discoverable, it needs to be encoded
in the device tree, but where? I can see two approaches:
1. For each IOMMU node, list phandles to the devices connected to it
and have a corresponding list of StreamIDs.
or
2. For each device wishing to use an IOMMU, have a phandle to the
IOMMU node and a separate StreamID property. The IOMMU would then
parse this information when the device is added to the bus.
Although I prefer the second approach, it has the downside of affecting
all device bindings that wish to use an IOMMU, so I'm open to any other
ideas.
> +Required properties:
> +- compatible: Should be "samsung,exynos-sysmmu".
> +- reg: Tuples of base address and size of System MMU registers. The number of
> + tuples can be more than one if two or more System MMUs are controlled
> + by a single device descriptor.
> +- interrupt-parent: The phandle of the interrupt controller of System MMU
> +- interrupts: Tuples of numbers that indicates the interrupt source. The
> + number of elements in the tuple is dependent upon
> + 'interrupt-parent' property. The number of tuples in this property
> + must be the same with 'reg' property.
> +
> +Optional properties:
> +- mmuname: Strings of the name of System MMU for debugging purpose. The number
> + of strings must be the same with the number of tuples in 'reg'
> + property.
> +- mmu-master: phandle to the device node that owns System MMU. Only the device
> + that is specified whith this property can control System MMU with
> + IOMMU API.
> +
> +Examples:
> +
> +MFC has 2 System MMUs for each port that MFC is attached. Thus it seems natural
> +to define 2 System MMUs for each port of the MFC:
> +
> + sysmmu-mfc-l {
> + mmuname = "mfc_l";
> + reg = <0x11210000 0x1000>;
> + compatible = "samsung,exynos-sysmmu";
> + interrupt-parent = <&combiner>;
> + interrupts = <8 5>;
> + mmu-master = <&mfc>;
> + };
> +
> + sysmmu-mfc-r {
> + mmuname = "mfc_r";
> + reg = <0x11200000 0x1000>;
> + compatible = "samsung,exynos-sysmmu";
> + interrupt-parent = <&combiner>;
> + interrupts = <6 2>;
> + mmu-master = <&mfc>;
> + };
> +
> +Actually, MFC device driver requires sub-devices that represents each port and
> +above 'mmu-master' properties of sysmmu-mfc-l and sysmmu-mfc-r have the phandles
> +to those sub-devices.
> +
> +However, it is also a good idea that treats the above System MMUs as one System
> +MMU because those System MMUs are actually required by the MFC device:
> +
> + sysmmu-mfc {
> + mmuname = "mfc_l", "mfc_r";
> + reg = <0x11210000 0x1000
> + 0x11200000 0x1000>;
> + compatible = "samsung,exynos-sysmmu";
> + interrupt-parent = <&combiner>;
> + interrupts = <8 5
> + 6 2>;
> + mmu-master = <&mfc>;
> + };
Merging the nodes like this has strong requirements for uniform smmu
devices, which I think are unreasonable. I would not be surprised to see
some system MMUs have more interrupts than others, for example. Furthermore,
you may chain stage-1 System MMUs with a stage-2 System MMU and these would
need to be described as separate nodes (this actually complicates my two
suggestions earlier on...).
> +If System MMU of MFC is defined like the above, the number of elements and the
> +order of list in 'mmuname', 'reg' and 'interrupts' must be the same.
That's a really nasty restriction (the interrupts could even be ORd
together).
I appreciate that this is a samsung/exynos-specific binding but if we can
generalise at least some parts of it then they can be re-used by other
SoCs with multiple IOMMUs too.
Will
^ permalink raw reply
* [RFC PATCH 3/4] ARM: bL_entry: Match memory barriers to architectural requirements
From: Dave Martin @ 2013-01-16 11:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F64DC7.6040707@ti.com>
On Wed, Jan 16, 2013 at 12:20:47PM +0530, Santosh Shilimkar wrote:
> + Catalin, RMK
>
> Dave,
>
> On Tuesday 15 January 2013 10:18 PM, Dave Martin wrote:
> >For architectural correctness even Strongly-Ordered memory accesses
> >require barriers in order to guarantee that multiple CPUs have a
> >coherent view of the ordering of memory accesses.
> >
> >Virtually everything done by this early code is done via explicit
> >memory access only, so DSBs are seldom required. Existing barriers
> >are demoted to DMB, except where a DSB is needed to synchronise
> >non-memory signalling (i.e., before a SEV). If a particular
> >platform performs cache maintenance in its power_up_setup function,
> >it should force it to complete explicitly including a DSB, instead
> >of relying on the bL_head framework code to do it.
> >
> >Some additional DMBs are added to ensure all the memory ordering
> >properties required by the race avoidance algorithm. DMBs are also
> >moved out of loops, and for clarity some are moved so that most
> >directly follow the memory operation which needs to be
> >synchronised.
> >
> >The setting of a CPU's bL_entry_vectors[] entry is also required to
> >act as a synchronisation point, so a DMB is added after checking
> >that entry to ensure that other CPUs do not observe gated
> >operations leaking across the opening of the gate.
> >
> >Signed-off-by: Dave Martin <dave.martin@linaro.org>
> >---
>
> Sorry to pick on this again but I am not able to understand why
> the strongly ordered access needs barriers. At least from the
> ARM point of view, a strongly ordered write will be more of blocking
> write and the further interconnect also is suppose to respect that
This is what I originally assumed (hence the absence of barriers in
the initial patch).
> rule. SO read writes are like adding barrier after every load store
This assumption turns out to be wrong, unfortunately, although in
a uniprocessor scenario is makes no difference. A SO memory access
does block the CPU making the access, but explicitly does not
block the interconnect.
In a typical boot scenario for example, all secondary CPUs are
quiescent or powered down, so there's no problem. But we can't make
the same assumptions when we're trying to coordinate between
multiple active CPUs.
> so adding explicit barriers doesn't make sense. Is this a side
> effect of some "write early response" kind of optimizations at
> interconnect level ?
Strongly-Ordered accesses are always non-shareable, so there is
no explicit guarantee of coherency between multiple masters.
If there is only one master, it makes no difference, but if there
are multiple masters, there is no guarantee that they are conntected
to a slave device (DRAM controller in this case) via a single
slave port.
The architecture only guarantees global serialisation when there is a
single slave device, but provides no way to know whether two accesses
from different masters will reach the same slave port. This is in the
realms of "implementation defined."
Unfortunately, a high-performance component like a DRAM controller
is exactly the kind of component which may implement multiple
master ports, so you can't guarantee that accesses are serialised
in the same order from the perspective of all masters. There may
be some pipelining and caching between each master port and the actual
memory, for example. This is allowed, because there is no requirement
for the DMC to look like a single slave device from the perspective
of multiple masters.
A multi-ported slave might provide transparent coherency between master
ports, but it is only required to guarantee this when the accesses
are shareable (SO is always non-shared), or when explicit barriers
are used to force synchronisation between the device's master ports.
Of course, a given platform may have a DMC with only one slave
port, in which case the barriers should not be needed. But I wanted
this code to be generic enough to be reusable -- hence the
addition of the barriers. The CPU does not need to wait for a DMB
to "complete" in any sense, so this does not necessarily have a
meaningful impact on performance.
This is my understanding anyway.
> Will you be able to point to specs or documents which puts
> this requirement ?
Unfortunately, this is one of this things which we require not because
there is a statement in the ARM ARM to say that we need it -- rather,
there is no statement in the ARM ARM to say that we don't.
Cheers
---Dave
^ permalink raw reply
* [PATCH 3/4] Enable ecc-mode selection in the driver
From: Stefan Peter @ 2013-01-16 11:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130115123307.GA13433@titan.lakedaemon.net>
Hi Jason
on 15.01.2013 13:33, Jason Cooper wrote:
> Stefan,
>
> This is a good-looking patch series, nice and small! A few comments:
>
> On Tue, Jan 15, 2013 at 01:13:12PM +0100, Stefan Peter wrote:
>> In order to be able to use the ecc-mode, add the bch module to the default
>> settings for the kirwood boards and enable the activation in orin-nand.c
>
> s/orin-nand/orion-nand/
Will fix.
>
>>
>> Signed-off-by: Stefan Peter <s.peter@mpl.ch>
>> ---
>> diff --git a/arch/arm/configs/kirkwood_defconfig b/arch/arm/configs/kirkwood_defconfig
>> index 93f3794..4a9d3f7 100644
>> --- a/arch/arm/configs/kirkwood_defconfig
>> +++ b/arch/arm/configs/kirkwood_defconfig
>> @@ -84,6 +84,7 @@ CONFIG_MTD_CFI_STAA=y
>> CONFIG_MTD_PHYSMAP=y
>> CONFIG_MTD_M25P80=y
>> CONFIG_MTD_NAND=y
>> +CONFIG_MTD_NAND_ECC_BCH=y
>> CONFIG_MTD_NAND_ORION=y
>> CONFIG_BLK_DEV_LOOP=y
>> # CONFIG_SCSI_PROC_FS is not set
>
> Please make the defconfig change a separate patch.
Will do.
>
> As a general note (this series shouldn't be a problem), please sort the
> series so that you add the driver feature first, then enable it in
> defconfig, then use it in DT. This way, a future bisection won't run
> into any nasty surprises, other than what they're looking for. ;-)
Will do.
Thank you very much for your review.
Stefan Peter
--
MPL AG, Switzerland http://www.mpl.ch
Tel. +41 (0)56 483 34 34 Fax: +41(0)56 493 30 20
^ permalink raw reply
* [PATCH 3/4] Enable ecc-mode selection in the driver
From: Stefan Peter @ 2013-01-16 11:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130115125119.GH13482@lunn.ch>
Hi Andrew
on 15.01.2013 13:51, Andrew Lunn wrote:
> On Tue, Jan 15, 2013 at 01:13:12PM +0100, Stefan Peter wrote:
>> In order to be able to use the ecc-mode, add the bch module to the default
>> settings for the kirwood boards and enable the activation in orin-nand.c
>>
>> Signed-off-by: Stefan Peter <s.peter@mpl.ch>
>> ---
>> diff --git a/arch/arm/configs/kirkwood_defconfig b/arch/arm/configs/kirkwood_defconfig
>> index 93f3794..4a9d3f7 100644
>> --- a/arch/arm/configs/kirkwood_defconfig
>> +++ b/arch/arm/configs/kirkwood_defconfig
>> @@ -84,6 +84,7 @@ CONFIG_MTD_CFI_STAA=y
>> CONFIG_MTD_PHYSMAP=y
>> CONFIG_MTD_M25P80=y
>> CONFIG_MTD_NAND=y
>> +CONFIG_MTD_NAND_ECC_BCH=y
>> CONFIG_MTD_NAND_ORION=y
>> CONFIG_BLK_DEV_LOOP=y
>> # CONFIG_SCSI_PROC_FS is not set
>> diff --git a/drivers/mtd/nand/orion_nand.c b/drivers/mtd/nand/orion_nand.c
>> index cd72b92..1a35257 100644
>> --- a/drivers/mtd/nand/orion_nand.c
>> +++ b/drivers/mtd/nand/orion_nand.c
>> @@ -14,6 +14,7 @@
>> #include <linux/module.h>
>> #include <linux/platform_device.h>
>> #include <linux/of.h>
>> +#include <linux/of_mtd.h>
>> #include <linux/mtd/mtd.h>
>> #include <linux/mtd/nand.h>
>> #include <linux/mtd/partitions.h>
>> @@ -130,6 +131,7 @@ static int __init orion_nand_probe(struct platform_device *pdev)
>> if (!of_property_read_u32(pdev->dev.of_node,
>> "chip-delay", &val))
>> board->chip_delay = (u8)val;
>> + board->ecc_mode = of_get_nand_ecc_mode(pdev->dev.of_node);
>
>> } else
>> board = pdev->dev.platform_data;
>>
>> @@ -140,7 +142,8 @@ static int __init orion_nand_probe(struct platform_device *pdev)
>> nc->IO_ADDR_R = nc->IO_ADDR_W = io_base;
>> nc->cmd_ctrl = orion_nand_cmd_ctrl;
>> nc->read_buf = orion_nand_read_buf;
>> - nc->ecc.mode = NAND_ECC_SOFT;
>> + nc->ecc.mode = board->ecc_mode == NAND_ECC_SOFT_BCH ?
>> + NAND_ECC_SOFT_BCH : NAND_ECC_SOFT;
>
> Hi Stefan
>
> What about a user that wants one of the other valid values?
> NAND_ECC_OOB_FIRST, NAND_ECC_HW_SYNDROME, etc.
As far as I understand, NAND_ECC_NONE, NAND_ECC_SOFT and
NAND_ECC_SOFT_BCH are the only ECC modes that do not require
corresponding hardware support which is missing in the marvell
88F6180/88F619x/88F628x. From my point of view, NAND_ECC_NONE does not
make sense, too, because MLC NAND Flash requires ECC to be usable.
>
> Would:
>
> if (IS_ERR(board->ecc_mode)) {
> nc->ecc.mode = NAND_ECC_SOFT;
> dev_info(&pdev->dev, "Defaulting to NAND_ECC_SOFT");
> } else
> nc->ecc.mode = board->ecc_mode
>
> be better?
I feel safer by limiting the modes to what I could test.
Regards
Stefan Peter
--
MPL AG, Switzerland http://www.mpl.ch
Tel. +41 (0)56 483 34 34 Fax: +41(0)56 493 30 20
^ permalink raw reply
* [PATCH v2 2/9] ARM: PRIMA2: enable AUTO_ZRELADDR for SIRF in Kconfig
From: Mark Rutland @ 2013-01-16 11:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1358315610-25001-2-git-send-email-Barry.Song@csr.com>
On Wed, Jan 16, 2013 at 05:53:28AM +0000, Barry Song wrote:
> From: Barry Song <Baohua.Song@csr.com>
>
> prima2 and marco have different memory base address. prima2
> begins from 0 and marco begins from
Runaway commit message.
>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
> arch/arm/Kconfig | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index f95ba14..13f89a2 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -394,6 +394,7 @@ config ARCH_GEMINI
> config ARCH_SIRF
> bool "CSR SiRF"
> select ARCH_REQUIRE_GPIOLIB
> + select AUTO_ZRELADDR
> select COMMON_CLK
> select GENERIC_CLOCKEVENTS
> select GENERIC_IRQ_CHIP
> --
> 1.7.5.4
>
>
>
> Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
> More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Thanks,
Mark.
^ permalink raw reply
* [PATCH v2 1/9] ARM: PRIMA2: add CSR SiRFmarco device tree .dts
From: Mark Rutland @ 2013-01-16 11:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1358315610-25001-1-git-send-email-Barry.Song@csr.com>
Hello,
This looks pretty good. I've tried to give a more thorough review this time,
so hopefully these comments should be my last.
On Wed, Jan 16, 2013 at 05:53:27AM +0000, Barry Song wrote:
> From: Barry Song <Baohua.Song@csr.com>
>
> SiRFmarco is a dual-core cortex-a9 SMP SoC from CSR. this patch
> adds the .dtsi and a basic evb board .dts for it.
>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
> arch/arm/boot/dts/marco-evb.dts | 51 +++
> arch/arm/boot/dts/marco.dtsi | 756 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 807 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/boot/dts/marco-evb.dts
> create mode 100644 arch/arm/boot/dts/marco.dtsi
>
> diff --git a/arch/arm/boot/dts/marco-evb.dts b/arch/arm/boot/dts/marco-evb.dts
> new file mode 100644
> index 0000000..4e68d3c
> --- /dev/null
> +++ b/arch/arm/boot/dts/marco-evb.dts
> @@ -0,0 +1,51 @@
> +/*
> + * DTS file for CSR SiRFmarco Evaluation Board
> + *
> + * Copyright (c) 2012 Cambridge Silicon Radio Limited, a CSR plc group company.
> + *
> + * Licensed under GPLv2 or later.
> + */
> +
> +/dts-v1/;
> +
> +/include/ "marco.dtsi"
> +
> +/ {
> + model = "CSR SiRFmarco Evaluation Board";
> + compatible = "sirf,marco", "sirf,marco-cb";
Shouldn't "sirf,marco-cb" come before "sirf,marco", so we have the most
specific match first?
It would also be nice if both compatible strings were documented.
> +
> + memory {
> + reg = <0x40000000 0x60000000>;
> + };
> +
> + axi {
> + peri-iobg {
> + uart1: uart at cc060000 {
> + status = "okay";
> + };
> + i2c0: i2c at cc0e0000 {
> + status = "okay";
> + fpga-cpld at 4d {
> + compatible = "sirf,fpga-cpld";
> + reg = <0x4d>;
> + };
> + };
> + spi1: spi at cc170000 {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&spi1_pins_a>;
> + spi at 0 {
> + compatible = "spidev";
> + reg = <0>;
> + spi-max-frequency = <1000000>;
> + };
> + };
> + pci-iobg {
> + sd0: sdhci at cd000000 {
> + bus-width = <8>;
> + status = "okay";
> + };
> + };
> + };
> + };
> +};
> diff --git a/arch/arm/boot/dts/marco.dtsi b/arch/arm/boot/dts/marco.dtsi
> new file mode 100644
> index 0000000..d6bad50
> --- /dev/null
> +++ b/arch/arm/boot/dts/marco.dtsi
> @@ -0,0 +1,756 @@
> +/*
> + * DTS file for CSR SiRFmarco SoC
> + *
> + * Copyright (c) 2012 Cambridge Silicon Radio Limited, a CSR plc group company.
> + *
> + * Licensed under GPLv2 or later.
> + */
> +
> +/include/ "skeleton.dtsi"
> +/ {
> + compatible = "sirf,marco";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + interrupt-parent = <&gic>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu at 0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a9";
> + reg = <0>;
> + };
> + cpu at 1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a9";
> + reg = <1>;
> + };
> + };
Nice to see the reg properties present :)
Do the CPUs support the performance monitors extension, and if so are
interrupts wired up?
If so it'd be nice to see a pmu node:
pmu {
compatible = "arm,cortex-a9-pmu";
interrupts = <cpu0-irq>,
<cpu1-irq>;
};
> +
> + axi {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0x40000000 0x40000000 0xa0000000>;
> +
> + l2-cache-controller at c0030000 {
> + compatible = "sirf,marco-pl310-cache", "arm,pl310-cache";
> + reg = <0xc0030000 0x1000>;
> + interrupts = <0 59 0>;
> + arm,tag-latency = <1 1 1>;
> + arm,data-latency = <1 1 1>;
> + arm,filter-ranges = <0x40000000 0x80000000>;
> + };
> +
> + gic: interrupt-controller at c0011000 {
> + compatible = "arm,cortex-a9-gic";
> + interrupt-controller;
> + #interrupt-cells = <3>;
> + reg = <0xc0011000 0x1000>,
> + <0xc0010100 0x0100>;
> + };
> +
> + rstc-iobg {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0xc2000000 0xc2000000 0x1000000>;
> +
> + reset-controller at c2000000 {
> + compatible = "sirf,marco-rstc";
> + reg = <0xc2000000 0x10000>;
> + };
> + };
> +
> + sys-iobg {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0xc3000000 0xc3000000 0x1000000>;
> +
> + clock-controller at c3000000 {
> + compatible = "sirf,marco-clkc";
> + reg = <0xc3000000 0x1000>;
> + interrupts = <0 3 0>;
> + };
> +
> + rsc-controller at c3010000 {
> + compatible = "sirf,marco-rsc";
> + reg = <0xc3010000 0x1000>;
> + };
I assume an update for the clk-prima2 driver is going out in a separate series
to enable these compatible strings?
Is the hardware backwards compatible with the prima2 variant? If so, you could
append the sirf,prima2 variant to the compatible lists and save a lot of churn
in drivers.
> + };
> +
> + mem-iobg {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0xc4000000 0xc4000000 0x1000000>;
> +
> + memory-controller at c4000000 {
> + compatible = "sirf,marco-memc";
> + reg = <0xc4000000 0x10000>;
> + interrupts = <0 27 0>;
> + };
Again, if this is compatible with the prima2 variant, it'd be good to append
the prima2 variant's compatible string.
> + };
> +
> + disp-iobg0 {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0xc5000000 0xc5000000 0x1000000>;
> +
> + display0 at c5000000 {
> + compatible = "sirf,marco-lcd";
> + reg = <0xc5000000 0x10000>;
> + interrupts = <0 30 0>;
> + };
> +
> + vpp0 at c5010000 {
> + compatible = "sirf,marco-vpp";
> + reg = <0xc5010000 0x10000>;
> + interrupts = <0 31 0>;
> + };
And again, though I can't find any string matching "sirf,.*-lcd" or
"sirf,.*-vpp" in v3.8-rc3.
> + };
> +
> + disp-iobg1 {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0xc6000000 0xc6000000 0x1000000>;
> +
> + display1 at c6000000 {
> + compatible = "sirf,marco-lcd";
> + reg = <0xc6000000 0x10000>;
> + interrupts = <0 62 0>;
> + };
> +
> + vpp1 at c6010000 {
> + compatible = "sirf,marco-vpp";
> + reg = <0xc6010000 0x10000>;
> + interrupts = <0 63 0>;
> + };
And again.
> + };
> +
> + graphics-iobg {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0xc8000000 0xc8000000 0x1000000>;
> +
> + graphics at c8000000 {
> + compatible = "powervr,sgx540";
> + reg = <0xc8000000 0x1000000>;
> + interrupts = <0 6 0>;
> + };
> + };
> +
> + multimedia-iobg {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0xc9000000 0xc9000000 0x1000000>;
> +
> + multimedia at a0000000 {
> + compatible = "sirf,marco-video-codec";
And again. I'll stop with the compatible string pedantry here, but if any
hardware with a "sirf,marco-.*" string is compatible with a prima2 variant,
it'd be good to append the prima2 string to the end of the compatible list.
> + reg = <0xc9000000 0x1000000>;
> + interrupts = <0 5 0>;
> + };
> + };
> +
> + dsp-iobg {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0xca000000 0xca000000 0x2000000>;
> +
> + dspif at ca000000 {
> + compatible = "sirf,marco-dspif";
> + reg = <0xca000000 0x10000>;
> + interrupts = <0 9 0>;
> + };
> +
> + gps at ca010000 {
> + compatible = "sirf,marco-gps";
> + reg = <0xca010000 0x10000>;
> + interrupts = <0 7 0>;
> + };
> +
> + dsp at cb000000 {
> + compatible = "sirf,marco-dsp";
> + reg = <0xcb000000 0x1000000>;
> + interrupts = <0 8 0>;
> + };
> + };
> +
> + peri-iobg {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0xcc000000 0xcc000000 0x2000000>;
> +
> + timer at cc020000 {
> + compatible = "sirf,marco-tick";
> + reg = <0xcc020000 0x1000>;
> + interrupts = <0 0 0>,
> + <0 1 0>,
> + <0 2 0>,
> + <0 49 0>,
> + <0 50 0>,
> + <0 51 0>;
> + };
> +
> + nand at cc030000 {
> + compatible = "sirf,marco-nand";
> + reg = <0xcc030000 0x10000>;
> + interrupts = <0 41 0>;
> + };
> +
> + audio at cc040000 {
> + compatible = "sirf,marco-audio";
> + reg = <0xcc040000 0x10000>;
> + interrupts = <0 35 0>;
> + };
> +
> + uart0: uart at cc050000 {
> + cell-index = <0>;
> + compatible = "sirf,marco-uart";
> + reg = <0xcc050000 0x1000>;
> + interrupts = <0 17 0>;
> + fifosize = <128>;
> + status = "disabled";
> + };
> +
> + uart1: uart at cc060000 {
> + cell-index = <1>;
> + compatible = "sirf,marco-uart";
> + reg = <0xcc060000 0x1000>;
> + interrupts = <0 18 0>;
> + fifosize = <32>;
> + status = "disabled";
> + };
> +
> + uart2: uart at cc070000 {
> + cell-index = <2>;
> + compatible = "sirf,marco-uart";
> + reg = <0xcc070000 0x1000>;
> + interrupts = <0 19 0>;
> + fifosize = <128>;
> + status = "disabled";
> + };
> +
> + uart3: uart at cc190000 {
> + cell-index = <3>;
> + compatible = "sirf,marco-uart";
> + reg = <0xcc190000 0x1000>;
> + interrupts = <0 66 0>;
> + fifosize = <128>;
> + status = "disabled";
> + };
> +
> + uart4: uart at cc1a0000 {
> + cell-index = <4>;
> + compatible = "sirf,marco-uart";
> + reg = <0xcc1a0000 0x1000>;
> + interrupts = <0 69 0>;
> + fifosize = <128>;
> + status = "disabled";
> + };
> +
> + usp0: usp at cc080000 {
> + cell-index = <0>;
> + compatible = "sirf,marco-usp";
> + reg = <0xcc080000 0x10000>;
> + interrupts = <0 20 0>;
> + status = "disabled";
> + };
> +
> + usp1: usp at cc090000 {
> + cell-index = <1>;
> + compatible = "sirf,marco-usp";
> + reg = <0xcc090000 0x10000>;
> + interrupts = <0 21 0>;
> + status = "disabled";
> + };
> +
> + usp2: usp at cc0a0000 {
> + cell-index = <2>;
> + compatible = "sirf,marco-usp";
> + reg = <0xcc0a0000 0x10000>;
> + interrupts = <0 22 0>;
> + status = "disabled";
> + };
> +
> + dmac0: dma-controller at cc0b0000 {
> + cell-index = <0>;
> + compatible = "sirf,marco-dmac";
> + reg = <0xcc0b0000 0x10000>;
> + interrupts = <0 12 0>;
> + };
> +
> + dmac1: dma-controller at cc160000 {
> + cell-index = <1>;
> + compatible = "sirf,marco-dmac";
> + reg = <0xcc160000 0x10000>;
> + interrupts = <0 13 0>;
> + };
> +
> + vip at cc0c0000 {
> + compatible = "sirf,marco-vip";
> + reg = <0xcc0c0000 0x10000>;
> + };
> +
> + spi0: spi at cc0d0000 {
> + cell-index = <0>;
> + compatible = "sirf,marco-spi";
> + reg = <0xcc0d0000 0x10000>;
> + interrupts = <0 15 0>;
> + sirf,spi-num-chipselects = <1>;
> + cs-gpios = <&gpio 0 0>;
> + sirf,spi-dma-rx-channel = <25>;
> + sirf,spi-dma-tx-channel = <20>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + spi1: spi at cc170000 {
> + cell-index = <1>;
> + compatible = "sirf,marco-spi";
> + reg = <0xcc170000 0x10000>;
> + interrupts = <0 16 0>;
> + sirf,spi-num-chipselects = <1>;
> + cs-gpios = <&gpio 0 0>;
> + sirf,spi-dma-rx-channel = <12>;
> + sirf,spi-dma-tx-channel = <13>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + i2c0: i2c at cc0e0000 {
> + cell-index = <0>;
> + compatible = "sirf,marco-i2c";
> + reg = <0xcc0e0000 0x10000>;
> + interrupts = <0 24 0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + i2c1: i2c at cc0f0000 {
> + cell-index = <1>;
> + compatible = "sirf,marco-i2c";
> + reg = <0xcc0f0000 0x10000>;
> + interrupts = <0 25 0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + tsc at cc110000 {
> + compatible = "sirf,marco-tsc";
> + reg = <0xcc110000 0x10000>;
> + interrupts = <0 33 0>;
> + };
> +
> + gpio: pinctrl at cc120000 {
> + #gpio-cells = <2>;
> + #interrupt-cells = <2>;
> + compatible = "sirf,marco-pinctrl";
It would be nice if there were a patch adding the "sirf,marco-pinctrl"
compatible string to the pinctrl/pinctrl-sirf.txt devicetree binding doc (given
the pinctrl-sirf driver handles the string already).
> + reg = <0xcc120000 0x10000>;
> + interrupts = <0 43 0>,
> + <0 44 0>,
> + <0 45 0>,
> + <0 46 0>,
> + <0 47 0>;
> + gpio-controller;
> + interrupt-controller;
> +
> + lcd_16pins_a: lcd0 at 0 {
Is the lcd0 name special to the driver/subsystem?
If not, it would be nice to change lcd0 at N to lcd0_N -- ePAPR says the
unit-address should match the reg property, and if there's no reg property the
unit-address must be omitted.
The same goes for all the remaining nodes within gpio.
> + lcd {
> + sirf,pins = "lcd_16bitsgrp";
> + sirf,function = "lcd_16bits";
> + };
> + };
> + lcd_18pins_a: lcd0 at 1 {
> + lcd {
> + sirf,pins = "lcd_18bitsgrp";
> + sirf,function = "lcd_18bits";
> + };
> + };
> + lcd_24pins_a: lcd0 at 2 {
> + lcd {
> + sirf,pins = "lcd_24bitsgrp";
> + sirf,function = "lcd_24bits";
> + };
> + };
> + lcdrom_pins_a: lcdrom0 at 0 {
> + lcd {
> + sirf,pins = "lcdromgrp";
> + sirf,function = "lcdrom";
> + };
> + };
> + uart0_pins_a: uart0 at 0 {
> + uart {
> + sirf,pins = "uart0grp";
> + sirf,function = "uart0";
> + };
> + };
> + uart1_pins_a: uart1 at 0 {
> + uart {
> + sirf,pins = "uart1grp";
> + sirf,function = "uart1";
> + };
> + };
> + uart2_pins_a: uart2 at 0 {
> + uart {
> + sirf,pins = "uart2grp";
> + sirf,function = "uart2";
> + };
> + };
> + uart2_noflow_pins_a: uart2 at 1 {
> + uart {
> + sirf,pins = "uart2_nostreamctrlgrp";
> + sirf,function = "uart2_nostreamctrl";
> + };
> + };
> + spi0_pins_a: spi0 at 0 {
> + spi {
> + sirf,pins = "spi0grp";
> + sirf,function = "spi0";
> + };
> + };
> + spi1_pins_a: spi1 at 0 {
> + spi {
> + sirf,pins = "spi1grp";
> + sirf,function = "spi1";
> + };
> + };
> + i2c0_pins_a: i2c0 at 0 {
> + i2c {
> + sirf,pins = "i2c0grp";
> + sirf,function = "i2c0";
> + };
> + };
> + i2c1_pins_a: i2c1 at 0 {
> + i2c {
> + sirf,pins = "i2c1grp";
> + sirf,function = "i2c1";
> + };
> + };
> + pwm0_pins_a: pwm0 at 0 {
> + pwm {
> + sirf,pins = "pwm0grp";
> + sirf,function = "pwm0";
> + };
> + };
> + pwm1_pins_a: pwm1 at 0 {
> + pwm {
> + sirf,pins = "pwm1grp";
> + sirf,function = "pwm1";
> + };
> + };
> + pwm2_pins_a: pwm2 at 0 {
> + pwm {
> + sirf,pins = "pwm2grp";
> + sirf,function = "pwm2";
> + };
> + };
> + pwm3_pins_a: pwm3 at 0 {
> + pwm {
> + sirf,pins = "pwm3grp";
> + sirf,function = "pwm3";
> + };
> + };
> + gps_pins_a: gps at 0 {
> + gps {
> + sirf,pins = "gpsgrp";
> + sirf,function = "gps";
> + };
> + };
> + vip_pins_a: vip at 0 {
> + vip {
> + sirf,pins = "vipgrp";
> + sirf,function = "vip";
> + };
> + };
> + sdmmc0_pins_a: sdmmc0 at 0 {
> + sdmmc0 {
> + sirf,pins = "sdmmc0grp";
> + sirf,function = "sdmmc0";
> + };
> + };
> + sdmmc1_pins_a: sdmmc1 at 0 {
> + sdmmc1 {
> + sirf,pins = "sdmmc1grp";
> + sirf,function = "sdmmc1";
> + };
> + };
> + sdmmc2_pins_a: sdmmc2 at 0 {
> + sdmmc2 {
> + sirf,pins = "sdmmc2grp";
> + sirf,function = "sdmmc2";
> + };
> + };
> + sdmmc3_pins_a: sdmmc3 at 0 {
> + sdmmc3 {
> + sirf,pins = "sdmmc3grp";
> + sirf,function = "sdmmc3";
> + };
> + };
> + sdmmc4_pins_a: sdmmc4 at 0 {
> + sdmmc4 {
> + sirf,pins = "sdmmc4grp";
> + sirf,function = "sdmmc4";
> + };
> + };
> + sdmmc5_pins_a: sdmmc5 at 0 {
> + sdmmc5 {
> + sirf,pins = "sdmmc5grp";
> + sirf,function = "sdmmc5";
> + };
> + };
> + i2s_pins_a: i2s at 0 {
> + i2s {
> + sirf,pins = "i2sgrp";
> + sirf,function = "i2s";
> + };
> + };
> + ac97_pins_a: ac97 at 0 {
> + ac97 {
> + sirf,pins = "ac97grp";
> + sirf,function = "ac97";
> + };
> + };
> + nand_pins_a: nand at 0 {
> + nand {
> + sirf,pins = "nandgrp";
> + sirf,function = "nand";
> + };
> + };
> + usp0_pins_a: usp0 at 0 {
> + usp0 {
> + sirf,pins = "usp0grp";
> + sirf,function = "usp0";
> + };
> + };
> + usp1_pins_a: usp1 at 0 {
> + usp1 {
> + sirf,pins = "usp1grp";
> + sirf,function = "usp1";
> + };
> + };
> + usp2_pins_a: usp2 at 0 {
> + usp2 {
> + sirf,pins = "usp2grp";
> + sirf,function = "usp2";
> + };
> + };
> + usb0_utmi_drvbus_pins_a: usb0_utmi_drvbus at 0 {
> + usb0_utmi_drvbus {
> + sirf,pins = "usb0_utmi_drvbusgrp";
> + sirf,function = "usb0_utmi_drvbus";
> + };
> + };
> + usb1_utmi_drvbus_pins_a: usb1_utmi_drvbus at 0 {
> + usb1_utmi_drvbus {
> + sirf,pins = "usb1_utmi_drvbusgrp";
> + sirf,function = "usb1_utmi_drvbus";
> + };
> + };
> + warm_rst_pins_a: warm_rst at 0 {
> + warm_rst {
> + sirf,pins = "warm_rstgrp";
> + sirf,function = "warm_rst";
> + };
> + };
> + pulse_count_pins_a: pulse_count at 0 {
> + pulse_count {
> + sirf,pins = "pulse_countgrp";
> + sirf,function = "pulse_count";
> + };
> + };
> + cko0_rst_pins_a: cko0_rst at 0 {
> + cko0_rst {
> + sirf,pins = "cko0_rstgrp";
> + sirf,function = "cko0_rst";
> + };
> + };
> + cko1_rst_pins_a: cko1_rst at 0 {
> + cko1_rst {
> + sirf,pins = "cko1_rstgrp";
> + sirf,function = "cko1_rst";
> + };
> + };
> + };
> +
> + pwm at cc130000 {
> + compatible = "sirf,marco-pwm";
> + reg = <0xcc130000 0x10000>;
> + };
> +
> + efusesys at cc140000 {
> + compatible = "sirf,marco-efuse";
> + reg = <0xcc140000 0x10000>;
> + };
> +
> + pulsec at cc150000 {
> + compatible = "sirf,marco-pulsec";
> + reg = <0xcc150000 0x10000>;
> + interrupts = <0 48 0>;
> + };
> +
> + pci-iobg {
> + compatible = "sirf,marco-pciiobg", "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0xcd000000 0xcd000000 0x1000000>;
> +
> + sd0: sdhci at cd000000 {
> + cell-index = <0>;
> + compatible = "sirf,marco-sdhc";
> + reg = <0xcd000000 0x100000>;
> + interrupts = <0 38 0>;
> + status = "disabled";
> + };
> +
> + sd1: sdhci at cd100000 {
> + cell-index = <1>;
> + compatible = "sirf,marco-sdhc";
> + reg = <0xcd100000 0x100000>;
> + interrupts = <0 38 0>;
> + status = "disabled";
> + };
> +
> + sd2: sdhci at cd200000 {
> + cell-index = <2>;
> + compatible = "sirf,marco-sdhc";
> + reg = <0xcd200000 0x100000>;
> + interrupts = <0 23 0>;
> + status = "disabled";
> + };
> +
> + sd3: sdhci at cd300000 {
> + cell-index = <3>;
> + compatible = "sirf,marco-sdhc";
> + reg = <0xcd300000 0x100000>;
> + interrupts = <0 23 0>;
> + status = "disabled";
> + };
> +
> + sd4: sdhci at cd400000 {
> + cell-index = <4>;
> + compatible = "sirf,marco-sdhc";
> + reg = <0xcd400000 0x100000>;
> + interrupts = <0 39 0>;
> + status = "disabled";
> + };
> +
> + sd5: sdhci at cd500000 {
> + cell-index = <5>;
> + compatible = "sirf,marco-sdhc";
> + reg = <0xcd500000 0x100000>;
> + interrupts = <0 39 0>;
> + status = "disabled";
> + };
> +
> + pci-copy at cd900000 {
> + compatible = "sirf,marco-pcicp";
> + reg = <0xcd900000 0x100000>;
> + interrupts = <0 40 0>;
> + };
> +
> + rom-interface at cda00000 {
> + compatible = "sirf,marco-romif";
> + reg = <0xcda00000 0x100000>;
> + };
> + };
> + };
> +
> + rtc-iobg {
> + compatible = "sirf,marco-rtciobg", "sirf-marco-rtciobg-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + reg = <0xc1000000 0x10000>;
> +
> + gpsrtc at 1000 {
> + compatible = "sirf,marco-gpsrtc";
> + reg = <0x1000 0x1000>;
> + interrupts = <0 55 0>,
> + <0 56 0>,
> + <0 57 0>;
> + };
> +
> + sysrtc at 2000 {
> + compatible = "sirf,marco-sysrtc";
> + reg = <0x2000 0x1000>;
> + interrupts = <0 52 0>,
> + <0 53 0>,
> + <0 54 0>;
> + };
> +
> + pwrc at 3000 {
> + compatible = "sirf,marco-pwrc";
> + reg = <0x3000 0x1000>;
> + interrupts = <0 32 0>;
> + };
> + };
> +
> + uus-iobg {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0xce000000 0xce000000 0x1000000>;
> +
> + usb0: usb at ce000000 {
> + compatible = "chipidea,ci13611a-marco";
> + reg = <0xce000000 0x10000>;
> + interrupts = <0 10 0>;
> + };
> +
> + usb1: usb at ce010000 {
> + compatible = "chipidea,ci13611a-marco";
> + reg = <0xce010000 0x10000>;
> + interrupts = <0 11 0>;
> + };
> +
> + security at ce020000 {
> + compatible = "sirf,marco-security";
> + reg = <0xce020000 0x10000>;
> + interrupts = <0 42 0>;
> + };
> + };
> +
> + can-iobg {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0xd0000000 0xd0000000 0x1000000>;
> +
> + can0: can at d0000000 {
> + compatible = "sirf,marco-can";
> + reg = <0xd0000000 0x10000>;
> + };
> +
> + can1: can at d0010000 {
> + compatible = "sirf,marco-can";
> + reg = <0xd0010000 0x10000>;
> + };
> + };
> +
> + lvds-iobg {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0xd1000000 0xd1000000 0x1000000>;
> +
> + lvds at d1000000 {
> + compatible = "sirf,marco-lvds";
> + reg = <0xd1000000 0x10000>;
> + interrupts = <0 64 0>;
> + };
> + };
> + };
> +};
If you're able to deal with all that, you can add:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
For the compatible strings, it would be good if those devices which need
special treatment compared to prima2 (and thus can't fall back on the prima2
compatible strings) were listed in the commit message to aid review. Either
that or list the ones which are compatible, whichever list is shorter.
Thanks,
Mark.
^ permalink raw reply
* [PATCH v4 5/9] ARM: Tegra: Define Tegra30 CAR binding
From: Hiroshi Doyu @ 2013-01-16 11:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357890387-23245-6-git-send-email-pgaikwad@nvidia.com>
On Fri, 11 Jan 2013 08:46:23 +0100
Prashant Gaikwad <pgaikwad@nvidia.com> wrote:
> The device tree binding models Tegra30 CAR (Clock And Reset)
> as a single monolithic clock provider.
>
> Signed-off-by: Prashant Gaikwad <pgaikwad@nvidia.com>
> ---
> .../bindings/clock/nvidia,tegra30-car.txt | 262 ++++++++++++++++++++
> arch/arm/boot/dts/tegra30.dtsi | 6 +
> 2 files changed, 268 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra30-car.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra30-car.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra30-car.txt
> new file mode 100644
> index 0000000..121d203
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra30-car.txt
> @@ -0,0 +1,262 @@
> +NVIDIA Tegra30 Clock And Reset Controller
> +
> +This binding uses the common clock binding:
......
> + 215 cml0
> + 216 cml1
> + 217 hclk
> + 218 pclk
> +
> +Example SoC include file:
> +
> +/ {
> + tegra_car: clock {
> + compatible = "nvidia,tegra30-car";
> + reg = <0x60006000 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + usb at c5004000 {
> + clocks = <&tegra_car 58>; /* usb2 */
> + };
> +};
> +
> +Example board file:
> +
> +/ {
> + clocks {
> + compatible = "simple-bug";
typo, 's/simple-bug/simple-bus/' ?
^ permalink raw reply
* [PATCH 05/14] lib: Add I/O map cache implementation
From: Russell King - ARM Linux @ 2013-01-16 11:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130116101822.GA17706@avionic-0098.adnet.avionic-design.de>
On Wed, Jan 16, 2013 at 11:18:22AM +0100, Thierry Reding wrote:
> err = ioremap_page_range(virt, virt + SZ_64K - 1, phys,
Why -1 here?
^ permalink raw reply
* [PATCH V3] mmc: mmci: Fixup and cleanup code for DMA handling
From: Ulf Hansson @ 2013-01-16 11:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130116104947.GQ23505@n2100.arm.linux.org.uk>
On 16 January 2013 11:49, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Jan 16, 2013 at 11:23:57AM +0100, Ulf Hansson wrote:
>> On 13 January 2013 20:24, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Mon, Jan 07, 2013 at 03:58:27PM +0100, Ulf Hansson wrote:
>> >> @@ -374,19 +415,12 @@ static void mmci_dma_unmap(struct mmci_host *host, struct mmc_data *data)
>> >> * contiguous buffers. On TX, we'll get a FIFO underrun error.
>> >> */
>> >> if (status & MCI_RXDATAAVLBLMASK) {
>> >> - dmaengine_terminate_all(chan);
>> >> - if (!data->error)
>> >> - data->error = -EIO;
>> >> - }
>> >> -
>> >> - if (data->flags & MMC_DATA_WRITE) {
>> >> - dir = DMA_TO_DEVICE;
>> >> - } else {
>> >> - dir = DMA_FROM_DEVICE;
>> >> + data->error = -EIO;
>> >> + mmci_dma_data_error(host);
>> >
>> > Please explain the change of behaviour here. Before your change, we _only_
>> > set data->error if the error is not set. Here, we overwrite the error code
>> > no matter what. What is the reasoning for that change?
>> >
>> > The reason the code is like it _was_ is so that any bytes remaining in the
>> > FIFO are _only_ reported as an error if there wasn't a preceding error.
>> > That is the behaviour I desired when I wrote this code.
>>
>> Since we need to do dmaengine_terminate_all(chan), that will mean
>> another request could potentially already be prepared and thus it
>> could also be terminated.
>>
>> Then by always reporting an error the async request handling in the
>> mmc protocol layer, can do proper error handling and clean up the
>> previously prepared request.
>
> Sigh, I don't think you understand what this code is doing at all then.
>
> We will _always_ report _an_ error as the code originally stands if we
> encounter MCI_RXDATAAVLBLMASK being set. The error that we report will
> be the _correct_ one - either the one which preceeds this condition
> occuring _or_ an IO error because not all data was read from the FIFO
> by the DMA engine.
>
> If the DMA engine fails to read all the data from the FIFO, _and_ there
> was a preceeding error, we should _not_ overwrite the preceeding error.
> This is exactly what the original code does.
>
> Your version _always_ overwrites the previous error. This could result
> in FIFO/CRC errors always being reported as an IO error rather than
> their proper error codes, and therefore _breaking_ error handling.
Sorry, Russell I was talking junk. :-) Please ignore my last respond.
It shall be safe to keep the original behavior. The important thing is
that an error code get set, so the async request handling can clean up
a prepared request, which was already handled.
I will fixup the patch and send a v4, thanks a lot spotting this.
Kind regards
Ulf Hansson
^ permalink raw reply
* [PATCH 5/6] ARM: dts: Add basic dts include files for Samsung S3C64xx SoCs
From: Dave Martin @ 2013-01-16 10:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130114150531.GE21142@e102568-lin.cambridge.arm.com>
On Mon, Jan 14, 2013 at 03:05:32PM +0000, Lorenzo Pieralisi wrote:
> On Mon, Jan 14, 2013 at 02:48:41PM +0000, Mark Rutland wrote:
> > Hello,
> >
> > This all looks good. I just have a couple of comments about the cpus node.
> >
> > On Sun, Jan 13, 2013 at 01:10:57AM +0000, Tomasz Figa wrote:
> > > This patch adds basic device tree definitions for Samsung S3C64xx SoCs.
> > >
> > > Since all the SoCs in the series are very similar, the files are created
> > > hierarchically - one file for the whole series and then separate files
> > > for particular SoCs including the common one.
> > >
> > > Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com>
> >
> > [...]
> >
> > > diff --git a/arch/arm/boot/dts/s3c64xx.dtsi b/arch/arm/boot/dts/s3c64xx.dtsi
> > > new file mode 100644
> > > index 0000000..55d6e08
> > > --- /dev/null
> > > +++ b/arch/arm/boot/dts/s3c64xx.dtsi
> > > @@ -0,0 +1,97 @@
> > > +/*
> > > + * Samsung's S3C64xx SoC series common device tree source
> > > + *
> > > + * Copyright (c) 2013 Tomasz Figa <tomasz.figa@gmail.com>
> > > + *
> > > + * Samsung's S3C64xx SoC series device nodes are listed in this file.
> > > + * Particular SoCs from S3C64xx series can include this file and provide
> > > + * values for SoCs specfic bindings.
> > > + *
> > > + * Note: This file does not include device nodes for all the controllers in
> > > + * S3C64xx SoCs. As device tree coverage for S3C64xx increases, additional
> > > + * nodes can be added to this file.
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License version 2 as
> > > + * published by the Free Software Foundation.
> > > + */
> > > +
> > > +/include/ "skeleton.dtsi"
> > > +
> > > +/ {
> > > + cpus {
> > > + cpu at 0 {
> > > + compatible = "arm,arm1176jzf-s";
> > > + };
> > > + };
> >
> > You can drop the unit address from the cpu node - it's meant to be there to
> > differentiate multiple nodes (and is supposed to match the reg property, which
> > the 1176jzf-s can't have, as it doesn't have an MPIDR).
>
> Well, this is a point that I should consider since the kernel docs I wrote are
> misleading, they require the reg property that can not be there on UP.
> True, MPIDR does not exist in this case, but I have to document this in the
> bindings since it is unclear.
>
> >
> > Also, "arm,arm1176jzf-s" isn't listed in the binding doc. There was a question
> > about how to maintain this list [1], but I can't seem to find a conclusion, if
> > any were reached. It might be worth appending "arm,arm1176" to the compatible
> > list for the cpu node in case we want to enable something via dt for all 1176
> > variations.
> >
> > Dave, Lorenzo, any thoughts?
>
> Eh, frankly I do not know how to handle this. Either we add a compatible
> string to the bindings anytime a DT gets merged in the kernel but how
> to maintain it, it has to be defined. Happy to hear some feedback on
> this.
Well, the number of CPU types does not grow rapidly. It will be much
less than one per SoC -- so keeping the list up to date shouldn't be
that much effort.
For ARM1176JZF-S, it could make sense for the comatible list to be
"arm,arm1176jzf-s", "arm,arm1176"
...since the differences between 1176 variants are software probeable
(i.e., whether there is an FPU or not). AFAIK the J, Z apply to all
ARM1176, and the -S (synthesisable RTL) is nothing to do with software.
The kernel probably only really needs to know "arm,arm1176".
Cheers
---Dave
>
> Lorenzo
>
^ permalink raw reply
* [PATCH v6 00/12] iommu/exynos: Fixes and Enhancements of System MMU driver with DT
From: Will Deacon @ 2013-01-16 10:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <003301cde30b$c584c2d0$508e4870$%cho@samsung.com>
On Wed, Dec 26, 2012 at 01:53:15AM +0000, Cho KyongHo wrote:
> notice: v6 patch-set is rebased on next/iommu-exynos branch of
> linux-samsung.git. This patch-set does not include 2 patches (05 and 06
> patches in v5 patch-se) because they alread exist already in the branch.
Given that devicetree-discuss has been notably absent from discussion
surrounding the proposed binding, I think that including that patch in
linux-samsung.git and dropping it from this series is rather premature.
I have comments on the binding, so I'll dig up the version you posted in v5
and add devicetree-discuss to CC.
> The current exynos-iommu(System MMU) driver does not work autonomously
> since it is lack of support for power management of peripheral blocks.
> For example, MFC device driver must ensure that its System MMU is disabled
> before MFC block is power-down not to invalidate IOTLB in the System MMU
> when I/O memory mapping is changed. Because A System MMU is resides in the
> same H/W block, access to control registers of System MMU while the H/W
> block is turned off must be prohibited.
>
> This set of changes solves the above problem with setting each System MMUs
> as the parent of the device which owns the System MMU to recieve the
> information when the device is turned off or turned on.
>
> Another big change to the driver is the support for devicetree.
> The bindings for System MMU is described in
> Documentation/devicetree/bindings/arm/samsung/system-mmu.txt
This should probably be Documentation/devicetree/bindings/iommu/ no?
Will
^ permalink raw reply
* [PATCH V3] mmc: mmci: Fixup and cleanup code for DMA handling
From: Russell King - ARM Linux @ 2013-01-16 10:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFrV8h+2ebfT6AK-ACfr716CE-f7+HGuh_+rOjbVnGOdfw@mail.gmail.com>
On Wed, Jan 16, 2013 at 11:23:57AM +0100, Ulf Hansson wrote:
> On 13 January 2013 20:24, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Jan 07, 2013 at 03:58:27PM +0100, Ulf Hansson wrote:
> >> @@ -374,19 +415,12 @@ static void mmci_dma_unmap(struct mmci_host *host, struct mmc_data *data)
> >> * contiguous buffers. On TX, we'll get a FIFO underrun error.
> >> */
> >> if (status & MCI_RXDATAAVLBLMASK) {
> >> - dmaengine_terminate_all(chan);
> >> - if (!data->error)
> >> - data->error = -EIO;
> >> - }
> >> -
> >> - if (data->flags & MMC_DATA_WRITE) {
> >> - dir = DMA_TO_DEVICE;
> >> - } else {
> >> - dir = DMA_FROM_DEVICE;
> >> + data->error = -EIO;
> >> + mmci_dma_data_error(host);
> >
> > Please explain the change of behaviour here. Before your change, we _only_
> > set data->error if the error is not set. Here, we overwrite the error code
> > no matter what. What is the reasoning for that change?
> >
> > The reason the code is like it _was_ is so that any bytes remaining in the
> > FIFO are _only_ reported as an error if there wasn't a preceding error.
> > That is the behaviour I desired when I wrote this code.
>
> Since we need to do dmaengine_terminate_all(chan), that will mean
> another request could potentially already be prepared and thus it
> could also be terminated.
>
> Then by always reporting an error the async request handling in the
> mmc protocol layer, can do proper error handling and clean up the
> previously prepared request.
Sigh, I don't think you understand what this code is doing at all then.
We will _always_ report _an_ error as the code originally stands if we
encounter MCI_RXDATAAVLBLMASK being set. The error that we report will
be the _correct_ one - either the one which preceeds this condition
occuring _or_ an IO error because not all data was read from the FIFO
by the DMA engine.
If the DMA engine fails to read all the data from the FIFO, _and_ there
was a preceeding error, we should _not_ overwrite the preceeding error.
This is exactly what the original code does.
Your version _always_ overwrites the previous error. This could result
in FIFO/CRC errors always being reported as an IO error rather than
their proper error codes, and therefore _breaking_ error handling.
^ permalink raw reply
* [PATCH 0/5] W1: Support onewire master on i.MX53
From: Sascha Hauer @ 2013-01-16 10:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130116094821.9660.18278.stgit@localhost>
Hi Martin,
On Wed, Jan 16, 2013 at 10:48:21AM +0100, Martin Fuzzey wrote:
> W1: Support onewire master on i.MX53
> * Add device tree and pinctrl support to the MXC master driver
> * Add i.MX53 clocks
> * Add i.MX53 device tree entries
>
> The final patch converts the driver to use devm_
Looks mostly good. I think the patch converting to devm_ should come
before the one adding pinctrl for the driver, just because the driver
should first be switched to devm_ and afterwards the devm_ usage should
be extended.
Other than that I just noticed that the clk_prepare_enable/clk_disable_unprepare
is unbalanced in the error case of the driver (clk_disable_unprepare
missing after the failed_add label). Might be worth fixing this in this series.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [kvmarm] [PATCH v5 06/14] KVM: ARM: Inject IRQs and FIQs from userspace
From: Gleb Natapov @ 2013-01-16 10:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F582E9.40402@suse.de>
On Tue, Jan 15, 2013 at 05:25:13PM +0100, Alexander Graf wrote:
> On 01/15/2013 04:17 PM, Gleb Natapov wrote:
> >On Tue, Jan 15, 2013 at 02:04:47PM +0000, Peter Maydell wrote:
> >>On 15 January 2013 12:52, Gleb Natapov<gleb@redhat.com> wrote:
> >>>On Tue, Jan 15, 2013 at 12:15:01PM +0000, Peter Maydell wrote:
> >>>>On 15 January 2013 09:56, Gleb Natapov<gleb@redhat.com> wrote:
> >>>>>>ARM can signal an interrupt either at the CPU level, or at the in-kernel irqchip
> >>>>>CPU level interrupt should use KVM_INTERRUPT instead.
> >>>>No, that would be wrong. KVM_INTERRUPT is for interrupts which must be
> >>>>delivered synchronously to the CPU. KVM_IRQ_LINE is for interrupts which
> >>>>can be fed to the kernel asynchronously. It happens that on x86 "must be
> >>>>delivered synchronously" and "not going to in kernel irqchip" are the same, but
> >>>>this isn't true for other archs. For ARM all our interrupts can be fed
> >>>>to the kernel asynchronously, and so we use KVM_IRQ_LINE in all
> >>>>cases.
> >>>I do no quite understand what you mean by synchronously and
> >>>asynchronously.
> >>Synchronously: the vcpu has to be stopped and userspace then
> >>feeds in the interrupt to be taken when the guest is resumed.
> >>Asynchronously: any old thread can tell the kernel there's an
> >>interrupt, and the guest vcpu then deals with it when needed
> >>(the vcpu thread may leave the guest but doesn't come out of
> >>the host kernel to qemu).
> >>
> >>>The difference between KVM_INTERRUPT and KVM_IRQ_LINE line
> >>>is that former is used when destination cpu is known to userspace later
> >>>is used when kernel code is involved in figuring out the destination.
> >>This doesn't match up with Avi's explanation at all.
> >>
> >>>The
> >>>injections themselves are currently synchronous for both of them on x86
> >>>and ARM. i.e vcpu is kicked out from guest mode when interrupt need to
> >>>be injected into a guest and vcpu state is changed to inject interrupt
> >>>during next guest entry. In the near feature x86 will be able to inject
> >>>interrupt without kicking vcpu out from the guest mode does ARM plan to
> >>>do the same? For GIC interrupts or for IRQ/FIQ or for both?
> >>>
> >>>>There was a big discussion thread about this on kvm and qemu-devel last
> >>>>July (and we cleaned up some of the QEMU code to not smoosh together
> >>>>all these different concepts under "do I have an irqchip or not?").
> >>>Do you have a pointer?
> >> http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02460.html
> >>and there was a later longer (but less clear) thread which included
> >>this mail from Avi:
> >> http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02872.html
> >>basically explaining that the reason for the weird synchronous
> >>KVM_INTERRUPT API is that it's emulating a weird synchronous
> >>hardware interface which is specific to x86. ARM doesn't have
> >>a synchronous interface in the same way, so it's much more
> >>straightforward to use KVM_IRQ_LINE.
> >>
> >OK. I see. So basically Avi saw KVM_INTERRUPT as an oddball interface
> >required only for APIC emulation in userspace. It is used for PIC also,
> >where this is not strictly needed, but this is for historical reasons
> >(KVM_IRQ_LINE was introduces late and it is GSI centric on x86).
> >
> >Thank you for the pointer.
>
> Yeah, please keep in mind that KVM_INTERRUPT is not a unified
> interface either. In fact, it is asynchronous on PPC :). And it's
> called KVM_S390_INTERRUPT on s390 and also asynchronous. X86 is the
> oddball here.
>
KVM_INTERRUPT needs vcpu fd to be issues. Usually such ioctls are
issued only by vcpu thread which makes them synchronous and vcpu_load()
synchronise them anyway if the rule is not met. And sure enough those
KVM_S390_INTERRUPT/KVM_INTERRUPT are special cased in kvm_vcpu_ioctl()
to not call vcpu_load(), sigh :(
There was an idea to change vcpu ioctls to kvm syscall which would have
made it impossible to use KVM_INTERRUPT asynchronously.
> But I don't care whether we call the ioctl to steer CPU interrupt
> pins KVM_INTERRUPT, KVM_S390_INTERRUPT or KVM_IRQ_LINE, as long as
> the code makes it obvious what is happening.
>
Some consistency would be nice though. You do not always look at the
kernel code when you read userspace code and iothread calling KVM_INTERRUPT
would have made me suspicious.
--
Gleb.
^ permalink raw reply
* [PATCH v2 1/1] block: blk-merge: don't merge the pages with non-contiguous descriptors
From: James Bottomley @ 2013-01-16 10:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F64AC1.3040304@codeaurora.org>
On Wed, 2013-01-16 at 12:07 +0530, Subhash Jadavani wrote:
> Now consider this call stack from MMC block driver (this is on the ARmv7
> based board):
> [ 98.918174] [<c001b50c>] (v7_dma_inv_range+0x30/0x48) from
> [<c0017b8c>] (dma_cache_maint_page+0x1c4/0x24c)
> [ 98.927819] [<c0017b8c>] (dma_cache_maint_page+0x1c4/0x24c) from
> [<c0017c28>] (___dma_page_cpu_to_dev+0x14/0x1c)
> [ 98.937982] [<c0017c28>] (___dma_page_cpu_to_dev+0x14/0x1c) from
> [<c0017ff8>] (dma_map_sg+0x3c/0x114)
OK, so this is showing that ARM itself is making the assumption that the
pages are contiguous in the page offset map.
Fix this by doing the increment via the pfn, which will do the right
thing whatever the memory model.
Signed-off-by: James Bottomley <JBottomley@Parallels.com>
---
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 6b2fb87..ab88c5b 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -809,7 +809,7 @@ static void dma_cache_maint_page(struct page *page, unsigned long offset,
op(vaddr, len, dir);
}
offset = 0;
- page++;
+ page = pfn_to_page(page_to_pfn(page) + 1);
left -= len;
} while (left);
}
^ 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