Re: [PATCH v3 00/27] Use MSI chip framework to configure MSI/MSI-X in all platforms
>> v1->v2: >> Add a patch to make s390 MSI code build happy between patch "x86/xen/MSI: >> E.." >> and "s390/MSI: Use MSI..". Fix several typo problems found by Lucas. >> >> RFC->v1: >> Updated "[patch 4/21] x86/xen/MSI: Eliminate...", export msi_chip instead >> of #ifdef to fix MSI bug in xen running in x86. >> Rename arch_get_match_msi_chip() to arch_find_msi_chip(). >> Drop use struct device as the msi_chip argument, we will do that >> later in another patchset. >> >> Yijing Wang (27): > > This is a lot of stuff, and it's not all related, so putting it all > together in one series makes it hard for me to deal with it. > >> MSI: Remove the redundant irq_set_chip_data() OK, will post it out separately. > > This doesn't seem related to eliminating weak functions. > >> x86/xen/MSI: Eliminate arch_msix_mask_irq() and arch_msi_mask_irq() >> s390/MSI: Use __msi_mask_irq() instead of default_msi_mask_irq() Will update your comments and post these together. > > These two seem to go together. > >> arm/MSI: Save MSI chip in pci_sys_data >> PCI: tegra: Save msi chip in pci_sys_data >> PCI: designware: Save msi chip in pci_sys_data >> PCI: rcar: Save msi chip in pci_sys_data >> PCI: mvebu: Save msi chip in pci_sys_data >> arm/PCI: Clean unused pcibios_add_bus() and pcibios_remove_bus() >> PCI/MSI: Remove useless bus->msi assignment > > These seem to go together (it'd be nice if they were all capitalized the > same). OK, will update the titles and post together. > > I don't like the "msi_chip" name because the "chip" part doesn't convey > much information, other than telling us that apparently some sort of > semiconductor integrated circuit is involved. I sort of assumed that > much :) I think so, msi_chip easily make me confuse with x86 irq_chip msi_chip. > > Something like "msi_controller" would be more descriptive since we're > talking about an interrupt controller that accepts writes from a device and > turns them into CPU interrupts, e.g., a LAPIC. Hm, it's a better one, arm also use "msi-controller" to represent the msi related object in DTS file. > >> PCI/MSI: Refactor struct msi_chip to make it become more common >> x86/MSI: Use MSI chip framework to configure MSI/MSI-X irq >> x86/xen/MSI: Use MSI chip framework to configure MSI/MSI-X irq >> Irq_remapping/MSI: Use MSI chip framework to configure MSI/MSI-X irq >> x86/MSI: Remove unused MSI weak arch functions >> Mips/MSI: Save msi chip in pci sysdata >> MIPS/Octeon/MSI: Use MSI chip framework to configure MSI/MSI-X irq >> MIPS/Xlp: Remove the dead function destroy_irq() to fix build error >> MIPS/Xlp/MSI: Use MSI chip framework to configure MSI/MSI-X irq >> MIPS/Xlr/MSI: Use MSI chip framework to configure MSI/MSI-X irq >> Powerpc/MSI: Use MSI chip framework to configure MSI/MSI-X irq >> s390/MSI: Use MSI chip framework to configure MSI/MSI-X irq >> arm/iop13xx/MSI: Use MSI chip framework to configure MSI/MSI-X irq >> IA64/MSI: Use MSI chip framework to configure MSI/MSI-X irq >> Sparc/MSI: Use MSI chip framework to configure MSI/MSI-X irq >> tile/MSI: Use MSI chip framework to configure MSI/MSI-X irq >> PCI/MSI: Clean up unused MSI arch functions >> >> arch/arm/include/asm/mach/pci.h | 10 +- >> arch/arm/include/asm/pci.h |9 ++ >> arch/arm/kernel/bios32.c| 19 +--- >> arch/arm/mach-iop13xx/include/mach/pci.h|4 + >> arch/arm/mach-iop13xx/iq81340mc.c |3 + >> arch/arm/mach-iop13xx/iq81340sc.c |5 +- >> arch/arm/mach-iop13xx/msi.c | 11 ++- >> arch/ia64/include/asm/pci.h | 10 ++ >> arch/ia64/kernel/msi_ia64.c | 14 ++- >> arch/ia64/pci/pci.c |1 + >> arch/mips/include/asm/netlogic/xlp-hal/pcibus.h |1 + >> arch/mips/include/asm/octeon/pci-octeon.h |4 + >> arch/mips/include/asm/pci.h | 14 +++ >> arch/mips/pci/msi-octeon.c | 31 +++--- >> arch/mips/pci/msi-xlp.c | 15 ++- >> arch/mips/pci/pci-octeon.c |3 + >> arch/mips/pci/pci-xlp.c |3 + >> arch/mips/pci/pci-xlr.c | 17 +++- >> arch/powerpc/include/asm/pci-bridge.h | 15 +++ >> arch/powerpc/kernel/msi.c | 12 ++- >> arch/powerpc/kernel/pci-common.c|3 + >> arch/s390/include/asm/pci.h |9 ++ >> arch/s390/pci/pci.c | 16 ++- >> arch/sparc/kernel/pci.c | 14 ++- >> arch/sparc/kernel/pci_impl.h| 12 ++ >> arch/tile/include/asm/pci.h | 10 ++ >> arch/tile/kernel/pci_gx.c | 13 ++- >> arch/x86/include/asm/pci.h | 18 +++- >> arch/x86/inclu
Re: [PATCH V2 1/2] mm: Update generic gup implementation to handle hugepage directory
Andrew Morton writes:
> On Fri, 17 Oct 2014 10:08:06 +0530 "Aneesh Kumar K.V"
> wrote:
>
>> Update generic gup implementation with powerpc specific details.
>> On powerpc at pmd level we can have hugepte, normal pmd pointer
>> or a pointer to the hugepage directory.
>>
>> ...
>>
>> --- a/arch/arm/include/asm/pgtable.h
>> +++ b/arch/arm/include/asm/pgtable.h
>> @@ -181,6 +181,8 @@ extern pgd_t swapper_pg_dir[PTRS_PER_PGD];
>> /* to find an entry in a kernel page-table-directory */
>> #define pgd_offset_k(addr) pgd_offset(&init_mm, addr)
>>
>> +#define pgd_huge(pgd) (0)
>> +
>> #define pmd_none(pmd) (!pmd_val(pmd))
>> #define pmd_present(pmd)(pmd_val(pmd))
>>
>> diff --git a/arch/arm64/include/asm/pgtable.h
>> b/arch/arm64/include/asm/pgtable.h
>> index cefd3e825612..ed8f42497ac4 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -464,6 +464,8 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t
>> newprot)
>> extern pgd_t swapper_pg_dir[PTRS_PER_PGD];
>> extern pgd_t idmap_pg_dir[PTRS_PER_PGD];
>>
>> +#define pgd_huge(pgd) (0)
>> +
>
> So only arm, arm64 and powerpc implement CONFIG_HAVE_GENERIC_RCU_GUP
> and only powerpc impements pgd_huge().
>
> Could we get a bit of documentation in place for pgd_huge() so that
> people who aren't familiar with powerpc can understand what's going
> on?
I ended up moving that to include/linux/hugetlb.h with the below
comments added. Let me know if the below is ok
/*
* hugepages at page global directory. If arch support
* hugepages at pgd level, they need to define this.
*/
#ifndef pgd_huge
#define pgd_huge(x) 0
#endif
#ifndef is_hugepd
/*
* Some architectures requires a hugepage directory format that is
* required to support multiple hugepage sizes. For example
* a4fe3ce7699bfe1bd88f816b55d42d8fe1dac655 introduced the same
* on powerpc. This allows for a more flexible hugepage pagetable
* layout.
*/
typedef struct { unsigned long pd; } hugepd_t;
#define is_hugepd(hugepd) (0)
#define __hugepd(x) ((hugepd_t) { (x) })
static inline int gup_huge_pd(hugepd_t hugepd, unsigned long addr,
unsigned pdshift, unsigned long end,
int write, struct page **pages, int *nr)
{
return 0;
}
#else
extern int gup_huge_pd(hugepd_t hugepd, unsigned long addr,
unsigned pdshift, unsigned long end,
int write, struct page **pages, int *nr);
#endif
-aneesh
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] drivers: depend on instead of select BACKLIGHT_CLASS_DEVICE and ACPI_VIDEO
On Wed, Oct 22, 2014 at 11:02:26AM +0300, Tomi Valkeinen wrote: > On 18/10/14 00:13, Jani Nikula wrote: > > Documentation/kbuild/kconfig-language.txt warns to use select with care, > > and in general use select only for non-visible symbols and for symbols > > with no dependencies, because select will force a symbol to a value > > without visiting the dependencies. > > > > Select has become particularly problematic, interdependently, with the > > BACKLIGHT_CLASS_DEVICE and ACPI_VIDEO configs. For example: > > > > scripts/kconfig/conf --randconfig Kconfig > > KCONFIG_SEED=0x48312B00 > > warning: (DRM_RADEON && DRM_NOUVEAU && DRM_I915 && DRM_GMA500 && > > DRM_SHMOBILE && DRM_TILCDC && FB_BACKLIGHT && FB_MX3 && USB_APPLEDISPLAY > > && FB_OLPC_DCON && ASUS_LAPTOP && SONY_LAPTOP && THINKPAD_ACPI && > > EEEPC_LAPTOP && ACPI_CMPC && SAMSUNG_Q10) selects BACKLIGHT_CLASS_DEVICE > > which has unmet direct dependencies (HAS_IOMEM && BACKLIGHT_LCD_SUPPORT) > > warning: (DRM_RADEON && DRM_NOUVEAU && DRM_I915 && DRM_GMA500 && > > DRM_SHMOBILE && DRM_TILCDC && FB_BACKLIGHT && FB_MX3 && USB_APPLEDISPLAY > > && FB_OLPC_DCON && ASUS_LAPTOP && SONY_LAPTOP && THINKPAD_ACPI && > > EEEPC_LAPTOP && ACPI_CMPC && SAMSUNG_Q10) selects BACKLIGHT_CLASS_DEVICE > > which has unmet direct dependencies (HAS_IOMEM && BACKLIGHT_LCD_SUPPORT) > > > > With tristates it's possible to end up selecting FOO=y depending on > > BAR=m in the config, which gets discovered at build time, not config > > time, like reported in the thread referenced below. > > > > Do the following to fix the dependencies: > > > > * Depend on instead of select BACKLIGHT_CLASS_DEVICE everywhere. Drop > > select BACKLIGHT_LCD_SUPPORT in such cases, as it's a dependency of > > BACKLIGHT_CLASS_DEVICE. > > > > * Remove config FB_BACKLIGHT altogether, and replace it with a > > dependency on BACKLIGHT_CLASS_DEVICE. All configs selecting > > FB_BACKLIGHT select or depend on FB anyway, so we can simplify. > > > > * Depend on (ACPI && ACPI_VIDEO) || ACPI=n in several places instead of > > selecting ACPI_VIDEO and a number of its dependencies if ACPI is > > enabled. This is tied to backlight, as ACPI_VIDEO depends on > > BACKLIGHT_CLASS_DEVICE. > > > > * Replace a couple of select INPUT/VT with depends as it seemed to be > > necessary. > > While doing 'depends on' instead of 'select' is an "easy" fix for this, > I do dislike it quite a bit. It's a major pain to go around the kernel > config, trying to find all the dependencies that a particular driver > wants. If I need fb-foobar, I should just be able to enable it, instead > of first searching and selecting its minor dependencies individually. > > So, not a NACK, but a "isn't there an another way to fix this?". > > Looking at backlight... BACKLIGHT_LCD_SUPPORT seems to be a "meta" > option, it only enables a Kconfig submenu. > > So I think we could just remove the whole BACKLIGHT_LCD_SUPPORT option. > But if we do that, all the items in drivers/video/backlight/Kconfig with > default 'y' or 'm' would get enabled by default, so I think we should > remove the 'default's from that file. That makes sense in any case, as I > don't see why "HP Jornada 700 series LCD Driver" should be "default y". > > BACKLIGHT_CLASS_DEVICE doesn't depend on anything except > BACKLIGHT_LCD_SUPPORT, so after removing BACKLIGHT_LCD_SUPPORT it should > be safe to 'select' BACKLIGHT_CLASS_DEVICE. > > BACKLIGHT_CLASS_DEVICE could be made a hidden option, and the drivers in > drivers/video/backlight/Kconfig which are under BACKLIGHT_CLASS_DEVICE > could be made to select BACKLIGHT_CLASS_DEVICE instead. > > That doesn't exactly fix anything, but I think it makes sense as > BACKLIGHT_CLASS_DEVICE is something that's selected from all around the > kernel, so it should be a selectable "library" instead of a Kconfig menu > option. > > I didn't look at the ACPI_VIDEO side, so no idea how messy that is. If we want to make BACKLIGHT_CLASS_DEVICE into a library thing then I guess we could do that, but we must then also drag it out of all the other meta options to make sure it's always available. No need I think to ditch the entire BACKLIGHT_LCD_SUPPORT meta option. And then everyone could select it. The problem is if you mix depends and select Kconfig falls over and starts to see loops where there are none. So you really can't ever mix them for the same symbol. I think if we get the BACKLIGHT_CLASS_DEVICE out of the picture with that ACPI_VIDEO should be fixable with some select ACPI_VIDEO if ACPI or so. Currently that doesn't work because of the backlight class knob and select being broken. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/4] dt/bindings: Introduce the FSL QorIQ DPAA BMan portal(s)
On Wed, Oct 22, 2014 at 09:04:54PM +0100, Emil Medve wrote:
> Hello Mark,
>
>
> Thanks for having a look at this
>
> On 10/22/2014 09:29 AM, Mark Rutland wrote:
> > On Wed, Oct 22, 2014 at 03:09:30PM +0100, Emil Medve wrote:
> >> Portals are used by software running on processor cores, accelerators and
> >> network interfaces to communicate with the BMan
> >
> > What exactly is a portal?
> >
> > Is it a region of shared memory? A device?
>
> In a nutshell a (software, i.e. processor core accessible) portal is a
> memory mapped interface to the B/QMan that allows low latency, lock-less
> interaction by logically separated units of software. The original
> intent was to have one affine portal per core. As of now we're
> sprinkling portals to use from various (core affine) contexts:
> hypervisor, guests, user-space, containers, etc.
>
> I'll make the definition more palatable in the next round
Thanks,
> > I only received emails 2 and 3 of this series, so I'm lacking the
> > context necessary to understand the bindings.
>
> Some bubble in the pipe... As of now they all seem to have hit the
> e-mail list(s), patchwork and hopefully your Inbox
They've all come through now, yes.
I'd expected to see the relevant code too -- in isolation the bindings
are somewhat difficult to grok.
>
> >> Signed-off-by: Emil Medve
> >> Change-Id: I6d245ffc14ba3d0e91d403ac7c3b91b75a9e6a95
> >> ---
> >> .../bindings/powerpc/fsl/bman-portals.txt | 50
> >> ++
> >> 1 file changed, 50 insertions(+)
> >> create mode 100644
> >> Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
> >> b/Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
> >> new file mode 100644
> >> index 000..40e607e
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
> >> @@ -0,0 +1,50 @@
> >> +QorIQ DPAA Buffer Manager Portals Device Tree Binding
> >> +
> >> +Copyright (C) 2008 - 2014 Freescale Semiconductor Inc.
> >> +
> >> +CONTENTS
> >> +
> >> + - BMan Portal
> >> + - Example
> >> +
> >> +NOTE: The bindings described in this document are preliminary and
> >> subject to
> >> + change
> >
> > While we've tried that elsewhere, unstable DT bindings have been shown
> > to be a major source of pain.
>
> Agreed
>
> > I'd feel rather uncomfortable accepting a
> > binding that we already believe to be insufficient to describe the
> > hardware.
> >
> > What do you expect to change?
>
> Related bindings seem incomplete. As such, the PAMU binding (pamu.txt)
> covers incompletely a dynamic LIODN assignment/programming model. The
> current driver uses a static assignment scheme that the binding needs to
> include. I also suspect that once the driver starts supporting the
> dynamic LIODN assignment/programming we might find some wrinkles
Ok. Given that the driver doesn't seem to be in mainline (per a quick
grep), just put "DO NOT MERGE" or something like that in the commit
message. Once this is merged you're going to need to support it.
> >> +BMan Portal Node
> >> +
> >> +PROPERTIES
> >> +
> >> +- compatible
> >> + Usage: Required
> >> + Value type:
> >> + Definition: Must include "fsl,bman-portal-"
> >> + May include "fsl,-bman-portal" or "fsl,bman-portal"
> >> +
> >> +- reg
> >> + Usage: Required
> >> + Value type:
> >> + Definition: Two regions. The first is the cache-enabled region of
> >> + the portal. The second is the cache-inhibited region of
> >> + the portal
> >> +
> >> +EXAMPLE
> >> +
> >> +The example below shows a (P4080) BMan portals container/bus node with
> >> two portals
> >
> > Is there any particular reason to place these under a simple-bus?
>
> I think they fit the ePAPR definition for simple-bus container. They can
> be accessed directly and can be used independently. What are you suggesting?
>
> >> +
> >> + bman-portals@ff400 {
> >> + #address-cells = <1>;
> >> + #size-cells = <1>;
> >> + compatible = "simple-bus";
> >> + ranges = <0 0xf 0xf400 0x20>;
> >> +
> >> + bman-portal@0 {
> >> + compatible = "fsl,bman-portal-1.0.0", "fsl,bman-portal";
> >> + reg = <0x0 0x4000 0x10 0x1000>;
> >
> > It would be easier to read is each entry had its own set of brackets.
> > Initially this looked to me like a single 64-bit address/size pair.
>
> Something like <>, <>?
So here we'd have:
reg = <0x00 0x4000>,
<0x10 0x1000>;
> It doesn't seem widely used but I agree is more readable. I can
> include it in the the next spin
Thanks.
>
> >> + interrupts = <105 2 0 0>;
> >> + };
> >
> > Given the description above, surely you need to know what the portal is
> > used for? Or is that queried from the portal
Re: [PATCH 3/4] dt/bindings: Introduce the FSL QorIQ DPAA QMan
[...] > >> +QMan Node > >> + > >> +PROPERTIES > >> + > >> +- compatible > >> + Usage: Required > >> + Value type: > >> + Definition: Must include "fsl,qman" > >> + May include "fsl,-qman" > >> + > >> +- reg > >> + Usage: Required > >> + Value type: > >> + Definition: Registers region within the CCSR address space > >> + > >> +- fsl,liodn > >> + Usage: See pamu.txt > >> + Value type: > >> + Definition: PAMU property used for static LIODN assignment > >> + > >> +- fsl,iommu-parent > >> + Usage: See pamu.txt > >> + Value type: > >> + Definition: PAMU property used for dynamic LIODN assignment > >> + > >> + For additional details about the PAMU/LIODN binding(s) see pamu.txt > > > > This is not present in the example. Is this always required? > > Sort of. Initial hardware (and current documentation) programming > suggestion was to configure all the PAMU instances the same way > regardless of what devices were behind them. Given the PAMU internal > caches sizes, this proved suboptimal from a performance perspective so > we're trying to discover/describe/use the PAMU topology > > fsl,liodn is part of the undocumented static LIODN assignment binding > that the current PAMU driver uses. If fsl,iommu-parent is present, > fsl,liodn can be ignored and the LIODN can be assigned dynamically > and/or programmed only in the relevant PAMU instance Ok. > >> + > >> +- clocks > >> + Usage: See clock-bindings.txt and qoriq-clock.txt > >> + Value type: > >> + Definition: Half of the platform clock > >> + > > > > I don't understand the description here. What is the clock from the PoV > > of the QMan? Which input line on the QMan is this clock attached to? > > > > Is there only one clock input? Or jsut one that you need to manage at > > the moment? > > As part of the programming model (QoS features specifically) QMan needs > to know its clock speed. Prior to the existence of the > clock-bindings.txt, a "static" clock-frequency property was/is used > convey such information. Using clock-binding.txt to describe the > clocking hierarchy in the SoC makes it easier with DFS, power > management, etc. Ok. My concern is the phrase "Half of the platform clock" is meaingless. The property contains a phandle + clock-specifier pair that describe a single input clock by reference (some bindings just say "clock reference" for that, which is fine). This is not "half" of any particular clock. The description of the clock should describe what it logically is from the PoV of the consumer (i.e. _this_ device) rather than the provider. To me "platform clock" sounds like a description of the provider. Is there a name for the clock input line on this device? Is there only a single clock input? Or just one that you care about at the moment? > The platform clock/PLL binding is part of qoriq-clock.txt > > > You also seem to have an interrupt in the example. How many do you > > expect, and what are their their logical functions? > > That's the error interrupt and hopefully it never triggers. I didn't add > [many] words about it as it's a standard property While the interrupts property is standard, it is only standard in the sense that the name and format are standard. Not every bidning has an interrupts property, and the set of interrupts described are specific to the binding. Please describe the set of interrupts you expect. Does the device only have the error interrupt, or is that the only interrupt you care about? > >> +QMan Private Memory Nodes > >> + > >> +QMan requires two contiguous range of physical memory used for the > >> backing store > >> +for QMan Frame Queue Descriptor and Packed Frame Descriptor Record. This > >> memory > >> +is reserved/allocated as a nodes under the /reserved-memory node > >> + > >> +The QMan FQD memory node must be named "qman-fqd" > >> + > >> +PROPERTIES > >> + > >> +- compatible > >> + Usage: required > >> + Value type: > >> + Definition: Must inclide "fsl,qman-fqd" > >> + > >> +The QMan PFDR memory node must be named "qman-pfdr" > >> + > >> +PROPERTIES > >> + > >> +- compatible > >> + Usage: required > >> + Value type: > >> + Definition: Must inclide "fsl,qman-pfdr" > >> + > >> +The following constraints are relevant to the FQD and PFDR private memory: > >> + - The size must be 2^(size + 1), with size = 11..29. That is 4 KiB to > >> +1 GiB > >> + - The alignment must be a muliptle of the memory size > >> + > >> +The size of the FQD and PFDP must be chosen by observing the hardware > >> features > >> +configured via the RCW and that are relevant to a specific board (e.g. > >> number of > >> +MAC(s) pinned-out, number of offline/host command FMan ports, etc.). The > >> size > >> +configured in the DT must reflect the hardware capabilities and not the > >> specific > >> +needs of an application > >> + > >> +If the memory reserved
Re: [PATCH] drivers: depend on instead of select BACKLIGHT_CLASS_DEVICE and ACPI_VIDEO
On 23/10/14 11:10, Daniel Vetter wrote: > If we want to make BACKLIGHT_CLASS_DEVICE into a library thing then I > guess we could do that, but we must then also drag it out of all the other > meta options to make sure it's always available. No need I think to ditch BACKLIGHT_CLASS_DEVICE only depends on HAS_IOMEM and BACKLIGHT_LCD_SUPPORT so there are no other meta options to avoid. HAS_IOMEM comes from drivers/video/Kconfig's "Graphics support", and I guess we can ignore it. > the entire BACKLIGHT_LCD_SUPPORT meta option. And then everyone could > select it. I don't quite understand what purpose does BACKLIGHT_LCD_SUPPORT serve. It doesn't enable any code, it just opens up new Kconfig options. Why can't the Kconfig options be always available? It's just another option to 'select', without any reason I can see. Tomi signature.asc Description: OpenPGP digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/4] dt/bindings: Introduce the FSL QorIQ DPAA QMan
Thought I'd comment too on the interrupt question for the block-level (not portal-level) node; > -Original Message- > From: Mark Rutland [mailto:mark.rutl...@arm.com] > Sent: October-23-14 7:26 AM > To: Medve Emilian-EMMEDVE1 > > [...] > > > > You also seem to have an interrupt in the example. How many do you > > > expect, and what are their their logical functions? > > > > That's the error interrupt and hopefully it never triggers. I didn't > add > > [many] words about it as it's a standard property > > While the interrupts property is standard, it is only standard in the > sense that the name and format are standard. Not every bidning has an > interrupts property, and the set of interrupts described are specific > to > the binding. Please describe the set of interrupts you expect. > > Does the device only have the error interrupt, or is that the only > interrupt you care about? So as with the portals, the blocks themselves each have a single interrupt line, but a number of the (CCSR) registers are related to that interrupt to capture the codes and attributes of the various error conditions being reported, because a wide variety of events and information can be conveyed through that interrupt line. In general, yes, this interrupt line is only for "errors", but some errors are more equal than others. Eg. there is the usual class of catastrophic DMA-failure, ECC, hardware-internal-inconsistency types of errors that devices tend to shout about. But there are also threshold-based "resource starvation" types of "errors" too, which are not really errors per se, unless the system configuration and use-case is explicitly engineered and constrained such that the thresholds shouldn't ever get tripped. So the handling of the interrupt involves interrogating registers to classify the why, how, and what, and the reaction to it (whether a write-to-clear is needed, whether it should be masked, whether functionality should be halted, ...) is also a fairly elaborate question. I don't know what the expectations are for these "bindings" docs, but the request to "describe the set of interrupts you expect" does risk opening a fairly elaborate can of worms (that may be better confined to the driver). Unless I misunderstood the request, which is always quite possible before my second coffee... Cheers, Geoff ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/4] dt/bindings: Introduce the FSL QorIQ DPAA BMan portal(s)
Hi, Leaping in here a little after the fact, to add a bit of background info on the hardware in case it helps. > -Original Message- > From: Mark Rutland [mailto:mark.rutl...@arm.com] > Sent: October-23-14 7:16 AM > To: Medve Emilian-EMMEDVE1 > > On Wed, Oct 22, 2014 at 09:04:54PM +0100, Emil Medve wrote: > > Hello Mark, > > > > > > Thanks for having a look at this > > > > On 10/22/2014 09:29 AM, Mark Rutland wrote: > > > On Wed, Oct 22, 2014 at 03:09:30PM +0100, Emil Medve wrote: > > >> Portals are used by software running on processor cores, > accelerators and > > >> network interfaces to communicate with the BMan > > > > > > What exactly is a portal? > > > > > > Is it a region of shared memory? A device? Just to supplement what Emil already mentioned, the portals each contain two memory-mapped regions and a single interrupt. One of the regions is a conventional 4K page of 32-bit cache-inhibited registers (and this includes all the controls and status for the portal's interrupt line, which conveys a variety of individually-configurable interrupt source conditions). The other region is a 16K cache-enabled space, so the "registers" are cacheline width - interaction with this relies on cacheline manipulation instructions (flushing, prefetching, etc), and in some configurations hardware will initiate stashing (cache-warming) for some of those cachelines, which is one of the reasons that these portals have "LIODN" information configured into them (stashing transactions go through the IOMMU). The other reason for LIODN/IOMMU context is that all the accelerator and networking functions accessed via a portal will be subjected to the IOMMU context info of that portal. In this way, a portal that is mapped/used by a given software context can be setup such that DMA generated by its hardware interactions will respect the same guest-physical translations and restrictions as apply to the software context itself. BTW, a subset of the CCSR registers for the QMan and BMan blocks are used to configure these LIODN/IOMMU attributes of the portals, as opposed to those attributes being exposed in the register space of the portals themselves. In this way, the user of a memory-mapped portal cannot set their own LIODN/IOMMU attributes, that decision belongs to whoever twiddles with CCSR. > > Given the description above, surely you need to know what the > portal is > > > used for? Or is that queried from the portal? > > > > We don't need/want to include such information in the DT. Portals > are > > "allocated" dynamically and used by the respective context > > Ok. So they have no fixed purpose and software allocates as necessary? > There are no constraints on how each portal may be used, or which > portal > another agetn might use? Yes, precisely. They're interchangeable windows/contexts into the hardware, for issuing various kinds of commands and obtaining various kinds of results/status. The SoC provides a certain number of these portals, and software can allocate them out and configure them in whatever manner (static, dynamic, ...). Because of the CCSR-controlled LIODN/IOMMU portal attributes, two portals can be set up to have different "access" and operate in different contexts (eg. apps, virtual machines), but that is determined by control software (kernel driver) configuration of those portals before their use. In short, there's nothing intrinsic to the portals (ie. from a hardware perspective) that distinguishes them or predetermines their use. Cheers, Geoff ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2] powernv/iommu: disable IOMMU bypass with param iommu=nobypass
When IOMMU bypass is enabled, a PCI device can read and write memory
that was not mapped by the driver without causing an EEH. That might
cause memory corruption, for example.
When we disable bypass, DMA reads and writes to addresses not mapped by
the IOMMU will cause an EEH, allowing us to debug such issues.
Signed-off-by: Thadeu Lima de Souza Cascardo
Reviewed-by: Gavin Shan
---
Documentation/kernel-parameters.txt |2 ++
arch/powerpc/platforms/powernv/pci-ioda.c | 24 +++-
2 files changed, 25 insertions(+), 1 deletions(-)
diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
index 988160a..b03322a 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1454,6 +1454,8 @@ bytes respectively. Such letter suffixes can also be
entirely omitted.
forcesac
soft
pt [x86, IA-64]
+ nobypass[PPC/POWERNV]
+ Disable IOMMU bypass, using IOMMU for PCI devices.
io7=[HW] IO7 for Marvel based alpha systems
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index 468a0f2..a7d2f32 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -75,6 +75,27 @@ static void pe_level_printk(const struct pnv_ioda_pe *pe,
const char *level,
#define pe_info(pe, fmt, ...) \
pe_level_printk(pe, KERN_INFO, fmt, ##__VA_ARGS__)
+static bool pnv_iommu_bypass_disabled __read_mostly;
+
+static int __init iommu_setup(char *str)
+{
+ if (!str)
+ return -EINVAL;
+ while (*str) {
+ if (!strncmp(str, "nobypass", 8)) {
+ pnv_iommu_bypass_disabled = true;
+ pr_info("PowerNV: IOMMU bypass window disabled.\n");
+ }
+ str += strcspn(str, ",");
+ if (*str == ',')
+ str++;
+ }
+
+ return 0;
+}
+
+early_param("iommu", iommu_setup);
+
/*
* stdcix is only supposed to be used in hypervisor real mode as per
* the architecture spec
@@ -1243,7 +1264,8 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb
*phb,
pnv_ioda_setup_bus_dma(pe, pe->pbus, true);
/* Also create a bypass window */
- pnv_pci_ioda2_setup_bypass_pe(phb, pe);
+ if (!pnv_iommu_bypass_disabled)
+ pnv_pci_ioda2_setup_bypass_pe(phb, pe);
return;
fail:
if (pe->tce32_seg >= 0)
--
1.7.1
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V2 1/2] mm: Update generic gup implementation to handle hugepage directory
Hey guys, was looking over the generic GUP while working on a sparc64 issue and I noticed that you guys do speculative page gets, and after talking with Johannes Weiner (CC:'d) about this we don't see how it could be necessary. If interrupts are disabled during the page table scan (which they are), no IPI tlb flushes can arrive. Therefore any removal from the page tables is guarded by interrupts being re-enabled. And as a result, page counts of pages we see in the page tables must always have a count > 0. x86 does direct atomic_add() on &page->_count because of this invariant and I would rather see the generic version do this too. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V2 1/2] mm: Update generic gup implementation to handle hugepage directory
On Thu, 2014-10-23 at 18:40 -0400, David Miller wrote: > Hey guys, was looking over the generic GUP while working on a sparc64 > issue and I noticed that you guys do speculative page gets, and after > talking with Johannes Weiner (CC:'d) about this we don't see how it > could be necessary. > > If interrupts are disabled during the page table scan (which they > are), no IPI tlb flushes can arrive. Therefore any removal from the > page tables is guarded by interrupts being re-enabled. And as a > result, page counts of pages we see in the page tables must always > have a count > 0. > > x86 does direct atomic_add() on &page->_count because of this > invariant and I would rather see the generic version do this too. This is of course only true of archs who use IPIs for TLB flushes, so if we are going down the path of not being speculative, powerpc would have to go back to doing its own since our broadcast TLB flush means we aren't protected (we are only protected vs. the page tables themselves being freed since we do that via sched RCU). AFAIK, ARM also broadcasts TLB flushes... Another option would be to make the generic code use something defined by the arch to decide whether to use speculative get or not. I like the idea of keeping the bulk of that code generic... Cheers, Ben. > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majord...@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: mailto:"d...@kvack.org";> em...@kvack.org ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V2 1/2] mm: Update generic gup implementation to handle hugepage directory
From: Benjamin Herrenschmidt Date: Fri, 24 Oct 2014 10:40:35 +1100 > Another option would be to make the generic code use something defined > by the arch to decide whether to use speculative get or > not. I like the idea of keeping the bulk of that code generic... Me too. We could have inlines that do either speculative or non-speculative gets on the pages in some header file and hide the ifdefs in there. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
