* Re: [PATCH] dt: Remove booting-without-of.txt
From: Frank Rowand @ 2020-03-05 22:48 UTC (permalink / raw)
To: Rob Herring, devicetree
Cc: Geert Uytterhoeven, linux-kernel, Mauro Carvalho Chehab,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200304184525.7643-1-robh@kernel.org>
On 3/4/20 12:45 PM, Rob Herring wrote:
> Well, not quite removed yet... Mauro is looking at moving this to ReST,
> but I think it would be better to trim or remove it.
>
> boot-without-of.txt is an ancient document that first outlined
> Flattened DeviceTree. The DT world has evolved a lot in the 15 years
> since and boot-without-of.txt is pretty stale. The name of the document
> itself is confusing if you don't understand the evolution from real
> 'OpenFirmware'. Much of what booting-without-of.txt contains is now in
> the DT specification (which evolved out of the ePAPR).
>
> This is a first pass of removing everything that has a DT spec
> equivalent or is no longer standard practice (e.g. soc<SoCName> for SoC
> nodes) in order to see what's left. This is what I have:
>
> TODO
> - Move boot interface details to arch specific docs
> - Document 'serial-number' property in DT spec
> - Document the 'hotpluggable' memory property in DT spec
> - Document the 'sleep' property (PPC only)
> - Document the 'dma-coherent' property in DT spec
> - Need the history of node names and 'name' property?
> - Need how addresses work?
Looks good. Since this is a first pass, I'm expecting that polishing
(things like updating section numbers) would happen in subsequent
patches after more of the content changes are done, so no need
to do so in this patch.
Reviewed-by: Frank Rowand <frank.rowand@sony.com>
-Frank
>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> .../devicetree/booting-without-of.txt | 1027 +----------------
> 1 file changed, 1 insertion(+), 1026 deletions(-)
>
> diff --git a/Documentation/devicetree/booting-without-of.txt b/Documentation/devicetree/booting-without-of.txt
> index 4660ccee35a3..97beee828ba4 100644
> --- a/Documentation/devicetree/booting-without-of.txt
> +++ b/Documentation/devicetree/booting-without-of.txt
> @@ -19,44 +19,17 @@ Table of Contents
> 5) Entry point for arch/sh
>
> II - The DT block format
> - 1) Header
> 2) Device tree generalities
> - 3) Device tree "structure" block
> - 4) Device tree "strings" block
>
> III - Required content of the device tree
> 1) Note about cells and address representation
> - 2) Note about "compatible" properties
> - 3) Note about "name" properties
> - 4) Note about node and property names and character set
> 5) Required nodes and properties
> a) The root node
> - b) The /cpus node
> - c) The /cpus/* nodes
> - d) the /memory node(s)
> - e) The /chosen node
> - f) the /soc<SOCname> node
> -
> - IV - "dtc", the device tree compiler
> -
> - V - Recommendations for a bootloader
> -
> - VI - System-on-a-chip devices and nodes
> - 1) Defining child nodes of an SOC
> - 2) Representing devices without a current OF specification
> -
> - VII - Specifying interrupt information for devices
> - 1) interrupts property
> - 2) interrupt-parent property
> - 3) OpenPIC Interrupt Controllers
> - 4) ISA Interrupt Controllers
>
> VIII - Specifying device power management information (sleep property)
>
> IX - Specifying dma bus information
>
> - Appendix A - Sample SOC node for MPC8540
> -
>
> Revision Information
> ====================
> @@ -105,19 +78,6 @@ Revision Information
> - Added chapter VI
>
>
> - ToDo:
> - - Add some definitions of interrupt tree (simple/complex)
> - - Add some definitions for PCI host bridges
> - - Add some common address format examples
> - - Add definitions for standard properties and "compatible"
> - names for cells that are not already defined by the existing
> - OF spec.
> - - Compare FSL SOC use of PCI to standard and make sure no new
> - node definition required.
> - - Add more information about node definitions for SOC devices
> - that currently have no standard, like the FSL CPM.
> -
> -
> I - Introduction
> ================
>
> @@ -333,196 +293,17 @@ II - The DT block format
> ========================
>
>
> -This chapter defines the actual format of the flattened device-tree
> -passed to the kernel. The actual content of it and kernel requirements
> -are described later. You can find example of code manipulating that
> -format in various places, including arch/powerpc/kernel/prom_init.c
> -which will generate a flattened device-tree from the Open Firmware
> -representation, or the fs2dt utility which is part of the kexec tools
> -which will generate one from a filesystem representation. It is
> -expected that a bootloader like uboot provides a bit more support,
> -that will be discussed later as well.
> -
> Note: The block has to be in main memory. It has to be accessible in
> both real mode and virtual mode with no mapping other than main
> memory. If you are writing a simple flash bootloader, it should copy
> the block to RAM before passing it to the kernel.
>
>
> -1) Header
> ----------
> -
> - The kernel is passed the physical address pointing to an area of memory
> - that is roughly described in include/linux/of_fdt.h by the structure
> - boot_param_header:
> -
> -struct boot_param_header {
> - u32 magic; /* magic word OF_DT_HEADER */
> - u32 totalsize; /* total size of DT block */
> - u32 off_dt_struct; /* offset to structure */
> - u32 off_dt_strings; /* offset to strings */
> - u32 off_mem_rsvmap; /* offset to memory reserve map
> - */
> - u32 version; /* format version */
> - u32 last_comp_version; /* last compatible version */
> -
> - /* version 2 fields below */
> - u32 boot_cpuid_phys; /* Which physical CPU id we're
> - booting on */
> - /* version 3 fields below */
> - u32 size_dt_strings; /* size of the strings block */
> -
> - /* version 17 fields below */
> - u32 size_dt_struct; /* size of the DT structure block */
> -};
> -
> - Along with the constants:
> -
> -/* Definitions used by the flattened device tree */
> -#define OF_DT_HEADER 0xd00dfeed /* 4: version,
> - 4: total size */
> -#define OF_DT_BEGIN_NODE 0x1 /* Start node: full name
> - */
> -#define OF_DT_END_NODE 0x2 /* End node */
> -#define OF_DT_PROP 0x3 /* Property: name off,
> - size, content */
> -#define OF_DT_END 0x9
> -
> - All values in this header are in big endian format, the various
> - fields in this header are defined more precisely below. All
> - "offset" values are in bytes from the start of the header; that is
> - from the physical base address of the device tree block.
> -
> - - magic
> -
> - This is a magic value that "marks" the beginning of the
> - device-tree block header. It contains the value 0xd00dfeed and is
> - defined by the constant OF_DT_HEADER
> -
> - - totalsize
> -
> - This is the total size of the DT block including the header. The
> - "DT" block should enclose all data structures defined in this
> - chapter (who are pointed to by offsets in this header). That is,
> - the device-tree structure, strings, and the memory reserve map.
> -
> - - off_dt_struct
> -
> - This is an offset from the beginning of the header to the start
> - of the "structure" part the device tree. (see 2) device tree)
> -
> - - off_dt_strings
> -
> - This is an offset from the beginning of the header to the start
> - of the "strings" part of the device-tree
> -
> - - off_mem_rsvmap
> -
> - This is an offset from the beginning of the header to the start
> - of the reserved memory map. This map is a list of pairs of 64-
> - bit integers. Each pair is a physical address and a size. The
> - list is terminated by an entry of size 0. This map provides the
> - kernel with a list of physical memory areas that are "reserved"
> - and thus not to be used for memory allocations, especially during
> - early initialization. The kernel needs to allocate memory during
> - boot for things like un-flattening the device-tree, allocating an
> - MMU hash table, etc... Those allocations must be done in such a
> - way to avoid overriding critical things like, on Open Firmware
> - capable machines, the RTAS instance, or on some pSeries, the TCE
> - tables used for the iommu. Typically, the reserve map should
> - contain _at least_ this DT block itself (header,total_size). If
> - you are passing an initrd to the kernel, you should reserve it as
> - well. You do not need to reserve the kernel image itself. The map
> - should be 64-bit aligned.
> -
> - - version
> -
> - This is the version of this structure. Version 1 stops
> - here. Version 2 adds an additional field boot_cpuid_phys.
> - Version 3 adds the size of the strings block, allowing the kernel
> - to reallocate it easily at boot and free up the unused flattened
> - structure after expansion. Version 16 introduces a new more
> - "compact" format for the tree itself that is however not backward
> - compatible. Version 17 adds an additional field, size_dt_struct,
> - allowing it to be reallocated or moved more easily (this is
> - particularly useful for bootloaders which need to make
> - adjustments to a device tree based on probed information). You
> - should always generate a structure of the highest version defined
> - at the time of your implementation. Currently that is version 17,
> - unless you explicitly aim at being backward compatible.
> -
> - - last_comp_version
> -
> - Last compatible version. This indicates down to what version of
> - the DT block you are backward compatible. For example, version 2
> - is backward compatible with version 1 (that is, a kernel build
> - for version 1 will be able to boot with a version 2 format). You
> - should put a 1 in this field if you generate a device tree of
> - version 1 to 3, or 16 if you generate a tree of version 16 or 17
> - using the new unit name format.
> -
> - - boot_cpuid_phys
> -
> - This field only exist on version 2 headers. It indicate which
> - physical CPU ID is calling the kernel entry point. This is used,
> - among others, by kexec. If you are on an SMP system, this value
> - should match the content of the "reg" property of the CPU node in
> - the device-tree corresponding to the CPU calling the kernel entry
> - point (see further chapters for more information on the required
> - device-tree contents)
> -
> - - size_dt_strings
> -
> - This field only exists on version 3 and later headers. It
> - gives the size of the "strings" section of the device tree (which
> - starts at the offset given by off_dt_strings).
> -
> - - size_dt_struct
> -
> - This field only exists on version 17 and later headers. It gives
> - the size of the "structure" section of the device tree (which
> - starts at the offset given by off_dt_struct).
> -
> - So the typical layout of a DT block (though the various parts don't
> - need to be in that order) looks like this (addresses go from top to
> - bottom):
> -
> -
> - ------------------------------
> - base -> | struct boot_param_header |
> - ------------------------------
> - | (alignment gap) (*) |
> - ------------------------------
> - | memory reserve map |
> - ------------------------------
> - | (alignment gap) |
> - ------------------------------
> - | |
> - | device-tree structure |
> - | |
> - ------------------------------
> - | (alignment gap) |
> - ------------------------------
> - | |
> - | device-tree strings |
> - | |
> - -----> ------------------------------
> - |
> - |
> - --- (base + totalsize)
> -
> - (*) The alignment gaps are not necessarily present; their presence
> - and size are dependent on the various alignment requirements of
> - the individual data blocks.
>
>
> 2) Device tree generalities
> ---------------------------
>
> -This device-tree itself is separated in two different blocks, a
> -structure block and a strings block. Both need to be aligned to a 4
> -byte boundary.
> -
> First, let's quickly describe the device-tree concept before detailing
> the storage format. This chapter does _not_ describe the detail of the
> required types of nodes & properties for the kernel, this is done
> @@ -574,128 +355,6 @@ is) is also required to have a "compatible" property indicating the
> specific hardware and an optional list of devices it is fully
> backwards compatible with.
>
> -Finally, every node that can be referenced from a property in another
> -node is required to have either a "phandle" or a "linux,phandle"
> -property. Real Open Firmware implementations provide a unique
> -"phandle" value for every node that the "prom_init()" trampoline code
> -turns into "linux,phandle" properties. However, this is made optional
> -if the flattened device tree is used directly. An example of a node
> -referencing another node via "phandle" is when laying out the
> -interrupt tree which will be described in a further version of this
> -document.
> -
> -The "phandle" property is a 32-bit value that uniquely
> -identifies a node. You are free to use whatever values or system of
> -values, internal pointers, or whatever to generate these, the only
> -requirement is that every node for which you provide that property has
> -a unique value for it.
> -
> -Here is an example of a simple device-tree. In this example, an "o"
> -designates a node followed by the node unit name. Properties are
> -presented with their name followed by their content. "content"
> -represents an ASCII string (zero terminated) value, while <content>
> -represents a 32-bit value, specified in decimal or hexadecimal (the
> -latter prefixed 0x). The various nodes in this example will be
> -discussed in a later chapter. At this point, it is only meant to give
> -you a idea of what a device-tree looks like. I have purposefully kept
> -the "name" and "linux,phandle" properties which aren't necessary in
> -order to give you a better idea of what the tree looks like in
> -practice.
> -
> - / o device-tree
> - |- name = "device-tree"
> - |- model = "MyBoardName"
> - |- compatible = "MyBoardFamilyName"
> - |- #address-cells = <2>
> - |- #size-cells = <2>
> - |- linux,phandle = <0>
> - |
> - o cpus
> - | | - name = "cpus"
> - | | - linux,phandle = <1>
> - | | - #address-cells = <1>
> - | | - #size-cells = <0>
> - | |
> - | o PowerPC,970@0
> - | |- name = "PowerPC,970"
> - | |- device_type = "cpu"
> - | |- reg = <0>
> - | |- clock-frequency = <0x5f5e1000>
> - | |- 64-bit
> - | |- linux,phandle = <2>
> - |
> - o memory@0
> - | |- name = "memory"
> - | |- device_type = "memory"
> - | |- reg = <0x00000000 0x00000000 0x00000000 0x20000000>
> - | |- linux,phandle = <3>
> - |
> - o chosen
> - |- name = "chosen"
> - |- bootargs = "root=/dev/sda2"
> - |- linux,phandle = <4>
> -
> -This tree is almost a minimal tree. It pretty much contains the
> -minimal set of required nodes and properties to boot a linux kernel;
> -that is, some basic model information at the root, the CPUs, and the
> -physical memory layout. It also includes misc information passed
> -through /chosen, like in this example, the platform type (mandatory)
> -and the kernel command line arguments (optional).
> -
> -The /cpus/PowerPC,970@0/64-bit property is an example of a
> -property without a value. All other properties have a value. The
> -significance of the #address-cells and #size-cells properties will be
> -explained in chapter IV which defines precisely the required nodes and
> -properties and their content.
> -
> -
> -3) Device tree "structure" block
> -
> -The structure of the device tree is a linearized tree structure. The
> -"OF_DT_BEGIN_NODE" token starts a new node, and the "OF_DT_END_NODE"
> -ends that node definition. Child nodes are simply defined before
> -"OF_DT_END_NODE" (that is nodes within the node). A 'token' is a 32
> -bit value. The tree has to be "finished" with a OF_DT_END token
> -
> -Here's the basic structure of a single node:
> -
> - * token OF_DT_BEGIN_NODE (that is 0x00000001)
> - * for version 1 to 3, this is the node full path as a zero
> - terminated string, starting with "/". For version 16 and later,
> - this is the node unit name only (or an empty string for the
> - root node)
> - * [align gap to next 4 bytes boundary]
> - * for each property:
> - * token OF_DT_PROP (that is 0x00000003)
> - * 32-bit value of property value size in bytes (or 0 if no
> - value)
> - * 32-bit value of offset in string block of property name
> - * property value data if any
> - * [align gap to next 4 bytes boundary]
> - * [child nodes if any]
> - * token OF_DT_END_NODE (that is 0x00000002)
> -
> -So the node content can be summarized as a start token, a full path,
> -a list of properties, a list of child nodes, and an end token. Every
> -child node is a full node structure itself as defined above.
> -
> -NOTE: The above definition requires that all property definitions for
> -a particular node MUST precede any subnode definitions for that node.
> -Although the structure would not be ambiguous if properties and
> -subnodes were intermingled, the kernel parser requires that the
> -properties come first (up until at least 2.6.22). Any tools
> -manipulating a flattened tree must take care to preserve this
> -constraint.
> -
> -4) Device tree "strings" block
> -
> -In order to save space, property names, which are generally redundant,
> -are stored separately in the "strings" block. This block is simply the
> -whole bunch of zero terminated strings for all property names
> -concatenated together. The device-tree property definitions in the
> -structure block will contain offset values from the beginning of the
> -strings block.
> -
>
> III - Required content of the device tree
> =========================================
> @@ -792,568 +451,14 @@ registers are visible on the parent bus using an identity mapping
> translation. In other words, the parent bus address space is the same
> as the child bus address space.
>
> -2) Note about "compatible" properties
> --------------------------------------
> -
> -These properties are optional, but recommended in devices and the root
> -node. The format of a "compatible" property is a list of concatenated
> -zero terminated strings. They allow a device to express its
> -compatibility with a family of similar devices, in some cases,
> -allowing a single driver to match against several devices regardless
> -of their actual names.
> -
> -3) Note about "name" properties
> --------------------------------
> -
> -While earlier users of Open Firmware like OldWorld macintoshes tended
> -to use the actual device name for the "name" property, it's nowadays
> -considered a good practice to use a name that is closer to the device
> -class (often equal to device_type). For example, nowadays, Ethernet
> -controllers are named "ethernet", an additional "model" property
> -defining precisely the chip type/model, and "compatible" property
> -defining the family in case a single driver can driver more than one
> -of these chips. However, the kernel doesn't generally put any
> -restriction on the "name" property; it is simply considered good
> -practice to follow the standard and its evolutions as closely as
> -possible.
> -
> -Note also that the new format version 16 makes the "name" property
> -optional. If it's absent for a node, then the node's unit name is then
> -used to reconstruct the name. That is, the part of the unit name
> -before the "@" sign is used (or the entire unit name if no "@" sign
> -is present).
> -
> -4) Note about node and property names and character set
> --------------------------------------------------------
> -
> -While Open Firmware provides more flexible usage of 8859-1, this
> -specification enforces more strict rules. Nodes and properties should
> -be comprised only of ASCII characters 'a' to 'z', '0' to
> -'9', ',', '.', '_', '+', '#', '?', and '-'. Node names additionally
> -allow uppercase characters 'A' to 'Z' (property names should be
> -lowercase. The fact that vendors like Apple don't respect this rule is
> -irrelevant here). Additionally, node and property names should always
> -begin with a character in the range 'a' to 'z' (or 'A' to 'Z' for node
> -names).
> -
> -The maximum number of characters for both nodes and property names
> -is 31. In the case of node names, this is only the leftmost part of
> -a unit name (the pure "name" property), it doesn't include the unit
> -address which can extend beyond that limit.
> -
>
> 5) Required nodes and properties
> --------------------------------
> - These are all that are currently required. However, it is strongly
> - recommended that you expose PCI host bridges as documented in the
> - PCI binding to Open Firmware, and your interrupt tree as documented
> - in OF interrupt tree specification.
> -
> - a) The root node
> -
> - The root node requires some properties to be present:
> -
> - - model : this is your board name/model
> - - #address-cells : address representation for "root" devices
> - - #size-cells: the size representation for "root" devices
> - - compatible : the board "family" generally finds its way here,
> - for example, if you have 2 board models with a similar layout,
> - that typically get driven by the same platform code in the
> - kernel, you would specify the exact board model in the
> - compatible property followed by an entry that represents the SoC
> - model.
> -
> - The root node is also generally where you add additional properties
> - specific to your board like the serial number if any, that sort of
> - thing. It is recommended that if you add any "custom" property whose
> - name may clash with standard defined ones, you prefix them with your
> - vendor name and a comma.
>
> Additional properties for the root node:
>
> - serial-number : a string representing the device's serial number
>
> - b) The /cpus node
> -
> - This node is the parent of all individual CPU nodes. It doesn't
> - have any specific requirements, though it's generally good practice
> - to have at least:
> -
> - #address-cells = <00000001>
> - #size-cells = <00000000>
> -
> - This defines that the "address" for a CPU is a single cell, and has
> - no meaningful size. This is not necessary but the kernel will assume
> - that format when reading the "reg" properties of a CPU node, see
> - below
> -
> - c) The /cpus/* nodes
> -
> - So under /cpus, you are supposed to create a node for every CPU on
> - the machine. There is no specific restriction on the name of the
> - CPU, though it's common to call it <architecture>,<core>. For
> - example, Apple uses PowerPC,G5 while IBM uses PowerPC,970FX.
> - However, the Generic Names convention suggests that it would be
> - better to simply use 'cpu' for each cpu node and use the compatible
> - property to identify the specific cpu core.
> -
> - Required properties:
> -
> - - device_type : has to be "cpu"
> - - reg : This is the physical CPU number, it's a single 32-bit cell
> - and is also used as-is as the unit number for constructing the
> - unit name in the full path. For example, with 2 CPUs, you would
> - have the full path:
> - /cpus/PowerPC,970FX@0
> - /cpus/PowerPC,970FX@1
> - (unit addresses do not require leading zeroes)
> - - d-cache-block-size : one cell, L1 data cache block size in bytes (*)
> - - i-cache-block-size : one cell, L1 instruction cache block size in
> - bytes
> - - d-cache-size : one cell, size of L1 data cache in bytes
> - - i-cache-size : one cell, size of L1 instruction cache in bytes
> -
> -(*) The cache "block" size is the size on which the cache management
> -instructions operate. Historically, this document used the cache
> -"line" size here which is incorrect. The kernel will prefer the cache
> -block size and will fallback to cache line size for backward
> -compatibility.
> -
> - Recommended properties:
> -
> - - timebase-frequency : a cell indicating the frequency of the
> - timebase in Hz. This is not directly used by the generic code,
> - but you are welcome to copy/paste the pSeries code for setting
> - the kernel timebase/decrementer calibration based on this
> - value.
> - - clock-frequency : a cell indicating the CPU core clock frequency
> - in Hz. A new property will be defined for 64-bit values, but if
> - your frequency is < 4Ghz, one cell is enough. Here as well as
> - for the above, the common code doesn't use that property, but
> - you are welcome to re-use the pSeries or Maple one. A future
> - kernel version might provide a common function for this.
> - - d-cache-line-size : one cell, L1 data cache line size in bytes
> - if different from the block size
> - - i-cache-line-size : one cell, L1 instruction cache line size in
> - bytes if different from the block size
> -
> - You are welcome to add any property you find relevant to your board,
> - like some information about the mechanism used to soft-reset the
> - CPUs. For example, Apple puts the GPIO number for CPU soft reset
> - lines in there as a "soft-reset" property since they start secondary
> - CPUs by soft-resetting them.
> -
> -
> - d) the /memory node(s)
> -
> - To define the physical memory layout of your board, you should
> - create one or more memory node(s). You can either create a single
> - node with all memory ranges in its reg property, or you can create
> - several nodes, as you wish. The unit address (@ part) used for the
> - full path is the address of the first range of memory defined by a
> - given node. If you use a single memory node, this will typically be
> - @0.
> -
> - Required properties:
> -
> - - device_type : has to be "memory"
> - - reg : This property contains all the physical memory ranges of
> - your board. It's a list of addresses/sizes concatenated
> - together, with the number of cells of each defined by the
> - #address-cells and #size-cells of the root node. For example,
> - with both of these properties being 2 like in the example given
> - earlier, a 970 based machine with 6Gb of RAM could typically
> - have a "reg" property here that looks like:
> -
> - 00000000 00000000 00000000 80000000
> - 00000001 00000000 00000001 00000000
> -
> - That is a range starting at 0 of 0x80000000 bytes and a range
> - starting at 0x100000000 and of 0x100000000 bytes. You can see
> - that there is no memory covering the IO hole between 2Gb and
> - 4Gb. Some vendors prefer splitting those ranges into smaller
> - segments, but the kernel doesn't care.
> -
> - Additional properties:
> -
> - - hotpluggable : The presence of this property provides an explicit
> - hint to the operating system that this memory may potentially be
> - removed later. The kernel can take this into consideration when
> - doing nonmovable allocations and when laying out memory zones.
> -
> - e) The /chosen node
> -
> - This node is a bit "special". Normally, that's where Open Firmware
> - puts some variable environment information, like the arguments, or
> - the default input/output devices.
> -
> - This specification makes a few of these mandatory, but also defines
> - some linux-specific properties that would be normally constructed by
> - the prom_init() trampoline when booting with an OF client interface,
> - but that you have to provide yourself when using the flattened format.
> -
> - Recommended properties:
> -
> - - bootargs : This zero-terminated string is passed as the kernel
> - command line
> - - linux,stdout-path : This is the full path to your standard
> - console device if any. Typically, if you have serial devices on
> - your board, you may want to put the full path to the one set as
> - the default console in the firmware here, for the kernel to pick
> - it up as its own default console.
> -
> - Note that u-boot creates and fills in the chosen node for platforms
> - that use it.
> -
> - (Note: a practice that is now obsolete was to include a property
> - under /chosen called interrupt-controller which had a phandle value
> - that pointed to the main interrupt controller)
> -
> - f) the /soc<SOCname> node
> -
> - This node is used to represent a system-on-a-chip (SoC) and must be
> - present if the processor is a SoC. The top-level soc node contains
> - information that is global to all devices on the SoC. The node name
> - should contain a unit address for the SoC, which is the base address
> - of the memory-mapped register set for the SoC. The name of an SoC
> - node should start with "soc", and the remainder of the name should
> - represent the part number for the soc. For example, the MPC8540's
> - soc node would be called "soc8540".
> -
> - Required properties:
> -
> - - ranges : Should be defined as specified in 1) to describe the
> - translation of SoC addresses for memory mapped SoC registers.
> - - bus-frequency: Contains the bus frequency for the SoC node.
> - Typically, the value of this field is filled in by the boot
> - loader.
> - - compatible : Exact model of the SoC
> -
> -
> - Recommended properties:
> -
> - - reg : This property defines the address and size of the
> - memory-mapped registers that are used for the SOC node itself.
> - It does not include the child device registers - these will be
> - defined inside each child node. The address specified in the
> - "reg" property should match the unit address of the SOC node.
> - - #address-cells : Address representation for "soc" devices. The
> - format of this field may vary depending on whether or not the
> - device registers are memory mapped. For memory mapped
> - registers, this field represents the number of cells needed to
> - represent the address of the registers. For SOCs that do not
> - use MMIO, a special address format should be defined that
> - contains enough cells to represent the required information.
> - See 1) above for more details on defining #address-cells.
> - - #size-cells : Size representation for "soc" devices
> - - #interrupt-cells : Defines the width of cells used to represent
> - interrupts. Typically this value is <2>, which includes a
> - 32-bit number that represents the interrupt number, and a
> - 32-bit number that represents the interrupt sense and level.
> - This field is only needed if the SOC contains an interrupt
> - controller.
> -
> - The SOC node may contain child nodes for each SOC device that the
> - platform uses. Nodes should not be created for devices which exist
> - on the SOC but are not used by a particular platform. See chapter VI
> - for more information on how to specify devices that are part of a SOC.
> -
> - Example SOC node for the MPC8540:
> -
> - soc8540@e0000000 {
> - #address-cells = <1>;
> - #size-cells = <1>;
> - #interrupt-cells = <2>;
> - device_type = "soc";
> - ranges = <0x00000000 0xe0000000 0x00100000>
> - reg = <0xe0000000 0x00003000>;
> - bus-frequency = <0>;
> - }
> -
> -
> -
> -IV - "dtc", the device tree compiler
> -====================================
> -
> -
> -dtc source code can be found at
> -<http://git.jdl.com/gitweb/?p=dtc.git>
> -
> -WARNING: This version is still in early development stage; the
> -resulting device-tree "blobs" have not yet been validated with the
> -kernel. The current generated block lacks a useful reserve map (it will
> -be fixed to generate an empty one, it's up to the bootloader to fill
> -it up) among others. The error handling needs work, bugs are lurking,
> -etc...
> -
> -dtc basically takes a device-tree in a given format and outputs a
> -device-tree in another format. The currently supported formats are:
> -
> - Input formats:
> - -------------
> -
> - - "dtb": "blob" format, that is a flattened device-tree block
> - with
> - header all in a binary blob.
> - - "dts": "source" format. This is a text file containing a
> - "source" for a device-tree. The format is defined later in this
> - chapter.
> - - "fs" format. This is a representation equivalent to the
> - output of /proc/device-tree, that is nodes are directories and
> - properties are files
> -
> - Output formats:
> - ---------------
> -
> - - "dtb": "blob" format
> - - "dts": "source" format
> - - "asm": assembly language file. This is a file that can be
> - sourced by gas to generate a device-tree "blob". That file can
> - then simply be added to your Makefile. Additionally, the
> - assembly file exports some symbols that can be used.
> -
> -
> -The syntax of the dtc tool is
> -
> - dtc [-I <input-format>] [-O <output-format>]
> - [-o output-filename] [-V output_version] input_filename
> -
> -
> -The "output_version" defines what version of the "blob" format will be
> -generated. Supported versions are 1,2,3 and 16. The default is
> -currently version 3 but that may change in the future to version 16.
> -
> -Additionally, dtc performs various sanity checks on the tree, like the
> -uniqueness of linux, phandle properties, validity of strings, etc...
> -
> -The format of the .dts "source" file is "C" like, supports C and C++
> -style comments.
> -
> -/ {
> -}
> -
> -The above is the "device-tree" definition. It's the only statement
> -supported currently at the toplevel.
> -
> -/ {
> - property1 = "string_value"; /* define a property containing a 0
> - * terminated string
> - */
> -
> - property2 = <0x1234abcd>; /* define a property containing a
> - * numerical 32-bit value (hexadecimal)
> - */
> -
> - property3 = <0x12345678 0x12345678 0xdeadbeef>;
> - /* define a property containing 3
> - * numerical 32-bit values (cells) in
> - * hexadecimal
> - */
> - property4 = [0x0a 0x0b 0x0c 0x0d 0xde 0xea 0xad 0xbe 0xef];
> - /* define a property whose content is
> - * an arbitrary array of bytes
> - */
> -
> - childnode@address { /* define a child node named "childnode"
> - * whose unit name is "childnode at
> - * address"
> - */
> -
> - childprop = "hello\n"; /* define a property "childprop" of
> - * childnode (in this case, a string)
> - */
> - };
> -};
> -
> -Nodes can contain other nodes etc... thus defining the hierarchical
> -structure of the tree.
> -
> -Strings support common escape sequences from C: "\n", "\t", "\r",
> -"\(octal value)", "\x(hex value)".
> -
> -It is also suggested that you pipe your source file through cpp (gcc
> -preprocessor) so you can use #include's, #define for constants, etc...
> -
> -Finally, various options are planned but not yet implemented, like
> -automatic generation of phandles, labels (exported to the asm file so
> -you can point to a property content and change it easily from whatever
> -you link the device-tree with), label or path instead of numeric value
> -in some cells to "point" to a node (replaced by a phandle at compile
> -time), export of reserve map address to the asm file, ability to
> -specify reserve map content at compile time, etc...
> -
> -We may provide a .h include file with common definitions of that
> -proves useful for some properties (like building PCI properties or
> -interrupt maps) though it may be better to add a notion of struct
> -definitions to the compiler...
> -
> -
> -V - Recommendations for a bootloader
> -====================================
> -
> -
> -Here are some various ideas/recommendations that have been proposed
> -while all this has been defined and implemented.
> -
> - - The bootloader may want to be able to use the device-tree itself
> - and may want to manipulate it (to add/edit some properties,
> - like physical memory size or kernel arguments). At this point, 2
> - choices can be made. Either the bootloader works directly on the
> - flattened format, or the bootloader has its own internal tree
> - representation with pointers (similar to the kernel one) and
> - re-flattens the tree when booting the kernel. The former is a bit
> - more difficult to edit/modify, the later requires probably a bit
> - more code to handle the tree structure. Note that the structure
> - format has been designed so it's relatively easy to "insert"
> - properties or nodes or delete them by just memmoving things
> - around. It contains no internal offsets or pointers for this
> - purpose.
> -
> - - An example of code for iterating nodes & retrieving properties
> - directly from the flattened tree format can be found in the kernel
> - file drivers/of/fdt.c. Look at the of_scan_flat_dt() function,
> - its usage in early_init_devtree(), and the corresponding various
> - early_init_dt_scan_*() callbacks. That code can be re-used in a
> - GPL bootloader, and as the author of that code, I would be happy
> - to discuss possible free licensing to any vendor who wishes to
> - integrate all or part of this code into a non-GPL bootloader.
> - (reference needed; who is 'I' here? ---gcl Jan 31, 2011)
> -
> -
> -
> -VI - System-on-a-chip devices and nodes
> -=======================================
> -
> -Many companies are now starting to develop system-on-a-chip
> -processors, where the processor core (CPU) and many peripheral devices
> -exist on a single piece of silicon. For these SOCs, an SOC node
> -should be used that defines child nodes for the devices that make
> -up the SOC. While platforms are not required to use this model in
> -order to boot the kernel, it is highly encouraged that all SOC
> -implementations define as complete a flat-device-tree as possible to
> -describe the devices on the SOC. This will allow for the
> -genericization of much of the kernel code.
> -
> -
> -1) Defining child nodes of an SOC
> ----------------------------------
> -
> -Each device that is part of an SOC may have its own node entry inside
> -the SOC node. For each device that is included in the SOC, the unit
> -address property represents the address offset for this device's
> -memory-mapped registers in the parent's address space. The parent's
> -address space is defined by the "ranges" property in the top-level soc
> -node. The "reg" property for each node that exists directly under the
> -SOC node should contain the address mapping from the child address space
> -to the parent SOC address space and the size of the device's
> -memory-mapped register file.
> -
> -For many devices that may exist inside an SOC, there are predefined
> -specifications for the format of the device tree node. All SOC child
> -nodes should follow these specifications, except where noted in this
> -document.
> -
> -See appendix A for an example partial SOC node definition for the
> -MPC8540.
> -
> -
> -2) Representing devices without a current OF specification
> -----------------------------------------------------------
> -
> -Currently, there are many devices on SoCs that do not have a standard
> -representation defined as part of the Open Firmware specifications,
> -mainly because the boards that contain these SoCs are not currently
> -booted using Open Firmware. Binding documentation for new devices
> -should be added to the Documentation/devicetree/bindings directory.
> -That directory will expand as device tree support is added to more and
> -more SoCs.
> -
> -
> -VII - Specifying interrupt information for devices
> -===================================================
> -
> -The device tree represents the buses and devices of a hardware
> -system in a form similar to the physical bus topology of the
> -hardware.
> -
> -In addition, a logical 'interrupt tree' exists which represents the
> -hierarchy and routing of interrupts in the hardware.
> -
> -The interrupt tree model is fully described in the
> -document "Open Firmware Recommended Practice: Interrupt
> -Mapping Version 0.9". The document is available at:
> -<http://www.devicetree.org/open-firmware/practice/>
> -
> -1) interrupts property
> -----------------------
> -
> -Devices that generate interrupts to a single interrupt controller
> -should use the conventional OF representation described in the
> -OF interrupt mapping documentation.
> -
> -Each device which generates interrupts must have an 'interrupt'
> -property. The interrupt property value is an arbitrary number of
> -of 'interrupt specifier' values which describe the interrupt or
> -interrupts for the device.
> -
> -The encoding of an interrupt specifier is determined by the
> -interrupt domain in which the device is located in the
> -interrupt tree. The root of an interrupt domain specifies in
> -its #interrupt-cells property the number of 32-bit cells
> -required to encode an interrupt specifier. See the OF interrupt
> -mapping documentation for a detailed description of domains.
> -
> -For example, the binding for the OpenPIC interrupt controller
> -specifies an #interrupt-cells value of 2 to encode the interrupt
> -number and level/sense information. All interrupt children in an
> -OpenPIC interrupt domain use 2 cells per interrupt in their interrupts
> -property.
> -
> -The PCI bus binding specifies a #interrupt-cells value of 1 to encode
> -which interrupt pin (INTA,INTB,INTC,INTD) is used.
> -
> -2) interrupt-parent property
> -----------------------------
> -
> -The interrupt-parent property is specified to define an explicit
> -link between a device node and its interrupt parent in
> -the interrupt tree. The value of interrupt-parent is the
> -phandle of the parent node.
> -
> -If the interrupt-parent property is not defined for a node, its
> -interrupt parent is assumed to be an ancestor in the node's
> -_device tree_ hierarchy.
> -
> -3) OpenPIC Interrupt Controllers
> ---------------------------------
> -
> -OpenPIC interrupt controllers require 2 cells to encode
> -interrupt information. The first cell defines the interrupt
> -number. The second cell defines the sense and level
> -information.
> -
> -Sense and level information should be encoded as follows:
> -
> - 0 = low to high edge sensitive type enabled
> - 1 = active low level sensitive type enabled
> - 2 = active high level sensitive type enabled
> - 3 = high to low edge sensitive type enabled
> -
> -4) ISA Interrupt Controllers
> -----------------------------
> -
> -ISA PIC interrupt controllers require 2 cells to encode
> -interrupt information. The first cell defines the interrupt
> -number. The second cell defines the sense and level
> -information.
> -
> -ISA PIC interrupt controllers should adhere to the ISA PIC
> -encodings listed below:
> -
> - 0 = active low level sensitive type enabled
> - 1 = active high level sensitive type enabled
> - 2 = high to low edge sensitive type enabled
> - 3 = low to high edge sensitive type enabled
>
> VIII - Specifying Device Power Management Information (sleep property)
> ===================================================================
> @@ -1386,6 +491,7 @@ reasonably grouped in this manner, then create a virtual sleep controller
> (similar to an interrupt nexus, except that defining a standardized
> sleep-map should wait until its necessity is demonstrated).
>
> +
> IX - Specifying dma bus information
>
> Some devices may have DMA memory range shifted relatively to the beginning of
> @@ -1420,134 +526,3 @@ Optional property:
> - dma-ranges: <empty> value. if present - It means that DMA addresses
> translation has to be enabled for this device.
> - dma-coherent: Present if dma operations are coherent
> -
> -Example:
> -soc {
> - compatible = "ti,keystone","simple-bus";
> - ranges = <0x0 0x0 0x0 0xc0000000>;
> - dma-ranges = <0x80000000 0x8 0x00000000 0x80000000>;
> -
> - [...]
> -
> - usb: usb@2680000 {
> - compatible = "ti,keystone-dwc3";
> -
> - [...]
> - dma-coherent;
> - };
> -};
> -
> -Appendix A - Sample SOC node for MPC8540
> -========================================
> -
> - soc@e0000000 {
> - #address-cells = <1>;
> - #size-cells = <1>;
> - compatible = "fsl,mpc8540-ccsr", "simple-bus";
> - device_type = "soc";
> - ranges = <0x00000000 0xe0000000 0x00100000>
> - bus-frequency = <0>;
> - interrupt-parent = <&pic>;
> -
> - ethernet@24000 {
> - #address-cells = <1>;
> - #size-cells = <1>;
> - device_type = "network";
> - model = "TSEC";
> - compatible = "gianfar", "simple-bus";
> - reg = <0x24000 0x1000>;
> - local-mac-address = [ 0x00 0xE0 0x0C 0x00 0x73 0x00 ];
> - interrupts = <0x29 2 0x30 2 0x34 2>;
> - phy-handle = <&phy0>;
> - sleep = <&pmc 0x00000080>;
> - ranges;
> -
> - mdio@24520 {
> - reg = <0x24520 0x20>;
> - compatible = "fsl,gianfar-mdio";
> -
> - phy0: ethernet-phy@0 {
> - interrupts = <5 1>;
> - reg = <0>;
> - };
> -
> - phy1: ethernet-phy@1 {
> - interrupts = <5 1>;
> - reg = <1>;
> - };
> -
> - phy3: ethernet-phy@3 {
> - interrupts = <7 1>;
> - reg = <3>;
> - };
> - };
> - };
> -
> - ethernet@25000 {
> - device_type = "network";
> - model = "TSEC";
> - compatible = "gianfar";
> - reg = <0x25000 0x1000>;
> - local-mac-address = [ 0x00 0xE0 0x0C 0x00 0x73 0x01 ];
> - interrupts = <0x13 2 0x14 2 0x18 2>;
> - phy-handle = <&phy1>;
> - sleep = <&pmc 0x00000040>;
> - };
> -
> - ethernet@26000 {
> - device_type = "network";
> - model = "FEC";
> - compatible = "gianfar";
> - reg = <0x26000 0x1000>;
> - local-mac-address = [ 0x00 0xE0 0x0C 0x00 0x73 0x02 ];
> - interrupts = <0x41 2>;
> - phy-handle = <&phy3>;
> - sleep = <&pmc 0x00000020>;
> - };
> -
> - serial@4500 {
> - #address-cells = <1>;
> - #size-cells = <1>;
> - compatible = "fsl,mpc8540-duart", "simple-bus";
> - sleep = <&pmc 0x00000002>;
> - ranges;
> -
> - serial@4500 {
> - device_type = "serial";
> - compatible = "ns16550";
> - reg = <0x4500 0x100>;
> - clock-frequency = <0>;
> - interrupts = <0x42 2>;
> - };
> -
> - serial@4600 {
> - device_type = "serial";
> - compatible = "ns16550";
> - reg = <0x4600 0x100>;
> - clock-frequency = <0>;
> - interrupts = <0x42 2>;
> - };
> - };
> -
> - pic: pic@40000 {
> - interrupt-controller;
> - #address-cells = <0>;
> - #interrupt-cells = <2>;
> - reg = <0x40000 0x40000>;
> - compatible = "chrp,open-pic";
> - device_type = "open-pic";
> - };
> -
> - i2c@3000 {
> - interrupts = <0x43 2>;
> - reg = <0x3000 0x100>;
> - compatible = "fsl-i2c";
> - dfsrr;
> - sleep = <&pmc 0x00000004>;
> - };
> -
> - pmc: power@e0070 {
> - compatible = "fsl,mpc8540-pmc", "fsl,mpc8548-pmc";
> - reg = <0xe0070 0x20>;
> - };
> - };
>
^ permalink raw reply
* [Bug 206695] kmemleak reports leaks in drivers/macintosh/windfarm
From: bugzilla-daemon @ 2020-03-05 22:09 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206695-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206695
--- Comment #4 from Erhard F. (erhard_f@mailbox.org) ---
(In reply to mpe from comment #3)
> Can you try this patch?
Applied your patch on top of 5.6-rc4 +
https://patchwork.ozlabs.org/patch/1248350/ and let the G5 do a few hours
compiling.
Only getting those nice memleaks from bug #206203 but no windfarm_pm112 memleak
any longer. So your patch works well it seems. Thanks!
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [RFC 00/11] perf: Enhancing perf to export processor hazard information
From: Kim Phillips @ 2020-03-05 22:06 UTC (permalink / raw)
To: Ravi Bangoria
Cc: Mark Rutland, Andi Kleen, maddy, Peter Zijlstra, Jiri Olsa, LKML,
Stephane Eranian, Adrian Hunter, Alexander Shishkin, yao.jin,
Ingo Molnar, Paul Mackerras, Arnaldo Carvalho de Melo,
Robert Richter, Namhyung Kim, linuxppc-dev, Alexey Budankov,
Liang, Kan
In-Reply-To: <2550ec4d-a015-4625-ca24-ff10632dbe2e@linux.ibm.com>
On 3/4/20 10:46 PM, Ravi Bangoria wrote:
> Hi Kim,
Hi Ravi,
> On 3/3/20 3:55 AM, Kim Phillips wrote:
>> On 3/2/20 2:21 PM, Stephane Eranian wrote:
>>> On Mon, Mar 2, 2020 at 2:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
>>>>
>>>> On Mon, Mar 02, 2020 at 10:53:44AM +0530, Ravi Bangoria wrote:
>>>>> Modern processors export such hazard data in Performance
>>>>> Monitoring Unit (PMU) registers. Ex, 'Sampled Instruction Event
>>>>> Register' on IBM PowerPC[1][2] and 'Instruction-Based Sampling' on
>>>>> AMD[3] provides similar information.
>>>>>
>>>>> Implementation detail:
>>>>>
>>>>> A new sample_type called PERF_SAMPLE_PIPELINE_HAZ is introduced.
>>>>> If it's set, kernel converts arch specific hazard information
>>>>> into generic format:
>>>>>
>>>>> struct perf_pipeline_haz_data {
>>>>> /* Instruction/Opcode type: Load, Store, Branch .... */
>>>>> __u8 itype;
>>>>> /* Instruction Cache source */
>>>>> __u8 icache;
>>>>> /* Instruction suffered hazard in pipeline stage */
>>>>> __u8 hazard_stage;
>>>>> /* Hazard reason */
>>>>> __u8 hazard_reason;
>>>>> /* Instruction suffered stall in pipeline stage */
>>>>> __u8 stall_stage;
>>>>> /* Stall reason */
>>>>> __u8 stall_reason;
>>>>> __u16 pad;
>>>>> };
>>>>
>>>> Kim, does this format indeed work for AMD IBS?
>>
>> It's not really 1:1, we don't have these separations of stages
>> and reasons, for example: we have missed in L2 cache, for example.
>> So IBS output is flatter, with more cycle latency figures than
>> IBM's AFAICT.
>
> AMD IBS captures pipeline latency data incase Fetch sampling like the
> Fetch latency, tag to retire latency, completion to retire latency and
> so on. Yes, Ops sampling do provide more data on load/store centric
> information. But it also captures more detailed data for Branch instructions.
> And we also looked at ARM SPE, which also captures more details pipeline
> data and latency information.
>
>>> Personally, I don't like the term hazard. This is too IBM Power
>>> specific. We need to find a better term, maybe stall or penalty.
>>
>> Right, IBS doesn't have a filter to only count stalled or otherwise
>> bad events. IBS' PPR descriptions has one occurrence of the
>> word stall, and no penalty. The way I read IBS is it's just
>> reporting more sample data than just the precise IP: things like
>> hits, misses, cycle latencies, addresses, types, etc., so words
>> like 'extended', or the 'auxiliary' already used today even
>> are more appropriate for IBS, although I'm the last person to
>> bikeshed.
>
> We are thinking of using "pipeline" word instead of Hazard.
Hm, the word 'pipeline' occurs 0 times in IBS documentation.
I realize there are a couple of core pipeline-specific pieces
of information coming out of it, but the vast majority
are addresses, latencies of various components in the memory
hierarchy, and various component hit/miss bits.
What's needed here is a vendor-specific extended
sample information that all these technologies gather,
of which things like e.g., 'L1 TLB cycle latency' we
all should have in common.
I'm not sure why a new PERF_SAMPLE_PIPELINE_HAZ is needed
either. Can we use PERF_SAMPLE_AUX instead? Take a look at
commit 98dcf14d7f9c "perf tools: Add kernel AUX area sampling
definitions". The sample identifier can be used to determine
which vendor's sampling IP's data is in it, and events can
be recorded just by copying the content of the SIER, etc.
registers, and then events get synthesized from the aux
sample at report/inject/annotate etc. time. This allows
for less sample recording overhead, and moves all the vendor
specific decoding and common event conversions for userspace
to figure out.
>>> Also worth considering is the support of ARM SPE (Statistical
>>> Profiling Extension) which is their version of IBS.
>>> Whatever gets added need to cover all three with no limitations.
>>
>> I thought Intel's various LBR, PEBS, and PT supported providing
>> similar sample data in perf already, like with perf mem/c2c?
>
> perf-mem is more of data centric in my opinion. It is more towards
> memory profiling. So proposal here is to expose pipeline related
> details like stalls and latencies.
Like I said, I don't see it that way, I see it as "any particular
vendor's event's extended details', and these pipeline details
have overlap with existing infrastructure within perf, e.g., L2
cache misses.
Kim
^ permalink raw reply
* Re: [PATCH v2] powerpc/mm: Fix missing KUAP disable in flush_coherent_icache()
From: Russell Currey @ 2020-03-05 20:19 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
In-Reply-To: <20200303235708.26004-1-mpe@ellerman.id.au>
On Wed, 2020-03-04 at 10:57 +1100, Michael Ellerman wrote:
> We received a report of strange kernel faults which turned out to be
> due to a missing KUAP disable in flush_coherent_icache() called
> from flush_icache_range().
>
> The fault looks like:
>
> Kernel attempted to access user page (7fffc30d9c00) - exploit
> attempt? (uid: 1009)
> BUG: Unable to handle kernel data access on read at 0x7fffc30d9c00
> Faulting instruction address: 0xc00000000007232c
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV
> CPU: 35 PID: 5886 Comm: sigtramp Not tainted 5.6.0-rc2-gcc-8.2.0-
> 00003-gfc37a1632d40 #79
> NIP: c00000000007232c LR: c00000000003b7fc CTR: 0000000000000000
> REGS: c000001e11093940 TRAP: 0300 Not tainted (5.6.0-rc2-gcc-
> 8.2.0-00003-gfc37a1632d40)
> MSR: 900000000280b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR:
> 28000884 XER: 00000000
> CFAR: c0000000000722fc DAR: 00007fffc30d9c00 DSISR: 08000000
> IRQMASK: 0
> GPR00: c00000000003b7fc c000001e11093bd0 c0000000023ac200
> 00007fffc30d9c00
> GPR04: 00007fffc30d9c18 0000000000000000 c000001e11093bd4
> 0000000000000000
> GPR08: 0000000000000000 0000000000000001 0000000000000000
> c000001e1104ed80
> GPR12: 0000000000000000 c000001fff6ab380 c0000000016be2d0
> 4000000000000000
> GPR16: c000000000000000 bfffffffffffffff 0000000000000000
> 0000000000000000
> GPR20: 00007fffc30d9c00 00007fffc30d8f58 00007fffc30d9c18
> 00007fffc30d9c20
> GPR24: 00007fffc30d9c18 0000000000000000 c000001e11093d90
> c000001e1104ed80
> GPR28: c000001e11093e90 0000000000000000 c0000000023d9d18
> 00007fffc30d9c00
> NIP flush_icache_range+0x5c/0x80
> LR handle_rt_signal64+0x95c/0xc2c
> Call Trace:
> 0xc000001e11093d90 (unreliable)
> handle_rt_signal64+0x93c/0xc2c
> do_notify_resume+0x310/0x430
> ret_from_except_lite+0x70/0x74
> Instruction dump:
> 409e002c 7c0802a6 3c62ff31 3863f6a0 f8010080 48195fed 60000000
> 48fe4c8d
> 60000000 e8010080 7c0803a6 7c0004ac <7c00ffac> 7c0004ac 4c00012c
> 38210070
>
> This path through handle_rt_signal64() to setup_trampoline() and
> flush_icache_range() is only triggered by 64-bit processes that have
> unmapped their VDSO, which is rare.
>
> flush_icache_range() takes a range of addresses to flush. In
> flush_coherent_icache() we implement an optimisation for CPUs where
> we
> know we don't actually have to flush the whole range, we just need to
> do a single icbi.
>
> However we still execute the icbi on the user address of the start of
> the range we're flushing. On CPUs that also implement KUAP (Power9)
> that leads to the spurious fault above.
>
> We should be able to pass any address, including a kernel address, to
> the icbi on these CPUs, which would avoid any interaction with KUAP.
> But I don't want to make that change in a bug fix, just in case it
> surfaces some strange behaviour on some CPU.
>
> So for now just disable KUAP around the icbi. Note the icbi is
> treated
> as a load, so we allow read access, not write as you'd expect.
>
> Fixes: 890274c2dc4c ("powerpc/64s: Implement KUAP for Radix MMU")
> Cc: stable@vger.kernel.org # v5.2+
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
Reviewed-by: Russell Currey <ruscur@russell.cc>
^ permalink raw reply
* Re: [PATCH -next v2] powerpc/64s/pgtable: fix an undefined behaviour
From: Christophe Leroy @ 2020-03-05 19:22 UTC (permalink / raw)
To: Qian Cai, mpe, akpm; +Cc: linux-mm, linuxppc-dev, rashmicy, linux-kernel
In-Reply-To: <1583418759-16105-1-git-send-email-cai@lca.pw>
Le 05/03/2020 à 15:32, Qian Cai a écrit :
> Booting a power9 server with hash MMU could trigger an undefined
> behaviour because pud_offset(p4d, 0) will do,
>
> 0 >> (PAGE_SHIFT:16 + PTE_INDEX_SIZE:8 + H_PMD_INDEX_SIZE:10)
>
> Fix it by converting pud_offset() and friends to static inline
> functions.
I was suggesting to convert pud_index() to static inline, because that's
where the shift sits. Is it not possible ?
Here you seems to fix the problem for now, but if someone reuses
pud_index() in another macro one day, the same problem may happen again.
Christophe
>
> UBSAN: shift-out-of-bounds in arch/powerpc/mm/ptdump/ptdump.c:282:15
> shift exponent 34 is too large for 32-bit type 'int'
> CPU: 6 PID: 1 Comm: swapper/0 Not tainted 5.6.0-rc4-next-20200303+ #13
> Call Trace:
> dump_stack+0xf4/0x164 (unreliable)
> ubsan_epilogue+0x18/0x78
> __ubsan_handle_shift_out_of_bounds+0x160/0x21c
> walk_pagetables+0x2cc/0x700
> walk_pud at arch/powerpc/mm/ptdump/ptdump.c:282
> (inlined by) walk_pagetables at arch/powerpc/mm/ptdump/ptdump.c:311
> ptdump_check_wx+0x8c/0xf0
> mark_rodata_ro+0x48/0x80
> kernel_init+0x74/0x194
> ret_from_kernel_thread+0x5c/0x74
>
> Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Qian Cai <cai@lca.pw>
> ---
> arch/powerpc/include/asm/book3s/64/pgtable.h | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index fa60e8594b9f..4967bc9e25e2 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -1016,12 +1016,20 @@ static inline bool p4d_access_permitted(p4d_t p4d, bool write)
>
> #define pgd_offset(mm, address) ((mm)->pgd + pgd_index(address))
>
> -#define pud_offset(p4dp, addr) \
> - (((pud_t *) p4d_page_vaddr(*(p4dp))) + pud_index(addr))
> -#define pmd_offset(pudp,addr) \
> - (((pmd_t *) pud_page_vaddr(*(pudp))) + pmd_index(addr))
> -#define pte_offset_kernel(dir,addr) \
> - (((pte_t *) pmd_page_vaddr(*(dir))) + pte_index(addr))
> +static inline pud_t *pud_offset(p4d_t *p4d, unsigned long address)
> +{
> + return (pud_t *)p4d_page_vaddr(*p4d) + pud_index(address);
> +}
> +
> +static inline pmd_t *pmd_offset(pud_t *pud, unsigned long address)
> +{
> + return (pmd_t *)pud_page_vaddr(*pud) + pmd_index(address);
> +}
> +
> +static inline pte_t *pte_offset_kernel(pmd_t *pmd, unsigned long address)
> +{
> + return (pte_t *)pmd_page_vaddr(*pmd) + pte_index(address);
> +}
>
> #define pte_offset_map(dir,addr) pte_offset_kernel((dir), (addr))
>
>
^ permalink raw reply
* Re: [PATCH rebased 1/2] powerpc: reserve memory for capture kernel after hugepages init
From: Michal Suchánek @ 2020-03-05 18:42 UTC (permalink / raw)
To: Hari Bathini, Mahesh Jagannath Salgaonkar,
Ananth N Mavinakayanahalli, linuxppc-dev
In-Reply-To: <f0e3acbcd3ba16d06d4f3e51b90655c69004768c.1582043081.git.msuchanek@suse.de>
Hello,
This seems to cause crash with kdump reservation 1GB quite reliably.
Thanks
Michal
On Tue, Feb 18, 2020 at 05:28:34PM +0100, Michal Suchanek wrote:
> From: Hari Bathini <hbathini@linux.ibm.com>
>
> Sometimes, memory reservation for KDump/FADump can overlap with memory
> marked for hugepages. This overlap leads to error, hang in KDump case
> and copy error reported by f/w in case of FADump, while trying to
> capture dump. Report error while setting up memory for the capture
> kernel instead of running into issues while capturing dump, by moving
> KDump/FADump reservation below MMU early init and failing gracefully
> when hugepages memory overlaps with capture kernel memory.
>
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
> arch/powerpc/kernel/prom.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 6620f37abe73..0f14dc9c4dab 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -735,14 +735,6 @@ void __init early_init_devtree(void *params)
> if (PHYSICAL_START > MEMORY_START)
> memblock_reserve(MEMORY_START, 0x8000);
> reserve_kdump_trampoline();
> -#if defined(CONFIG_FA_DUMP) || defined(CONFIG_PRESERVE_FA_DUMP)
> - /*
> - * If we fail to reserve memory for firmware-assisted dump then
> - * fallback to kexec based kdump.
> - */
> - if (fadump_reserve_mem() == 0)
> -#endif
> - reserve_crashkernel();
> early_reserve_mem();
>
> /* Ensure that total memory size is page-aligned. */
> @@ -781,6 +773,14 @@ void __init early_init_devtree(void *params)
> #endif
>
> mmu_early_init_devtree();
> +#if defined(CONFIG_FA_DUMP) || defined(CONFIG_PRESERVE_FA_DUMP)
> + /*
> + * If we fail to reserve memory for firmware-assisted dump then
> + * fallback to kexec based kdump.
> + */
> + if (fadump_reserve_mem() == 0)
> +#endif
> + reserve_crashkernel();
>
> #ifdef CONFIG_PPC_POWERNV
> /* Scan and build the list of machine check recoverable ranges */
> --
> 2.23.0
>
^ permalink raw reply
* Re: [PATCH 2/2] powerpc: Suppress .eh_frame generation
From: Segher Boessenkool @ 2020-03-05 16:39 UTC (permalink / raw)
To: Naveen N. Rao; +Cc: Rasmus Villemoes, linuxppc-dev
In-Reply-To: <1ed7cd84a7d1a3180b30c0c60e70eed8bb8b40c3.1583415544.git.naveen.n.rao@linux.vnet.ibm.com>
On Thu, Mar 05, 2020 at 08:05:30PM +0530, Naveen N. Rao wrote:
> GCC v8 defaults to enabling -fasynchronous-unwind-tables due to
> https://gcc.gnu.org/r259298, which results in .eh_frame section being
> generated. This results in additional disk usage by the build, as well
> as the kernel modules. Since the kernel has no use for this, this
> section is discarded.
>
> Add -fno-asynchronous-unwind-tables to KBUILD_CFLAGS to suppress
> generation of .eh_frame section. Note that our VDSOs need .eh_frame, but
> are not affected by this change since our VDSO code are all in assembly.
That may change, but it is easy to change again for just the VDSOs.
> Reported-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org>
Thanks!
Segher
^ permalink raw reply
* RE: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: Cédric Le Goater @ 2020-03-05 15:36 UTC (permalink / raw)
To: Ram Pai, David Gibson
Cc: aik, andmike, Greg Kurz, kvm-ppc, sukadev, linuxppc-dev, bauerman
In-Reply-To: <20200305151530.GJ5416@oc0525413822.ibm.com>
On 3/5/20 4:15 PM, Ram Pai wrote:
> On Thu, Mar 05, 2020 at 10:55:45AM +1100, David Gibson wrote:
>> On Wed, Mar 04, 2020 at 04:56:09PM +0100, Cédric Le Goater wrote:
>>> [ ... ]
>>>
>>>> (1) applied the patch which shares the EQ-page with the hypervisor.
>>>> (2) set "kernel_irqchip=off"
>>>> (3) set "ic-mode=xive"
>>>
>>> you don't have to set the interrupt mode. xive should be negotiated
>>> by default.
>>>
>>>> (4) set "svm=on" on the kernel command line.
>>>> (5) no changes to the hypervisor and ultravisor.
>>>>
>>>> And Boom it works!. So you were right.
>>>
>>> Excellent.
>>>
>>>> I am sending out the patch for (1) above ASAP.
>>>
>>> Next step, could you please try to do the same with the TIMA and ESB pfn ?
>>> and use KVM.
>>
>> I'm a bit confused by this. Aren't the TIMA and ESB pages essentially
>> IO pages, rather than memory pages from the guest's point of view? I
>> assume only memory pages are protected with PEF - I can't even really
>> see what protecting an IO page would even mean.
>
> It means, that the hypervisor and qemu cannot access the addresses used
> to access the I/O pages. It can only be accessed by Ultravisor and the
> SVM.
>
> As it stands today, those pages are accessible from the hypervisor
> and not from the SVM or the ultravisor.
>
> To make it work, we need to enable acccess to those pages from the SVM
> and from the ultravisor. One thing I am not clear is should we block
> access to those pages from the hypervisor. If yes, than there is no> good way to do that, without hardware help. If no, than those GPA pages
> can be shared, so that hypervisor/ultravisor/qemu/SVM can all access
> those pages.
They are shared.
KVM will also access them, at interrupt creation, device reset and
passthrough. QEMU will use them to mask on/off the interrupts in
case of guest migration or machine stop/continue.
C.
^ permalink raw reply
* RE: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: Ram Pai @ 2020-03-05 15:15 UTC (permalink / raw)
To: David Gibson
Cc: aik, andmike, Greg Kurz, kvm-ppc, Cédric Le Goater, sukadev,
linuxppc-dev, bauerman
In-Reply-To: <20200304235545.GE593957@umbus.fritz.box>
On Thu, Mar 05, 2020 at 10:55:45AM +1100, David Gibson wrote:
> On Wed, Mar 04, 2020 at 04:56:09PM +0100, Cédric Le Goater wrote:
> > [ ... ]
> >
> > > (1) applied the patch which shares the EQ-page with the hypervisor.
> > > (2) set "kernel_irqchip=off"
> > > (3) set "ic-mode=xive"
> >
> > you don't have to set the interrupt mode. xive should be negotiated
> > by default.
> >
> > > (4) set "svm=on" on the kernel command line.
> > > (5) no changes to the hypervisor and ultravisor.
> > >
> > > And Boom it works!. So you were right.
> >
> > Excellent.
> >
> > > I am sending out the patch for (1) above ASAP.
> >
> > Next step, could you please try to do the same with the TIMA and ESB pfn ?
> > and use KVM.
>
> I'm a bit confused by this. Aren't the TIMA and ESB pages essentially
> IO pages, rather than memory pages from the guest's point of view? I
> assume only memory pages are protected with PEF - I can't even really
> see what protecting an IO page would even mean.
It means, that the hypervisor and qemu cannot access the addresses used
to access the I/O pages. It can only be accessed by Ultravisor and the
SVM.
As it stands today, those pages are accessible from the hypervisor
and not from the SVM or the ultravisor.
To make it work, we need to enable acccess to those pages from the SVM
and from the ultravisor. One thing I am not clear is should we block
access to those pages from the hypervisor. If yes, than there is no
good way to do that, without hardware help. If no, than those GPA pages
can be shared, so that hypervisor/ultravisor/qemu/SVM can all access
those pages.
RP
^ permalink raw reply
* [PATCH 2/2] powerpc: Suppress .eh_frame generation
From: Naveen N. Rao @ 2020-03-05 14:35 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Rasmus Villemoes
In-Reply-To: <9b22a064de6eb1301d92177eb3a38559df7005d3.1583415544.git.naveen.n.rao@linux.vnet.ibm.com>
GCC v8 defaults to enabling -fasynchronous-unwind-tables due to
https://gcc.gnu.org/r259298, which results in .eh_frame section being
generated. This results in additional disk usage by the build, as well
as the kernel modules. Since the kernel has no use for this, this
section is discarded.
Add -fno-asynchronous-unwind-tables to KBUILD_CFLAGS to suppress
generation of .eh_frame section. Note that our VDSOs need .eh_frame, but
are not affected by this change since our VDSO code are all in assembly.
Reported-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/Makefile | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 89956c4f1ce3..f310c32e88a4 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -239,6 +239,9 @@ KBUILD_CFLAGS += $(call cc-option,-mno-vsx)
KBUILD_CFLAGS += $(call cc-option,-mno-spe)
KBUILD_CFLAGS += $(call cc-option,-mspe=no)
+# Don't emit .eh_frame since we have no use for it
+KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
+
# Never use string load/store instructions as they are
# often slow when they are implemented at all
KBUILD_CFLAGS += $(call cc-option,-mno-string)
--
2.24.1
^ permalink raw reply related
* [PATCH 1/2] powerpc: Drop -fno-dwarf2-cfi-asm
From: Naveen N. Rao @ 2020-03-05 14:35 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Rasmus Villemoes
The original commit/discussion adding -fno-dwarf2-cfi-asm refers to
R_PPC64_REL32 relocations not being handled by our module loader:
http://lkml.kernel.org/r/20090224065112.GA6690@bombadil.infradead.org
However, that is now handled thanks to commit 9f751b82b491d
("powerpc/module: Add support for R_PPC64_REL32 relocations").
So, drop this flag from our Makefile.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/Makefile | 5 -----
1 file changed, 5 deletions(-)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index cbe5ca4f0ee5..89956c4f1ce3 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -239,11 +239,6 @@ KBUILD_CFLAGS += $(call cc-option,-mno-vsx)
KBUILD_CFLAGS += $(call cc-option,-mno-spe)
KBUILD_CFLAGS += $(call cc-option,-mspe=no)
-# FIXME: the module load should be taught about the additional relocs
-# generated by this.
-# revert to pre-gcc-4.4 behaviour of .eh_frame
-KBUILD_CFLAGS += $(call cc-option,-fno-dwarf2-cfi-asm)
-
# Never use string load/store instructions as they are
# often slow when they are implemented at all
KBUILD_CFLAGS += $(call cc-option,-mno-string)
--
2.24.1
^ permalink raw reply related
* [PATCH -next v2] powerpc/64s/pgtable: fix an undefined behaviour
From: Qian Cai @ 2020-03-05 14:32 UTC (permalink / raw)
To: mpe, akpm; +Cc: rashmicy, linux-kernel, linux-mm, Qian Cai, linuxppc-dev
Booting a power9 server with hash MMU could trigger an undefined
behaviour because pud_offset(p4d, 0) will do,
0 >> (PAGE_SHIFT:16 + PTE_INDEX_SIZE:8 + H_PMD_INDEX_SIZE:10)
Fix it by converting pud_offset() and friends to static inline
functions.
UBSAN: shift-out-of-bounds in arch/powerpc/mm/ptdump/ptdump.c:282:15
shift exponent 34 is too large for 32-bit type 'int'
CPU: 6 PID: 1 Comm: swapper/0 Not tainted 5.6.0-rc4-next-20200303+ #13
Call Trace:
dump_stack+0xf4/0x164 (unreliable)
ubsan_epilogue+0x18/0x78
__ubsan_handle_shift_out_of_bounds+0x160/0x21c
walk_pagetables+0x2cc/0x700
walk_pud at arch/powerpc/mm/ptdump/ptdump.c:282
(inlined by) walk_pagetables at arch/powerpc/mm/ptdump/ptdump.c:311
ptdump_check_wx+0x8c/0xf0
mark_rodata_ro+0x48/0x80
kernel_init+0x74/0x194
ret_from_kernel_thread+0x5c/0x74
Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Qian Cai <cai@lca.pw>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index fa60e8594b9f..4967bc9e25e2 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1016,12 +1016,20 @@ static inline bool p4d_access_permitted(p4d_t p4d, bool write)
#define pgd_offset(mm, address) ((mm)->pgd + pgd_index(address))
-#define pud_offset(p4dp, addr) \
- (((pud_t *) p4d_page_vaddr(*(p4dp))) + pud_index(addr))
-#define pmd_offset(pudp,addr) \
- (((pmd_t *) pud_page_vaddr(*(pudp))) + pmd_index(addr))
-#define pte_offset_kernel(dir,addr) \
- (((pte_t *) pmd_page_vaddr(*(dir))) + pte_index(addr))
+static inline pud_t *pud_offset(p4d_t *p4d, unsigned long address)
+{
+ return (pud_t *)p4d_page_vaddr(*p4d) + pud_index(address);
+}
+
+static inline pmd_t *pmd_offset(pud_t *pud, unsigned long address)
+{
+ return (pmd_t *)pud_page_vaddr(*pud) + pmd_index(address);
+}
+
+static inline pte_t *pte_offset_kernel(pmd_t *pmd, unsigned long address)
+{
+ return (pte_t *)pmd_page_vaddr(*pmd) + pte_index(address);
+}
#define pte_offset_map(dir,addr) pte_offset_kernel((dir), (addr))
--
1.8.3.1
^ permalink raw reply related
* eh_frame confusion
From: Naveen N. Rao @ 2020-03-05 12:47 UTC (permalink / raw)
To: Linux Kbuild mailing list, LKML, linuxppc-dev@lists.ozlabs.org,
Rasmus Villemoes, Michael Ellerman
In-Reply-To: <1583169883.zo43kx69lm.naveen@linux.ibm.com>
Naveen N. Rao wrote:
> Naveen N. Rao wrote:
>> Rasmus Villemoes wrote:
<snip>
> Can you check if the below patch works? I am yet to test this in more
> detail, but would be good to know the implications for ppc32.
>
> - Naveen
>
>
> ---
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index f35730548e42..5b5bf98b8217 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -239,10 +239,7 @@ KBUILD_CFLAGS += $(call cc-option,-mno-vsx)
> KBUILD_CFLAGS += $(call cc-option,-mno-spe)
> KBUILD_CFLAGS += $(call cc-option,-mspe=no)
>
> -# FIXME: the module load should be taught about the additional relocs
> -# generated by this.
> -# revert to pre-gcc-4.4 behaviour of .eh_frame
> -KBUILD_CFLAGS += $(call cc-option,-fno-dwarf2-cfi-asm)
> +KBUILD_CFLAGS += $(call cc-option,-fno-asynchronous-unwind-tables)
In terms of the CFI information, the primary difference I see with
-fno-dwarf2-cfi-asm is that when dumping the debug frames, CIE indicates
version 3, while otherwise (i.e., without -fno-dwarf2-cfi-asm and
with/without -fasynchronous-unwind-tables), it is version 1, regardless
of -gdwarf-2/-gdwarf-4. There are few more minor changes, but none of
these looked significant to me.
>
> # Never use string load/store instructions as they are
> # often slow when they are implemented at all
> diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile
> index e147bbdc12cd..d43b0b18137c 100644
> --- a/arch/powerpc/kernel/vdso32/Makefile
> +++ b/arch/powerpc/kernel/vdso32/Makefile
> @@ -25,6 +25,7 @@ KCOV_INSTRUMENT := n
> UBSAN_SANITIZE := n
>
> ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
> + -fasynchronous-unwind-tables \
> -Wl,-soname=linux-vdso32.so.1 -Wl,--hash-style=both
> asflags-y := -D__VDSO32__ -s
>
> diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
> index 32ebb3522ea1..b2cbb5c49bad 100644
> --- a/arch/powerpc/kernel/vdso64/Makefile
> +++ b/arch/powerpc/kernel/vdso64/Makefile
> @@ -13,6 +13,7 @@ KCOV_INSTRUMENT := n
> UBSAN_SANITIZE := n
>
> ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
> + -fasynchronous-unwind-tables \
> -Wl,-soname=linux-vdso64.so.1 -Wl,--hash-style=both
> asflags-y := -D__VDSO64__ -s
The above vdso hunks can be dropped since all our VDSO are assembly, so
the above have no impact.
- Naveen
^ permalink raw reply
* Re: eh_frame confusion
From: Naveen N. Rao @ 2020-03-05 12:41 UTC (permalink / raw)
To: Linux Kbuild mailing list, LKML, linuxppc-dev@lists.ozlabs.org,
Rasmus Villemoes, Michael Ellerman
In-Reply-To: <877e01spfa.fsf@mpe.ellerman.id.au>
Michael Ellerman wrote:
> "Naveen N. Rao" <naveen.n.rao@linux.ibm.com> writes:
>> Rasmus Villemoes wrote:
>>> I'm building a ppc32 kernel, and noticed that after upgrading from gcc-7
>>> to gcc-8 all object files now end up having .eh_frame section. For
>>> vmlinux, that's not a problem, because they all get discarded in
>>> arch/powerpc/kernel/vmlinux.lds.S . However, they stick around in
>>> modules, which doesn't seem to be useful - given that everything worked
>>> just fine with gcc-7, and I don't see anything in the module loader that
>>> handles .eh_frame.
>>>
>>> The reason I care is that my target has a rather tight rootfs budget,
>>> and the .eh_frame section seem to occupy 10-30% of the file size
>>> (obviously very depending on the particular module).
>>>
>>> Comparing the .foo.o.cmd files, I don't see change in options that might
>>> explain this (there's a bunch of new -Wno-*, and the -mspe=no spelling
>>> is apparently no longer supported in gcc-8). Both before and after, there's
>>>
>>> -fno-dwarf2-cfi-asm
>>>
>>> about which gcc's documentation says
>>>
>>> '-fno-dwarf2-cfi-asm'
>>> Emit DWARF unwind info as compiler generated '.eh_frame' section
>>> instead of using GAS '.cfi_*' directives.
>>>
>>> Looking into where that comes from got me even more confused, because
>>> both arm and unicore32 say
>>>
>>> # Never generate .eh_frame
>>> KBUILD_CFLAGS += $(call cc-option,-fno-dwarf2-cfi-asm)
>>>
>>> while the ppc32 case at hand says
>>>
>>> # FIXME: the module load should be taught about the additional relocs
>>> # generated by this.
>>> # revert to pre-gcc-4.4 behaviour of .eh_frame
>>
>> Michael opened a task to look into this recently and I had spent some
>> time last week on this. The original commit/discussion adding
>> -fno-dwarf2-cfi-asm refers to R_PPC64_REL32 relocations not being
>> handled by our module loader:
>> http://lkml.kernel.org/r/20090224065112.GA6690@bombadil.infradead.org
>
> I opened that issue purely based on noticing the wart in the Makefile,
> not because I'd actually tested it.
>
>> However, that is now handled thanks to commit 9f751b82b491d:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9f751b82b491d
>
> Haha, written by me, what an idiot.
>
> So the Makefile hack can presumably be dropped, because the module
> loader can handle the relocations.
>
> And then maybe we also want to turn off the unwind tables, but that
> would be a separate patch.
>
>> I did a test build and a simple module loaded fine, so I think
>> -fno-dwarf2-cfi-asm is not required anymore, unless Michael has seen
>> some breakages with it. Michael?
>
> No, as I said above it was just reading the Makefile.
Ok, thanks for clarifying. To test, I did 'allmodconfig' builds across
three environments:
- gcc (Ubuntu 9.2.1-9ubuntu2) 9.2.1 20191008 -- ppc64le
- gcc (SUSE Linux) 7.5.0 -- ppc64le
- gcc (GCC) 8.2.1 20181215 (Red Hat 8.2.1-6) -- ppc64 (BE)
Then, used the below command to list all relocations in the modules:
$ find . -name '*.ko' | xargs -n 1 readelf -Wr | grep -v "Relocation " | grep -v "Offset " | cut -d' ' -f4 | sort | uniq
R_PPC64_ADDR32
R_PPC64_ADDR64
R_PPC64_ENTRY
R_PPC64_REL24
R_PPC64_REL32
R_PPC64_REL64
R_PPC64_TOC
R_PPC64_TOC16_HA
R_PPC64_TOC16_LO
R_PPC64_TOC16_LO_DS
All three environments show up similar set of relocations, all of which
we handle in the module loader today.
If Rasmus/Christophe can confirm that this is true for ppc32 as well,
then we should be fine.
- Naveen
^ permalink raw reply
* [Bug 206695] kmemleak reports leaks in drivers/macintosh/windfarm
From: bugzilla-daemon @ 2020-03-05 12:22 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206695-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206695
--- Comment #3 from mpe@ellerman.id.au ---
Can you try this patch?
diff --git a/drivers/macintosh/windfarm_pm112.c
b/drivers/macintosh/windfarm_pm112.c
index 4150301a89a5..a16f43a1def9 100644
--- a/drivers/macintosh/windfarm_pm112.c
+++ b/drivers/macintosh/windfarm_pm112.c
@@ -125,7 +125,7 @@ static int create_cpu_loop(int cpu)
{
int chip = cpu / 2;
int core = cpu & 1;
- struct smu_sdbp_header *hdr;
+ struct smu_sdbp_header *hdr, *hdr2;
struct smu_sdbp_cpupiddata *piddata;
struct wf_cpu_pid_param pid;
struct wf_control *main_fan = cpu_fans[0];
@@ -141,9 +141,9 @@ static int create_cpu_loop(int cpu)
piddata = (struct smu_sdbp_cpupiddata *)&hdr[1];
/* Get FVT params to get Tmax; if not found, assume default */
- hdr = smu_sat_get_sdb_partition(chip, 0xC4 + core, NULL);
- if (hdr) {
- struct smu_sdbp_fvt *fvt = (struct smu_sdbp_fvt *)&hdr[1];
+ hdr2 = smu_sat_get_sdb_partition(chip, 0xC4 + core, NULL);
+ if (hdr2) {
+ struct smu_sdbp_fvt *fvt = (struct smu_sdbp_fvt *)&hdr2[1];
tmax = fvt->maxtemp << 16;
} else
tmax = 95 << 16; /* default to 95 degrees C */
@@ -174,6 +174,10 @@ static int create_cpu_loop(int cpu)
pid.min = fmin;
wf_cpu_pid_init(&cpu_pid[cpu], &pid);
+
+ kfree(hdr);
+ kfree(hdr2);
+
return 0;
}
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply related
* Re: [Bug 206695] New: kmemleak reports leaks in drivers/macintosh/windfarm
From: Michael Ellerman @ 2020-03-05 12:22 UTC (permalink / raw)
To: bugzilla-daemon, linuxppc-dev
In-Reply-To: <bug-206695-206035@https.bugzilla.kernel.org/>
Can you try this patch?
diff --git a/drivers/macintosh/windfarm_pm112.c b/drivers/macintosh/windfarm_pm112.c
index 4150301a89a5..a16f43a1def9 100644
--- a/drivers/macintosh/windfarm_pm112.c
+++ b/drivers/macintosh/windfarm_pm112.c
@@ -125,7 +125,7 @@ static int create_cpu_loop(int cpu)
{
int chip = cpu / 2;
int core = cpu & 1;
- struct smu_sdbp_header *hdr;
+ struct smu_sdbp_header *hdr, *hdr2;
struct smu_sdbp_cpupiddata *piddata;
struct wf_cpu_pid_param pid;
struct wf_control *main_fan = cpu_fans[0];
@@ -141,9 +141,9 @@ static int create_cpu_loop(int cpu)
piddata = (struct smu_sdbp_cpupiddata *)&hdr[1];
/* Get FVT params to get Tmax; if not found, assume default */
- hdr = smu_sat_get_sdb_partition(chip, 0xC4 + core, NULL);
- if (hdr) {
- struct smu_sdbp_fvt *fvt = (struct smu_sdbp_fvt *)&hdr[1];
+ hdr2 = smu_sat_get_sdb_partition(chip, 0xC4 + core, NULL);
+ if (hdr2) {
+ struct smu_sdbp_fvt *fvt = (struct smu_sdbp_fvt *)&hdr2[1];
tmax = fvt->maxtemp << 16;
} else
tmax = 95 << 16; /* default to 95 degrees C */
@@ -174,6 +174,10 @@ static int create_cpu_loop(int cpu)
pid.min = fmin;
wf_cpu_pid_init(&cpu_pid[cpu], &pid);
+
+ kfree(hdr);
+ kfree(hdr2);
+
return 0;
}
^ permalink raw reply related
* Re: [PATCH 2/2] mm/vma: Introduce VM_ACCESS_FLAGS
From: Vlastimil Babka @ 2020-03-05 12:11 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm
Cc: devel, linux-s390, linuxppc-dev, Yoshinori Sato, Nick Hu,
Greg Kroah-Hartman, Catalin Marinas, Dave Hansen, hughd,
Russell King, Ley Foon Tan, linux-kernel, Heiko Carstens,
Rob Springer, Mark Salter, Thomas Gleixner, Guan Xuetao,
Andrew Morton, linux-arm-kernel
In-Reply-To: <1583391014-8170-3-git-send-email-anshuman.khandual@arm.com>
On 3/5/20 7:50 AM, Anshuman Khandual wrote:
> There are many places where all basic VMA access flags (read, write, exec)
> are initialized or checked against as a group. One such example is during
> page fault. Existing vma_is_accessible() wrapper already creates the notion
> of VMA accessibility as a group access permissions. Hence lets just create
> VM_ACCESS_FLAGS (VM_READ|VM_WRITE|VM_EXEC) which will not only reduce code
> duplication but also extend the VMA accessibility concept in general.
>
> Cc: Russell King <linux@armlinux.org.uk>
> CC: Catalin Marinas <catalin.marinas@arm.com>
> CC: Mark Salter <msalter@redhat.com>
> Cc: Nick Hu <nickhu@andestech.com>
> CC: Ley Foon Tan <ley.foon.tan@intel.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Guan Xuetao <gxt@pku.edu.cn>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Rob Springer <rspringer@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-s390@vger.kernel.org
> Cc: devel@driverdev.osuosl.org
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
Thanks.
^ permalink raw reply
* Re: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: Cédric Le Goater @ 2020-03-05 11:41 UTC (permalink / raw)
To: Ram Pai
Cc: aik, andmike, groug, kvm-ppc, sukadev, linuxppc-dev, bauerman,
David Gibson
In-Reply-To: <20200303202918.GC5416@oc0525413822.ibm.com>
[ ... ]
>> yes because you also need to share the XIVE TIMA and ESB pages mapped
>> in xive_native_esb_fault() and xive_native_tima_fault().
>
> These pages belong to the xive memory slot right? If that is the case,
> they are implicitly shared. The Ultravisor will set them up to be
> shared. The guest kernel should not be doing anything.
>
> We still need some fixes in KVM and Ultravisor to correctly map the
> hardware pages to GPA ranges of the xive memory slot. Work is in progress...
ok. Since this is already done for KVM, I suppose it's not too hard.
the VMA has VM_IO | VM_PFNMAP flags.
Otherwise you could still pick up the XIVE ESB and TIMA HW MMIO ranges
in OPAL and brutally declare the whole as shared, if that's possible.
C.
^ permalink raw reply
* Re: [PATCH] powerpc/64: BE option to use ELFv2 ABI for big endian kernels
From: Segher Boessenkool @ 2020-03-05 10:55 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <1583379060.p6od1jalr3.astroid@bobo.none>
On Thu, Mar 05, 2020 at 01:34:22PM +1000, Nicholas Piggin wrote:
> Segher Boessenkool's on March 4, 2020 9:09 am:
> >> +override flavour := linux-ppc64v2
> >
> > That isn't a good name, heh. This isn't "v2" of anything... Spell out
> > the name "ELFv2"? Or as "elfv2"? It is just a name after all, it is
> > version 1 in all three version fields in the ELF headers!
>
> Yeah okay. This part is only for some weird little perl asm generator
> script, but probably better to be careful. linux-ppc64-elfv2 ?
That generator is from openssl, or inspired by it, it is everywhere.
So it is more important to get it right than it would seem at first
glance ;-)
That name looks perfect to me. You'll have to update REs expecting the
arch at the end (like /le$/), but you had to already I think?
Segher
^ permalink raw reply
* Re: [PATCH v3 17/27] powerpc/powernv/pmem: Implement the Read Error Log command
From: Frederic Barrat @ 2020-03-05 9:33 UTC (permalink / raw)
To: Alastair D'Silva
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
Vishal Verma, Paul Mackerras, Andrew Morton, linuxppc-dev,
David S. Miller
In-Reply-To: <739066a997f83e7aa27dc364071223936fa753ef.camel@au1.ibm.com>
>>> + if (rc)
>>> + goto out;
>>> +
>>> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
>>> + ocxlpmem-
>>>> admin_command.data_offset + 0x28,
>>> + OCXL_HOST_ENDIAN, &log->wwid[1]);
>>> + if (rc)
>>> + goto out;
>>> +
>>> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
>>> + ocxlpmem-
>>>> admin_command.data_offset + 0x30,
>>> + OCXL_HOST_ENDIAN, (u64 *)log-
>>>> fw_revision);
>>> + if (rc)
>>> + goto out;
>>> + log->fw_revision[8] = '\0';
>>> +
>>> + buf_length = (user_buf_length < log->buf_size) ?
>>> + user_buf_length : log->buf_size;
>>> + for (i = 0; i < buf_length + 0x48; i += 8) {
>>> + u64 val;
>>> +
>>> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
>>> + ocxlpmem-
>>>> admin_command.data_offset + i,
>>> + OCXL_HOST_ENDIAN, &val);
>>> + if (rc)
>>> + goto out;
>>> +
>>> + if (buf_is_user) {
>>> + if (copy_to_user(&log->buf[i], &val,
>>> sizeof(u64))) {
>>> + rc = -EFAULT;
>>> + goto out;
>>> + }
>>> + } else
>>> + log->buf[i] = val;
>>> + }
>>
>>
>> I think it could be a bit simplified by keeping the handling of the
>> user
>> buffer out of this function. Always call it with a kernel buffer.
>> And
>> have only one copy_to_user() call on the ioctl() path. You'd need to
>> allocate a kernel buf on the ioctl path, but you're already doing it
>> on
>> the probe() path, so it should be doable to share code.
>
> Hmm, the problem then is that on the IOCTL side, I'll have to save,
> modify, then restore the buf member of struct
> ioctl_ocxl_pmem_error_log, which would be uglier.
buf is just an output buffer. All you'd need to do is allocate a kernel
buf, like it's already done for the "probe" case in dump_error_log().
And add a global copy_to_user() of the buf at the end of the ioctl path,
instead of having multiple smaller copy_to_user() in the loop here.
copy_to_user() is a bit expensive so it's usually better to regroup
them. I think it's easy here and make sense since that function is also
trying to handle both a kernel and user space bufffers.
But we're not in a critical path, and after this patch, there are others
copying out mmio content to user buffers and those don't have a kernel
buffer to handle, so the copy_to_user() in a loop makes things easier.
So I guess the conclusion is whatever you think is the easiest...
>>
>>
>>> +
>>> + rc = admin_response_handled(ocxlpmem);
>>> + if (rc)
>>> + goto out;
>>> +
>>> +out:
>>> + mutex_unlock(&ocxlpmem->admin_command.lock);
>>> + return rc;
>>> +
>>> +}
>>> +
>>> +static int ioctl_error_log(struct ocxlpmem *ocxlpmem,
>>> + struct ioctl_ocxl_pmem_error_log __user *uarg)
>>> +{
>>> + struct ioctl_ocxl_pmem_error_log args;
>>> + int rc;
>>> +
>>> + if (copy_from_user(&args, uarg, sizeof(args)))
>>> + return -EFAULT;
>>> +
>>> + rc = read_error_log(ocxlpmem, &args, true);
>>> + if (rc)
>>> + return rc;
>>> +
>>> + if (copy_to_user(uarg, &args, sizeof(args)))
>>> + return -EFAULT;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static long file_ioctl(struct file *file, unsigned int cmd,
>>> unsigned long args)
>>> +{
>>> + struct ocxlpmem *ocxlpmem = file->private_data;
>>> + int rc = -EINVAL;
>>> +
>>> + switch (cmd) {
>>> + case IOCTL_OCXL_PMEM_ERROR_LOG:
>>> + rc = ioctl_error_log(ocxlpmem,
>>> + (struct ioctl_ocxl_pmem_error_log
>>> __user *)args);
>>> + break;
>>> + }
>>> + return rc;
>>> +}
>>> +
>>> static const struct file_operations fops = {
>>> .owner = THIS_MODULE,
>>> .open = file_open,
>>> .release = file_release,
>>> + .unlocked_ioctl = file_ioctl,
>>> + .compat_ioctl = file_ioctl,
>>> };
>>>
>>> /**
>>> @@ -527,6 +736,60 @@ static int read_device_metadata(struct
>>> ocxlpmem *ocxlpmem)
>>> return 0;
>>> }
>>>
>>> +static const char *decode_error_log_type(u8 error_log_type)
>>> +{
>>> + switch (error_log_type) {
>>> + case 0x00:
>>> + return "general";
>>> + case 0x01:
>>> + return "predictive failure";
>>> + case 0x02:
>>> + return "thermal warning";
>>> + case 0x03:
>>> + return "data loss";
>>> + case 0x04:
>>> + return "health & performance";
>>> + default:
>>> + return "unknown";
>>> + }
>>> +}
>>> +
>>> +static void dump_error_log(struct ocxlpmem *ocxlpmem)
>>> +{
>>> + struct ioctl_ocxl_pmem_error_log log;
>>> + u32 buf_size;
>>> + u8 *buf;
>>> + int rc;
>>> +
>>> + if (ocxlpmem->admin_command.data_size == 0)
>>> + return;
>>> +
>>> + buf_size = ocxlpmem->admin_command.data_size - 0x48;
>>> + buf = kzalloc(buf_size, GFP_KERNEL);
>>> + if (!buf)
>>> + return;
>>> +
>>> + log.buf = buf;
>>> + log.buf_size = buf_size;
>>> +
>>> + rc = read_error_log(ocxlpmem, &log, false);
>>> + if (rc < 0)
>>> + goto out;
>>> +
>>> + dev_warn(&ocxlpmem->dev,
>>> + "OCXL PMEM Error log: WWID=0x%016llx%016llx LID=0x%x
>>> PRC=%x type=0x%x %s, Uptime=%u seconds timestamp=0x%llx\n",
>>> + log.wwid[0], log.wwid[1],
>>> + log.log_identifier, log.program_reference_code,
>>> + log.error_log_type,
>>> + decode_error_log_type(log.error_log_type),
>>> + log.power_on_seconds, log.timestamp);
>>> + print_hex_dump(KERN_WARNING, "buf", DUMP_PREFIX_OFFSET, 16, 1,
>>> buf,
>>> + log.buf_size, false);
>>
>> dev_warn already logs a warning. Isn't KERN_DEBUG more appropriate
>> for
>> the hex dump?
>>
>>
>
> The hex dump is associated binary data for the warning, it doesn't
> replicate the contents of the message.
My point is not about duplicating, it's about exposing an hexadecimal
dump where it makes sense. Those DEBUG and WARNING tags are used for
filtering content. For example to know what to display on the console. A
warning to mention that a device hits a serious error is perfectly fine.
A hexadecimal dump which is going to be meaningless to most everybody is
not. The system is not crashing, so it's not like the console is our
last hope. I think the dump is debug data and should be tagged as such.
Fred
>>
>>> +
>>> +out:
>>> + kfree(buf);
>>> +}
>>> +
>>> /**
>>> * probe_function0() - Set up function 0 for an OpenCAPI
>>> persistent memory device
>>> * This is important as it enables templates higher than 0 across
>>> all other functions,
>>> @@ -568,6 +831,7 @@ static int probe(struct pci_dev *pdev, const
>>> struct pci_device_id *ent)
>>> struct ocxlpmem *ocxlpmem;
>>> int rc;
>>> u16 elapsed, timeout;
>>> + u64 chi;
>>>
>>> if (PCI_FUNC(pdev->devfn) == 0)
>>> return probe_function0(pdev);
>>> @@ -667,6 +931,11 @@ static int probe(struct pci_dev *pdev, const
>>> struct pci_device_id *ent)
>>> return 0;
>>>
>>> err:
>>> + if (ocxlpmem &&
>>> + (ocxlpmem_chi(ocxlpmem, &chi) == 0) &&
>>> + (chi & GLOBAL_MMIO_CHI_ELA))
>>> + dump_error_log(ocxlpmem);
>>> +
>>> /*
>>> * Further cleanup is done in the release handler via
>>> free_ocxlpmem()
>>> * This allows us to keep the character device live to handle
>>> IOCTLs to
>>> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
>>> b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
>>> index d2d81fec7bb1..b953ee522ed4 100644
>>> --- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
>>> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
>>> @@ -5,6 +5,7 @@
>>> #include <linux/cdev.h>
>>> #include <misc/ocxl.h>
>>> #include <linux/libnvdimm.h>
>>> +#include <uapi/nvdimm/ocxl-pmem.h>
>>
>> Can't we limit the extra include to ocxl.c?
>>
>
> Yes, there are no consumers referred to in ocxl_interal.[hc]
>
>> Completely unrelated, but ocxl.c contains most of the code for this
>> driver. We should consider renaming it to ocxlpmem.c or something
>> along
>> those lines, since it does a lot more than just interfacing with the
>> opencapi interface. And would avoid confusion with an other already
>> existing ocxl.c file.
>>
>
> Ok, my thinking was that it's already in a pmem directory, but I can
> see arguments both ways.
>
>>
>>> #include <linux/mm.h>
>>>
>>> #define LABEL_AREA_SIZE (1UL << PA_SECTION_SHIFT)
>>> diff --git a/include/uapi/nvdimm/ocxl-pmem.h
>>> b/include/uapi/nvdimm/ocxl-pmem.h
>>> new file mode 100644
>>> index 000000000000..b10f8ac0c20f
>>> --- /dev/null
>>> +++ b/include/uapi/nvdimm/ocxl-pmem.h
>>> @@ -0,0 +1,46 @@
>>> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
>>> +/* Copyright 2017 IBM Corp. */
>>> +#ifndef _UAPI_OCXL_SCM_H
>>> +#define _UAPI_OCXL_SCM_H
>>> +
>>> +#include <linux/types.h>
>>> +#include <linux/ioctl.h>
>>> +
>>> +#define OCXL_PMEM_ERROR_LOG_ACTION_RESET (1 << (32-32))
>>> +#define OCXL_PMEM_ERROR_LOG_ACTION_CHKFW (1 << (53-32))
>>> +#define OCXL_PMEM_ERROR_LOG_ACTION_REPLACE (1 << (54-32))
>>> +#define OCXL_PMEM_ERROR_LOG_ACTION_DUMP (1 << (55-32))
>>> +
>>> +#define OCXL_PMEM_ERROR_LOG_TYPE_GENERAL (0x00)
>>> +#define OCXL_PMEM_ERROR_LOG_TYPE_PREDICTIVE_FAILURE (0x01)
>>> +#define OCXL_PMEM_ERROR_LOG_TYPE_THERMAL_WARNING (0x02)
>>> +#define OCXL_PMEM_ERROR_LOG_TYPE_DATA_LOSS (0x03)
>>> +#define OCXL_PMEM_ERROR_LOG_TYPE_HEALTH_PERFORMANCE (0x04)
>>> +
>>> +struct ioctl_ocxl_pmem_error_log {
>>> + __u32 log_identifier; /* out */
>>> + __u32 program_reference_code; /* out */
>>> + __u32 action_flags; /* out, recommended course of action */
>>> + __u32 power_on_seconds; /* out, Number of seconds the
>>> controller has been on when the error occurred */
>>> + __u64 timestamp; /* out, relative time since the current IPL */
>>> + __u64 wwid[2]; /* out, the NAA formatted WWID associated with
>>> the controller */
>>> + char fw_revision[8+1]; /* out, firmware revision as null
>>> terminated text */
>>
>> The 8+1 size will make the compiler add some padding here. Are we
>> confident that all the compilers, at least on powerpc, will do the
>> same
>> thing and we can guarantee a kernel ABI? I would play it safe and
>> have a
>> discussion with folks who understand compilers better.
>>
>
> I'll add some explicit padding.
>
>>
>>
>>> + __u16 buf_size; /* in/out, buffer size provided/required.
>>> + * If required is greater than provided, the
>>> buffer
>>> + * will be truncated to the amount provided. If
>>> its
>>> + * less, then only the required bytes will be
>>> populated.
>>> + * If it is 0, then there are no more error log
>>> entries.
>>> + */
>>> + __u8 error_log_type;
>>> + __u8 reserved1;
>>> + __u32 reserved2;
>>> + __u64 reserved3[2];
>>> + __u8 *buf; /* pointer to output buffer */
>>> +};
>>> +
>>> +/* ioctl numbers */
>>> +#define OCXL_PMEM_MAGIC 0x5C
>>
>> Randomly picked?
>> See (and add entry in) Documentation/userspace-api/ioctl/ioctl-
>> number.rst
>>
> Ok
>
>>
>> Fred
>>
>>
>>
>>> +/* SCM devices */
>>> +#define IOCTL_OCXL_PMEM_ERROR_LOG _IOWR(OCXL_PMEM
>>> _MAGIC, 0x01, struct ioctl_ocxl_pmem_error_log)
>>> +
>>> +#endif /* _UAPI_OCXL_SCM_H */
>>>
^ permalink raw reply
* Re: [PATCH] selftests/powerpc: Turn off timeout setting for benchmarks, dscr, signal, tm
From: Po-Hsu Lin @ 2020-03-05 9:02 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev, linux-kernel, paulus, linux-kselftest, shuah
In-Reply-To: <87zhcvp89e.fsf@mpe.ellerman.id.au>
On Thu, Mar 5, 2020 at 3:32 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Po-Hsu Lin <po-hsu.lin@canonical.com> writes:
> > Some specific tests in powerpc can take longer than the default 45
> > seconds that added in commit 852c8cbf (selftests/kselftest/runner.sh:
> > Add 45 second timeout per test) to run, the following test result was
> > collected across 2 Power8 nodes and 1 Power9 node in our pool:
> > powerpc/benchmarks/futex_bench - 52s
> > powerpc/dscr/dscr_sysfs_test - 116s
> > powerpc/signal/signal_fuzzer - 88s
> > powerpc/tm/tm_unavailable_test - 168s
> > powerpc/tm/tm-poison - 240s
> >
> > Thus they will fail with TIMEOUT error. Disable the timeout setting
> > for these sub-tests to allow them finish properly.
>
> I run the powerpc tests with run-parts, rather than the kselftest
> script, we already have our own test runner with a 120s timeout.
>
> I didn't think the kselftests runner actually worked with the powerpc
> tests? Because we override RUN_TESTS.
>
Hello Michael,
I have done a small experiment with timeout=1 in settings and use
run-parts to run the executables, it looks like this change won't
affect run-parts.
Not quite sure about the RUN_TESTS you mentioned here, we're testing
it with command like:
sudo make -C linux/tools/testing/selftests TARGETS=powerpc run_tests
And the timeout setting will take effect with this.
Thanks
> cheers
>
>
> > https://bugs.launchpad.net/bugs/1864642
> > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> > ---
> > tools/testing/selftests/powerpc/benchmarks/Makefile | 2 ++
> > tools/testing/selftests/powerpc/benchmarks/settings | 1 +
> > tools/testing/selftests/powerpc/dscr/Makefile | 2 ++
> > tools/testing/selftests/powerpc/dscr/settings | 1 +
> > tools/testing/selftests/powerpc/signal/Makefile | 2 ++
> > tools/testing/selftests/powerpc/signal/settings | 1 +
> > tools/testing/selftests/powerpc/tm/Makefile | 2 ++
> > tools/testing/selftests/powerpc/tm/settings | 1 +
> > 8 files changed, 12 insertions(+)
> > create mode 100644 tools/testing/selftests/powerpc/benchmarks/settings
> > create mode 100644 tools/testing/selftests/powerpc/dscr/settings
> > create mode 100644 tools/testing/selftests/powerpc/signal/settings
> > create mode 100644 tools/testing/selftests/powerpc/tm/settings
> >
> > diff --git a/tools/testing/selftests/powerpc/benchmarks/Makefile b/tools/testing/selftests/powerpc/benchmarks/Makefile
> > index d40300a..a32a6ab 100644
> > --- a/tools/testing/selftests/powerpc/benchmarks/Makefile
> > +++ b/tools/testing/selftests/powerpc/benchmarks/Makefile
> > @@ -2,6 +2,8 @@
> > TEST_GEN_PROGS := gettimeofday context_switch fork mmap_bench futex_bench null_syscall
> > TEST_GEN_FILES := exec_target
> >
> > +TEST_FILES := settings
> > +
> > CFLAGS += -O2
> >
> > top_srcdir = ../../../../..
> > diff --git a/tools/testing/selftests/powerpc/benchmarks/settings b/tools/testing/selftests/powerpc/benchmarks/settings
> > new file mode 100644
> > index 0000000..e7b9417
> > --- /dev/null
> > +++ b/tools/testing/selftests/powerpc/benchmarks/settings
> > @@ -0,0 +1 @@
> > +timeout=0
> > diff --git a/tools/testing/selftests/powerpc/dscr/Makefile b/tools/testing/selftests/powerpc/dscr/Makefile
> > index 5df4763..cfa6eed 100644
> > --- a/tools/testing/selftests/powerpc/dscr/Makefile
> > +++ b/tools/testing/selftests/powerpc/dscr/Makefile
> > @@ -3,6 +3,8 @@ TEST_GEN_PROGS := dscr_default_test dscr_explicit_test dscr_user_test \
> > dscr_inherit_test dscr_inherit_exec_test dscr_sysfs_test \
> > dscr_sysfs_thread_test
> >
> > +TEST_FILES := settings
> > +
> > top_srcdir = ../../../../..
> > include ../../lib.mk
> >
> > diff --git a/tools/testing/selftests/powerpc/dscr/settings b/tools/testing/selftests/powerpc/dscr/settings
> > new file mode 100644
> > index 0000000..e7b9417
> > --- /dev/null
> > +++ b/tools/testing/selftests/powerpc/dscr/settings
> > @@ -0,0 +1 @@
> > +timeout=0
> > diff --git a/tools/testing/selftests/powerpc/signal/Makefile b/tools/testing/selftests/powerpc/signal/Makefile
> > index 113838f..153fafc 100644
> > --- a/tools/testing/selftests/powerpc/signal/Makefile
> > +++ b/tools/testing/selftests/powerpc/signal/Makefile
> > @@ -5,6 +5,8 @@ CFLAGS += -maltivec
> > $(OUTPUT)/signal_tm: CFLAGS += -mhtm
> > $(OUTPUT)/sigfuz: CFLAGS += -pthread -m64
> >
> > +TEST_FILES := settings
> > +
> > top_srcdir = ../../../../..
> > include ../../lib.mk
> >
> > diff --git a/tools/testing/selftests/powerpc/signal/settings b/tools/testing/selftests/powerpc/signal/settings
> > new file mode 100644
> > index 0000000..e7b9417
> > --- /dev/null
> > +++ b/tools/testing/selftests/powerpc/signal/settings
> > @@ -0,0 +1 @@
> > +timeout=0
> > diff --git a/tools/testing/selftests/powerpc/tm/Makefile b/tools/testing/selftests/powerpc/tm/Makefile
> > index b15a1a3..7b99d09 100644
> > --- a/tools/testing/selftests/powerpc/tm/Makefile
> > +++ b/tools/testing/selftests/powerpc/tm/Makefile
> > @@ -7,6 +7,8 @@ TEST_GEN_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv tm-signal-stack
> > $(SIGNAL_CONTEXT_CHK_TESTS) tm-sigreturn tm-signal-sigreturn-nt \
> > tm-signal-context-force-tm tm-poison
> >
> > +TEST_FILES := settings
> > +
> > top_srcdir = ../../../../..
> > include ../../lib.mk
> >
> > diff --git a/tools/testing/selftests/powerpc/tm/settings b/tools/testing/selftests/powerpc/tm/settings
> > new file mode 100644
> > index 0000000..e7b9417
> > --- /dev/null
> > +++ b/tools/testing/selftests/powerpc/tm/settings
> > @@ -0,0 +1 @@
> > +timeout=0
> > --
> > 2.7.4
^ permalink raw reply
* Re: [PATCH] selftests/powerpc: Turn off timeout setting for benchmarks, dscr, signal, tm
From: Michael Ellerman @ 2020-03-05 7:31 UTC (permalink / raw)
To: Po-Hsu Lin, linux-kselftest; +Cc: shuah, linuxppc-dev, linux-kernel, paulus
In-Reply-To: <20200304131553.27582-1-po-hsu.lin@canonical.com>
Po-Hsu Lin <po-hsu.lin@canonical.com> writes:
> Some specific tests in powerpc can take longer than the default 45
> seconds that added in commit 852c8cbf (selftests/kselftest/runner.sh:
> Add 45 second timeout per test) to run, the following test result was
> collected across 2 Power8 nodes and 1 Power9 node in our pool:
> powerpc/benchmarks/futex_bench - 52s
> powerpc/dscr/dscr_sysfs_test - 116s
> powerpc/signal/signal_fuzzer - 88s
> powerpc/tm/tm_unavailable_test - 168s
> powerpc/tm/tm-poison - 240s
>
> Thus they will fail with TIMEOUT error. Disable the timeout setting
> for these sub-tests to allow them finish properly.
I run the powerpc tests with run-parts, rather than the kselftest
script, we already have our own test runner with a 120s timeout.
I didn't think the kselftests runner actually worked with the powerpc
tests? Because we override RUN_TESTS.
cheers
> https://bugs.launchpad.net/bugs/1864642
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> ---
> tools/testing/selftests/powerpc/benchmarks/Makefile | 2 ++
> tools/testing/selftests/powerpc/benchmarks/settings | 1 +
> tools/testing/selftests/powerpc/dscr/Makefile | 2 ++
> tools/testing/selftests/powerpc/dscr/settings | 1 +
> tools/testing/selftests/powerpc/signal/Makefile | 2 ++
> tools/testing/selftests/powerpc/signal/settings | 1 +
> tools/testing/selftests/powerpc/tm/Makefile | 2 ++
> tools/testing/selftests/powerpc/tm/settings | 1 +
> 8 files changed, 12 insertions(+)
> create mode 100644 tools/testing/selftests/powerpc/benchmarks/settings
> create mode 100644 tools/testing/selftests/powerpc/dscr/settings
> create mode 100644 tools/testing/selftests/powerpc/signal/settings
> create mode 100644 tools/testing/selftests/powerpc/tm/settings
>
> diff --git a/tools/testing/selftests/powerpc/benchmarks/Makefile b/tools/testing/selftests/powerpc/benchmarks/Makefile
> index d40300a..a32a6ab 100644
> --- a/tools/testing/selftests/powerpc/benchmarks/Makefile
> +++ b/tools/testing/selftests/powerpc/benchmarks/Makefile
> @@ -2,6 +2,8 @@
> TEST_GEN_PROGS := gettimeofday context_switch fork mmap_bench futex_bench null_syscall
> TEST_GEN_FILES := exec_target
>
> +TEST_FILES := settings
> +
> CFLAGS += -O2
>
> top_srcdir = ../../../../..
> diff --git a/tools/testing/selftests/powerpc/benchmarks/settings b/tools/testing/selftests/powerpc/benchmarks/settings
> new file mode 100644
> index 0000000..e7b9417
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/benchmarks/settings
> @@ -0,0 +1 @@
> +timeout=0
> diff --git a/tools/testing/selftests/powerpc/dscr/Makefile b/tools/testing/selftests/powerpc/dscr/Makefile
> index 5df4763..cfa6eed 100644
> --- a/tools/testing/selftests/powerpc/dscr/Makefile
> +++ b/tools/testing/selftests/powerpc/dscr/Makefile
> @@ -3,6 +3,8 @@ TEST_GEN_PROGS := dscr_default_test dscr_explicit_test dscr_user_test \
> dscr_inherit_test dscr_inherit_exec_test dscr_sysfs_test \
> dscr_sysfs_thread_test
>
> +TEST_FILES := settings
> +
> top_srcdir = ../../../../..
> include ../../lib.mk
>
> diff --git a/tools/testing/selftests/powerpc/dscr/settings b/tools/testing/selftests/powerpc/dscr/settings
> new file mode 100644
> index 0000000..e7b9417
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/dscr/settings
> @@ -0,0 +1 @@
> +timeout=0
> diff --git a/tools/testing/selftests/powerpc/signal/Makefile b/tools/testing/selftests/powerpc/signal/Makefile
> index 113838f..153fafc 100644
> --- a/tools/testing/selftests/powerpc/signal/Makefile
> +++ b/tools/testing/selftests/powerpc/signal/Makefile
> @@ -5,6 +5,8 @@ CFLAGS += -maltivec
> $(OUTPUT)/signal_tm: CFLAGS += -mhtm
> $(OUTPUT)/sigfuz: CFLAGS += -pthread -m64
>
> +TEST_FILES := settings
> +
> top_srcdir = ../../../../..
> include ../../lib.mk
>
> diff --git a/tools/testing/selftests/powerpc/signal/settings b/tools/testing/selftests/powerpc/signal/settings
> new file mode 100644
> index 0000000..e7b9417
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/signal/settings
> @@ -0,0 +1 @@
> +timeout=0
> diff --git a/tools/testing/selftests/powerpc/tm/Makefile b/tools/testing/selftests/powerpc/tm/Makefile
> index b15a1a3..7b99d09 100644
> --- a/tools/testing/selftests/powerpc/tm/Makefile
> +++ b/tools/testing/selftests/powerpc/tm/Makefile
> @@ -7,6 +7,8 @@ TEST_GEN_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv tm-signal-stack
> $(SIGNAL_CONTEXT_CHK_TESTS) tm-sigreturn tm-signal-sigreturn-nt \
> tm-signal-context-force-tm tm-poison
>
> +TEST_FILES := settings
> +
> top_srcdir = ../../../../..
> include ../../lib.mk
>
> diff --git a/tools/testing/selftests/powerpc/tm/settings b/tools/testing/selftests/powerpc/tm/settings
> new file mode 100644
> index 0000000..e7b9417
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/tm/settings
> @@ -0,0 +1 @@
> +timeout=0
> --
> 2.7.4
^ permalink raw reply
* RE: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: Cédric Le Goater @ 2020-03-05 7:15 UTC (permalink / raw)
To: David Gibson
Cc: aik, andmike, Ram Pai, Greg Kurz, kvm-ppc, sukadev, linuxppc-dev,
bauerman
In-Reply-To: <20200304235545.GE593957@umbus.fritz.box>
On 3/5/20 12:55 AM, David Gibson wrote:
> On Wed, Mar 04, 2020 at 04:56:09PM +0100, Cédric Le Goater wrote:
>> [ ... ]
>>
>>> (1) applied the patch which shares the EQ-page with the hypervisor.
>>> (2) set "kernel_irqchip=off"
>>> (3) set "ic-mode=xive"
>>
>> you don't have to set the interrupt mode. xive should be negotiated
>> by default.
>>
>>> (4) set "svm=on" on the kernel command line.
>>> (5) no changes to the hypervisor and ultravisor.
>>>
>>> And Boom it works!. So you were right.
>>
>> Excellent.
>>
>>> I am sending out the patch for (1) above ASAP.
>>
>> Next step, could you please try to do the same with the TIMA and ESB pfn ?
>> and use KVM.
>
> I'm a bit confused by this. Aren't the TIMA and ESB pages essentially
> IO pages, rather than memory pages from the guest's point of view?
yes.
> I assume only memory pages are protected with PEF - I can't even really
> see what protecting an IO page would even mean.
AFAIUI, the ultravisor needs to be aware of these IO page frames. We have
the information in KVM but we need to inform the ultravisor in some ways.
It could be done earlier than in the page fault handler.
C.
^ permalink raw reply
* Re: [PATCH -next] powerpc/mm/ptdump: fix an undefined behaviour
From: Michael Ellerman @ 2020-03-05 6:53 UTC (permalink / raw)
To: Christophe Leroy, Qian Cai, akpm
Cc: linux-mm, linuxppc-dev, rashmicy, linux-kernel
In-Reply-To: <3b724167-6bd2-f281-c6ee-fcb39cb9e24b@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Le 05/03/2020 à 05:47, Qian Cai a écrit :
>> Booting a power9 server with hash MMU could trigger an undefined
>> behaviour because pud_offset(p4d, 0) will do,
>>
>> 0 >> (PAGE_SHIFT:16 + PTE_INDEX_SIZE:8 + H_PMD_INDEX_SIZE:10)
>>
>> UBSAN: shift-out-of-bounds in arch/powerpc/mm/ptdump/ptdump.c:282:15
>> shift exponent 34 is too large for 32-bit type 'int'
>> CPU: 6 PID: 1 Comm: swapper/0 Not tainted 5.6.0-rc4-next-20200303+ #13
>> Call Trace:
>> dump_stack+0xf4/0x164 (unreliable)
>> ubsan_epilogue+0x18/0x78
>> __ubsan_handle_shift_out_of_bounds+0x160/0x21c
>> walk_pagetables+0x2cc/0x700
>> walk_pud at arch/powerpc/mm/ptdump/ptdump.c:282
>> (inlined by) walk_pagetables at arch/powerpc/mm/ptdump/ptdump.c:311
>> ptdump_check_wx+0x8c/0xf0
>> mark_rodata_ro+0x48/0x80
>> kernel_init+0x74/0x194
>> ret_from_kernel_thread+0x5c/0x74
>>
>> Fixes: 8eb07b187000 ("powerpc/mm: Dump linux pagetables")
>> Signed-off-by: Qian Cai <cai@lca.pw>
>> ---
>>
>> Notes for maintainers:
>>
>> This is on the top of the linux-next commit "powerpc: add support for
>> folded p4d page tables" which is in the Andrew's tree.
>>
>> arch/powerpc/mm/ptdump/ptdump.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
>> index 9d6256b61df3..b530f81398a7 100644
>> --- a/arch/powerpc/mm/ptdump/ptdump.c
>> +++ b/arch/powerpc/mm/ptdump/ptdump.c
>> @@ -279,7 +279,7 @@ static void walk_pmd(struct pg_state *st, pud_t *pud, unsigned long start)
>>
>> static void walk_pud(struct pg_state *st, p4d_t *p4d, unsigned long start)
>> {
>> - pud_t *pud = pud_offset(p4d, 0);
>> + pud_t *pud = pud_offset(p4d, 0UL);
>
> Is that the only place we have to do this ?
>
> (In 5.6-rc) I see the same in:
> /arch/powerpc/mm/ptdump/hashpagetable.c
> /arch/powerpc/kvm/book3s_64_mmu_radix.c
>
> Wouldn't it be better to:
> - Either cast addr to unsigned long in pud_index() macro
> - Or change pud_index() macro to a static inline function as x86 ?
Yes, either would be better, but preferably the latter.
It's hostile to require the caller to pass an unsigned long when there's
no way they can know that's required.
cheers
^ permalink raw reply
* [PATCH 2/2] mm/vma: Introduce VM_ACCESS_FLAGS
From: Anshuman Khandual @ 2020-03-05 6:50 UTC (permalink / raw)
To: linux-mm
Cc: Catalin Marinas, Dave Hansen, Heiko Carstens, devel, linux-s390,
Yoshinori Sato, hughd, Russell King, Ley Foon Tan, vbabka,
Mark Salter, Anshuman Khandual, Rob Springer, Thomas Gleixner,
Guan Xuetao, linux-arm-kernel, Nick Hu, Greg Kroah-Hartman,
linux-kernel, Andrew Morton, linuxppc-dev
In-Reply-To: <1583391014-8170-1-git-send-email-anshuman.khandual@arm.com>
There are many places where all basic VMA access flags (read, write, exec)
are initialized or checked against as a group. One such example is during
page fault. Existing vma_is_accessible() wrapper already creates the notion
of VMA accessibility as a group access permissions. Hence lets just create
VM_ACCESS_FLAGS (VM_READ|VM_WRITE|VM_EXEC) which will not only reduce code
duplication but also extend the VMA accessibility concept in general.
Cc: Russell King <linux@armlinux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Mark Salter <msalter@redhat.com>
Cc: Nick Hu <nickhu@andestech.com>
CC: Ley Foon Tan <ley.foon.tan@intel.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Guan Xuetao <gxt@pku.edu.cn>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rob Springer <rspringer@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: devel@driverdev.osuosl.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
arch/arm/mm/fault.c | 2 +-
arch/arm64/mm/fault.c | 2 +-
arch/nds32/mm/fault.c | 2 +-
arch/powerpc/mm/book3s64/pkeys.c | 2 +-
arch/s390/mm/fault.c | 2 +-
arch/unicore32/mm/fault.c | 2 +-
arch/x86/mm/pkeys.c | 2 +-
drivers/staging/gasket/gasket_core.c | 2 +-
include/linux/mm.h | 6 +++++-
mm/mmap.c | 2 +-
mm/mprotect.c | 4 ++--
11 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index bd0f4821f7e1..2c71028d9d6b 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -189,7 +189,7 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
*/
static inline bool access_error(unsigned int fsr, struct vm_area_struct *vma)
{
- unsigned int mask = VM_READ | VM_WRITE | VM_EXEC;
+ unsigned int mask = VM_ACCESS_FLAGS;
if ((fsr & FSR_WRITE) && !(fsr & FSR_CM))
mask = VM_WRITE;
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 85566d32958f..63f31206a12e 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -445,7 +445,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
const struct fault_info *inf;
struct mm_struct *mm = current->mm;
vm_fault_t fault, major = 0;
- unsigned long vm_flags = VM_READ | VM_WRITE | VM_EXEC;
+ unsigned long vm_flags = VM_ACCESS_FLAGS;
unsigned int mm_flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
if (kprobe_page_fault(regs, esr))
diff --git a/arch/nds32/mm/fault.c b/arch/nds32/mm/fault.c
index 906dfb25353c..55387a31bf42 100644
--- a/arch/nds32/mm/fault.c
+++ b/arch/nds32/mm/fault.c
@@ -79,7 +79,7 @@ void do_page_fault(unsigned long entry, unsigned long addr,
struct vm_area_struct *vma;
int si_code;
vm_fault_t fault;
- unsigned int mask = VM_READ | VM_WRITE | VM_EXEC;
+ unsigned int mask = VM_ACCESS_FLAGS;
unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
error_code = error_code & (ITYPE_mskINST | ITYPE_mskETYPE);
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index 59e0ebbd8036..11fd52b24f68 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -315,7 +315,7 @@ int __execute_only_pkey(struct mm_struct *mm)
static inline bool vma_is_pkey_exec_only(struct vm_area_struct *vma)
{
/* Do this check first since the vm_flags should be hot */
- if ((vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)) != VM_EXEC)
+ if ((vma->vm_flags & VM_ACCESS_FLAGS) != VM_EXEC)
return false;
return (vma_pkey(vma) == vma->vm_mm->context.execute_only_pkey);
diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index 7b0bb475c166..b2cb3c0d0e1a 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -584,7 +584,7 @@ void do_dat_exception(struct pt_regs *regs)
int access;
vm_fault_t fault;
- access = VM_READ | VM_EXEC | VM_WRITE;
+ access = VM_ACCESS_FLAGS;
fault = do_exception(regs, access);
if (unlikely(fault))
do_fault_error(regs, access, fault);
diff --git a/arch/unicore32/mm/fault.c b/arch/unicore32/mm/fault.c
index 76342de9cf8c..fc27c274d358 100644
--- a/arch/unicore32/mm/fault.c
+++ b/arch/unicore32/mm/fault.c
@@ -149,7 +149,7 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
*/
static inline bool access_error(unsigned int fsr, struct vm_area_struct *vma)
{
- unsigned int mask = VM_READ | VM_WRITE | VM_EXEC;
+ unsigned int mask = VM_ACCESS_FLAGS;
if (!(fsr ^ 0x12)) /* write? */
mask = VM_WRITE;
diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c
index c6f84c0b5d7a..8873ed1438a9 100644
--- a/arch/x86/mm/pkeys.c
+++ b/arch/x86/mm/pkeys.c
@@ -63,7 +63,7 @@ int __execute_only_pkey(struct mm_struct *mm)
static inline bool vma_is_pkey_exec_only(struct vm_area_struct *vma)
{
/* Do this check first since the vm_flags should be hot */
- if ((vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)) != VM_EXEC)
+ if ((vma->vm_flags & VM_ACCESS_FLAGS) != VM_EXEC)
return false;
if (vma_pkey(vma) != vma->vm_mm->context.execute_only_pkey)
return false;
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index be6b50f454b4..81bb7d58dc49 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -689,7 +689,7 @@ static bool gasket_mmap_has_permissions(struct gasket_dev *gasket_dev,
/* Make sure that no wrong flags are set. */
requested_permissions =
- (vma->vm_flags & (VM_WRITE | VM_READ | VM_EXEC));
+ (vma->vm_flags & VM_ACCESS_FLAGS);
if (requested_permissions & ~(bar_permissions)) {
dev_dbg(gasket_dev->dev,
"Attempting to map a region with requested permissions "
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 3861524368a4..e89512f1c170 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -368,6 +368,10 @@ extern unsigned int kobjsize(const void *objp);
#define VM_STACK_FLAGS (VM_STACK | VM_STACK_DEFAULT_FLAGS | VM_ACCOUNT)
+/* VMA basic access permission flags */
+#define VM_ACCESS_FLAGS (VM_READ | VM_WRITE | VM_EXEC)
+
+
/*
* Special vmas that are non-mergable, non-mlock()able.
* Note: mm/huge_memory.c VM_NO_THP depends on this definition.
@@ -557,7 +561,7 @@ static inline bool vma_is_anonymous(struct vm_area_struct *vma)
static inline bool vma_is_accessible(struct vm_area_struct *vma)
{
- return vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
+ return vma->vm_flags & VM_ACCESS_FLAGS;
}
#ifdef CONFIG_SHMEM
diff --git a/mm/mmap.c b/mm/mmap.c
index 0d295f49b24d..57f74ade19a0 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1221,7 +1221,7 @@ static int anon_vma_compatible(struct vm_area_struct *a, struct vm_area_struct *
return a->vm_end == b->vm_start &&
mpol_equal(vma_policy(a), vma_policy(b)) &&
a->vm_file == b->vm_file &&
- !((a->vm_flags ^ b->vm_flags) & ~(VM_READ|VM_WRITE|VM_EXEC|VM_SOFTDIRTY)) &&
+ !((a->vm_flags ^ b->vm_flags) & ~(VM_ACCESS_FLAGS | VM_SOFTDIRTY)) &&
b->vm_pgoff == a->vm_pgoff + ((b->vm_start - a->vm_start) >> PAGE_SHIFT);
}
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 7a8e84f86831..8fbb7e7c08a5 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -359,7 +359,7 @@ mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
*/
if (arch_has_pfn_modify_check() &&
(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP)) &&
- (newflags & (VM_READ|VM_WRITE|VM_EXEC)) == 0) {
+ (newflags & VM_ACCESS_FLAGS) == 0) {
pgprot_t new_pgprot = vm_get_page_prot(newflags);
error = walk_page_range(current->mm, start, end,
@@ -538,7 +538,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
newflags |= (vma->vm_flags & ~mask_off_old_flags);
/* newflags >> 4 shift VM_MAY% in place of VM_% */
- if ((newflags & ~(newflags >> 4)) & (VM_READ | VM_WRITE | VM_EXEC)) {
+ if ((newflags & ~(newflags >> 4)) & VM_ACCESS_FLAGS) {
error = -EACCES;
goto out;
}
--
2.20.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox