* [PATCH 3/4 v7] iommu/fsl: Add iommu domain attributes required by fsl PAMU driver.
From: Varun Sethi @ 2012-12-14 13:51 UTC (permalink / raw)
To: joerg.roedel, iommu, linuxppc-dev, linux-kernel, timur, scottwood
Cc: Varun Sethi
In-Reply-To: <1355493114-21776-1-git-send-email-Varun.Sethi@freescale.com>
Added the following domain attributes required by FSL PAMU driver:
1. Subwindows field added to the iommu domain geometry attribute.
2. Added new iommu stash attribute, which allows setting of the
LIODN specific stash id parameter through IOMMU API.
3. Added an attribute for enabling/disabling DMA to a particular
memory window.
4. Added max_subwindows field to the geometry attribute. This is
used to determine the maximum number sub windows available
for the geometry.
Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
---
changes in v7:
- Added max_subwindows field to the geometry attribute.
changes in v5:
- Updated description of the subwindows field.
changes in v4:
- Updated comment explaining subwindows(as mentioned by Scott).
change in v3:
-renamed the stash attribute targets
include/linux/iommu.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 49 insertions(+), 0 deletions(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index f3b99e1..01ca1de 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -44,6 +44,47 @@ struct iommu_domain_geometry {
dma_addr_t aperture_start; /* First address that can be mapped */
dma_addr_t aperture_end; /* Last address that can be mapped */
bool force_aperture; /* DMA only allowed in mappable range? */
+
+ /*
+ * A geometry mapping can be created in one of the following ways
+ * for an IOMMU:
+ * 1. A single contiguous window
+ * 2. Through arbritary paging throughout the aperture.
+ * 3. Using multiple subwindows
+ *
+ * In absence of arbritary paging, subwindows allow for supporting
+ * physically discontiguous mappings.
+ *
+ * This attribute indicates number of DMA subwindows supported by
+ * the geometry. If there is a single window that maps the entire
+ * geometry, attribute must be set to "1". A value of "0" implies
+ * that this mechanism is not used at all(normal paging is used).
+ * Value other than* "0" or "1" indicates the actual number of
+ * subwindows.
+ */
+ u32 subwindows;
+ /*
+ * This is a read only field and indicates the maximum number of
+ * subwindows that are permitted for the geometry. The user is
+ * not allowed to write to this field.
+ */
+ u32 max_subwindows;
+};
+
+/* cache stash targets */
+#define IOMMU_ATTR_CACHE_L1 1
+#define IOMMU_ATTR_CACHE_L2 2
+#define IOMMU_ATTR_CACHE_L3 3
+
+/* This attribute corresponds to IOMMUs capable of generating
+ * a stash transaction. A stash transaction is typically a
+ * hardware initiated prefetch of data from memory to cache.
+ * This attribute allows configuring stashig specific parameters
+ * in the IOMMU hardware.
+ */
+struct iommu_stash_attribute {
+ u32 cpu; /* cpu number */
+ u32 cache; /* cache to stash to: L1,L2,L3 */
};
struct iommu_domain {
@@ -60,6 +101,14 @@ struct iommu_domain {
enum iommu_attr {
DOMAIN_ATTR_MAX,
DOMAIN_ATTR_GEOMETRY,
+ /* Set the IOMMU hardware stashing
+ * parameters.
+ */
+ DOMAIN_ATTR_STASH,
+ /* Explicity enable/disable DMA for a
+ * particular memory window.
+ */
+ DOMAIN_ATTR_ENABLE,
};
#ifdef CONFIG_IOMMU_API
--
1.7.4.1
^ permalink raw reply related
* [PATCH 0/4] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
From: Varun Sethi @ 2012-12-14 13:51 UTC (permalink / raw)
To: joerg.roedel, iommu, linuxppc-dev, linux-kernel, timur, scottwood
Cc: Varun Sethi
This patchset provides the Freescale PAMU (Peripheral Access Management Unit) driver
and the corresponding IOMMU API implementation. PAMU is the IOMMU present on Freescale
QorIQ platforms. PAMU can authorize memory access, remap the memory address, and remap
the I/O transaction type.
This set consists of the following patches:
1. Addition of new field in the device (powerpc) archdata structure for storing iommu domain information
pointer. This pointer is stored when the device is attached to a particular iommu domain.
2. Add PAMU bypass enable register to the ccsr_guts structure.
3. Addition of domain attributes required by the PAMU driver IOMMU API.
4. PAMU driver and IOMMU API implementation.
This patch set is based on the next branch of the iommu git tree maintained by Joerg.
Varun Sethi (4):
store iommu domain info in device arch data.
add pamu bypass enable register to guts.
Add iommu attributes for PAMU
FSL PAMU driver.
arch/powerpc/include/asm/device.h | 4 +
arch/powerpc/include/asm/fsl_guts.h | 4 +-
drivers/iommu/Kconfig | 8 +
drivers/iommu/Makefile | 1 +
drivers/iommu/fsl_pamu.c | 1152 +++++++++++++++++++++++++++++++++++
drivers/iommu/fsl_pamu.h | 398 ++++++++++++
drivers/iommu/fsl_pamu_domain.c | 1033 +++++++++++++++++++++++++++++++
drivers/iommu/fsl_pamu_domain.h | 96 +++
include/linux/iommu.h | 49 ++
9 files changed, 2744 insertions(+), 1 deletions(-)
create mode 100644 drivers/iommu/fsl_pamu.c
create mode 100644 drivers/iommu/fsl_pamu.h
create mode 100644 drivers/iommu/fsl_pamu_domain.c
create mode 100644 drivers/iommu/fsl_pamu_domain.h
--
1.7.4.1
^ permalink raw reply
* [PATCH 1/4 v7] iommu/fsl: Store iommu domain information pointer in archdata.
From: Varun Sethi @ 2012-12-14 13:51 UTC (permalink / raw)
To: joerg.roedel, iommu, linuxppc-dev, linux-kernel, timur, scottwood
Cc: Varun Sethi
In-Reply-To: <1355493114-21776-1-git-send-email-Varun.Sethi@freescale.com>
Add a new field in the device (powerpc) archdata structure for storing iommu domain
information pointer. This pointer is stored when the device is attached to a particular
domain.
Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
---
- no change in this version
arch/powerpc/include/asm/device.h | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/include/asm/device.h b/arch/powerpc/include/asm/device.h
index 77e97dd..6dc79fe 100644
--- a/arch/powerpc/include/asm/device.h
+++ b/arch/powerpc/include/asm/device.h
@@ -28,6 +28,10 @@ struct dev_archdata {
void *iommu_table_base;
} dma_data;
+ /* IOMMU domain information pointer. This would be set
+ * when this device is attached to an iommu_domain.
+ */
+ void *iommu_domain;
#ifdef CONFIG_SWIOTLB
dma_addr_t max_direct_dma_addr;
#endif
--
1.7.4.1
^ permalink raw reply related
* PS3 platform is broken on Linux 3.7.0
From: Phileas Fogg @ 2012-12-14 12:35 UTC (permalink / raw)
To: linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 2304 bytes --]
Hi,
I wanted to bring to your attention the fact that the PS3 platform is broken on Linux 3.7.0.
i'm not able to boot Linux 3.7.0 on my PS3 slim. Linux 3.6.10 boots just fine but not 3.7.0
When i try to boot Linux 3.7.0 then my PS3 shuts down.
So i cloned the Linux powerpc GIT repository and tried to find out which commits broke the PS3 platform.
After some time I tracked it down to 2 commits:
---------------------------------------------------------------------------------------------
commit 407821a34fce89b4f0b031dbab5cec7d059f46bc
Author: Michael Ellerman <michael@ellerman.id.au>
Date: Fri Sep 7 15:31:44 2012 +0000
powerpc: Initialise paca.data_offset with poison
It's possible for the cpu_possible_mask to change between the time we
initialise the pacas and the time we setup per_cpu areas.
Obviously impossible cpus shouldn't ever be running, but stranger things
have happened. So be paranoid and initialise data_offset with a poison
value in case we don't set it up later.
Based on a patch from Anton Blanchard.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
------------------------------------------------------------------------------------------------
commit 048ee0993ec8360abb0b51bdf8f8721e9ed62ec4
Author: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Date: Mon Sep 10 02:52:55 2012 +0000
powerpc/mm: Add 64TB support
Increase max addressable range to 64TB. This is not tested on
real hardware yet.
Reviewed-by: Paul Mackerras <paulus@samba.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
----------------------------------------------------------------------------------------------------------
The first commit causes my PS3 to shut down. If i revert it then i'm able to boot Linux 3.7.0 and even see some boot messages
on my screen. But then it hangs. The second commit is the reason for the hang as i figured it out.
I reverted both commits in current Linux 3.7.0 and was able to boot Linux 3.7.0 on my PS3 slim successfully.
Regards
[-- Attachment #2: Type: text/html, Size: 2829 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 00/11] Hot-plug and Online/Offline framework
From: Toshi Kani @ 2012-12-14 1:51 UTC (permalink / raw)
To: Greg KH
Cc: linux-s390, jiang.liu@huawei.com, wency@cn.fujitsu.com,
linux-mm@kvack.org, linuxppc-dev, linux-kernel@vger.kernel.org,
rjw@sisk.pl, linux-acpi@vger.kernel.org,
isimatu.yasuaki@jp.fujitsu.com, srivatsa.bhat@linux.vnet.ibm.com,
guohanjun@huawei.com, bhelgaas@google.com,
akpm@linux-foundation.org, yinghai@kernel.org, lenb@kernel.org
In-Reply-To: <20121213183021.GC9606@kroah.com>
On Thu, 2012-12-13 at 10:30 -0800, Greg KH wrote:
> On Thu, Dec 13, 2012 at 09:03:54AM -0700, Toshi Kani wrote:
> > On Thu, 2012-12-13 at 04:16 +0000, Greg KH wrote:
> > > On Wed, Dec 12, 2012 at 08:37:44PM -0700, Toshi Kani wrote:
> > > > On Wed, 2012-12-12 at 16:55 -0800, Greg KH wrote:
> > > > > On Wed, Dec 12, 2012 at 05:39:36PM -0700, Toshi Kani wrote:
> > > > > > On Wed, 2012-12-12 at 15:56 -0800, Greg KH wrote:
> > > > > > > On Wed, Dec 12, 2012 at 04:17:12PM -0700, Toshi Kani wrote:
> > > > > > > > This patchset is an initial prototype of proposed hot-plug framework
> > > > > > > > for design review. The hot-plug framework is designed to provide
> > > > > > > > the common framework for hot-plugging and online/offline operations
> > > > > > > > of system devices, such as CPU, Memory and Node. While this patchset
> > > > > > > > only supports ACPI-based hot-plug operations, the framework itself is
> > > > > > > > designed to be platform-neural and can support other FW architectures
> > > > > > > > as necessary.
> > > > > > > >
> > > > > > > > The patchset has not been fully tested yet, esp. for memory hot-plug.
> > > > > > > > Any help for testing will be very appreciated since my test setup
> > > > > > > > is limited.
> > > > > > > >
> > > > > > > > The patchset is based on the linux-next branch of linux-pm.git tree.
> > > > > > > >
> > > > > > > > Overview of the Framework
> > > > > > > > =========================
> > > > > > >
> > > > > > > <snip>
> > > > > > >
> > > > > > > Why all the new framework, doesn't the existing bus infrastructure
> > > > > > > provide everything you need here? Shouldn't you just be putting your
> > > > > > > cpus and memory sticks on a bus and handle stuff that way? What makes
> > > > > > > these types of devices so unique from all other devices that Linux has
> > > > > > > been handling in a dynamic manner (i.e. hotplugging them) for many many
> > > > > > > years?
> > > > > > >
> > > > > > > Why are you reinventing the wheel?
> > > > > >
> > > > > > Good question. Yes, USB and PCI hotplug operate based on their bus
> > > > > > structures. USB and PCI cards only work under USB and PCI bus
> > > > > > controllers. So, their framework can be composed within the bus
> > > > > > structures as you pointed out.
> > > > > >
> > > > > > However, system devices such CPU and memory do not have their standard
> > > > > > bus. ACPI allows these system devices to be enumerated, but it does not
> > > > > > make ACPI as the HW bus hierarchy for CPU and memory, unlike PCI and
> > > > > > USB. Therefore, CPU and memory modules manage CPU and memory outside of
> > > > > > ACPI. This makes sense because CPU and memory can be used without ACPI.
> > > > > >
> > > > > > This leads us an issue when we try to manage system device hotplug
> > > > > > within ACPI, because ACPI does not control everything. This patchset
> > > > > > provides a common hotplug framework for system devices, which both ACPI
> > > > > > and non-ACPI modules (i.e. CPU and memory modules) can participate and
> > > > > > are coordinated for their hotplug operations. This is analogous to the
> > > > > > boot-up sequence, which ACPI and non-ACPI modules can participate to
> > > > > > enable CPU and memory.
> > > > >
> > > > > Then create a "virtual" bus and put the devices you wish to control on
> > > > > that. That is what the "system bus" devices were supposed to be, it's
> > > > > about time someone took that code and got it all working properly in
> > > > > this way, that is why it was created oh so long ago.
> > > >
> > > > It may be the ideal, but it will take us great effort to make such
> > > > things to happen based on where we are now. It is going to be a long
> > > > way. I believe the first step is to make the boot-up flow and hot-plug
> > > > flow consistent for system devices. This is what this patchset is
> > > > trying to do.
> > >
> > > If you use the system "bus" for this, the "flow" will be identical, that
> > > is what the driver core provides for you. I don't see why you need to
> > > implement something that sits next to it and not just use what we
> > > already have here.
> >
> > Here is very brief boot-up flow.
> >
> > start_kernel()
> > boot_cpu_init() // init cpu0
> > setup_arch()
> > x86_init.paging.pagetable_init() // init mem pagetable
> > :
> > kernel_init()
> > kernel_init_freeable()
> > smp_init() // init other CPUs
> > :
> > do_basic_setup()
> > driver_init()
> > cpu_dev_init() // build system/cpu tree
> > memory_dev_init() // build system/memory tree
> > do_initcalls()
> > acpi_init() // build ACPI device tree
> >
> > CPU and memory are initialized at early boot. The system device tree is
> > built at the last step of the boot sequence and is only used for
> > providing sysfs interfaces.
>
> Then fix that and create the system device tree earlier.
# I added ppc and s390 to the list as you suggested... and because
# I may be wrong in my code reading... This thread is:
# https://lkml.org/lkml/2012/12/13/452
I looked at s390 and powerpc hotplug code. Their boot flow is
consistent as above since most of the funcs above are actually common.
For hotplug, pSeries (powerpc) supports CPU, Memory and I/O hotplug.
s390 seems to support less capability, so I looked at pSeries mostly.
pSeries supports DLPAR, and all hotplug code is put under
pSeries-specific code, such as arch/powerpc/platforms/pseries/dlpar.c.
The code is implemented outside of the OF module. Therefore, I think
the OF module itself works consistently at boot and hot-add. It has its
own hotplug framework with pSeries_reconfig_chain, which calls all
registered handlers for reconfig events.
So, it looks to me that pSeries has somewhat a similar framework, but
put everything under pSeries specific code. pSeries and s390 hotplug
implementations do not use the bus structure you suggested, either.
> > That is, the system bus structure has nothing to do with the actual
> > CPU and memory initialization at boot.
>
> Then that should be fixed, right?
The boot-up sequence is shared by all architectures, and it is nearly
impossible to make such changes to work on all architectures.
> > Similarly, ACPI drivers do not initialize actual CPU and memory at boot
> > as they are also called at the last step.
>
> That should also probably be fixed, right?
Since the boot flow is consistent for all architectures, I do not think
other FW modules initialize CPU and memory, either.
> > Further, the ACPI device tree and system bus tree are separate
> > entities.
>
> That's because ACPI seems to be getting crazy these days, and creating
> lots of different devices and tieing it back into the existing device
> trees. Which is fine, see how it's being done with USB for one example
> of how this can be done correctly, _if_ you want to keep them separate
> (doing so is your own choice, nothing that I'm saying is necessary.)
>
> > Hotplug events are sent to ACPI.
>
> Your hotplug events are being sent there, that's your decision to do so,
> it doesn't happen that way with other subsystems that get hotplug events
> from ACPI (i.e. PCI hotplug, right?)
ACPICA sends a notification to an ACPI notify handler, so it has to go
with ACPI. The "system/cpu" and "system/memory" sysfs drivers are
platform neutral and do not depend on ACPI. PCIe hotplug is based on
PCIe spec, and does not use ACPI. ACPI PCI hotplug (for PCIx) uses ACPI
and its notification is sent to an ACPI notify handler.
> > In order to keep the boot flow and hotplug flow consistent, I believe
> > the first step is to keep the role of modules consistent between boot
> > and hotplug.
>
> I agree, see above for how to resolve that :)
>
> > For instance, acpi_init() only builds ACPI tree at boot, so ACPI
> > should only build ACPI tree at hot-add as well. This keeps ACPI
> > drivers to do the same for both boot and hot-add.
>
> Agreed.
Great! Yes, we just need to agree on a solution. :)
> > The framework is designed to provide the consistency along with other
> > high-availability features such as rollback.
>
> I want my tiny, USB-powered device to have "high-availability", don't
> think of that type of functionality as somehow being special, it's what
> we have been doing with other subsystems for _years_ now.
>
> Again, I think if you properly tie the system bus code into the CPU work
> at the correct location, you can achieve everything you need. I base
> this on the fact that this is what other subsystems and architectures
> have been doing for years. Just because this is ACPI is no reason to
> think that it needs to be done differently.
>
> Odds are, s390 has been doing this for 10+ years and none of us realize
> this, they are usually that far ahead of the curve if history is any
> lesson.
I looked at pSeries and s390, and they have their own framework. Their
cases are also unique because their implementations are tied with
specific platforms / products. In high-level, however, their approach
is similar to mine. I have also worked on hotplug for years on other
OS, so I am not coming from nowhere, either. :)
If your concern is having a common hotplug framework, I can try to
address it by putting this framework under ACPI. This makes it less
capable/cleaner, but it keeps it within ACPI. That makes it more
similar to how pSeries and s390 did.
Thanks,
-Toshi
^ permalink raw reply
* Re: Understanding how kernel updates MMU hash table
From: Benjamin Herrenschmidt @ 2012-12-13 21:48 UTC (permalink / raw)
To: pegasus; +Cc: linuxppc-dev
In-Reply-To: <1355388520814-67313.post@n7.nabble.com>
On Thu, 2012-12-13 at 00:48 -0800, pegasus wrote:
> 1. Linux page table structure (PGD, PUD, PMD and PTE) is directly used in
> case of architecture that lend themselves to such a tree structure for
> maintaining virtual memory information. Otherwise Linux needs to maintain
> two seperate constructs like it does in case of PowerPC. Right?
Linux always maintains a tree structure, it can be 2, 3 or 4 levels, and
there's some flexibility on the actual details of the structure and PTE
format. If that can be made to match a HW construct, then it's used
directly (x86, ARM), else, there's some other mechanism to load the HW
construct.
I believe some sparcs have some kind of hash table as well (though a
different one).
> 2. PowerPC's hash table as you said is pretty large. However isn't it still
> smaller than Linux's VM infrastructure such that the chances of it being
> 'FULL' are a lot more. It is also possible that there could be two entries
> in the table that points to the same Real address. Like a page being shared
> by two processes?
Yes and yes.
> My main concern here is to understand if having such an inverted page table
> aka the hash table helps us in any way when doing TLB flushes. You mentioned
> and I also read in a paper by Paul Mackerras that every Linux PTE (LPTE) in
> case of ppc64 contains 4 extra bits that help us to get to the very slot in
> the hash table that houses the corresponding hashtable PTE (HPTE). Now this
> (at least to me) is smartness on the part of the kernel and I do not think
> the architecture per se is doing us any favor by having that hash table
> right? Or am I missing something here?
Right.
> His paper is (or rather was) on how one can optimize the Linux ppc kernel
> and time and again he mentions the fact that one can first record the LPTEs
> being invalidated and then remove the corresponding HPTEs in a batched
> format. In his own words "Alternatively, it would be possible to make a list
> of virtual addresses when LPTEs are changed and then use that list in the
> TLB flush routines to avoid the search through the Linux page tables". So do
> we skip looking for the corresponding LPTEs or perhaps we've already
> invalidated them and we remove the corresponding HPTEs in a batch as you
> mentioned earlier?? Could you shed some light on how this optimization
> actually developed over time?
Currently we batch within arch_lazy_mmu sections. We do that because we
require a batch to be fully contained within a page table spinlock
section, ie, we must guarantee that we have performed the hash
invalidations before there's a chance that a new PTE for that same VA
gets faulted in (or we would run the risk of creating duplicates in the
hash which is fatal).
For the details, I'd say look at the code (and not 2.6.10, that's quite
uninteresting).
> He had results for an "immediate update"
> kernel
> and "batched update" kernel for both ppc32 and ppc64. For ppc32 the batched
> update is actually a bit worse than immediate update however for ppc64, the
> batched update performs better than immediate update. What exactly is
> helping ppc64 perform better with the so called "batched update"? Is it the
> encoding of the HPTE address in the LPTE as mentioned above? Or some aspect
> of ppc64 that I am unaware of?
Possibly the fact that we know which slot which means we don't search.
> Also on a generic note, how come we have 4 spare bits in the PTE for 64bit
> address space? Large pages perhaps?
We don't exploit the entire 64-bit address space. Up until recently we
only gave 16T to processes though we just bumped that a bit.
Cheers,
Ben.
>
>
> --
> View this message in context: http://linuxppc.10917.n7.nabble.com/Understanding-how-kernel-updates-MMU-hash-table-tp59509p67313.html
> Sent from the linuxppc-dev mailing list archive at Nabble.com.
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: Build regressions/improvements in v3.7
From: Geert Uytterhoeven @ 2012-12-13 19:27 UTC (permalink / raw)
To: linux-kernel; +Cc: the arch/x86 maintainers, linuxppc-dev
In-Reply-To: <1355426667-3060-1-git-send-email-geert@linux-m68k.org>
On Thu, Dec 13, 2012 at 8:24 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> JFYI, when comparing v3.7 to v3.7-rc8[3], the summaries are:
> - build errors: +2/-0
+ error: No rule to make target include/config/auto.conf: => N/A
x86_64-randconfig
+ error: vga16fb.c: undefined reference to `vgacon_remap_base': =>
.devinit.text+0x16b52), .devinit.text+0x16b5a)
powerpc-randconfig
> [1] http://kisskb.ellerman.id.au/kisskb/head/5700/ (all 117 configs)
> [3] http://kisskb.ellerman.id.au/kisskb/head/5670/ (all 117 configs)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [RFC PATCH] powerpc/fsl: add timer wakeup source
From: Tabi Timur-B04825 @ 2012-12-13 15:51 UTC (permalink / raw)
To: Wang Dongsheng
Cc: Wood Scott-B07421, Wang Dongsheng-B40534,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1349260948-15828-1-git-send-email-dongsheng.wds@gmail.com>
On Wed, Oct 3, 2012 at 5:42 AM, Wang Dongsheng <dongsheng.wds@gmail.com> wr=
ote:
>
>
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
Wang,
Your patches must always have a From: line of your Freescale email
address. Do not use your gmail.com address to send patches.
This needs to be fixed when this patch is applied.
--=20
Timur Tabi
Linux kernel developer at Freescale=
^ permalink raw reply
* Re: [PATCH] powerpc/mpic: add global timer support
From: Tabi Timur-B04825 @ 2012-12-13 15:49 UTC (permalink / raw)
To: Wang Dongsheng
Cc: Wood Scott-B07421, Wang Dongsheng-B40534,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1349260710-15718-1-git-send-email-dongsheng.wds@gmail.com>
On Wed, Oct 3, 2012 at 5:38 AM, Wang Dongsheng <dongsheng.wds@gmail.com> wr=
ote:
> diff --git a/arch/powerpc/include/asm/mpic_timer.h b/arch/powerpc/include=
/asm/mpic_timer.h
> new file mode 100644
> index 0000000..2428972
> --- /dev/null
> +++ b/arch/powerpc/include/asm/mpic_timer.h
> @@ -0,0 +1,39 @@
> +/*
> + * arch/powerpc/include/asm/mpic_timer.h
> + *
> + * Mpic Global Timer Header
> + *
> + * Copyright 2012 Freescale Semicondutor, Inc.
When this patch is applied, please fix the misspelling of "Semiconductor".
--=20
Timur Tabi
Linux kernel developer at Freescale=
^ permalink raw reply
* Re: [TRIVIAL PATCH 11/26] powerpc: Convert print_symbol to %pSR
From: Arnd Bergmann @ 2012-12-13 11:58 UTC (permalink / raw)
To: Joe Perches
Cc: cbe-oss-dev, Jiri Kosina, linux-kernel, Paul Mackerras,
linuxppc-dev
In-Reply-To: <8c901014a0f571011023ed98b9a22bc690925b73.1355335228.git.joe@perches.com>
On Wednesday 12 December 2012, Joe Perches wrote:
> Use the new vsprintf extension to avoid any possible
> message interleaving.
>
> Convert the #ifdef DEBUG block to a single pr_debug.
>
> Signed-off-by: Joe Perches <joe@perches.com>
nice cleanup!
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply
* Re: [PATCH] powerpc+of: Rename and fix OF reconfig notifier error inject module
From: Akinobu Mita @ 2012-12-13 11:05 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Stephen Rothwell, linuxppc-dev
In-Reply-To: <1355353492.19932.59.camel@pasglop>
2012/12/13 Benjamin Herrenschmidt <benh@kernel.crashing.org>:
> This module used to inject errors in the pSeries specific dynamic
> reconfiguration notifiers. Those are gone however, replaced by
> generic notifiers for changes to the device-tree. So let's update
> the module to deal with these instead and rename it along the way.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Looks good.
Acked-by: Akinobu Mita <akinobu.mita@gmail.com>
^ permalink raw reply
* Re: [PATCH] pci: Provide support for parsing PCI DT ranges property
From: Andrew Murray @ 2012-12-13 10:34 UTC (permalink / raw)
To: Thierry Reding
Cc: Michal Simek, linux-pci@vger.kernel.org, devicetree-discuss,
Liviu Dudau, rob.herring@calxeda.com, Rob Herring, linuxppc-dev
In-Reply-To: <20121213100318.GB13055@avionic-0098.adnet.avionic-design.de>
On Thu, Dec 13, 2012 at 10:03:18AM +0000, Thierry Reding wrote:
> On Thu, Dec 13, 2012 at 09:45:43AM +0000, Andrew Murray wrote:
> > On Thu, Dec 13, 2012 at 09:13:33AM +0000, Thierry Reding wrote:
> > > Hi Andrew,
> > >=20
> > > I don't like iterator interfaces too much, but I can live with that.
> > > Other than that the patch looks good to me and I'll try to work it in=
to
> > > my Tegra PCIe patch series.
> > >=20
> > > Just two minor comments below.
> > >=20
> > > > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > > [...]
> > > > @@ -421,7 +472,7 @@ u64 __of_translate_address(struct device_node *=
dev, const __be32 *in_addr,
> > > > =09=09goto bail;
> > > > =09bus =3D of_match_bus(parent);
> > > > =20
> > > > -=09/* Cound address cells & copy address locally */
> > > > +=09/* Count address cells & copy address locally */
> > > > =09bus->count_cells(dev, &na, &ns);
> > > > =09if (!OF_CHECK_COUNTS(na, ns)) {
> > > > =09=09printk(KERN_ERR "prom_parse: Bad cell count for %s\n",
> > >=20
> > > This is really minor, but it should still go into a separate patch.
> > >=20
> > > > diff --git a/include/linux/of_address.h b/include/linux/of_address.=
h
> > > > index 01b925a..4582b20 100644
> > > > --- a/include/linux/of_address.h
> > > > +++ b/include/linux/of_address.h
> > > > @@ -26,6 +26,8 @@ static inline unsigned long pci_address_to_pio(ph=
ys_addr_t addr) { return -1; }
> > > > #define pci_address_to_pio pci_address_to_pio
> > > > #endif
> > > > =20
> > > > +const __be32 *of_pci_process_ranges(struct device_node *node,
> > > > +=09=09=09=09 struct resource *res, const __be32 *from);
> > > > #else /* CONFIG_OF_ADDRESS */
> > > > static inline int of_address_to_resource(struct device_node *dev, =
int index,
> > > > =09=09=09=09=09 struct resource *r)
> > > > @@ -48,6 +50,11 @@ static inline const u32 *of_get_address(struct d=
evice_node *dev, int index,
> > > > {
> > > > =09return NULL;
> > > > }
> > > > +const __be32 *of_pci_process_ranges(struct device_node *node,
> > >=20
> > > There should be a blank line to separate the above two lines.
> > >=20
> >=20
> > Thanks for the feedback.
> >=20
> > I will send another patch for the typo and leave this patch with you fo=
r
> > working into your existing series.
>=20
> I suppose you have your own series that uses this patch?
Not yet, it may be some time before I submit my PCI host bridge driver. Tho=
ugh
I am making changes else where (e.g. this patch) which I'm hoping to submit=
as
early as possible. I can rebase my work for these upstream dependencies.
I can re-spin this patch with your suggested changes if you prefer?
Andrew Murray
^ permalink raw reply
* Re: [PATCH] pci: Provide support for parsing PCI DT ranges property
From: Thierry Reding @ 2012-12-13 10:03 UTC (permalink / raw)
To: Andrew Murray
Cc: Michal Simek, linux-pci@vger.kernel.org, devicetree-discuss,
Liviu Dudau, rob.herring@calxeda.com, Rob Herring, linuxppc-dev
In-Reply-To: <20121213094543.GA23446@arm.com>
[-- Attachment #1: Type: text/plain, Size: 2154 bytes --]
On Thu, Dec 13, 2012 at 09:45:43AM +0000, Andrew Murray wrote:
> On Thu, Dec 13, 2012 at 09:13:33AM +0000, Thierry Reding wrote:
> > Hi Andrew,
> >
> > I don't like iterator interfaces too much, but I can live with that.
> > Other than that the patch looks good to me and I'll try to work it into
> > my Tegra PCIe patch series.
> >
> > Just two minor comments below.
> >
> > > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > [...]
> > > @@ -421,7 +472,7 @@ u64 __of_translate_address(struct device_node *dev, const __be32 *in_addr,
> > > goto bail;
> > > bus = of_match_bus(parent);
> > >
> > > - /* Cound address cells & copy address locally */
> > > + /* Count address cells & copy address locally */
> > > bus->count_cells(dev, &na, &ns);
> > > if (!OF_CHECK_COUNTS(na, ns)) {
> > > printk(KERN_ERR "prom_parse: Bad cell count for %s\n",
> >
> > This is really minor, but it should still go into a separate patch.
> >
> > > diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> > > index 01b925a..4582b20 100644
> > > --- a/include/linux/of_address.h
> > > +++ b/include/linux/of_address.h
> > > @@ -26,6 +26,8 @@ static inline unsigned long pci_address_to_pio(phys_addr_t addr) { return -1; }
> > > #define pci_address_to_pio pci_address_to_pio
> > > #endif
> > >
> > > +const __be32 *of_pci_process_ranges(struct device_node *node,
> > > + struct resource *res, const __be32 *from);
> > > #else /* CONFIG_OF_ADDRESS */
> > > static inline int of_address_to_resource(struct device_node *dev, int index,
> > > struct resource *r)
> > > @@ -48,6 +50,11 @@ static inline const u32 *of_get_address(struct device_node *dev, int index,
> > > {
> > > return NULL;
> > > }
> > > +const __be32 *of_pci_process_ranges(struct device_node *node,
> >
> > There should be a blank line to separate the above two lines.
> >
>
> Thanks for the feedback.
>
> I will send another patch for the typo and leave this patch with you for
> working into your existing series.
I suppose you have your own series that uses this patch?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] pci: Provide support for parsing PCI DT ranges property
From: Andrew Murray @ 2012-12-13 9:45 UTC (permalink / raw)
To: Thierry Reding
Cc: Michal Simek, linux-pci@vger.kernel.org, devicetree-discuss,
Liviu Dudau, rob.herring@calxeda.com, Rob Herring, linuxppc-dev
In-Reply-To: <20121213091333.GA14828@avionic-0098.adnet.avionic-design.de>
On Thu, Dec 13, 2012 at 09:13:33AM +0000, Thierry Reding wrote:
> Hi Andrew,
>=20
> I don't like iterator interfaces too much, but I can live with that.
> Other than that the patch looks good to me and I'll try to work it into
> my Tegra PCIe patch series.
>=20
> Just two minor comments below.
>=20
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> [...]
> > @@ -421,7 +472,7 @@ u64 __of_translate_address(struct device_node *dev,=
const __be32 *in_addr,
> > =09=09goto bail;
> > =09bus =3D of_match_bus(parent);
> > =20
> > -=09/* Cound address cells & copy address locally */
> > +=09/* Count address cells & copy address locally */
> > =09bus->count_cells(dev, &na, &ns);
> > =09if (!OF_CHECK_COUNTS(na, ns)) {
> > =09=09printk(KERN_ERR "prom_parse: Bad cell count for %s\n",
>=20
> This is really minor, but it should still go into a separate patch.
>=20
> > diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> > index 01b925a..4582b20 100644
> > --- a/include/linux/of_address.h
> > +++ b/include/linux/of_address.h
> > @@ -26,6 +26,8 @@ static inline unsigned long pci_address_to_pio(phys_a=
ddr_t addr) { return -1; }
> > #define pci_address_to_pio pci_address_to_pio
> > #endif
> > =20
> > +const __be32 *of_pci_process_ranges(struct device_node *node,
> > +=09=09=09=09 struct resource *res, const __be32 *from);
> > #else /* CONFIG_OF_ADDRESS */
> > static inline int of_address_to_resource(struct device_node *dev, int =
index,
> > =09=09=09=09=09 struct resource *r)
> > @@ -48,6 +50,11 @@ static inline const u32 *of_get_address(struct devic=
e_node *dev, int index,
> > {
> > =09return NULL;
> > }
> > +const __be32 *of_pci_process_ranges(struct device_node *node,
>=20
> There should be a blank line to separate the above two lines.
>=20
Thanks for the feedback.
I will send another patch for the typo and leave this patch with you for
working into your existing series.
Andrew Murray
^ permalink raw reply
* Re: [PATCH] pci: Provide support for parsing PCI DT ranges property
From: Thierry Reding @ 2012-12-13 9:13 UTC (permalink / raw)
To: Andrew Murray
Cc: Michal Simek, linux-pci, devicetree-discuss, Liviu Dudau,
Rob Herring, Rob Herring, linuxppc-dev
In-Reply-To: <20121212163749.GA17371@arm.com>
[-- Attachment #1: Type: text/plain, Size: 3320 bytes --]
On Wed, Dec 12, 2012 at 04:37:50PM +0000, Andrew Murray wrote:
> DT bindings for PCI host bridges often use the ranges property to describe
> memory and IO ranges - this binding tends to be the same across architectures
> yet several parsing implementations exist, e.g. arch/mips/pci/pci.c,
> arch/powerpc/kernel/pci-common.c, arch/sparc/kernel/pci.c and
> arch/microblaze/pci/pci-common.c (clone of PPC). Some of these duplicate
> functionality provided by drivers/of/address.c.
>
> This patch provides a common iterator-based parser for the ranges property, it
> is hoped this will reduce DT representation differences between architectures
> and that architectures will migrate in part to this new parser.
>
> It is also hoped (and the motativation for the patch) that this patch will
> reduce duplication of code when writing host bridge drivers that are supported
> by multiple architectures.
>
> This patch provides struct resources from a device tree node, e.g.:
>
> u32 *last = NULL;
> struct resource res;
> while ((last = of_pci_process_ranges(np, res, last))) {
> //do something with res
> }
>
> Platforms with quirks can then do what they like with the resource or migrate
> common quirk handling to the parser. In an ideal world drivers can just request
> the obtained resources and pass them on (e.g. pci_add_resource_offset).
>
> Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> ---
> drivers/of/address.c | 53 +++++++++++++++++++++++++++++++++++++++++++-
> include/linux/of_address.h | 7 +++++
> 2 files changed, 59 insertions(+), 1 deletions(-)
Hi Andrew,
I don't like iterator interfaces too much, but I can live with that.
Other than that the patch looks good to me and I'll try to work it into
my Tegra PCIe patch series.
Just two minor comments below.
> diff --git a/drivers/of/address.c b/drivers/of/address.c
[...]
> @@ -421,7 +472,7 @@ u64 __of_translate_address(struct device_node *dev, const __be32 *in_addr,
> goto bail;
> bus = of_match_bus(parent);
>
> - /* Cound address cells & copy address locally */
> + /* Count address cells & copy address locally */
> bus->count_cells(dev, &na, &ns);
> if (!OF_CHECK_COUNTS(na, ns)) {
> printk(KERN_ERR "prom_parse: Bad cell count for %s\n",
This is really minor, but it should still go into a separate patch.
> diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> index 01b925a..4582b20 100644
> --- a/include/linux/of_address.h
> +++ b/include/linux/of_address.h
> @@ -26,6 +26,8 @@ static inline unsigned long pci_address_to_pio(phys_addr_t addr) { return -1; }
> #define pci_address_to_pio pci_address_to_pio
> #endif
>
> +const __be32 *of_pci_process_ranges(struct device_node *node,
> + struct resource *res, const __be32 *from);
> #else /* CONFIG_OF_ADDRESS */
> static inline int of_address_to_resource(struct device_node *dev, int index,
> struct resource *r)
> @@ -48,6 +50,11 @@ static inline const u32 *of_get_address(struct device_node *dev, int index,
> {
> return NULL;
> }
> +const __be32 *of_pci_process_ranges(struct device_node *node,
There should be a blank line to separate the above two lines.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: Understanding how kernel updates MMU hash table
From: pegasus @ 2012-12-13 8:48 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <1355087457.28585.61.camel@pasglop>
Hi Ben
There has been quite much confusion with my post disappearing from the new
nabble system to it having getting posted twice..Im sorry for all this.
Nevertheless, Id like to continue where we left off. Here I again repost my
response which initially disappeared and then showed up twice. Ive removed
the duplicate. So here it goes:
Now that many things are becoming clear let me sum up my understanding until
this point. Do correct it if there are mistakes.
1. Linux page table structure (PGD, PUD, PMD and PTE) is directly used in
case of architecture that lend themselves to such a tree structure for
maintaining virtual memory information. Otherwise Linux needs to maintain
two seperate constructs like it does in case of PowerPC. Right?
2. PowerPC's hash table as you said is pretty large. However isn't it still
smaller than Linux's VM infrastructure such that the chances of it being
'FULL' are a lot more. It is also possible that there could be two entries
in the table that points to the same Real address. Like a page being shared
by two processes?
My main concern here is to understand if having such an inverted page table
aka the hash table helps us in any way when doing TLB flushes. You mentioned
and I also read in a paper by Paul Mackerras that every Linux PTE (LPTE) in
case of ppc64 contains 4 extra bits that help us to get to the very slot in
the hash table that houses the corresponding hashtable PTE (HPTE). Now this
(at least to me) is smartness on the part of the kernel and I do not think
the architecture per se is doing us any favor by having that hash table
right? Or am I missing something here?
His paper is (or rather was) on how one can optimize the Linux ppc kernel
and time and again he mentions the fact that one can first record the LPTEs
being invalidated and then remove the corresponding HPTEs in a batched
format. In his own words "Alternatively, it would be possible to make a list
of virtual addresses when LPTEs are changed and then use that list in the
TLB flush routines to avoid the search through the Linux page tables". So do
we skip looking for the corresponding LPTEs or perhaps we've already
invalidated them and we remove the corresponding HPTEs in a batch as you
mentioned earlier?? Could you shed some light on how this optimization
actually developed over time? He had results for an "immediate update"
kernel
and "batched update" kernel for both ppc32 and ppc64. For ppc32 the batched
update is actually a bit worse than immediate update however for ppc64, the
batched update performs better than immediate update. What exactly is
helping ppc64 perform better with the so called "batched update"? Is it the
encoding of the HPTE address in the LPTE as mentioned above? Or some aspect
of ppc64 that I am unaware of?
Also on a generic note, how come we have 4 spare bits in the PTE for 64bit
address space? Large pages perhaps?
--
View this message in context: http://linuxppc.10917.n7.nabble.com/Understanding-how-kernel-updates-MMU-hash-table-tp59509p67313.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.
^ permalink raw reply
* Re: [PATCH] vfio powerpc: enabled on powernv platform
From: Alexey Kardashevskiy @ 2012-12-13 6:27 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: kvm, linux-kernel, Alex Williamson, Paul Mackerras, linuxppc-dev,
David Gibson
In-Reply-To: <1355365763.19932.75.camel@pasglop>
On 13/12/12 13:29, Benjamin Herrenschmidt wrote:
> On Wed, 2012-12-12 at 07:34 -0700, Alex Williamson wrote:
>>> But what would I put there?... IOMMU ID is more than enough at the moment
>>> and struct iommu_table does not have anything what would have made sense to
>>> show in the sysfs...
>>
>> I believe David mentioned that PEs had user visible names. Perhaps they
>> match an enclosure location or something. Group numbers are rather
>> arbitrary and really have no guarantee of persistence. Thanks,
>
> I agree. Make up something, for example domain[PE] or something like
> that.
To be able to add a PE number, I need to call iommu_group_alloc() in the
correct place where I know this number OR I have to carry it in iommu_table
till the moment the iommu_group_alloc() is called (acceptable but not cool).
I will post a patch which would help as a response to this mail.
--
Alexey
^ permalink raw reply
* [PATCH] powerpc: added DSCR support to ptrace
From: Alexey Kardashevskiy @ 2012-12-13 5:34 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Alexey Kardashevskiy, linuxppc-dev, Anton Blanchard, linux-kernel
The DSCR (aka Data Stream Control Register) is supported on some
server PowerPC chips and allow some control over the prefetch
of data streams.
The kernel already supports DSCR value per thread but there is also
a need in a ability to change it from an external process for
the specific pid.
The patch adds new register index PT_DSCR (index=44) which can be
set/get by:
ptrace(PTRACE_POKEUSER, traced_process, PT_DSCR << 3, dscr);
dscr = ptrace(PTRACE_PEEKUSER, traced_process, PT_DSCR << 3, NULL);
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/include/asm/ptrace.h | 1 +
arch/powerpc/kernel/ptrace.c | 17 +++++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 9c21ed4..340fe36 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -276,6 +276,7 @@ static inline unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs,
#define PT_DAR 41
#define PT_DSISR 42
#define PT_RESULT 43
+#define PT_DSCR 44
#define PT_REGS_COUNT 44
#define PT_FPR0 48 /* each FP reg occupies 2 slots in this space */
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index c10fc28..aa19389 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -179,6 +179,18 @@ static int set_user_msr(struct task_struct *task, unsigned long msr)
return 0;
}
+static unsigned long get_user_dscr(struct task_struct *task)
+{
+ return task->thread.dscr;
+}
+
+static int set_user_dscr(struct task_struct *task, unsigned long dscr)
+{
+ task->thread.dscr = dscr;
+ task->thread.dscr_inherit = 1;
+ return 0;
+}
+
/*
* We prevent mucking around with the reserved area of trap
* which are used internally by the kernel.
@@ -200,6 +212,9 @@ unsigned long ptrace_get_reg(struct task_struct *task, int regno)
if (regno == PT_MSR)
return get_user_msr(task);
+ if (regno == PT_DSCR)
+ return get_user_dscr(task);
+
if (regno < (sizeof(struct pt_regs) / sizeof(unsigned long)))
return ((unsigned long *)task->thread.regs)[regno];
@@ -218,6 +233,8 @@ int ptrace_put_reg(struct task_struct *task, int regno, unsigned long data)
return set_user_msr(task, data);
if (regno == PT_TRAP)
return set_user_trap(task, data);
+ if (regno == PT_DSCR)
+ return set_user_dscr(task, data);
if (regno <= PT_MAX_PUT_REG) {
((unsigned long *)task->thread.regs)[regno] = data;
--
1.7.10.4
^ permalink raw reply related
* RE: [PATCH] Revert "crypto: caam - Updated SEC-4.0 device tree binding for ERA information."
From: Garg Vakul-B16394 @ 2012-12-13 4:04 UTC (permalink / raw)
To: Kumar Gala
Cc: linuxppc-dev@ozlabs.org, devicetree-discuss@lists.ozlabs.org,
linux-crypto@vger.kernel.org
In-Reply-To: <C38A27FC-51EF-44F3-8474-33162DE1A272@kernel.crashing.org>
Hello Kumar
This has been applied to:=20
git://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git.
Regards
Vakul
> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Thursday, December 13, 2012 3:00 AM
> To: Garg Vakul-B16394
> Cc: linux-crypto@vger.kernel.org; linuxppc-dev@ozlabs.org; devicetree-
> discuss@lists.ozlabs.org
> Subject: Re: [PATCH] Revert "crypto: caam - Updated SEC-4.0 device tree
> binding for ERA information."
>=20
>=20
> On Dec 7, 2012, at 2:57 AM, Vakul Garg wrote:
>=20
> > This reverts commit a2c0911c09190125f52c9941b9d187f601c2f7be.
> >
> > Signed-off-by: Vakul Garg <vakul@freescale.com>
> > ---
> > Instead of adding SEC era information in crypto node's compatible, a
> > new property 'fsl,sec-era' is being introduced into crypto node.
> >
> > .../devicetree/bindings/crypto/fsl-sec4.txt | 5 ++---
> > 1 files changed, 2 insertions(+), 3 deletions(-)
>=20
> What tree do you think this has been applied to?
>=20
> - k
^ permalink raw reply
* [PATCH] powerpc: added DSCR support to ptrace
From: Alexey Kardashevskiy @ 2012-12-13 3:50 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Alexey Kardashevskiy, linuxppc-dev, linux-kernel
The DSCR (aka Data Stream Control Register) is supported on some
server PowerPC chips and allow some control over the prefetch
of data streams.
The kernel already supports DSCR value per thread but there is also
a need in a ability to change it from an external process for
the specific pid.
The patch adds new register index PT_DSCR (index=44) which can be
set/get by:
ptrace(PTRACE_POKEUSER, traced_process, PT_DSCR << 3, dscr);
dscr = ptrace(PTRACE_PEEKUSER, traced_process, PT_DSCR << 3, NULL);
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/include/asm/ptrace.h | 1 +
arch/powerpc/kernel/ptrace.c | 16 ++++++++++++++++
2 files changed, 17 insertions(+)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 9c21ed4..340fe36 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -276,6 +276,7 @@ static inline unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs,
#define PT_DAR 41
#define PT_DSISR 42
#define PT_RESULT 43
+#define PT_DSCR 44
#define PT_REGS_COUNT 44
#define PT_FPR0 48 /* each FP reg occupies 2 slots in this space */
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index c10fc28..d3ba67b 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -179,6 +179,17 @@ static int set_user_msr(struct task_struct *task, unsigned long msr)
return 0;
}
+static unsigned long get_user_dscr(struct task_struct *task)
+{
+ return task->thread.dscr;
+}
+
+static int set_user_dscr(struct task_struct *task, unsigned long dscr)
+{
+ task->thread.dscr = dscr;
+ return 0;
+}
+
/*
* We prevent mucking around with the reserved area of trap
* which are used internally by the kernel.
@@ -200,6 +211,9 @@ unsigned long ptrace_get_reg(struct task_struct *task, int regno)
if (regno == PT_MSR)
return get_user_msr(task);
+ if (regno == PT_DSCR)
+ return get_user_dscr(task);
+
if (regno < (sizeof(struct pt_regs) / sizeof(unsigned long)))
return ((unsigned long *)task->thread.regs)[regno];
@@ -218,6 +232,8 @@ int ptrace_put_reg(struct task_struct *task, int regno, unsigned long data)
return set_user_msr(task, data);
if (regno == PT_TRAP)
return set_user_trap(task, data);
+ if (regno == PT_DSCR)
+ return set_user_dscr(task, data);
if (regno <= PT_MAX_PUT_REG) {
((unsigned long *)task->thread.regs)[regno] = data;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] vfio powerpc: enabled on powernv platform
From: Alex Williamson @ 2012-12-13 3:22 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: kvm, Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
linuxppc-dev, David Gibson
In-Reply-To: <1355367458.19932.84.camel@pasglop>
On Thu, 2012-12-13 at 13:57 +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2012-12-12 at 16:30 -0700, Alex Williamson wrote:
> > Locked page accounting in this version is very, very broken. How do
> > powerpc folks feel about seemingly generic kernel iommu interfaces
> > messing with the current task mm? Besides that, more problems
> > below...
>
> After a second look & thought...
>
> This whole accounting business is fucked. First, we simply can't just
> randomly return errors from H_PUT_TCE because the process reached some
> rlimit. This is not a proper failure mode. That means that the guest
> will probably panic() ... possibly right in the middle of some disk
> writeback or god knows what. Not good.
>
> Also the overhead of doing all that crap on every TCE map/unmap is
> ridiculous.
>
> Finally, it's just not going to work for real mode which we really want,
> since we can't take the mmap-sem in real mode anyway, so unless we
> convert that counter to an atomic, we can't do it.
>
> I'd suggest just not bothering, or if you want to bother, check once
> when creating a TCE table that the rlimit is enough to bolt as many
> pages as can be populated in that table and fail to create *that*. The
> failure mode is much better, ie, qemu failing to create a PCI bus due to
> insufficient rlimits.
I agree, we don't seem to be headed in the right direction. x86 needs
to track rlimits or else a user can exploit the interface to pin all the
memory in the system. On power, only the iova window can be pinned, so
it's a fixed amount. I could see it as granting access to a group
implicitly grants access to pinning the iova window. We can still make
it more explicit by handling the rlimit accounting upfront. Thanks,
Alex
^ permalink raw reply
* Re: [PATCH] vfio powerpc: enabled on powernv platform
From: Benjamin Herrenschmidt @ 2012-12-13 2:57 UTC (permalink / raw)
To: Alex Williamson
Cc: kvm, Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
linuxppc-dev, David Gibson
In-Reply-To: <1355355035.3224.343.camel@bling.home>
On Wed, 2012-12-12 at 16:30 -0700, Alex Williamson wrote:
> Locked page accounting in this version is very, very broken. How do
> powerpc folks feel about seemingly generic kernel iommu interfaces
> messing with the current task mm? Besides that, more problems
> below...
After a second look & thought...
This whole accounting business is fucked. First, we simply can't just
randomly return errors from H_PUT_TCE because the process reached some
rlimit. This is not a proper failure mode. That means that the guest
will probably panic() ... possibly right in the middle of some disk
writeback or god knows what. Not good.
Also the overhead of doing all that crap on every TCE map/unmap is
ridiculous.
Finally, it's just not going to work for real mode which we really want,
since we can't take the mmap-sem in real mode anyway, so unless we
convert that counter to an atomic, we can't do it.
I'd suggest just not bothering, or if you want to bother, check once
when creating a TCE table that the rlimit is enough to bolt as many
pages as can be populated in that table and fail to create *that*. The
failure mode is much better, ie, qemu failing to create a PCI bus due to
insufficient rlimits.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] vfio powerpc: enabled on powernv platform
From: Benjamin Herrenschmidt @ 2012-12-13 2:39 UTC (permalink / raw)
To: Alex Williamson
Cc: kvm, Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
linuxppc-dev, David Gibson
In-Reply-To: <1355355035.3224.343.camel@bling.home>
On Wed, 2012-12-12 at 16:30 -0700, Alex Williamson wrote:
> Locked page accounting in this version is very, very broken. How do
> powerpc folks feel about seemingly generic kernel iommu interfaces
> messing with the current task mm? Besides that, more problems below...
Not good at all :-)
I don't understand tho ... H_PUT_TCE calls should be in the qemu context
(or the guest) as current at the point of the call, so everything should
be accounted fine on the *current* task when those calls occur, what's
the point of the work queue Alexey ?
This code looks horribly complicated ... where does it come from ?
> > +/*
> > + * iommu_reset_table is called when it started/stopped being used.
> > + *
> > + * restore==true says to bring the iommu_table into the state as it was
> > + * before being used by VFIO.
> > + */
> > +void iommu_reset_table(struct iommu_table *tbl, bool restore)
> > +{
> > + /* Page#0 is marked as used in iommu_init_table, so we clear it... */
> > + if (!restore && (tbl->it_offset == 0))
> > + clear_bit(0, tbl->it_map);
> > +
> > + iommu_clear_tces(tbl, tbl->it_offset, tbl->it_size);
>
> This does locked page accounting and unpins pages, even on startup when
> the pages aren't necessarily pinned or accounted against the current
> process.
Not sure what you mean Alex, and not sure either what Alexey
implementation actually does but indeed, pages inside an iommu table
that was used by the host don't have their refcount elevated by the fact
that they are there.
So when taking ownership of an iommu for vfio, you probably need to FAIL
if any page is already mapped. Only once you know the iommu is clear for
use, then you can start populating it and account for anything you put
in it (and de-account anything you remove from it when cleaning things
up).
> > +
> > + /* ... or restore */
> > + if (restore && (tbl->it_offset == 0))
> > + set_bit(0, tbl->it_map);
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_reset_table);
> > +
> > +/*
> > + * Returns the number of used IOMMU pages (4K) within
> > + * the same system page (4K or 64K).
> > + *
> > + * syspage_weight_zero is optimized for expected case == 0
> > + * syspage_weight_one is optimized for expected case > 1
> > + * Other case are not used in this file.
> > + */
> > +#if PAGE_SIZE == IOMMU_PAGE_SIZE
> > +
> > +#define syspage_weight_zero(map, offset) test_bit((map), (offset))
> > +#define syspage_weight_one(map, offset) test_bit((map), (offset))
> > +
> > +#elif PAGE_SIZE/IOMMU_PAGE_SIZE == 16
> > +
> > +static int syspage_weight_zero(unsigned long *map, unsigned long offset)
> > +{
> > + offset &= PAGE_MASK >> IOMMU_PAGE_SHIFT;
> > + return 0xffffUL & (map[BIT_WORD(offset)] >>
> > + (offset & (BITS_PER_LONG-1)));
> > +}
>
> I would have expected these to be bools and return true if the weight
> matches the value.
What is that business anyway ? It's very obscure.
> If you replaced 0xffff above w/ this, would you need the #error below?
>
> (1UL << (PAGE_SIZE/IOMMU_PAGE_SIZE)) - 1)
>
> > +
> > +static int syspage_weight_one(unsigned long *map, unsigned long offset)
> > +{
> > + int ret = 0, nbits = PAGE_SIZE/IOMMU_PAGE_SIZE;
> > +
> > + /* Aligns TCE entry number to system page boundary */
> > + offset &= PAGE_MASK >> IOMMU_PAGE_SHIFT;
> > +
> > + /* Count used 4K pages */
> > + while (nbits && (ret < 2)) {
>
> Don't you have a ffs()? Could also be used for _zero. Surely there are
> some bitops helpers that could help here even on big endian. hweight
> really doesn't work?
>
> > + if (test_bit(offset, map))
> > + ++ret;
> > +
> > + --nbits;
> > + ++offset;
> > + }
> > +
> > + return ret;
> > +}
> > +#else
> > +#error TODO: support other page size
> > +#endif
What combinations do you support ?
> > +static void tce_flush(struct iommu_table *tbl)
> > +{
> > + /* Flush/invalidate TLB caches if necessary */
> > + if (ppc_md.tce_flush)
> > + ppc_md.tce_flush(tbl);
> > +
> > + /* Make sure updates are seen by hardware */
> > + mb();
> > +}
>> +
> > +/*
> > + * iommu_clear_tces clears tces and returned the number of system pages
> > + * which it called put_page() on
> > + */
> > +static long clear_tces_nolock(struct iommu_table *tbl, unsigned long entry,
> > + unsigned long pages)
> > +{
> > + int i, retpages = 0, clr;
> > + unsigned long oldtce, oldweight;
> > + struct page *page;
> > +
> > + for (i = 0; i < pages; ++i, ++entry) {
> > + if (!test_bit(entry - tbl->it_offset, tbl->it_map))
> > + continue;
> > +
> > + oldtce = ppc_md.tce_get(tbl, entry);
> > + ppc_md.tce_free(tbl, entry, 1);
> > +
> > + oldweight = syspage_weight_one(tbl->it_map,
> > + entry - tbl->it_offset);
> > + clr = __test_and_clear_bit(entry - tbl->it_offset,
> > + tbl->it_map);
> > +
> > + if (WARN_ON(!(oldtce & (TCE_PCI_WRITE | TCE_PCI_READ))))
> > + continue;
> > +
> > + page = pfn_to_page(oldtce >> PAGE_SHIFT);
> > +
> > + if (WARN_ON(!page))
> > + continue;
> > +
> > + if (oldtce & TCE_PCI_WRITE)
> > + SetPageDirty(page);
> > +
> > + put_page(page);
> > +
> > + /* That was the last IOMMU page within the system page */
> > + if ((oldweight == 1) && clr)
> > + ++retpages;
> > + }
> > +
> > + return retpages;
> > +}
> > +
> > +/*
> > + * iommu_clear_tces clears tces and returned the number
> > + * of released system pages
> > + */
> > +long iommu_clear_tces(struct iommu_table *tbl, unsigned long ioba,
> > + unsigned long size)
> > +{
> > + int ret;
> > + unsigned long entry = ioba >> IOMMU_PAGE_SHIFT;
> > + unsigned long npages = size >> IOMMU_PAGE_SHIFT;
> > + struct iommu_pool *pool = get_pool(tbl, entry);
> > +
> > + if ((size & ~IOMMU_PAGE_MASK) || (ioba & ~IOMMU_PAGE_MASK))
> > + return -EINVAL;
> > +
> > + if ((ioba + size) > ((tbl->it_offset + tbl->it_size)
> > + << IOMMU_PAGE_SHIFT))
> > + return -EINVAL;
> > +
> > + if (ioba < (tbl->it_offset << IOMMU_PAGE_SHIFT))
> > + return -EINVAL;
> > +
> > + spin_lock(&(pool->lock));
> > + ret = clear_tces_nolock(tbl, entry, npages);
> > + tce_flush(tbl);
> > + spin_unlock(&(pool->lock));
Why are you messing with the pools and their locks ? These are only
relevant for the in-kernel use of the table. The table should be locked
out of kernel use when given to vfio (we could add a flag to make any
kernel dma mapping attempt to fail).
> > + if (ret > 0) {
> > + lock_acct(-ret);
> > + return 0;
> > + }
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_clear_tces);
> > +
> > +static int put_tce(struct iommu_table *tbl, unsigned long entry,
> > + uint64_t tce, enum dma_data_direction direction)
> > +{
> > + int ret;
> > + struct page *page = NULL;
> > + unsigned long kva, offset, oldweight;
> > +
> > + /* Map new TCE */
> > + offset = tce & IOMMU_PAGE_MASK & ~PAGE_MASK;
> > + ret = get_user_pages_fast(tce & PAGE_MASK, 1,
> > + direction != DMA_TO_DEVICE, &page);
> > + if (ret != 1) {
> > + pr_err("tce_vfio: get_user_pages_fast failed tce=%llx ioba=%lx ret=%d\n",
> > + tce, entry << IOMMU_PAGE_SHIFT, ret);
> > + return -EFAULT;
> > + }
> > +
> > + kva = (unsigned long) page_address(page);
> > + kva += offset;
> > +
> > + /* tce_build receives a virtual address */
> > + ret = ppc_md.tce_build(tbl, entry, 1, kva, direction, NULL);
> > +
> > + /* tce_build() only returns non-zero for transient errors */
> > + if (unlikely(ret)) {
> > + pr_err("tce_vfio: tce_put failed on tce=%llx ioba=%lx kva=%lx ret=%d\n",
> > + tce, entry << IOMMU_PAGE_SHIFT, kva, ret);
> > + put_page(page);
> > + return -EIO;
> > + }
> > +
> > + /* Calculate if new system page has been locked */
> > + oldweight = syspage_weight_zero(tbl->it_map, entry - tbl->it_offset);
> > + __set_bit(entry - tbl->it_offset, tbl->it_map);
> > +
> > + return (oldweight == 0) ? 1 : 0;
> > +}
> > +
> > +/*
> > + * iommu_put_tces builds tces and returned the number of actually
> > + * locked system pages
> > + */
> > +long iommu_put_tces(struct iommu_table *tbl, unsigned long ioba,
> > + uint64_t tce, enum dma_data_direction direction,
> > + unsigned long size)
> > +{
> > + int i, ret = 0, retpages = 0;
> > + unsigned long entry = ioba >> IOMMU_PAGE_SHIFT;
> > + unsigned long npages = size >> IOMMU_PAGE_SHIFT;
> > + struct iommu_pool *pool = get_pool(tbl, entry);
> > + unsigned long locked, lock_limit;
> > +
> > + BUILD_BUG_ON(PAGE_SIZE < IOMMU_PAGE_SIZE);
> > + BUG_ON(direction == DMA_NONE);
> > +
> > + if ((size & ~IOMMU_PAGE_MASK) ||
> > + (ioba & ~IOMMU_PAGE_MASK) ||
> > + (tce & ~IOMMU_PAGE_MASK))
> > + return -EINVAL;
> > +
> > + if ((ioba + size) > ((tbl->it_offset + tbl->it_size)
> > + << IOMMU_PAGE_SHIFT))
> > + return -EINVAL;
> > +
> > + if (ioba < (tbl->it_offset << IOMMU_PAGE_SHIFT))
> > + return -EINVAL;
> > +
> > + /* Account for locked pages */
> > + locked = current->mm->locked_vm +
> > + (_ALIGN_UP(size, PAGE_SIZE) >> PAGE_SHIFT);
>
> Looks like we just over penalize upfront and correct when mapped, that's
> better, but not great.
>
> > + lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> > + if (locked > lock_limit && !capable(CAP_IPC_LOCK)) {
> > + pr_warn("RLIMIT_MEMLOCK (%ld) exceeded\n",
> > + rlimit(RLIMIT_MEMLOCK));
> > + return -ENOMEM;
> > + }
> > +
> > + spin_lock(&(pool->lock));
> > +
> > + /* Check if any is in use */
> > + for (i = 0; i < npages; ++i) {
> > + if (test_bit(entry + i - tbl->it_offset, tbl->it_map)) {
> > + spin_unlock(&(pool->lock));
> > + return -EBUSY;
> > + }
> > + }
> > +
> > + /* Put tces to the table */
> > + for (i = 0; (i < npages) && (ret >= 0); ++i, tce += IOMMU_PAGE_SIZE) {
> > + ret = put_tce(tbl, entry + i, tce, direction);
> > + if (ret == 1)
> > + ++retpages;
> > + }
> > +
> > + /*
> > + * If failed, release locked pages, otherwise return the number
> > + * of locked system pages
> > + */
> > + if (ret < 0) {
> > + clear_tces_nolock(tbl, entry, i);
> > + } else {
> > + if (retpages)
> > + lock_acct(retpages);
> > + ret = 0;
> > + }
>
> Bug, if it fails we clear, which decrements our locked pages, but we
> haven't incremented them yet. Thanks,
>
> Alex
>
> > +
> > + tce_flush(tbl);
> > + spin_unlock(&(pool->lock));
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_put_tces);
> > +
> > +#endif /* CONFIG_IOMMU_API */
> > diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> > index 05205cf..1b970bf 100644
> > --- a/arch/powerpc/platforms/powernv/pci.c
> > +++ b/arch/powerpc/platforms/powernv/pci.c
> > @@ -20,6 +20,7 @@
> > #include <linux/irq.h>
> > #include <linux/io.h>
> > #include <linux/msi.h>
> > +#include <linux/iommu.h>
> >
> > #include <asm/sections.h>
> > #include <asm/io.h>
> > @@ -613,3 +614,136 @@ void __init pnv_pci_init(void)
> > ppc_md.teardown_msi_irqs = pnv_teardown_msi_irqs;
> > #endif
> > }
> > +
> > +#ifdef CONFIG_IOMMU_API
> > +/*
> > + * IOMMU groups support required by VFIO
> > + */
> > +static int add_device(struct device *dev)
> > +{
> > + struct iommu_table *tbl;
> > + int ret = 0;
> > +
> > + if (WARN_ON(dev->iommu_group)) {
> > + pr_warn("tce_vfio: device %s is already in iommu group %d, skipping\n",
> > + dev_name(dev),
> > + iommu_group_id(dev->iommu_group));
> > + return -EBUSY;
> > + }
> > +
> > + tbl = get_iommu_table_base(dev);
> > + if (!tbl) {
> > + pr_debug("tce_vfio: skipping device %s with no tbl\n",
> > + dev_name(dev));
> > + return 0;
> > + }
> > +
> > + pr_debug("tce_vfio: adding %s to iommu group %d\n",
> > + dev_name(dev), iommu_group_id(tbl->it_group));
> > +
> > + ret = iommu_group_add_device(tbl->it_group, dev);
> > + if (ret < 0)
> > + pr_err("tce_vfio: %s has not been added, ret=%d\n",
> > + dev_name(dev), ret);
> > +
> > + return ret;
> > +}
> > +
> > +static void del_device(struct device *dev)
> > +{
> > + iommu_group_remove_device(dev);
> > +}
> > +
> > +static int iommu_bus_notifier(struct notifier_block *nb,
> > + unsigned long action, void *data)
> > +{
> > + struct device *dev = data;
> > +
> > + switch (action) {
> > + case BUS_NOTIFY_ADD_DEVICE:
> > + return add_device(dev);
> > + case BUS_NOTIFY_DEL_DEVICE:
> > + del_device(dev);
> > + return 0;
> > + default:
> > + return 0;
> > + }
> > +}
> > +
> > +static struct notifier_block tce_iommu_bus_nb = {
> > + .notifier_call = iommu_bus_notifier,
> > +};
> > +
> > +static void group_release(void *iommu_data)
> > +{
> > + struct iommu_table *tbl = iommu_data;
> > + tbl->it_group = NULL;
> > +}
> > +
> > +static int __init tce_iommu_init(void)
> > +{
> > + struct pci_dev *pdev = NULL;
> > + struct iommu_table *tbl;
> > + struct iommu_group *grp;
> > +
> > + /* Allocate and initialize IOMMU groups */
> > + for_each_pci_dev(pdev) {
> > + tbl = get_iommu_table_base(&pdev->dev);
> > + if (!tbl)
> > + continue;
> > +
> > + /* Skip already initialized */
> > + if (tbl->it_group)
> > + continue;
> > +
> > + grp = iommu_group_alloc();
> > + if (IS_ERR(grp)) {
> > + pr_info("tce_vfio: cannot create new IOMMU group, ret=%ld\n",
> > + PTR_ERR(grp));
> > + return PTR_ERR(grp);
> > + }
> > + tbl->it_group = grp;
> > + iommu_group_set_iommudata(grp, tbl, group_release);
> > + }
> > +
> > + bus_register_notifier(&pci_bus_type, &tce_iommu_bus_nb);
> > +
> > + /* Add PCI devices to VFIO groups */
> > + for_each_pci_dev(pdev)
> > + add_device(&pdev->dev);
> > +
> > + return 0;
> > +}
> > +
> > +static void __exit tce_iommu_cleanup(void)
> > +{
> > + struct pci_dev *pdev = NULL;
> > + struct iommu_table *tbl;
> > + struct iommu_group *grp = NULL;
> > +
> > + bus_unregister_notifier(&pci_bus_type, &tce_iommu_bus_nb);
> > +
> > + /* Delete PCI devices from VFIO groups */
> > + for_each_pci_dev(pdev)
> > + del_device(&pdev->dev);
> > +
> > + /* Release VFIO groups */
> > + for_each_pci_dev(pdev) {
> > + tbl = get_iommu_table_base(&pdev->dev);
> > + if (!tbl)
> > + continue;
> > + grp = tbl->it_group;
> > +
> > + /* Skip (already) uninitialized */
> > + if (!grp)
> > + continue;
> > +
> > + /* Do actual release, group_release() is expected to work */
> > + iommu_group_put(grp);
> > + BUG_ON(tbl->it_group);
> > + }
> > +}
> > +
> > +module_init(tce_iommu_init);
> > +module_exit(tce_iommu_cleanup);
> > +#endif /* CONFIG_IOMMU_API */
> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > index 9f69b56..29d11dc 100644
> > --- a/drivers/iommu/Kconfig
> > +++ b/drivers/iommu/Kconfig
> > @@ -187,4 +187,12 @@ config EXYNOS_IOMMU_DEBUG
> >
> > Say N unless you need kernel log message for IOMMU debugging
> >
> > +config SPAPR_TCE_IOMMU
> > + bool "sPAPR TCE IOMMU Support"
> > + depends on PPC_POWERNV
> > + select IOMMU_API
> > + help
> > + Enables bits of IOMMU API required by VFIO. The iommu_ops is
> > + still not implemented.
> > +
> > endif # IOMMU_SUPPORT
>
>
^ permalink raw reply
* Re: [PATCH] vfio powerpc: enabled on powernv platform
From: Benjamin Herrenschmidt @ 2012-12-13 2:29 UTC (permalink / raw)
To: Alex Williamson
Cc: kvm, Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
linuxppc-dev, David Gibson
In-Reply-To: <1355322875.3224.201.camel@bling.home>
On Wed, 2012-12-12 at 07:34 -0700, Alex Williamson wrote:
> > But what would I put there?... IOMMU ID is more than enough at the moment
> > and struct iommu_table does not have anything what would have made sense to
> > show in the sysfs...
>
> I believe David mentioned that PEs had user visible names. Perhaps they
> match an enclosure location or something. Group numbers are rather
> arbitrary and really have no guarantee of persistence. Thanks,
I agree. Make up something, for example domain[PE] or something like
that.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] vfio powerpc: enabled on powernv platform
From: Alexey Kardashevskiy @ 2012-12-13 2:24 UTC (permalink / raw)
To: Alex Williamson
Cc: kvm, linux-kernel, Paul Mackerras, linuxppc-dev, David Gibson
In-Reply-To: <1355355035.3224.343.camel@bling.home>
On 13/12/12 10:30, Alex Williamson wrote:
> On Wed, 2012-12-12 at 23:34 +1100, Alexey Kardashevskiy wrote:
>> This patch initializes IOMMU groups based on the IOMMU
>> configuration discovered during the PCI scan on POWERNV
>> (POWER non virtualized) platform. The IOMMU groups are
>> to be used later by VFIO driver (PCI pass through).
>>
>> It also implements an API for mapping/unmapping pages for
>> guest PCI drivers and providing DMA window properties.
>> This API is going to be used later by QEMU-VFIO to handle
>> h_put_tce hypercalls from the KVM guest.
>>
>> Although this driver has been tested only on the POWERNV
>> platform, it should work on any platform which supports
>> TCE tables.
>>
>> To enable VFIO on POWER, enable SPAPR_TCE_IOMMU config
>> option and configure VFIO as required.
>>
>> Cc: David Gibson <david@gibson.dropbear.id.au>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> arch/powerpc/include/asm/iommu.h | 10 ++
>> arch/powerpc/kernel/iommu.c | 329 ++++++++++++++++++++++++++++++++++
>> arch/powerpc/platforms/powernv/pci.c | 134 ++++++++++++++
>> drivers/iommu/Kconfig | 8 +
>> 4 files changed, 481 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
>> index cbfe678..3c861ae 100644
>> --- a/arch/powerpc/include/asm/iommu.h
>> +++ b/arch/powerpc/include/asm/iommu.h
>> @@ -76,6 +76,9 @@ struct iommu_table {
>> struct iommu_pool large_pool;
>> struct iommu_pool pools[IOMMU_NR_POOLS];
>> unsigned long *it_map; /* A simple allocation bitmap for now */
>> +#ifdef CONFIG_IOMMU_API
>> + struct iommu_group *it_group;
>> +#endif
>> };
>>
>> struct scatterlist;
>> @@ -147,5 +150,12 @@ static inline void iommu_restore(void)
>> }
>> #endif
>>
>> +extern void iommu_reset_table(struct iommu_table *tbl, bool restore);
>> +extern long iommu_clear_tces(struct iommu_table *tbl, unsigned long ioba,
>> + unsigned long size);
>> +extern long iommu_put_tces(struct iommu_table *tbl, unsigned long ioba,
>> + uint64_t tce, enum dma_data_direction direction,
>> + unsigned long size);
>> +
>> #endif /* __KERNEL__ */
>> #endif /* _ASM_IOMMU_H */
>> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
>> index ff5a6ce..f3bb2e7 100644
>> --- a/arch/powerpc/kernel/iommu.c
>> +++ b/arch/powerpc/kernel/iommu.c
>> @@ -36,6 +36,7 @@
>> #include <linux/hash.h>
>> #include <linux/fault-inject.h>
>> #include <linux/pci.h>
>> +#include <linux/uaccess.h>
>> #include <asm/io.h>
>> #include <asm/prom.h>
>> #include <asm/iommu.h>
>> @@ -44,6 +45,7 @@
>> #include <asm/kdump.h>
>> #include <asm/fadump.h>
>> #include <asm/vio.h>
>> +#include <asm/tce.h>
>>
>> #define DBG(...)
>>
>> @@ -856,3 +858,330 @@ void iommu_free_coherent(struct iommu_table *tbl, size_t size,
>> free_pages((unsigned long)vaddr, get_order(size));
>> }
>> }
>> +
>> +#ifdef CONFIG_IOMMU_API
>> +/*
>> + * SPAPR TCE API
>> + */
>> +
>> +struct vwork {
>> + struct mm_struct *mm;
>> + long npage;
>> + struct work_struct work;
>> +};
>> +
>> +/* delayed decrement/increment for locked_vm */
>> +static void lock_acct_bg(struct work_struct *work)
>> +{
>> + struct vwork *vwork = container_of(work, struct vwork, work);
>> + struct mm_struct *mm;
>> +
>> + mm = vwork->mm;
>> + down_write(&mm->mmap_sem);
>> + mm->locked_vm += vwork->npage;
>> + up_write(&mm->mmap_sem);
>> + mmput(mm);
>> + kfree(vwork);
>> +}
>> +
>> +static void lock_acct(long npage)
>> +{
>> + struct vwork *vwork;
>> + struct mm_struct *mm;
>> +
>> + if (!current->mm)
>> + return; /* process exited */
>> +
>> + if (down_write_trylock(¤t->mm->mmap_sem)) {
>> + current->mm->locked_vm += npage;
>> + up_write(¤t->mm->mmap_sem);
>> + return;
>> + }
>> +
>> + /*
>> + * Couldn't get mmap_sem lock, so must setup to update
>> + * mm->locked_vm later. If locked_vm were atomic, we
>> + * wouldn't need this silliness
>> + */
>> + vwork = kmalloc(sizeof(struct vwork), GFP_KERNEL);
>> + if (!vwork)
>> + return;
>> + mm = get_task_mm(current);
>> + if (!mm) {
>> + kfree(vwork);
>> + return;
>> + }
>> + INIT_WORK(&vwork->work, lock_acct_bg);
>> + vwork->mm = mm;
>> + vwork->npage = npage;
>> + schedule_work(&vwork->work);
>> +}
>
> Locked page accounting in this version is very, very broken. How do
> powerpc folks feel about seemingly generic kernel iommu interfaces
> messing with the current task mm? Besides that, more problems below...
>
>> +
>> +/*
>> + * iommu_reset_table is called when it started/stopped being used.
>> + *
>> + * restore==true says to bring the iommu_table into the state as it was
>> + * before being used by VFIO.
>> + */
>> +void iommu_reset_table(struct iommu_table *tbl, bool restore)
>> +{
>> + /* Page#0 is marked as used in iommu_init_table, so we clear it... */
>> + if (!restore && (tbl->it_offset == 0))
>> + clear_bit(0, tbl->it_map);
>> +
>> + iommu_clear_tces(tbl, tbl->it_offset, tbl->it_size);
>
> This does locked page accounting and unpins pages, even on startup when
> the pages aren't necessarily pinned or accounted against the current
> process.
>
>> +
>> + /* ... or restore */
>> + if (restore && (tbl->it_offset == 0))
>> + set_bit(0, tbl->it_map);
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_reset_table);
>> +
>> +/*
>> + * Returns the number of used IOMMU pages (4K) within
>> + * the same system page (4K or 64K).
>> + *
>> + * syspage_weight_zero is optimized for expected case == 0
>> + * syspage_weight_one is optimized for expected case > 1
>> + * Other case are not used in this file.
>> + */
>> +#if PAGE_SIZE == IOMMU_PAGE_SIZE
>> +
>> +#define syspage_weight_zero(map, offset) test_bit((map), (offset))
>> +#define syspage_weight_one(map, offset) test_bit((map), (offset))
>> +
>> +#elif PAGE_SIZE/IOMMU_PAGE_SIZE == 16
>> +
>> +static int syspage_weight_zero(unsigned long *map, unsigned long offset)
>> +{
>> + offset &= PAGE_MASK >> IOMMU_PAGE_SHIFT;
>> + return 0xffffUL & (map[BIT_WORD(offset)] >>
>> + (offset & (BITS_PER_LONG-1)));
>> +}
>
> I would have expected these to be bools and return true if the weight
> matches the value.
My expectation was different but ok, I'll fix :)
> If you replaced 0xffff above w/ this, would you need the #error below?
> (1UL << (PAGE_SIZE/IOMMU_PAGE_SIZE)) - 1)
We have 3 pages size on POWER - 4K, 64K and 16MB. We already handle 4K and
64K and the 16MB case will require much different approach and I am not
sure how/when we will add this so I'd keep it as an error.
>> +
>> +static int syspage_weight_one(unsigned long *map, unsigned long offset)
>> +{
>> + int ret = 0, nbits = PAGE_SIZE/IOMMU_PAGE_SIZE;
>> +
>> + /* Aligns TCE entry number to system page boundary */
>> + offset &= PAGE_MASK >> IOMMU_PAGE_SHIFT;
>> +
>> + /* Count used 4K pages */
>> + while (nbits && (ret < 2)) {
>
> Don't you have a ffs()? Could also be used for _zero. Surely there are
> some bitops helpers that could help here even on big endian. hweight
> really doesn't work?
>
>> + if (test_bit(offset, map))
>> + ++ret;
>> +
>> + --nbits;
>> + ++offset;
>> + }
>> +
>> + return ret;
>> +}
>> +#else
>> +#error TODO: support other page size
>> +#endif
>> +
>> +static void tce_flush(struct iommu_table *tbl)
>> +{
>> + /* Flush/invalidate TLB caches if necessary */
>> + if (ppc_md.tce_flush)
>> + ppc_md.tce_flush(tbl);
>> +
>> + /* Make sure updates are seen by hardware */
>> + mb();
>> +}
>> +
>> +/*
>> + * iommu_clear_tces clears tces and returned the number of system pages
>> + * which it called put_page() on
>> + */
>> +static long clear_tces_nolock(struct iommu_table *tbl, unsigned long entry,
>> + unsigned long pages)
>> +{
>> + int i, retpages = 0, clr;
>> + unsigned long oldtce, oldweight;
>> + struct page *page;
>> +
>> + for (i = 0; i < pages; ++i, ++entry) {
>> + if (!test_bit(entry - tbl->it_offset, tbl->it_map))
>> + continue;
>> +
>> + oldtce = ppc_md.tce_get(tbl, entry);
>> + ppc_md.tce_free(tbl, entry, 1);
>> +
>> + oldweight = syspage_weight_one(tbl->it_map,
>> + entry - tbl->it_offset);
>> + clr = __test_and_clear_bit(entry - tbl->it_offset,
>> + tbl->it_map);
>> +
>> + if (WARN_ON(!(oldtce & (TCE_PCI_WRITE | TCE_PCI_READ))))
>> + continue;
>> +
>> + page = pfn_to_page(oldtce >> PAGE_SHIFT);
>> +
>> + if (WARN_ON(!page))
>> + continue;
>> +
>> + if (oldtce & TCE_PCI_WRITE)
>> + SetPageDirty(page);
>> +
>> + put_page(page);
>> +
>> + /* That was the last IOMMU page within the system page */
>> + if ((oldweight == 1) && clr)
>> + ++retpages;
>> + }
>> +
>> + return retpages;
>> +}
>> +
>> +/*
>> + * iommu_clear_tces clears tces and returned the number
>> + * of released system pages
>> + */
>> +long iommu_clear_tces(struct iommu_table *tbl, unsigned long ioba,
>> + unsigned long size)
>> +{
>> + int ret;
>> + unsigned long entry = ioba >> IOMMU_PAGE_SHIFT;
>> + unsigned long npages = size >> IOMMU_PAGE_SHIFT;
>> + struct iommu_pool *pool = get_pool(tbl, entry);
>> +
>> + if ((size & ~IOMMU_PAGE_MASK) || (ioba & ~IOMMU_PAGE_MASK))
>> + return -EINVAL;
>> +
>> + if ((ioba + size) > ((tbl->it_offset + tbl->it_size)
>> + << IOMMU_PAGE_SHIFT))
>> + return -EINVAL;
>> +
>> + if (ioba < (tbl->it_offset << IOMMU_PAGE_SHIFT))
>> + return -EINVAL;
>> +
>> + spin_lock(&(pool->lock));
>> + ret = clear_tces_nolock(tbl, entry, npages);
>> + tce_flush(tbl);
>> + spin_unlock(&(pool->lock));
>> +
>> + if (ret > 0) {
>> + lock_acct(-ret);
>> + return 0;
>> + }
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_clear_tces);
>> +
>> +static int put_tce(struct iommu_table *tbl, unsigned long entry,
>> + uint64_t tce, enum dma_data_direction direction)
>> +{
>> + int ret;
>> + struct page *page = NULL;
>> + unsigned long kva, offset, oldweight;
>> +
>> + /* Map new TCE */
>> + offset = tce & IOMMU_PAGE_MASK & ~PAGE_MASK;
>> + ret = get_user_pages_fast(tce & PAGE_MASK, 1,
>> + direction != DMA_TO_DEVICE, &page);
>> + if (ret != 1) {
>> + pr_err("tce_vfio: get_user_pages_fast failed tce=%llx ioba=%lx ret=%d\n",
>> + tce, entry << IOMMU_PAGE_SHIFT, ret);
>> + return -EFAULT;
>> + }
>> +
>> + kva = (unsigned long) page_address(page);
>> + kva += offset;
>> +
>> + /* tce_build receives a virtual address */
>> + ret = ppc_md.tce_build(tbl, entry, 1, kva, direction, NULL);
>> +
>> + /* tce_build() only returns non-zero for transient errors */
>> + if (unlikely(ret)) {
>> + pr_err("tce_vfio: tce_put failed on tce=%llx ioba=%lx kva=%lx ret=%d\n",
>> + tce, entry << IOMMU_PAGE_SHIFT, kva, ret);
>> + put_page(page);
>> + return -EIO;
>> + }
>> +
>> + /* Calculate if new system page has been locked */
>> + oldweight = syspage_weight_zero(tbl->it_map, entry - tbl->it_offset);
>> + __set_bit(entry - tbl->it_offset, tbl->it_map);
>> +
>> + return (oldweight == 0) ? 1 : 0;
>> +}
>> +
>> +/*
>> + * iommu_put_tces builds tces and returned the number of actually
>> + * locked system pages
>> + */
>> +long iommu_put_tces(struct iommu_table *tbl, unsigned long ioba,
>> + uint64_t tce, enum dma_data_direction direction,
>> + unsigned long size)
>> +{
>> + int i, ret = 0, retpages = 0;
>> + unsigned long entry = ioba >> IOMMU_PAGE_SHIFT;
>> + unsigned long npages = size >> IOMMU_PAGE_SHIFT;
>> + struct iommu_pool *pool = get_pool(tbl, entry);
>> + unsigned long locked, lock_limit;
>> +
>> + BUILD_BUG_ON(PAGE_SIZE < IOMMU_PAGE_SIZE);
>> + BUG_ON(direction == DMA_NONE);
>> +
>> + if ((size & ~IOMMU_PAGE_MASK) ||
>> + (ioba & ~IOMMU_PAGE_MASK) ||
>> + (tce & ~IOMMU_PAGE_MASK))
>> + return -EINVAL;
>> +
>> + if ((ioba + size) > ((tbl->it_offset + tbl->it_size)
>> + << IOMMU_PAGE_SHIFT))
>> + return -EINVAL;
>> +
>> + if (ioba < (tbl->it_offset << IOMMU_PAGE_SHIFT))
>> + return -EINVAL;
>> +
>> + /* Account for locked pages */
>> + locked = current->mm->locked_vm +
>> + (_ALIGN_UP(size, PAGE_SIZE) >> PAGE_SHIFT);
>
> Looks like we just over penalize upfront and correct when mapped, that's
> better, but not great.
What would be great? :)
>> + lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>> + if (locked > lock_limit && !capable(CAP_IPC_LOCK)) {
>> + pr_warn("RLIMIT_MEMLOCK (%ld) exceeded\n",
>> + rlimit(RLIMIT_MEMLOCK));
>> + return -ENOMEM;
>> + }
>> +
>> + spin_lock(&(pool->lock));
>> +
>> + /* Check if any is in use */
>> + for (i = 0; i < npages; ++i) {
>> + if (test_bit(entry + i - tbl->it_offset, tbl->it_map)) {
>> + spin_unlock(&(pool->lock));
>> + return -EBUSY;
>> + }
>> + }
>> +
>> + /* Put tces to the table */
>> + for (i = 0; (i < npages) && (ret >= 0); ++i, tce += IOMMU_PAGE_SIZE) {
>> + ret = put_tce(tbl, entry + i, tce, direction);
>> + if (ret == 1)
>> + ++retpages;
>> + }
>> +
>> + /*
>> + * If failed, release locked pages, otherwise return the number
>> + * of locked system pages
>> + */
>> + if (ret < 0) {
>> + clear_tces_nolock(tbl, entry, i);
>> + } else {
>> + if (retpages)
>> + lock_acct(retpages);
>> + ret = 0;
>> + }
>
> Bug, if it fails we clear, which decrements our locked pages, but we
> haven't incremented them yet. Thanks,
static clear_tces_nolock does not touch the counter, extern
iommu_clear_tces does or I missed your point.
--
Alexey
^ permalink raw reply
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