* [PATCH v5 5/6] powernv/iommu: add support for generic boot option iommu.dma_mode
From: Zhen Lei @ 2019-04-09 12:53 UTC (permalink / raw)
To: Jean-Philippe Brucker, John Garry, Robin Murphy, Will Deacon,
Joerg Roedel, Jonathan Corbet, linux-doc, Sebastian Ott,
Gerald Schaefer, Martin Schwidefsky, Heiko Carstens,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Tony Luck, Fenghua Yu, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, David Woodhouse, iommu,
linux-kernel, linux-s390, linuxppc-dev, x86, linux-ia64
Cc: Hanjun Guo, Zhen Lei
In-Reply-To: <20190409125308.18304-1-thunder.leizhen@huawei.com>
iommu=nobypass can be replaced with iommu.dma_mode=strict.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
Documentation/admin-guide/kernel-parameters.txt | 2 +-
arch/powerpc/platforms/powernv/pci-ioda.c | 5 ++---
drivers/iommu/Kconfig | 1 +
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index ab8e3c4798c0a2a..176f96032d9d62a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1815,7 +1815,7 @@
options(such as CONFIG_IOMMU_DEFAULT_PASSTHROUGH) to
choose which mode to be used.
Note: For historical reasons, ARM64/S390/PPC/X86 have
- their specific options. Currently, only ARM64/S390
+ their specific options. Currently, only ARM64/S390/PPC
support this boot option, and hope other ARCHs to use
this as generic boot option.
passthrough
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 3ead4c237ed0ec9..8862885d866418f 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -85,7 +85,6 @@ void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level,
va_end(args);
}
-static bool pnv_iommu_bypass_disabled __read_mostly;
static bool pci_reset_phbs __read_mostly;
static int __init iommu_setup(char *str)
@@ -95,7 +94,7 @@ static int __init iommu_setup(char *str)
while (*str) {
if (!strncmp(str, "nobypass", 8)) {
- pnv_iommu_bypass_disabled = true;
+ iommu_default_dma_mode_set(IOMMU_DMA_MODE_STRICT);
pr_info("PowerNV: IOMMU bypass window disabled.\n");
break;
}
@@ -2456,7 +2455,7 @@ static long pnv_pci_ioda2_setup_default_config(struct pnv_ioda_pe *pe)
return rc;
}
- if (!pnv_iommu_bypass_disabled)
+ if (IOMMU_DMA_MODE_IS_PASSTHROUGH())
pnv_pci_ioda2_set_bypass(pe, true);
return 0;
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index b7173b106cd816a..5dca666b22e6cd5 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -77,6 +77,7 @@ config IOMMU_DEBUGFS
choice
prompt "IOMMU dma mode"
depends on IOMMU_API
+ default IOMMU_DEFAULT_PASSTHROUGH if (PPC_POWERNV && PCI)
default IOMMU_DEFAULT_LAZY if S390_IOMMU
default IOMMU_DEFAULT_STRICT
help
--
1.8.3
^ permalink raw reply related
* [PATCH v5 6/6] x86/iommu: add support for generic boot option iommu.dma_mode
From: Zhen Lei @ 2019-04-09 12:53 UTC (permalink / raw)
To: Jean-Philippe Brucker, John Garry, Robin Murphy, Will Deacon,
Joerg Roedel, Jonathan Corbet, linux-doc, Sebastian Ott,
Gerald Schaefer, Martin Schwidefsky, Heiko Carstens,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Tony Luck, Fenghua Yu, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, David Woodhouse, iommu,
linux-kernel, linux-s390, linuxppc-dev, x86, linux-ia64
Cc: Hanjun Guo, Zhen Lei
In-Reply-To: <20190409125308.18304-1-thunder.leizhen@huawei.com>
The following equivalence or replacement relationship exists:
iommu=pt <--> iommu.dma_mode=passthrough.
iommu=nopt can be replaced with iommu.dma_mode=lazy.
intel_iommu=strict <--> iommu.dma_mode=strict.
amd_iommu=fullflush <--> iommu.dma_mode=strict.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
Documentation/admin-guide/kernel-parameters.txt | 6 ++---
arch/ia64/include/asm/iommu.h | 2 --
arch/ia64/kernel/pci-dma.c | 2 --
arch/x86/include/asm/iommu.h | 1 -
arch/x86/kernel/pci-dma.c | 35 ++++++++++++-------------
drivers/iommu/Kconfig | 2 +-
drivers/iommu/amd_iommu.c | 10 +++----
drivers/iommu/amd_iommu_init.c | 4 +--
drivers/iommu/amd_iommu_types.h | 6 -----
drivers/iommu/intel-iommu.c | 9 +++----
10 files changed, 31 insertions(+), 46 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 176f96032d9d62a..ca6edc529d6a614 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1815,9 +1815,9 @@
options(such as CONFIG_IOMMU_DEFAULT_PASSTHROUGH) to
choose which mode to be used.
Note: For historical reasons, ARM64/S390/PPC/X86 have
- their specific options. Currently, only ARM64/S390/PPC
- support this boot option, and hope other ARCHs to use
- this as generic boot option.
+ their specific options, but strongly recommended switch
+ to use this one, the new ARCHs should use this generic
+ boot option.
passthrough
Configure DMA to bypass the IOMMU by default.
lazy
diff --git a/arch/ia64/include/asm/iommu.h b/arch/ia64/include/asm/iommu.h
index 7429a72f3f92199..92aceef63710861 100644
--- a/arch/ia64/include/asm/iommu.h
+++ b/arch/ia64/include/asm/iommu.h
@@ -8,10 +8,8 @@
extern void no_iommu_init(void);
#ifdef CONFIG_INTEL_IOMMU
extern int force_iommu, no_iommu;
-extern int iommu_pass_through;
extern int iommu_detected;
#else
-#define iommu_pass_through (0)
#define no_iommu (1)
#define iommu_detected (0)
#endif
diff --git a/arch/ia64/kernel/pci-dma.c b/arch/ia64/kernel/pci-dma.c
index fe988c49f01ce6a..f5d49cd3fbb01a9 100644
--- a/arch/ia64/kernel/pci-dma.c
+++ b/arch/ia64/kernel/pci-dma.c
@@ -22,8 +22,6 @@
int force_iommu __read_mostly;
#endif
-int iommu_pass_through;
-
static int __init pci_iommu_init(void)
{
if (iommu_detected)
diff --git a/arch/x86/include/asm/iommu.h b/arch/x86/include/asm/iommu.h
index baedab8ac5385f7..b91623d521d9f0f 100644
--- a/arch/x86/include/asm/iommu.h
+++ b/arch/x86/include/asm/iommu.h
@@ -4,7 +4,6 @@
extern int force_iommu, no_iommu;
extern int iommu_detected;
-extern int iommu_pass_through;
/* 10 seconds */
#define DMAR_OPERATION_TIMEOUT ((cycles_t) tsc_khz*10*1000)
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index d460998ae828514..fc64928e47cb860 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -6,6 +6,7 @@
#include <linux/memblock.h>
#include <linux/gfp.h>
#include <linux/pci.h>
+#include <linux/iommu.h>
#include <asm/proto.h>
#include <asm/dma.h>
@@ -34,21 +35,6 @@
/* Set this to 1 if there is a HW IOMMU in the system */
int iommu_detected __read_mostly = 0;
-/*
- * This variable becomes 1 if iommu=pt is passed on the kernel command line.
- * If this variable is 1, IOMMU implementations do no DMA translation for
- * devices and allow every device to access to whole physical memory. This is
- * useful if a user wants to use an IOMMU only for KVM device assignment to
- * guests and not for driver dma translation.
- * It is also possible to disable by default in kernel config, and enable with
- * iommu=nopt at boot time.
- */
-#ifdef CONFIG_IOMMU_DEFAULT_PASSTHROUGH
-int iommu_pass_through __read_mostly = 1;
-#else
-int iommu_pass_through __read_mostly;
-#endif
-
extern struct iommu_table_entry __iommu_table[], __iommu_table_end[];
/* Dummy device used for NULL arguments (normally ISA). */
@@ -139,10 +125,23 @@ static __init int iommu_setup(char *p)
if (!strncmp(p, "soft", 4))
swiotlb = 1;
#endif
+
+ /*
+ * IOMMU implementations do no DMA translation for devices and
+ * allow every device to access to whole physical memory. This
+ * is useful if a user wants to use an IOMMU only for KVM
+ * device assignment to guests and not for driver dma
+ * translation.
+ */
if (!strncmp(p, "pt", 2))
- iommu_pass_through = 1;
- if (!strncmp(p, "nopt", 4))
- iommu_pass_through = 0;
+ iommu_default_dma_mode_set(IOMMU_DMA_MODE_PASSTHROUGH);
+
+ /*
+ * The default dma mode is lazy on X86. And if dma mode is
+ * already nopt, keep it no change.
+ */
+ if (!strncmp(p, "nopt", 4) && IOMMU_DMA_MODE_IS_PASSTHROUGH())
+ iommu_default_dma_mode_set(IOMMU_DMA_MODE_LAZY);
gart_parse_options(p);
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 5dca666b22e6cd5..ace8cb467d742f9 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -78,7 +78,7 @@ choice
prompt "IOMMU dma mode"
depends on IOMMU_API
default IOMMU_DEFAULT_PASSTHROUGH if (PPC_POWERNV && PCI)
- default IOMMU_DEFAULT_LAZY if S390_IOMMU
+ default IOMMU_DEFAULT_LAZY if (AMD_IOMMU || INTEL_IOMMU || S390_IOMMU)
default IOMMU_DEFAULT_STRICT
help
This option allows IOMMU dma mode to be chose at build time, to
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index f7cdd2ab7f11f6c..44be42f1e887ea4 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -448,7 +448,7 @@ static int iommu_init_device(struct device *dev)
* invalid address), we ignore the capability for the device so
* it'll be forced to go into translation mode.
*/
- if ((iommu_pass_through || !amd_iommu_force_isolation) &&
+ if ((IOMMU_DMA_MODE_IS_PASSTHROUGH() || !amd_iommu_force_isolation) &&
dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev))) {
struct amd_iommu *iommu;
@@ -2274,7 +2274,7 @@ static int amd_iommu_add_device(struct device *dev)
BUG_ON(!dev_data);
- if (iommu_pass_through || dev_data->iommu_v2)
+ if (IOMMU_DMA_MODE_IS_PASSTHROUGH() || dev_data->iommu_v2)
iommu_request_dm_for_dev(dev);
/* Domains are initialized for this device - have a look what we ended up with */
@@ -2479,7 +2479,7 @@ static void __unmap_single(struct dma_ops_domain *dma_dom,
start += PAGE_SIZE;
}
- if (amd_iommu_unmap_flush) {
+ if (IOMMU_DMA_MODE_IS_STRICT()) {
domain_flush_tlb(&dma_dom->domain);
domain_flush_complete(&dma_dom->domain);
dma_ops_free_iova(dma_dom, dma_addr, pages);
@@ -2853,10 +2853,10 @@ int __init amd_iommu_init_api(void)
int __init amd_iommu_init_dma_ops(void)
{
- swiotlb = (iommu_pass_through || sme_me_mask) ? 1 : 0;
+ swiotlb = (IOMMU_DMA_MODE_IS_PASSTHROUGH() || sme_me_mask) ? 1 : 0;
iommu_detected = 1;
- if (amd_iommu_unmap_flush)
+ if (IOMMU_DMA_MODE_IS_STRICT())
pr_info("IO/TLB flush on unmap enabled\n");
else
pr_info("Lazy IO/TLB flushing enabled\n");
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 1b1378619fc9ec2..eae18aae2bafd39 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -166,8 +166,6 @@ struct ivmd_header {
to handle */
LIST_HEAD(amd_iommu_unity_map); /* a list of required unity mappings
we find in ACPI */
-bool amd_iommu_unmap_flush; /* if true, flush on every unmap */
-
LIST_HEAD(amd_iommu_list); /* list of all AMD IOMMUs in the
system */
@@ -2857,7 +2855,7 @@ static int __init parse_amd_iommu_options(char *str)
{
for (; *str; ++str) {
if (strncmp(str, "fullflush", 9) == 0)
- amd_iommu_unmap_flush = true;
+ iommu_default_dma_mode_set(IOMMU_DMA_MODE_STRICT);
if (strncmp(str, "off", 3) == 0)
amd_iommu_disabled = true;
if (strncmp(str, "force_isolation", 15) == 0)
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 87965e4d964771b..724182f158523a1 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -743,12 +743,6 @@ struct unity_map_entry {
/* allocation bitmap for domain ids */
extern unsigned long *amd_iommu_pd_alloc_bitmap;
-/*
- * If true, the addresses will be flushed on unmap time, not when
- * they are reused
- */
-extern bool amd_iommu_unmap_flush;
-
/* Smallest max PASID supported by any IOMMU in the system */
extern u32 amd_iommu_max_pasid;
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 28cb713d728ceef..57203f895382831 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -362,7 +362,6 @@ static int domain_detach_iommu(struct dmar_domain *domain,
static int dmar_map_gfx = 1;
static int dmar_forcedac;
-static int intel_iommu_strict;
static int intel_iommu_superpage = 1;
static int intel_iommu_sm;
static int iommu_identity_mapping;
@@ -453,7 +452,7 @@ static int __init intel_iommu_setup(char *str)
dmar_forcedac = 1;
} else if (!strncmp(str, "strict", 6)) {
pr_info("Disable batched IOTLB flush\n");
- intel_iommu_strict = 1;
+ iommu_default_dma_mode_set(IOMMU_DMA_MODE_STRICT);
} else if (!strncmp(str, "sp_off", 6)) {
pr_info("Disable supported super page\n");
intel_iommu_superpage = 0;
@@ -3408,7 +3407,7 @@ static int __init init_dmars(void)
iommu->flush.flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH);
}
- if (iommu_pass_through)
+ if (IOMMU_DMA_MODE_IS_PASSTHROUGH())
iommu_identity_mapping |= IDENTMAP_ALL;
#ifdef CONFIG_INTEL_IOMMU_BROKEN_GFX_WA
@@ -3749,7 +3748,7 @@ static void intel_unmap(struct device *dev, dma_addr_t dev_addr, size_t size)
freelist = domain_unmap(domain, start_pfn, last_pfn);
- if (intel_iommu_strict) {
+ if (IOMMU_DMA_MODE_IS_STRICT()) {
iommu_flush_iotlb_psi(iommu, domain, start_pfn,
nrpages, !freelist, 0);
/* free iova */
@@ -5460,7 +5459,7 @@ static void quirk_calpella_no_shadow_gtt(struct pci_dev *dev)
} else if (dmar_map_gfx) {
/* we have to ensure the gfx device is idle before we flush */
pci_info(dev, "Disabling batched IOTLB flush on Ironlake\n");
- intel_iommu_strict = 1;
+ iommu_default_dma_mode_set(IOMMU_DMA_MODE_STRICT);
}
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0040, quirk_calpella_no_shadow_gtt);
--
1.8.3
^ permalink raw reply related
* [PATCH v5 0/6] add generic boot option for IOMMU dma mode
From: Zhen Lei @ 2019-04-09 12:53 UTC (permalink / raw)
To: Jean-Philippe Brucker, John Garry, Robin Murphy, Will Deacon,
Joerg Roedel, Jonathan Corbet, linux-doc, Sebastian Ott,
Gerald Schaefer, Martin Schwidefsky, Heiko Carstens,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Tony Luck, Fenghua Yu, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, David Woodhouse, iommu,
linux-kernel, linux-s390, linuxppc-dev, x86, linux-ia64
Cc: Hanjun Guo, Zhen Lei
v4 --> v5:
As Hanjun and Thomas Gleixner's suggestion:
1. Keep the old ARCH specific boot options no change.
2. Keep build option CONFIG_IOMMU_DEFAULT_PASSTHROUGH no change.
v4:
As Robin Murphy's suggestion:
"It's also not necessarily obvious to the user how this interacts with
IOMMU_DEFAULT_PASSTHROUGH, so if we really do go down this route, maybe it
would be better to refactor the whole lot into a single selection of something
like IOMMU_DEFAULT_MODE anyway."
In this version, I tried to normalize the IOMMU dma mode boot options for all
ARCHs. When IOMMU is enabled, there are 3 dma modes: paasthrough(bypass),
lazy(mapping but defer the IOTLB invalidation), strict. But currently each
ARCHs defined their private boot options, different with each other. For
example, to enable/disable "passthrough", ARM64 use iommu.passthrough=1/0,
X86 use iommu=pt/nopt, PPC/POWERNV use iommu=nobypass.
Zhen Lei (6):
iommu: add generic boot option iommu.dma_mode
iommu: add build options corresponding to iommu.dma_mode
iommu: add iommu_default_dma_mode_get/set() helper
s390/pci: add support for generic boot option iommu.dma_mode
powernv/iommu: add support for generic boot option iommu.dma_mode
x86/iommu: add support for generic boot option iommu.dma_mode
Documentation/admin-guide/kernel-parameters.txt | 19 +++++++
arch/ia64/include/asm/iommu.h | 2 -
arch/ia64/kernel/pci-dma.c | 2 -
arch/powerpc/platforms/powernv/pci-ioda.c | 5 +-
arch/s390/pci/pci_dma.c | 14 ++---
arch/x86/include/asm/iommu.h | 1 -
arch/x86/kernel/pci-dma.c | 35 ++++++------
drivers/iommu/Kconfig | 45 ++++++++++++---
drivers/iommu/amd_iommu.c | 10 ++--
drivers/iommu/amd_iommu_init.c | 4 +-
drivers/iommu/amd_iommu_types.h | 6 --
drivers/iommu/intel-iommu.c | 9 ++-
drivers/iommu/iommu.c | 73 ++++++++++++++++++++-----
include/linux/iommu.h | 23 ++++++++
14 files changed, 176 insertions(+), 72 deletions(-)
--
1.8.3
^ permalink raw reply
* [PATCH v5 3/6] iommu: add iommu_default_dma_mode_get/set() helper
From: Zhen Lei @ 2019-04-09 12:53 UTC (permalink / raw)
To: Jean-Philippe Brucker, John Garry, Robin Murphy, Will Deacon,
Joerg Roedel, Jonathan Corbet, linux-doc, Sebastian Ott,
Gerald Schaefer, Martin Schwidefsky, Heiko Carstens,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Tony Luck, Fenghua Yu, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, David Woodhouse, iommu,
linux-kernel, linux-s390, linuxppc-dev, x86, linux-ia64
Cc: Hanjun Guo, Zhen Lei
In-Reply-To: <20190409125308.18304-1-thunder.leizhen@huawei.com>
Also add IOMMU_DMA_MODE_IS_{STRICT|LAZT|PASSTHROUGH}() to make the code
looks cleaner.
There is no functional change, just prepare for the following patches.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
drivers/iommu/iommu.c | 18 ++++++++++++++----
include/linux/iommu.h | 18 ++++++++++++++++++
2 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f4171bf4b46eaeb..86239dd46003fd4 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -195,6 +195,17 @@ static int __init iommu_dma_mode_setup(char *str)
}
early_param("iommu.dma_mode", iommu_dma_mode_setup);
+int iommu_default_dma_mode_get(void)
+{
+ return iommu_default_dma_mode;
+}
+
+void iommu_default_dma_mode_set(int mode)
+{
+ WARN_ON(mode > IOMMU_DMA_MODE_PASSTHROUGH);
+ iommu_default_dma_mode = mode;
+}
+
static ssize_t iommu_group_attr_show(struct kobject *kobj,
struct attribute *__attr, char *buf)
{
@@ -1136,9 +1147,8 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
*/
if (!group->default_domain) {
struct iommu_domain *dom;
- int def_domain_type =
- (iommu_default_dma_mode == IOMMU_DMA_MODE_PASSTHROUGH)
- ? IOMMU_DOMAIN_IDENTITY : IOMMU_DOMAIN_DMA;
+ int def_domain_type = IOMMU_DMA_MODE_IS_PASSTHROUGH() ?
+ IOMMU_DOMAIN_IDENTITY : IOMMU_DOMAIN_DMA;
dom = __iommu_domain_alloc(dev->bus, def_domain_type);
if (!dom && def_domain_type != IOMMU_DOMAIN_DMA) {
@@ -1154,7 +1164,7 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
if (!group->domain)
group->domain = dom;
- if (dom && (iommu_default_dma_mode == IOMMU_DMA_MODE_LAZY)) {
+ if (dom && IOMMU_DMA_MODE_IS_LAZY()) {
int attr = 1;
iommu_domain_set_attr(dom,
DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index c3f4e3416176496..3668a8b3846996a 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -46,6 +46,12 @@
#define IOMMU_DMA_MODE_STRICT 0x0
#define IOMMU_DMA_MODE_LAZY 0x1
#define IOMMU_DMA_MODE_PASSTHROUGH 0x2
+#define IOMMU_DMA_MODE_IS_STRICT() \
+ (iommu_default_dma_mode_get() == IOMMU_DMA_MODE_STRICT)
+#define IOMMU_DMA_MODE_IS_LAZY() \
+ (iommu_default_dma_mode_get() == IOMMU_DMA_MODE_LAZY)
+#define IOMMU_DMA_MODE_IS_PASSTHROUGH() \
+ (iommu_default_dma_mode_get() == IOMMU_DMA_MODE_PASSTHROUGH)
struct iommu_ops;
struct iommu_group;
@@ -421,6 +427,9 @@ static inline void dev_iommu_fwspec_set(struct device *dev,
int iommu_probe_device(struct device *dev);
void iommu_release_device(struct device *dev);
+extern int iommu_default_dma_mode_get(void);
+extern void iommu_default_dma_mode_set(int mode);
+
#else /* CONFIG_IOMMU_API */
struct iommu_ops {};
@@ -705,6 +714,15 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
return NULL;
}
+static inline int iommu_default_dma_mode_get(void)
+{
+ return IOMMU_DMA_MODE_PASSTHROUGH;
+}
+
+static inline void iommu_default_dma_mode_set(int mode)
+{
+}
+
#endif /* CONFIG_IOMMU_API */
#ifdef CONFIG_IOMMU_DEBUGFS
--
1.8.3
^ permalink raw reply related
* [PATCH v5 2/6] iommu: add build options corresponding to iommu.dma_mode
From: Zhen Lei @ 2019-04-09 12:53 UTC (permalink / raw)
To: Jean-Philippe Brucker, John Garry, Robin Murphy, Will Deacon,
Joerg Roedel, Jonathan Corbet, linux-doc, Sebastian Ott,
Gerald Schaefer, Martin Schwidefsky, Heiko Carstens,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Tony Luck, Fenghua Yu, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, David Woodhouse, iommu,
linux-kernel, linux-s390, linuxppc-dev, x86, linux-ia64
Cc: Hanjun Guo, Zhen Lei
In-Reply-To: <20190409125308.18304-1-thunder.leizhen@huawei.com>
First, add build option IOMMU_DEFAULT_{LAZY|STRICT}, so that we have the
opportunity to set {lazy|strict} mode as default at build time. Then put
the three config options in an choice, make people can only choose one of
the three at a time, the same to the boot options iommu.dma_mode.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
Documentation/admin-guide/kernel-parameters.txt | 6 ++--
drivers/iommu/Kconfig | 43 +++++++++++++++++++++----
drivers/iommu/iommu.c | 4 ++-
3 files changed, 42 insertions(+), 11 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f7766f8ac8b9084..92d1b3151d003c2 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1811,9 +1811,9 @@
1 - Bypass the IOMMU for DMA.
unset - Use value of CONFIG_IOMMU_DEFAULT_PASSTHROUGH.
- iommu.dma_mode= Configure default dma mode. if unset, use the value
- of CONFIG_IOMMU_DEFAULT_PASSTHROUGH to determine
- passthrough or not.
+ iommu.dma_mode= Configure default dma mode. if unset, use the build
+ options(such as CONFIG_IOMMU_DEFAULT_PASSTHROUGH) to
+ choose which mode to be used.
Note: For historical reasons, ARM64/S390/PPC/X86 have
their specific options. Currently, only ARM64 support
this boot option, and hope other ARCHs to use this as
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 6f07f3b21816c64..1986f9767da488b 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -74,17 +74,46 @@ config IOMMU_DEBUGFS
debug/iommu directory, and then populate a subdirectory with
entries as required.
-config IOMMU_DEFAULT_PASSTHROUGH
- bool "IOMMU passthrough by default"
+choice
+ prompt "IOMMU dma mode"
depends on IOMMU_API
- help
- Enable passthrough by default, removing the need to pass in
- iommu.passthrough=on or iommu=pt through command line. If this
- is enabled, you can still disable with iommu.passthrough=off
- or iommu=nopt depending on the architecture.
+ default IOMMU_DEFAULT_STRICT
+ help
+ This option allows IOMMU dma mode to be chose at build time, to
+ override the default dma mode of each ARCHs, removing the need to
+ pass in kernel parameters through command line. You can still use the
+ generic boot option iommu.dma_mode or ARCHs specific boot options to
+ override this option again.
+
+config IOMMU_DEFAULT_PASSTHROUGH
+ bool "passthrough"
+ help
+ In this mode, the dma access through IOMMU without any addresses
+ transformation. That means, the wrong or illegal dma access can not
+ be caught, no error information will be reported.
If unsure, say N here.
+config IOMMU_DEFAULT_LAZY
+ bool "lazy"
+ help
+ Support lazy mode, where for every IOMMU DMA unmap operation, the
+ flush operation of IOTLB and the free operation of IOVA are deferred.
+ They are only guaranteed to be done before the related IOVA will be
+ reused.
+
+config IOMMU_DEFAULT_STRICT
+ bool "strict"
+ help
+ For every IOMMU DMA unmap operation, the flush operation of IOTLB and
+ the free operation of IOVA are guaranteed to be done in the unmap
+ function.
+
+ This mode is safer than the two above, but it maybe slow in some high
+ performace scenarios.
+
+endchoice
+
config OF_IOMMU
def_bool y
depends on OF && IOMMU_API
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index df1ce8e22385b48..f4171bf4b46eaeb 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -39,8 +39,10 @@
static struct kset *iommu_group_kset;
static DEFINE_IDA(iommu_group_ida);
-#ifdef CONFIG_IOMMU_DEFAULT_PASSTHROUGH
+#if defined(CONFIG_IOMMU_DEFAULT_PASSTHROUGH)
#define IOMMU_DEFAULT_DMA_MODE IOMMU_DMA_MODE_PASSTHROUGH
+#elif defined(CONFIG_IOMMU_DEFAULT_LAZY)
+#define IOMMU_DEFAULT_DMA_MODE IOMMU_DMA_MODE_LAZY
#else
#define IOMMU_DEFAULT_DMA_MODE IOMMU_DMA_MODE_STRICT
#endif
--
1.8.3
^ permalink raw reply related
* [PATCH v5 1/6] iommu: add generic boot option iommu.dma_mode
From: Zhen Lei @ 2019-04-09 12:53 UTC (permalink / raw)
To: Jean-Philippe Brucker, John Garry, Robin Murphy, Will Deacon,
Joerg Roedel, Jonathan Corbet, linux-doc, Sebastian Ott,
Gerald Schaefer, Martin Schwidefsky, Heiko Carstens,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Tony Luck, Fenghua Yu, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, David Woodhouse, iommu,
linux-kernel, linux-s390, linuxppc-dev, x86, linux-ia64
Cc: Hanjun Guo, Zhen Lei
In-Reply-To: <20190409125308.18304-1-thunder.leizhen@huawei.com>
Currently the IOMMU dma contains 3 modes: passthrough, lazy, strict. The
passthrough mode bypass the IOMMU, the lazy mode defer the invalidation
of hardware TLBs, and the strict mode invalidate IOMMU hardware TLBs
synchronously. The three modes are mutually exclusive. But the current
boot options are confused, such as: iommu.passthrough and iommu.strict,
because they are no good to be coexist. So add iommu.dma_mode.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
Documentation/admin-guide/kernel-parameters.txt | 19 ++++++++
drivers/iommu/iommu.c | 59 ++++++++++++++++++++-----
include/linux/iommu.h | 5 +++
3 files changed, 71 insertions(+), 12 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2b8ee90bb64470d..f7766f8ac8b9084 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1811,6 +1811,25 @@
1 - Bypass the IOMMU for DMA.
unset - Use value of CONFIG_IOMMU_DEFAULT_PASSTHROUGH.
+ iommu.dma_mode= Configure default dma mode. if unset, use the value
+ of CONFIG_IOMMU_DEFAULT_PASSTHROUGH to determine
+ passthrough or not.
+ Note: For historical reasons, ARM64/S390/PPC/X86 have
+ their specific options. Currently, only ARM64 support
+ this boot option, and hope other ARCHs to use this as
+ generic boot option.
+ passthrough
+ Configure DMA to bypass the IOMMU by default.
+ lazy
+ Request that DMA unmap operations use deferred
+ invalidation of hardware TLBs, for increased
+ throughput at the cost of reduced device isolation.
+ Will fall back to strict mode if not supported by
+ the relevant IOMMU driver.
+ strict
+ DMA unmap operations invalidate IOMMU hardware TLBs
+ synchronously.
+
io7= [HW] IO7 for Marvel based alpha systems
See comment before marvel_specify_io7 in
arch/alpha/kernel/core_marvel.c.
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 109de67d5d727c2..df1ce8e22385b48 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -38,12 +38,13 @@
static struct kset *iommu_group_kset;
static DEFINE_IDA(iommu_group_ida);
+
#ifdef CONFIG_IOMMU_DEFAULT_PASSTHROUGH
-static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY;
+#define IOMMU_DEFAULT_DMA_MODE IOMMU_DMA_MODE_PASSTHROUGH
#else
-static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA;
+#define IOMMU_DEFAULT_DMA_MODE IOMMU_DMA_MODE_STRICT
#endif
-static bool iommu_dma_strict __read_mostly = true;
+static int iommu_default_dma_mode __read_mostly = IOMMU_DEFAULT_DMA_MODE;
struct iommu_callback_data {
const struct iommu_ops *ops;
@@ -147,20 +148,51 @@ static int __init iommu_set_def_domain_type(char *str)
int ret;
ret = kstrtobool(str, &pt);
- if (ret)
- return ret;
+ if (!ret && pt)
+ iommu_default_dma_mode = IOMMU_DMA_MODE_PASSTHROUGH;
- iommu_def_domain_type = pt ? IOMMU_DOMAIN_IDENTITY : IOMMU_DOMAIN_DMA;
- return 0;
+ return ret;
}
early_param("iommu.passthrough", iommu_set_def_domain_type);
static int __init iommu_dma_setup(char *str)
{
- return kstrtobool(str, &iommu_dma_strict);
+ bool strict;
+ int ret;
+
+ ret = kstrtobool(str, &strict);
+ if (!ret)
+ iommu_default_dma_mode = strict ?
+ IOMMU_DMA_MODE_STRICT : IOMMU_DMA_MODE_LAZY;
+
+ return ret;
}
early_param("iommu.strict", iommu_dma_setup);
+static int __init iommu_dma_mode_setup(char *str)
+{
+ if (!str)
+ goto fail;
+
+ if (!strncmp(str, "passthrough", 11))
+ iommu_default_dma_mode = IOMMU_DMA_MODE_PASSTHROUGH;
+ else if (!strncmp(str, "lazy", 4))
+ iommu_default_dma_mode = IOMMU_DMA_MODE_LAZY;
+ else if (!strncmp(str, "strict", 6))
+ iommu_default_dma_mode = IOMMU_DMA_MODE_STRICT;
+ else
+ goto fail;
+
+ pr_info("Force dma mode to be %d\n", iommu_default_dma_mode);
+
+ return 0;
+
+fail:
+ pr_debug("Boot option iommu.dma_mode is incorrect, ignored\n");
+ return -EINVAL;
+}
+early_param("iommu.dma_mode", iommu_dma_mode_setup);
+
static ssize_t iommu_group_attr_show(struct kobject *kobj,
struct attribute *__attr, char *buf)
{
@@ -1102,14 +1134,17 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
*/
if (!group->default_domain) {
struct iommu_domain *dom;
+ int def_domain_type =
+ (iommu_default_dma_mode == IOMMU_DMA_MODE_PASSTHROUGH)
+ ? IOMMU_DOMAIN_IDENTITY : IOMMU_DOMAIN_DMA;
- dom = __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
- if (!dom && iommu_def_domain_type != IOMMU_DOMAIN_DMA) {
+ dom = __iommu_domain_alloc(dev->bus, def_domain_type);
+ if (!dom && def_domain_type != IOMMU_DOMAIN_DMA) {
dom = __iommu_domain_alloc(dev->bus, IOMMU_DOMAIN_DMA);
if (dom) {
dev_warn(dev,
"failed to allocate default IOMMU domain of type %u; falling back to IOMMU_DOMAIN_DMA",
- iommu_def_domain_type);
+ def_domain_type);
}
}
@@ -1117,7 +1152,7 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
if (!group->domain)
group->domain = dom;
- if (dom && !iommu_dma_strict) {
+ if (dom && (iommu_default_dma_mode == IOMMU_DMA_MODE_LAZY)) {
int attr = 1;
iommu_domain_set_attr(dom,
DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index ffbbc7e39ceeba3..c3f4e3416176496 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -42,6 +42,11 @@
*/
#define IOMMU_PRIV (1 << 5)
+
+#define IOMMU_DMA_MODE_STRICT 0x0
+#define IOMMU_DMA_MODE_LAZY 0x1
+#define IOMMU_DMA_MODE_PASSTHROUGH 0x2
+
struct iommu_ops;
struct iommu_group;
struct bus_type;
--
1.8.3
^ permalink raw reply related
* Re: [RFC PATCH v2 10/14] kernel/watchdog: Add a function to obtain the watchdog_allowed_mask
From: Peter Zijlstra @ 2019-04-09 11:34 UTC (permalink / raw)
To: Ricardo Neri
Cc: Rafael J. Wysocki, Alexei Starovoitov, Kai-Heng Feng,
Paul Mackerras, H. Peter Anvin, sparclinux, Ingo Molnar,
Christoffer Dall, Davidlohr Bueso, Ashok Raj, x86, David Rientjes,
Andi Kleen, Waiman Long, Borislav Petkov, Don Zickus,
Ravi V. Shankar, Konrad Rzeszutek Wilk, Marc Zyngier,
Frederic Weisbecker, Nicholas Piggin, Ricardo Neri,
Byungchul Park, Mathieu Desnoyers, Josh Poimboeuf,
Paul E. McKenney, Thomas Gleixner, Tony Luck, Babu Moger,
Randy Dunlap, linux-kernel, Luis R. Rodriguez, Masami Hiramatsu,
Philippe Ombredanne, Colin Ian King, Andrew Morton, linuxppc-dev,
David S. Miller
In-Reply-To: <1551283518-18922-11-git-send-email-ricardo.neri-calderon@linux.intel.com>
On Wed, Feb 27, 2019 at 08:05:14AM -0800, Ricardo Neri wrote:
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 8fbfda94a67b..367aa81294ef 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -44,7 +44,7 @@ int __read_mostly soft_watchdog_user_enabled = 1;
> int __read_mostly watchdog_thresh = 10;
> int __read_mostly nmi_watchdog_available;
>
> -struct cpumask watchdog_allowed_mask __read_mostly;
> +static struct cpumask watchdog_allowed_mask __read_mostly;
>
> struct cpumask watchdog_cpumask __read_mostly;
> unsigned long *watchdog_cpumask_bits = cpumask_bits(&watchdog_cpumask);
Hurmph, more struct cpumask, ideally this would get converted to
cpumask_var_t, I don't think we need this before the allocators work, do
we?
^ permalink raw reply
* [PATCH v2 2/3] dt-bindings: fsl,audmix: remove "model" attribute
From: Viorel Suman @ 2019-04-09 11:27 UTC (permalink / raw)
To: Timur Tabi, Nicolin Chen, Xiubo Li, Fabio Estevam, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Rob Herring, Mark Rutland,
Takashi Iwai, Shawn Guo, Sascha Hauer, Julia Lawall
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
Viorel Suman, linux-kernel@vger.kernel.org, Viorel Suman,
dl-linux-imx, Pengutronix Kernel Team,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1554809242-27475-1-git-send-email-viorel.suman@nxp.com>
Remove "model" attribute.
Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
---
Documentation/devicetree/bindings/sound/fsl,audmix.txt | 4 ----
1 file changed, 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/fsl,audmix.txt b/Documentation/devicetree/bindings/sound/fsl,audmix.txt
index 45f807e..840b7e0 100644
--- a/Documentation/devicetree/bindings/sound/fsl,audmix.txt
+++ b/Documentation/devicetree/bindings/sound/fsl,audmix.txt
@@ -38,9 +38,6 @@ Device driver required properties:
to SAI interfaces to be provided, the first SAI in the
list being used to route the AUDMIX output.
- - model : Must contain machine driver name which will configure
- and instantiate the appropriate audio card.
-
Device driver configuration example:
======================================
audmix: audmix@59840000 {
@@ -50,5 +47,4 @@ Device driver configuration example:
clock-names = "ipg";
power-domains = <&pd_audmix>;
dais = <&sai4>, <&sai5>;
- model = "imx-audmix";
};
--
2.7.4
^ permalink raw reply related
* [PATCH v2 1/3] ASoC: fsl_audmix: remove "model" attribute
From: Viorel Suman @ 2019-04-09 11:27 UTC (permalink / raw)
To: Timur Tabi, Nicolin Chen, Xiubo Li, Fabio Estevam, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Rob Herring, Mark Rutland,
Takashi Iwai, Shawn Guo, Sascha Hauer, Julia Lawall
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
Viorel Suman, linux-kernel@vger.kernel.org, Viorel Suman,
dl-linux-imx, Pengutronix Kernel Team,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1554809242-27475-1-git-send-email-viorel.suman@nxp.com>
Use "of_device_id.data" to specify the machine driver
instead of "model" DTS attribute.
Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
---
sound/soc/fsl/fsl_audmix.c | 43 +++++++++++++++++++++++--------------------
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/sound/soc/fsl/fsl_audmix.c b/sound/soc/fsl/fsl_audmix.c
index dabde03..dc802d5 100644
--- a/sound/soc/fsl/fsl_audmix.c
+++ b/sound/soc/fsl/fsl_audmix.c
@@ -445,13 +445,29 @@ static const struct regmap_config fsl_audmix_regmap_config = {
.cache_type = REGCACHE_FLAT,
};
+static const struct of_device_id fsl_audmix_ids[] = {
+ {
+ .compatible = "fsl,imx8qm-audmix",
+ .data = "imx-audmix",
+ },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, fsl_audmix_ids);
+
static int fsl_audmix_probe(struct platform_device *pdev)
{
struct fsl_audmix *priv;
struct resource *res;
+ const char *mdrv;
+ const struct of_device_id *of_id;
void __iomem *regs;
int ret;
- const char *sprop;
+
+ of_id = of_match_device(fsl_audmix_ids, &pdev->dev);
+ if (!of_id || !of_id->data)
+ return -EINVAL;
+
+ mdrv = of_id->data;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
@@ -487,19 +503,12 @@ static int fsl_audmix_probe(struct platform_device *pdev)
return ret;
}
- sprop = of_get_property(pdev->dev.of_node, "model", NULL);
- if (sprop) {
- priv->pdev = platform_device_register_data(&pdev->dev, sprop, 0,
- NULL, 0);
- if (IS_ERR(priv->pdev)) {
- ret = PTR_ERR(priv->pdev);
- dev_err(&pdev->dev,
- "failed to register platform %s: %d\n", sprop,
- ret);
- }
- } else {
- dev_err(&pdev->dev, "[model] attribute missing.\n");
- ret = -EINVAL;
+ priv->pdev = platform_device_register_data(&pdev->dev, mdrv, 0, NULL,
+ 0);
+ if (IS_ERR(priv->pdev)) {
+ ret = PTR_ERR(priv->pdev);
+ dev_err(&pdev->dev, "failed to register platform %s: %d\n",
+ mdrv, ret);
}
return ret;
@@ -553,12 +562,6 @@ static const struct dev_pm_ops fsl_audmix_pm = {
pm_runtime_force_resume)
};
-static const struct of_device_id fsl_audmix_ids[] = {
- { .compatible = "fsl,imx8qm-audmix", },
- { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, fsl_audmix_ids);
-
static struct platform_driver fsl_audmix_driver = {
.probe = fsl_audmix_probe,
.remove = fsl_audmix_remove,
--
2.7.4
^ permalink raw reply related
* [PATCH v2 0/3] ASoC: fsl: audmix: remove "model" attribute and fix ref leaks
From: Viorel Suman @ 2019-04-09 11:27 UTC (permalink / raw)
To: Timur Tabi, Nicolin Chen, Xiubo Li, Fabio Estevam, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Rob Herring, Mark Rutland,
Takashi Iwai, Shawn Guo, Sascha Hauer, Julia Lawall
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
Viorel Suman, linux-kernel@vger.kernel.org, Viorel Suman,
dl-linux-imx, Pengutronix Kernel Team,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
The latest audmix patch-set (v5) had the "model" attribute removed as
requested by Nicolin Chen, but looks like (v4) version of DAI driver
reached "for-next" branch - fix this by removing "model" attribute.
Asside of this fix object reference leaks in machine probe reported by
Julia Lawall.
Viorel Suman (3):
ASoC: fsl_audmix: remove "model" attribute
dt-bindings: fsl,audmix: remove "model" attribute
ASoC: imx-audmix: fix object reference leaks in probe
Changes since V1:
a) Removed "model" attribute from dt-bindings documentation
b) Adressed Daniel's comments
.../devicetree/bindings/sound/fsl,audmix.txt | 4 --
sound/soc/fsl/fsl_audmix.c | 43 ++++++++++++----------
sound/soc/fsl/imx-audmix.c | 4 ++
3 files changed, 27 insertions(+), 24 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v2 3/3] ASoC: imx-audmix: fix object reference leaks in probe
From: Viorel Suman @ 2019-04-09 11:27 UTC (permalink / raw)
To: Timur Tabi, Nicolin Chen, Xiubo Li, Fabio Estevam, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Rob Herring, Mark Rutland,
Takashi Iwai, Shawn Guo, Sascha Hauer, Julia Lawall
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
Viorel Suman, linux-kernel@vger.kernel.org, Viorel Suman,
dl-linux-imx, Pengutronix Kernel Team,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1554809242-27475-1-git-send-email-viorel.suman@nxp.com>
Release the reference to the underlying device taken
by of_find_device_by_node() call.
Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
Reported-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
sound/soc/fsl/imx-audmix.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/sound/soc/fsl/imx-audmix.c b/sound/soc/fsl/imx-audmix.c
index 7983bd3..9aaf3e5 100644
--- a/sound/soc/fsl/imx-audmix.c
+++ b/sound/soc/fsl/imx-audmix.c
@@ -171,6 +171,7 @@ static int imx_audmix_probe(struct platform_device *pdev)
np->full_name);
return -EINVAL;
}
+ put_device(&audmix_pdev->dev);
num_dai = of_count_phandle_with_args(audmix_np, "dais", NULL);
if (num_dai != FSL_AUDMIX_MAX_DAIS) {
@@ -216,6 +217,7 @@ static int imx_audmix_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "failed to find SAI platform device\n");
return -EINVAL;
}
+ put_device(&cpu_pdev->dev);
dai_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s%s",
fe_name_pref, args.np->full_name + 1);
@@ -280,6 +282,8 @@ static int imx_audmix_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "failed to find SAI platform device\n");
return -EINVAL;
}
+ put_device(&cpu_pdev->dev);
+
priv->cpu_mclk = devm_clk_get(&cpu_pdev->dev, "mclk1");
if (IS_ERR(priv->cpu_mclk)) {
ret = PTR_ERR(priv->cpu_mclk);
--
2.7.4
^ permalink raw reply related
* [PATCH] Linux: Define struct termios2 in <termios.h> under _GNU_SOURCE [BZ #10339]
From: Florian Weimer @ 2019-04-09 10:47 UTC (permalink / raw)
To: libc-alpha; +Cc: linux-api, linuxppc-dev, adhemerval.zanella, hpa
struct termios2 is required for setting arbitrary baud rates on serial
ports. <sys/ioctl.h> and <linux/termios.h> have conflicting
definitions in the existing termios definitions, which means that it
is currently very difficult to use TCGETS2/TCSETS2 and struct termios2
with glibc. Providing a definition within glibc resolves this problem.
This does not completely address bug 10339, but it at least exposes
the current kernel functionality in this area.
Support for struct termios2 is architecture-specific in the kernel.
Support on alpha was only added in Linux 4.20. POWER support is
currently missing. The expectation is that the kernel will eventually
use the generic UAPI definition for struct termios2.
2019-04-09 Florian Weimer <fweimer@redhat.com>
[BZ #10339]
Linux: Define struct termios2 in <termios.h> under _GNU_SOURCE.
* sysdeps/unix/sysv/linux/Makefile [$(subdir) == termios] (tests):
Add tst-termios2.
* sysdeps/unix/sysv/linux/tst-termios2.c: New file.
* sysdeps/unix/sysv/linux/bits/termios2-struct.h: Likewise.
* sysdeps/unix/sysv/linux/bits/termios.h [__USE_GNU]: Include it.
* sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h: New file.
* sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h: Likewise.
diff --git a/NEWS b/NEWS
index b58e2469d4..5e6ecb9c7d 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,9 @@ Major new features:
* On Linux, the gettid function has been added.
+* On Linux, <termios.h> now provides a definition of struct termios2 with
+ the _GNU_SOURCE feature test macro.
+
* Minguo (Republic of China) calendar support has been added as an
alternative calendar for the following locales: zh_TW, cmn_TW, hak_TW,
nan_TW, lzh_TW.
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 52ac6ad484..4cb5e4f0d2 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -156,6 +156,7 @@ endif
ifeq ($(subdir),termios)
sysdep_headers += termio.h
+tests += tst-termios2
endif
ifeq ($(subdir),posix)
diff --git a/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h b/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
new file mode 100644
index 0000000000..5f09445e23
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
@@ -0,0 +1,33 @@
+/* struct termios2 definition. Linux/alpha version.
+ Copyright (C) 2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library. If not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _TERMIOS_H
+# error "Never include <bits/termios2-struct.h> directly; use <termios.h> instead."
+#endif
+
+struct termios2
+{
+ tcflag_t c_iflag;
+ tcflag_t c_oflag;
+ tcflag_t c_cflag;
+ tcflag_t c_lflag;
+ cc_t c_cc[NCCS];
+ cc_t c_line;
+ speed_t c_ispeed;
+ speed_t c_ospeed;
+};
diff --git a/sysdeps/unix/sysv/linux/bits/termios.h b/sysdeps/unix/sysv/linux/bits/termios.h
index 997231cd03..45ac7affdf 100644
--- a/sysdeps/unix/sysv/linux/bits/termios.h
+++ b/sysdeps/unix/sysv/linux/bits/termios.h
@@ -25,6 +25,10 @@ typedef unsigned int speed_t;
typedef unsigned int tcflag_t;
#include <bits/termios-struct.h>
+#ifdef __USE_GNU
+# include <bits/termios2-struct.h>
+#endif
+
#include <bits/termios-c_cc.h>
#include <bits/termios-c_iflag.h>
#include <bits/termios-c_oflag.h>
diff --git a/sysdeps/unix/sysv/linux/bits/termios2-struct.h b/sysdeps/unix/sysv/linux/bits/termios2-struct.h
new file mode 100644
index 0000000000..5a48e45ef3
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/bits/termios2-struct.h
@@ -0,0 +1,33 @@
+/* struct termios2 definition. Linux/generic version.
+ Copyright (C) 2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library. If not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _TERMIOS_H
+# error "Never include <bits/termios2-struct.h> directly; use <termios.h> instead."
+#endif
+
+struct termios2
+{
+ tcflag_t c_iflag;
+ tcflag_t c_oflag;
+ tcflag_t c_cflag;
+ tcflag_t c_lflag;
+ cc_t c_line;
+ cc_t c_cc[NCCS];
+ speed_t c_ispeed;
+ speed_t c_ospeed;
+};
diff --git a/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h b/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
new file mode 100644
index 0000000000..7c889e575c
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
@@ -0,0 +1,33 @@
+/* struct termios2 definition. Linux/sparc version.
+ Copyright (C) 2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library. If not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _TERMIOS_H
+# error "Never include <bits/termios2-struct.h> directly; use <termios.h> instead."
+#endif
+
+struct termios2
+{
+ tcflag_t c_iflag;
+ tcflag_t c_oflag;
+ tcflag_t c_cflag;
+ tcflag_t c_lflag;
+ cc_t c_line;
+ cc_t c_cc[NCCS + 2];
+ speed_t c_ispeed;
+ speed_t c_ospeed;
+};
diff --git a/sysdeps/unix/sysv/linux/tst-termios2.c b/sysdeps/unix/sysv/linux/tst-termios2.c
new file mode 100644
index 0000000000..82326a1288
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-termios2.c
@@ -0,0 +1,48 @@
+/* Minimal test of struct termios2 definition.
+ Copyright (C) 2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library. If not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <termios.h>
+#include <sys/ioctl.h>
+
+/* This function is never executed, but must be compiled successfully.
+ Accessing serial ports in the test suite is problematic because
+ they likely correspond with low-level system functionality. */
+void
+not_executed (int fd)
+{
+ /* Avoid a compilation failure if TCGETS2, TCSETS2 are not
+ defined. */
+#if defined (TCGETS2) && defined (TCSETS2)
+ struct termios2 ti;
+ ioctl (fd, TCGETS2, &ti);
+ ioctl (fd, TCSETS2, &ti);
+#endif
+}
+
+static int
+do_test (void)
+{
+ /* Fail at run time if TCGETS2 or TCSETS2 is not defined. */
+#if defined (TCGETS2) && defined (TCSETS2)
+ return 0;
+#else
+ return 1;
+#endif
+}
+
+#include <support/test-driver.c>
^ permalink raw reply related
* Re: [PATCH 2/2] ASoC: imx-audmix: fix object reference leaks in probe
From: Daniel Baluta @ 2019-04-09 10:08 UTC (permalink / raw)
To: Viorel Suman
Cc: linux-arm-kernel@lists.infradead.org, Viorel Suman,
alsa-devel@alsa-project.org, linuxppc-dev@lists.ozlabs.org,
Timur Tabi, Xiubo Li, Shawn Guo, Sascha Hauer, Takashi Iwai,
Liam Girdwood, Jaroslav Kysela, Nicolin Chen, Julia Lawall,
Mark Brown, dl-linux-imx, Pengutronix Kernel Team, Fabio Estevam,
linux-kernel@vger.kernel.org
In-Reply-To: <1554798876-20653-3-git-send-email-viorel.suman@nxp.com>
On Tue, Apr 9, 2019 at 11:36 AM Viorel Suman <viorel.suman@nxp.com> wrote:
>
> Release the reference to the underlying device taken
> by of_find_device_by_node() call.
>
> Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
Please add here the Reported-by tag pointing to Julia.
> ---
> sound/soc/fsl/imx-audmix.c | 31 +++++++++++--------------------
> 1 file changed, 11 insertions(+), 20 deletions(-)
>
> diff --git a/sound/soc/fsl/imx-audmix.c b/sound/soc/fsl/imx-audmix.c
> index 7983bd3..7c24095 100644
> --- a/sound/soc/fsl/imx-audmix.c
> +++ b/sound/soc/fsl/imx-audmix.c
> @@ -20,10 +20,7 @@
> #include "fsl_audmix.h"
>
> struct imx_audmix {
> - struct platform_device *pdev;
> struct snd_soc_card card;
> - struct platform_device *audmix_pdev;
> - struct platform_device *out_pdev;
I am not sure why are you removing these members here. It doesn't seem to match
with patch description. If these are needed to simplify the code please do it in
another patch.
This patch should only fix one problem and that is the refleak.
thanks,
Daniel.
^ permalink raw reply
* Re: [PATCH 1/2] ASoC: fsl_audmix: remove "model" attribute
From: Daniel Baluta @ 2019-04-09 10:05 UTC (permalink / raw)
To: Viorel Suman
Cc: linux-arm-kernel@lists.infradead.org, Viorel Suman,
alsa-devel@alsa-project.org, linuxppc-dev@lists.ozlabs.org,
Timur Tabi, Xiubo Li, Shawn Guo, Sascha Hauer, Takashi Iwai,
Liam Girdwood, Jaroslav Kysela, Nicolin Chen, Julia Lawall,
Mark Brown, dl-linux-imx, Pengutronix Kernel Team, Fabio Estevam,
linux-kernel@vger.kernel.org
In-Reply-To: <1554798876-20653-2-git-send-email-viorel.suman@nxp.com>
Hi Viorel,
Few comments inline.
On Tue, Apr 9, 2019 at 11:36 AM Viorel Suman <viorel.suman@nxp.com> wrote:
>
> Use "of_device_id.data" to specify the machine driver,
> instead of "model" DTS attribute.
<snip>
> static int fsl_audmix_probe(struct platform_device *pdev)
> {
> + struct device *dev = &pdev->dev;
You might want to remove this change from the patch because it touches
a lot of lines
and it makes the review harder.
I don't see any reason to have it at all.
thanks,
Daniel.
^ permalink raw reply
* [PATCH v1 4/4] mm/memory_hotplug: Make __remove_pages() and arch_remove_memory() never fail
From: David Hildenbrand @ 2019-04-09 10:01 UTC (permalink / raw)
To: linux-mm
Cc: Oscar Salvador, Michal Hocko, linux-ia64, David Hildenbrand,
Peter Zijlstra, Dave Hansen, Heiko Carstens, Stefan Agner,
Wei Yang, Masahiro Yamada, Rich Felker, Paul Mackerras,
H. Peter Anvin, Qian Cai, Rob Herring, Yoshinori Sato, linux-sh,
x86, Mike Rapoport, Ingo Molnar, Geert Uytterhoeven, Fenghua Yu,
Pavel Tatashin, Vasily Gorbik, linux-s390, Nicholas Piggin,
Borislav Petkov, Andy Lutomirski, Thomas Gleixner, Joonsoo Kim,
Tony Luck, Mathieu Malaterre, linux-kernel, Martin Schwidefsky,
Arun KS, Andrew Morton, linuxppc-dev, Kirill A. Shutemov
In-Reply-To: <20190409100148.24703-1-david@redhat.com>
All callers of arch_remove_memory() ignore errors. And we should really
try to remove any errors from the memory removal path.
No more errors are reported from __remove_pages(). BUG() in s390x code
in case arch_remove_memory() is triggered. We may implement that properly
later. WARN in case powerpc code failed to remove the section mapping,
which is better than ignoring the error completely right now.
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: Oscar Salvador <osalvador@suse.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Arun KS <arunks@codeaurora.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: Qian Cai <cai@lca.pw>
Cc: Mathieu Malaterre <malat@debian.org>
Cc: linux-ia64@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-sh@vger.kernel.org
Cc: linux-s390@vger.kernel.org
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/ia64/mm/init.c | 11 +++--------
arch/powerpc/mm/mem.c | 11 ++++-------
arch/s390/mm/init.c | 5 +++--
arch/sh/mm/init.c | 11 +++--------
arch/x86/mm/init_32.c | 5 +++--
arch/x86/mm/init_64.c | 10 +++-------
include/linux/memory_hotplug.h | 8 ++++----
mm/memory_hotplug.c | 5 ++---
8 files changed, 25 insertions(+), 41 deletions(-)
diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 379eb1f9adc9..d28e29103bdb 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -682,20 +682,15 @@ int arch_add_memory(int nid, u64 start, u64 size,
}
#ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap)
+void arch_remove_memory(int nid, u64 start, u64 size,
+ struct vmem_altmap *altmap)
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
struct zone *zone;
- int ret;
zone = page_zone(pfn_to_page(start_pfn));
- ret = __remove_pages(zone, start_pfn, nr_pages, altmap);
- if (ret)
- pr_warn("%s: Problem encountered in __remove_pages() as"
- " ret=%d\n", __func__, ret);
-
- return ret;
+ __remove_pages(zone, start_pfn, nr_pages, altmap);
}
#endif
#endif
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 76deaa8525db..cc9425fb9056 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -131,8 +131,8 @@ int __meminit arch_add_memory(int nid, u64 start, u64 size,
}
#ifdef CONFIG_MEMORY_HOTREMOVE
-int __meminit arch_remove_memory(int nid, u64 start, u64 size,
- struct vmem_altmap *altmap)
+void __meminit arch_remove_memory(int nid, u64 start, u64 size,
+ struct vmem_altmap *altmap)
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
@@ -147,14 +147,13 @@ int __meminit arch_remove_memory(int nid, u64 start, u64 size,
if (altmap)
page += vmem_altmap_offset(altmap);
- ret = __remove_pages(page_zone(page), start_pfn, nr_pages, altmap);
- if (ret)
- return ret;
+ __remove_pages(page_zone(page), start_pfn, nr_pages, altmap);
/* Remove htab bolted mappings for this section of memory */
start = (unsigned long)__va(start);
flush_inval_dcache_range(start, start + size);
ret = remove_section_mapping(start, start + size);
+ WARN_ON_ONCE(ret);
/* Ensure all vmalloc mappings are flushed in case they also
* hit that section of memory
@@ -162,8 +161,6 @@ int __meminit arch_remove_memory(int nid, u64 start, u64 size,
vm_unmap_aliases();
resize_hpt_for_hotplug(memblock_phys_mem_size());
-
- return ret;
}
#endif
#endif /* CONFIG_MEMORY_HOTPLUG */
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index f5db961ad792..31b1071315d7 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -234,14 +234,15 @@ int arch_add_memory(int nid, u64 start, u64 size,
}
#ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap)
+void arch_remove_memory(int nid, u64 start, u64 size,
+ struct vmem_altmap *altmap)
{
/*
* There is no hardware or firmware interface which could trigger a
* hot memory remove on s390. So there is nothing that needs to be
* implemented.
*/
- return -EBUSY;
+ BUG();
}
#endif
#endif /* CONFIG_MEMORY_HOTPLUG */
diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index 168d3a6b9358..5aeb4d7099a1 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -429,20 +429,15 @@ EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
#endif
#ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap)
+void arch_remove_memory(int nid, u64 start, u64 size,
+ struct vmem_altmap *altmap)
{
unsigned long start_pfn = PFN_DOWN(start);
unsigned long nr_pages = size >> PAGE_SHIFT;
struct zone *zone;
- int ret;
zone = page_zone(pfn_to_page(start_pfn));
- ret = __remove_pages(zone, start_pfn, nr_pages, altmap);
- if (unlikely(ret))
- pr_warn("%s: Failed, __remove_pages() == %d\n", __func__,
- ret);
-
- return ret;
+ __remove_pages(zone, start_pfn, nr_pages, altmap);
}
#endif
#endif /* CONFIG_MEMORY_HOTPLUG */
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index 755dbed85531..075e568098f2 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -860,14 +860,15 @@ int arch_add_memory(int nid, u64 start, u64 size,
}
#ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap)
+void arch_remove_memory(int nid, u64 start, u64 size,
+ struct vmem_altmap *altmap)
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
struct zone *zone;
zone = page_zone(pfn_to_page(start_pfn));
- return __remove_pages(zone, start_pfn, nr_pages, altmap);
+ __remove_pages(zone, start_pfn, nr_pages, altmap);
}
#endif
#endif
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index db42c11b48fb..20d14254b686 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1141,24 +1141,20 @@ kernel_physical_mapping_remove(unsigned long start, unsigned long end)
remove_pagetable(start, end, true, NULL);
}
-int __ref arch_remove_memory(int nid, u64 start, u64 size,
- struct vmem_altmap *altmap)
+void __ref arch_remove_memory(int nid, u64 start, u64 size,
+ struct vmem_altmap *altmap)
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
struct page *page = pfn_to_page(start_pfn);
struct zone *zone;
- int ret;
/* With altmap the first mapped page is offset from @start */
if (altmap)
page += vmem_altmap_offset(altmap);
zone = page_zone(page);
- ret = __remove_pages(zone, start_pfn, nr_pages, altmap);
- WARN_ON_ONCE(ret);
+ __remove_pages(zone, start_pfn, nr_pages, altmap);
kernel_physical_mapping_remove(start, start + size);
-
- return ret;
}
#endif
#endif /* CONFIG_MEMORY_HOTPLUG */
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index d0a145ffa4fe..9d0efac902ec 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -112,10 +112,10 @@ static inline bool movable_node_is_enabled(void)
}
#ifdef CONFIG_MEMORY_HOTREMOVE
-extern int arch_remove_memory(int nid, u64 start, u64 size,
- struct vmem_altmap *altmap);
-extern int __remove_pages(struct zone *zone, unsigned long start_pfn,
- unsigned long nr_pages, struct vmem_altmap *altmap);
+extern void arch_remove_memory(int nid, u64 start, u64 size,
+ struct vmem_altmap *altmap);
+extern void __remove_pages(struct zone *zone, unsigned long start_pfn,
+ unsigned long nr_pages, struct vmem_altmap *altmap);
#endif /* CONFIG_MEMORY_HOTREMOVE */
/*
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 17a60281c36f..52fef4a81e4c 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -548,8 +548,8 @@ static void __remove_section(struct zone *zone, struct mem_section *ms,
* sure that pages are marked reserved and zones are adjust properly by
* calling offline_pages().
*/
-int __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
- unsigned long nr_pages, struct vmem_altmap *altmap)
+void __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
+ unsigned long nr_pages, struct vmem_altmap *altmap)
{
unsigned long i;
unsigned long map_offset = 0;
@@ -580,7 +580,6 @@ int __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
}
set_zone_contiguous(zone);
- return 0;
}
#endif /* CONFIG_MEMORY_HOTREMOVE */
--
2.17.2
^ permalink raw reply related
* Re: [PATCH v2 1/2] cpuidle : auto-promotion for cpuidle states
From: Rafael J. Wysocki @ 2019-04-09 10:01 UTC (permalink / raw)
To: Abhishek Goel
Cc: Gautham R. Shenoy, Linux PM, Daniel Lezcano, Rafael J. Wysocki,
Linux Kernel Mailing List, linuxppc-dev
In-Reply-To: <20190405091647.4169-2-huntbag@linux.vnet.ibm.com>
On Fri, Apr 5, 2019 at 11:17 AM Abhishek Goel
<huntbag@linux.vnet.ibm.com> wrote:
>
> Currently, the cpuidle governors (menu /ladder) determine what idle state
There are three governors in 5.1-rc.
> an idling CPU should enter into based on heuristics that depend on the
> idle history on that CPU. Given that no predictive heuristic is perfect,
> there are cases where the governor predicts a shallow idle state, hoping
> that the CPU will be busy soon. However, if no new workload is scheduled
> on that CPU in the near future, the CPU will end up in the shallow state.
>
> In case of POWER, this is problematic, when the predicted state in the
> aforementioned scenario is a lite stop state, as such lite states will
> inhibit SMT folding, thereby depriving the other threads in the core from
> using the core resources.
>
> To address this, such lite states need to be autopromoted.
I don't quite agree with this statement and it doesn't even match what
the patch does AFAICS. "Autopromotion" would be going from the given
state to a deeper one without running state selection in between, but
that's not what's going on here.
> The cpuidle-core can queue timer to correspond with the residency value of the next
> available state. Thus leading to auto-promotion to a deeper idle state as
> soon as possible.
No, it doesn't automatically cause a deeper state to be used next
time. It simply kicks the CPU out of the idle state and one more
iteration of the idle loop runs on it. Whether or not a deeper state
will be selected in that iteration depends on the governor
computations carried out in it.
Now, this appears to be almost analogous to the "polling" state used
on x86 which uses the next idle state's target residency as a timeout.
While generally I'm not a big fan of setting up timers in the idle
loop (it sort of feels like pulling your own hair in order to get
yourself out of a swamp), if idle states like these are there in your
platform, setting up a timer to get out of them in the driver's
->enter() routine might not be particularly objectionable. Doing that
in the core is a whole different story, though.
Generally, this adds quite a bit of complexity (on the "ugly" side of
things IMO) to the core to cover a corner case present in one
platform, while IMO it can be covered in the driver for that platform
directly.
> Signed-off-by: Abhishek Goel <huntbag@linux.vnet.ibm.com>
> ---
>
> v1->v2 : Removed timeout_needed and rebased to current upstream kernel
>
> drivers/cpuidle/cpuidle.c | 68 +++++++++++++++++++++++++++++-
> drivers/cpuidle/governors/ladder.c | 3 +-
> drivers/cpuidle/governors/menu.c | 22 +++++++++-
> include/linux/cpuidle.h | 10 ++++-
> 4 files changed, 99 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 7f108309e..11ce43f19 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -36,6 +36,11 @@ static int enabled_devices;
> static int off __read_mostly;
> static int initialized __read_mostly;
>
> +struct auto_promotion {
> + struct hrtimer hrtimer;
> + unsigned long timeout_us;
> +};
> +
> int cpuidle_disabled(void)
> {
> return off;
> @@ -188,6 +193,54 @@ int cpuidle_enter_s2idle(struct cpuidle_driver *drv, struct cpuidle_device *dev)
> }
> #endif /* CONFIG_SUSPEND */
>
> +enum hrtimer_restart auto_promotion_hrtimer_callback(struct hrtimer *hrtimer)
> +{
> + return HRTIMER_NORESTART;
> +}
> +
> +#ifdef CONFIG_CPU_IDLE_AUTO_PROMOTION
> +DEFINE_PER_CPU(struct auto_promotion, ap);
> +
> +static void cpuidle_auto_promotion_start(int cpu, struct cpuidle_state *state)
> +{
> + struct auto_promotion *this_ap = &per_cpu(ap, cpu);
> +
> + if (state->flags & CPUIDLE_FLAG_AUTO_PROMOTION)
> + hrtimer_start(&this_ap->hrtimer, ns_to_ktime(this_ap->timeout_us
> + * 1000), HRTIMER_MODE_REL_PINNED);
> +}
> +
> +static void cpuidle_auto_promotion_cancel(int cpu)
> +{
> + struct hrtimer *hrtimer;
> +
> + hrtimer = &per_cpu(ap, cpu).hrtimer;
> + if (hrtimer_is_queued(hrtimer))
> + hrtimer_cancel(hrtimer);
> +}
> +
> +static void cpuidle_auto_promotion_update(int cpu, unsigned long timeout)
> +{
> + per_cpu(ap, cpu).timeout_us = timeout;
> +}
> +
> +static void cpuidle_auto_promotion_init(int cpu, struct cpuidle_driver *drv)
> +{
> + struct auto_promotion *this_ap = &per_cpu(ap, cpu);
> +
> + hrtimer_init(&this_ap->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> + this_ap->hrtimer.function = auto_promotion_hrtimer_callback;
> +}
> +#else
> +static inline void cpuidle_auto_promotion_start(int cpu, struct cpuidle_state
> + *state) { }
> +static inline void cpuidle_auto_promotion_cancel(int cpu) { }
> +static inline void cpuidle_auto_promotion_update(int cpu, unsigned long
> + timeout) { }
> +static inline void cpuidle_auto_promotion_init(int cpu, struct cpuidle_driver
> + *drv) { }
> +#endif
> +
> /**
> * cpuidle_enter_state - enter the state and update stats
> * @dev: cpuidle device for this cpu
> @@ -225,12 +278,17 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv,
> trace_cpu_idle_rcuidle(index, dev->cpu);
> time_start = ns_to_ktime(local_clock());
>
> + cpuidle_auto_promotion_start(dev->cpu, target_state);
First off, I wouldn't call it "auto-promotion", because it just adds a
timeout to trigger if the CPU spends too much time in the target
state.
Second, and more important, I don't see why this cannot be done in
target_state->enter() just for the state in which it is needed (in
analogy with the "polling" state).
> +
> stop_critical_timings();
> entered_state = target_state->enter(dev, drv, index);
> start_critical_timings();
>
> sched_clock_idle_wakeup_event();
> time_end = ns_to_ktime(local_clock());
> +
> + cpuidle_auto_promotion_cancel(dev->cpu);
> +
> trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu);
>
> /* The cpu is no longer idle or about to enter idle. */
> @@ -312,7 +370,13 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv,
> int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
> bool *stop_tick)
> {
> - return cpuidle_curr_governor->select(drv, dev, stop_tick);
> + unsigned long timeout_us, ret;
> +
> + timeout_us = UINT_MAX;
> + ret = cpuidle_curr_governor->select(drv, dev, stop_tick, &timeout_us);
> + cpuidle_auto_promotion_update(dev->cpu, timeout_us);
> +
> + return ret;
> }
>
> /**
> @@ -658,6 +722,8 @@ int cpuidle_register(struct cpuidle_driver *drv,
> device = &per_cpu(cpuidle_dev, cpu);
> device->cpu = cpu;
>
> + cpuidle_auto_promotion_init(cpu, drv);
> +
> #ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
> /*
> * On multiplatform for ARM, the coupled idle states could be
> diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
> index f0dddc66a..65b518dd7 100644
> --- a/drivers/cpuidle/governors/ladder.c
> +++ b/drivers/cpuidle/governors/ladder.c
> @@ -64,7 +64,8 @@ static inline void ladder_do_selection(struct ladder_device *ldev,
> * @dummy: not used
> */
> static int ladder_select_state(struct cpuidle_driver *drv,
> - struct cpuidle_device *dev, bool *dummy)
> + struct cpuidle_device *dev, bool *dummy,
> + unsigned long *unused)
> {
> struct ladder_device *ldev = this_cpu_ptr(&ladder_devices);
> struct ladder_device_state *last_state;
> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
> index 5951604e7..835e337de 100644
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -276,7 +276,7 @@ static unsigned int get_typical_interval(struct menu_device *data,
> * @stop_tick: indication on whether or not to stop the tick
> */
> static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
> - bool *stop_tick)
> + bool *stop_tick, unsigned long *timeout)
> {
> struct menu_device *data = this_cpu_ptr(&menu_devices);
> int latency_req = cpuidle_governor_latency_req(dev->cpu);
> @@ -442,6 +442,26 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
> }
> }
>
> +#ifdef CPUIDLE_FLAG_AUTO_PROMOTION
> + if (drv->states[idx].flags & CPUIDLE_FLAG_AUTO_PROMOTION) {
> + /*
> + * Timeout is intended to be defined as sum of target residency
> + * of next available state, entry latency and exit latency. If
> + * time interval equal to timeout is spent in current state,
> + * and if it is a shallow lite state, we may want to auto-
> + * promote from such state.
> + */
> + for (i = idx + 1; i < drv->state_count; i++) {
> + if (drv->states[i].disabled ||
> + dev->states_usage[i].disable)
> + continue;
> + *timeout = drv->states[i].target_residency +
> + 2 * drv->states[i].exit_latency;
> + break;
> + }
> + }
> +#endif
Why do you need to do the above in the governor at all?
The driver's ->enter() callback knows what state has been selected, it
can check what the next available state is and set up the timer
accordingly.
I don't see the need to pass the timeout from the governor to the core
anyway when the next thing the governor does is to return the selected
state index.
> +
> return idx;
> }
>
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 3b3947232..84d76d1ec 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -72,6 +72,13 @@ struct cpuidle_state {
> #define CPUIDLE_FLAG_POLLING BIT(0) /* polling state */
> #define CPUIDLE_FLAG_COUPLED BIT(1) /* state applies to multiple cpus */
> #define CPUIDLE_FLAG_TIMER_STOP BIT(2) /* timer is stopped on this state */
> +/*
> + * State with only and only fast state bit set don't even lose user context.
> + * But such states prevent other sibling threads from thread folding benefits.
> + * And hence we don't want to stay for too long in such states and want to
> + * auto-promote from it.
> + */
> +#define CPUIDLE_FLAG_AUTO_PROMOTION BIT(3)
>
> struct cpuidle_device_kobj;
> struct cpuidle_state_kobj;
> @@ -243,7 +250,8 @@ struct cpuidle_governor {
>
> int (*select) (struct cpuidle_driver *drv,
> struct cpuidle_device *dev,
> - bool *stop_tick);
> + bool *stop_tick, unsigned long
> + *timeout);
> void (*reflect) (struct cpuidle_device *dev, int index);
> };
>
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v2 1/2] cpuidle : auto-promotion for cpuidle states
From: Abhishek @ 2019-04-09 9:42 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Gautham R. Shenoy, Linux PM, Daniel Lezcano, Rafael J. Wysocki,
Linux Kernel Mailing List, linuxppc-dev, Daniel Axtens
In-Reply-To: <CAJZ5v0jS7HX3ka59OhYzbdgdAYDO_DTwp44gxar_xQV_AsscSw@mail.gmail.com>
On 04/09/2019 03:00 PM, Rafael J. Wysocki wrote:
> On Tue, Apr 9, 2019 at 11:29 AM Abhishek <huntbag@linux.vnet.ibm.com> wrote:
>> Hi Daniel,
>>
>> Thanks for such a descriptive review. I will include all the suggestions
>> made in my next iteration.
> Please give me some time to send comments before that.
Sure, I will wait for your review.
^ permalink raw reply
* Re: [PATCH v2 1/2] cpuidle : auto-promotion for cpuidle states
From: Abhishek @ 2019-04-09 9:36 UTC (permalink / raw)
To: Daniel Axtens, linux-kernel, linuxppc-dev, linux-pm
Cc: daniel.lezcano, rjw, ego
In-Reply-To: <87lg0kwp3t.fsf@dja-thinkpad.axtens.net>
Hi Daniel,
On 04/08/2019 07:55 PM, Daniel Axtens wrote:
> Hi,
>
> Sorry, just realised another thing I wanted to ask:
>
>> @@ -442,6 +442,26 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
>> }
>> }
>>
>>
>> +#ifdef CPUIDLE_FLAG_AUTO_PROMOTION
> Why is this based on CPUIDLE_FLAG_ rather than CONFIG_CPU_IDLE_? Won't
> this always be true, given that the flag is defined regardless of the
> config option in the header?
Yeah, You are right. This should have been CONFIG_CPU_IDLE_AUTO_PROMOTION.
--Abhishek
^ permalink raw reply
* Re: [PATCH v2 1/2] cpuidle : auto-promotion for cpuidle states
From: Rafael J. Wysocki @ 2019-04-09 9:30 UTC (permalink / raw)
To: Abhishek
Cc: Gautham R. Shenoy, Linux PM, Daniel Lezcano, Rafael J. Wysocki,
Linux Kernel Mailing List, linuxppc-dev, Daniel Axtens
In-Reply-To: <a8368cfa-9667-5f5b-cafe-f4830ece0f26@linux.vnet.ibm.com>
On Tue, Apr 9, 2019 at 11:29 AM Abhishek <huntbag@linux.vnet.ibm.com> wrote:
>
> Hi Daniel,
>
> Thanks for such a descriptive review. I will include all the suggestions
> made in my next iteration.
Please give me some time to send comments before that.
^ permalink raw reply
* Re: [PATCH v2 1/2] cpuidle : auto-promotion for cpuidle states
From: Abhishek @ 2019-04-09 9:28 UTC (permalink / raw)
To: Daniel Axtens, linux-kernel, linuxppc-dev, linux-pm
Cc: daniel.lezcano, rjw, ego
In-Reply-To: <87r2acwpp2.fsf@dja-thinkpad.axtens.net>
Hi Daniel,
Thanks for such a descriptive review. I will include all the suggestions
made in my next iteration.
--Abhishek
On 04/08/2019 07:42 PM, Daniel Axtens wrote:
> Hi Abhishek,
>
>> Currently, the cpuidle governors (menu /ladder) determine what idle state
>> an idling CPU should enter into based on heuristics that depend on the
>> idle history on that CPU. Given that no predictive heuristic is perfect,
>> there are cases where the governor predicts a shallow idle state, hoping
>> that the CPU will be busy soon. However, if no new workload is scheduled
>> on that CPU in the near future, the CPU will end up in the shallow state.
>>
>> In case of POWER, this is problematic, when the predicted state in the
>> aforementioned scenario is a lite stop state, as such lite states will
>> inhibit SMT folding, thereby depriving the other threads in the core from
>> using the core resources.
>>
>> To address this, such lite states need to be autopromoted. The cpuidle-
>> core can queue timer to correspond with the residency value of the next
>> available state. Thus leading to auto-promotion to a deeper idle state as
>> soon as possible.
>>
> This sounds sensible to me, although I'm not really qualified to offer a
> full power-management opinion on it. I have some general code questions
> and comments, however, which are below:
>
>> Signed-off-by: Abhishek Goel <huntbag@linux.vnet.ibm.com>
>> ---
>>
>> v1->v2 : Removed timeout_needed and rebased to current upstream kernel
>>
>> drivers/cpuidle/cpuidle.c | 68 +++++++++++++++++++++++++++++-
>> drivers/cpuidle/governors/ladder.c | 3 +-
>> drivers/cpuidle/governors/menu.c | 22 +++++++++-
>> include/linux/cpuidle.h | 10 ++++-
>> 4 files changed, 99 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index 7f108309e..11ce43f19 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -36,6 +36,11 @@ static int enabled_devices;
>> static int off __read_mostly;
>> static int initialized __read_mostly;
>>
>> +struct auto_promotion {
>> + struct hrtimer hrtimer;
>> + unsigned long timeout_us;
>> +};
>> +
>> int cpuidle_disabled(void)
>> {
>> return off;
>> @@ -188,6 +193,54 @@ int cpuidle_enter_s2idle(struct cpuidle_driver *drv, struct cpuidle_device *dev)
>> }
>> #endif /* CONFIG_SUSPEND */
>>
>> +enum hrtimer_restart auto_promotion_hrtimer_callback(struct hrtimer *hrtimer)
>> +{
>> + return HRTIMER_NORESTART;
>> +}
>> +
>> +#ifdef CONFIG_CPU_IDLE_AUTO_PROMOTION
> As far as I can tell, this config flag isn't defined until the next
> patch, making this dead code for now. Is this intentional?
>
>> +DEFINE_PER_CPU(struct auto_promotion, ap);
> A quick grep suggests that most per-cpu variable have more descriptive
> names, perhaps this one should too.
>
>> +
>> +static void cpuidle_auto_promotion_start(int cpu, struct cpuidle_state *state)
>> +{
>> + struct auto_promotion *this_ap = &per_cpu(ap, cpu);
>> +
>> + if (state->flags & CPUIDLE_FLAG_AUTO_PROMOTION)
>> + hrtimer_start(&this_ap->hrtimer, ns_to_ktime(this_ap->timeout_us
>> + * 1000), HRTIMER_MODE_REL_PINNED);
> Would it be clearer to have both sides of the multiplication on the same
> line? i.e.
> + hrtimer_start(&this_ap->hrtimer,
> + ns_to_ktime(this_ap->timeout_us * 1000),
> + HRTIMER_MODE_REL_PINNED);
>
>> +}
>> +
>> +static void cpuidle_auto_promotion_cancel(int cpu)
>> +{
>> + struct hrtimer *hrtimer;
>> +
>> + hrtimer = &per_cpu(ap, cpu).hrtimer;
>> + if (hrtimer_is_queued(hrtimer))
>> + hrtimer_cancel(hrtimer);
>> +}
>> +
>> +static void cpuidle_auto_promotion_update(int cpu, unsigned long timeout)
>> +{
>> + per_cpu(ap, cpu).timeout_us = timeout;
>> +}
>> +
>> +static void cpuidle_auto_promotion_init(int cpu, struct cpuidle_driver *drv)
>> +{
>> + struct auto_promotion *this_ap = &per_cpu(ap, cpu);
>> +
>> + hrtimer_init(&this_ap->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>> + this_ap->hrtimer.function = auto_promotion_hrtimer_callback;
>> +}
>> +#else
>> +static inline void cpuidle_auto_promotion_start(int cpu, struct cpuidle_state
>> + *state) { }
>> +static inline void cpuidle_auto_promotion_cancel(int cpu) { }
>> +static inline void cpuidle_auto_promotion_update(int cpu, unsigned long
>> + timeout) { }
>> +static inline void cpuidle_auto_promotion_init(int cpu, struct cpuidle_driver
>> + *drv) { }
> Several of these have the type, then a line break, and then the name
> (unsigned long\n timeout). This is a bit harder to read, they should
> probably all be on the same line.
>
>> +#endif
>> +
>> /**
>> * cpuidle_enter_state - enter the state and update stats
>> * @dev: cpuidle device for this cpu
>> @@ -225,12 +278,17 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv,
>> trace_cpu_idle_rcuidle(index, dev->cpu);
>> time_start = ns_to_ktime(local_clock());
>>
>> + cpuidle_auto_promotion_start(dev->cpu, target_state);
>> +
>> stop_critical_timings();
>> entered_state = target_state->enter(dev, drv, index);
>> start_critical_timings();
>>
>> sched_clock_idle_wakeup_event();
>> time_end = ns_to_ktime(local_clock());
>> +
>> + cpuidle_auto_promotion_cancel(dev->cpu);
>> +
>> trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu);
>>
>> /* The cpu is no longer idle or about to enter idle. */
>> @@ -312,7 +370,13 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv,
>> int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
>> bool *stop_tick)
>> {
>> - return cpuidle_curr_governor->select(drv, dev, stop_tick);
>> + unsigned long timeout_us, ret;
>> +
>> + timeout_us = UINT_MAX;
>> + ret = cpuidle_curr_governor->select(drv, dev, stop_tick, &timeout_us);
>> + cpuidle_auto_promotion_update(dev->cpu, timeout_us);
>> +
>> + return ret;
>> }
>>
>> /**
>> @@ -658,6 +722,8 @@ int cpuidle_register(struct cpuidle_driver *drv,
>> device = &per_cpu(cpuidle_dev, cpu);
>> device->cpu = cpu;
>>
>> + cpuidle_auto_promotion_init(cpu, drv);
>> +
>> #ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
>> /*
>> * On multiplatform for ARM, the coupled idle states could be
>> diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
>> index f0dddc66a..65b518dd7 100644
>> --- a/drivers/cpuidle/governors/ladder.c
>> +++ b/drivers/cpuidle/governors/ladder.c
>> @@ -64,7 +64,8 @@ static inline void ladder_do_selection(struct ladder_device *ldev,
>> * @dummy: not used
> I think you need an addition to the docstring for your new variable.
>
>> */
>> static int ladder_select_state(struct cpuidle_driver *drv,
>> - struct cpuidle_device *dev, bool *dummy)
>> + struct cpuidle_device *dev, bool *dummy,
>> + unsigned long *unused)
>> {
>> struct ladder_device *ldev = this_cpu_ptr(&ladder_devices);
>> struct ladder_device_state *last_state;
>> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
>> index 5951604e7..835e337de 100644
>> --- a/drivers/cpuidle/governors/menu.c
>> +++ b/drivers/cpuidle/governors/menu.c
>> @@ -276,7 +276,7 @@ static unsigned int get_typical_interval(struct menu_device *data,
>> * @stop_tick: indication on whether or not to stop the tick
> Likewise here.
>
>> */
>> static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
>> - bool *stop_tick)
>> + bool *stop_tick, unsigned long *timeout)
>> {
>> struct menu_device *data = this_cpu_ptr(&menu_devices);
>> int latency_req = cpuidle_governor_latency_req(dev->cpu);
>> @@ -442,6 +442,26 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
>> }
>> }
>>
>> +#ifdef CPUIDLE_FLAG_AUTO_PROMOTION
>> + if (drv->states[idx].flags & CPUIDLE_FLAG_AUTO_PROMOTION) {
>> + /*
>> + * Timeout is intended to be defined as sum of target residency
>> + * of next available state, entry latency and exit latency. If
>> + * time interval equal to timeout is spent in current state,
>> + * and if it is a shallow lite state, we may want to auto-
>> + * promote from such state.
> This comment makes sense if you already understand auto-promotion. That's
> fair enough - you wrote it and you presumably understand what your code
> does :) But for me it's a bit confusing! I think you want to start with
> a sentence about what autopromotion is (preferably not using
> power-specific terminology) and then explain the calculation of the
> timeouts.
>
>> + */
>> + for (i = idx + 1; i < drv->state_count; i++) {
>> + if (drv->states[i].disabled ||
>> + dev->states_usage[i].disable)
>> + continue;
>> + *timeout = drv->states[i].target_residency +
>> + 2 * drv->states[i].exit_latency;
>> + break;
>> + }
>> + }
>> +#endif
>> +
>> return idx;
>> }
>>
>> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
>> index 3b3947232..84d76d1ec 100644
>> --- a/include/linux/cpuidle.h
>> +++ b/include/linux/cpuidle.h
>> @@ -72,6 +72,13 @@ struct cpuidle_state {
>> #define CPUIDLE_FLAG_POLLING BIT(0) /* polling state */
>> #define CPUIDLE_FLAG_COUPLED BIT(1) /* state applies to multiple cpus */
>> #define CPUIDLE_FLAG_TIMER_STOP BIT(2) /* timer is stopped on this state */
>> +/*
>> + * State with only and only fast state bit set don't even lose user context.
> "only and only"?
>> + * But such states prevent other sibling threads from thread folding benefits.
>> + * And hence we don't want to stay for too long in such states and want to
>> + * auto-promote from it.
> I think this comment mixes Power-specific and generic concepts. (But I'm
> not a PM expert so tell me if I'm wrong here.) I think, if I've
> understood correctly: in the generic code, the bit represents a state
> that we do not want to linger in, which we want to definitely leave
> after some time. On Power, we have a state that doesn't lose user
> context but which prevents thread folding, so this is an example of a
> state where we want to auto-promote.
>
>> + */
>> +#define CPUIDLE_FLAG_AUTO_PROMOTION BIT(3)
>>
>> struct cpuidle_device_kobj;
>> struct cpuidle_state_kobj;
>> @@ -243,7 +250,8 @@ struct cpuidle_governor {
>>
>> int (*select) (struct cpuidle_driver *drv,
>> struct cpuidle_device *dev,
>> - bool *stop_tick);
>> + bool *stop_tick, unsigned long
>> + *timeout);
>> void (*reflect) (struct cpuidle_device *dev, int index);
>> };
>>
>> --
>> 2.17.1
^ permalink raw reply
* Re: [PATCH v4 10/17] KVM: PPC: Book3S HV: XIVE: add get/set accessors for the VP XIVE state
From: Cédric Le Goater @ 2019-04-09 9:18 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm, kvm-ppc, David Gibson
In-Reply-To: <20190409061916.GA3429@blackberry>
On 4/9/19 8:19 AM, Paul Mackerras wrote:
> On Wed, Mar 20, 2019 at 09:37:44AM +0100, Cédric Le Goater wrote:
>> The state of the thread interrupt management registers needs to be
>> collected for migration. These registers are cached under the
>> 'xive_saved_state.w01' field of the VCPU when the VPCU context is
>> pulled from the HW thread. An OPAL call retrieves the backup of the
>> IPB register in the underlying XIVE NVT structure and merges it in the
>> KVM state.
>
> Since you're adding a new one_reg identifier value, you need to update
> the list in Documentation/virtual/kvm/api.txt.
yes. I missed that file.
I will keep a description of the VP state register layout in the device
documentation file :
Documentation/virtual/kvm/devices/xive.txt
I don't think it belongs to kvm/api.txt file.
Thanks,
C.
^ permalink raw reply
* Re: [Qemu-ppc] pseries on qemu-system-ppc64le crashes in doorbell_core_ipi()
From: Nicholas Piggin @ 2019-04-09 9:25 UTC (permalink / raw)
To: Frederic Weisbecker, Peter Zijlstra
Cc: Sebastian Andrzej Siewior, Steven Rostedt, Paul Mackerras,
Cédric Le Goater, Suraj Jitindar Singh, Frederic Weisbecker,
qemu-ppc, tglx, linuxppc-dev, David? Gibson
In-Reply-To: <20190406000611.GA27782@lenoir>
Frederic Weisbecker's on April 6, 2019 10:06 am:
> On Mon, Apr 01, 2019 at 10:38:27AM +0200, Peter Zijlstra wrote:
>>
>> + fweisbec, who did the remote bits
>>
>> On Sat, Mar 30, 2019 at 01:10:28PM +1000, Nicholas Piggin wrote:
>> > diff --git a/kernel/irq_work.c b/kernel/irq_work.c
>> > index 6b7cdf17ccf8..f0e539d0f879 100644
>> > --- a/kernel/irq_work.c
>> > +++ b/kernel/irq_work.c
>> > -/* Enqueue the irq work @work on the current CPU */
>> > -bool irq_work_queue(struct irq_work *work)
>> > +/*
>> > + * Enqueue the irq_work @work on @cpu unless it's already pending
>> > + * somewhere.
>> > + *
>> > + * Can be re-enqueued while the callback is still in progress.
>> > + */
>> > +bool irq_work_queue_on(struct irq_work *work, int cpu)
>> > {
>> > +#ifndef CONFIG_SMP
>> > + return irq_work_queue(work);
>> > +
>
> I'd suggest to use "if (!IS_ENABLED(CONFIG_SMP))" here to avoid the large
> ifdeffery.
Sadly you can't do it because arch_send_call_function_single_ipi is
not defined for !SMP. I made your suggested name change though (with
the __ prefix because work needs to be claimed and preempt disabled).
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 0/2] ASoC: fsl: audmix: fix two issues
From: Viorel Suman @ 2019-04-09 8:55 UTC (permalink / raw)
To: timur@kernel.org, Xiubo.Lee@gmail.com, nicoleotsuka@gmail.com,
festevam@gmail.com, broonie@kernel.org, tiwai@suse.com,
lgirdwood@gmail.com, shawnguo@kernel.org, Julia.Lawall@lip6.fr,
perex@perex.cz, s.hauer@pengutronix.de
Cc: alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
viorel.suman@gmail.com, dl-linux-imx, kernel@pengutronix.de,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1554798876-20653-1-git-send-email-viorel.suman@nxp.com>
Please ignore this series, the device bindings documentation, [1],
still contains "model" attribute. Will send V2.
[1] Documentation/devicetree/bindings/sound/fsl,audmix.txt
Regards,
Viorel
On Ma, 2019-04-09 at 08:35 +0000, Viorel Suman wrote:
> The latest audmix patch-set (v5) had the "model" attribute
> removed as requested by Nicolin Chen, but looks like (v4)
> version of DAI driver reached "for-next" branch - fix this.
> Asside of this fix object reference leaks in machine probe reported
> by Julia Lawall.
>
> Viorel Suman (2):
> ASoC: fsl_audmix: remove "model" attribute
> ASoC: imx-audmix: fix object reference leaks in probe
>
> sound/soc/fsl/fsl_audmix.c | 61 ++++++++++++++++++++++++----------
> ------------
> sound/soc/fsl/imx-audmix.c | 31 +++++++++--------------
> 2 files changed, 43 insertions(+), 49 deletions(-)
>
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH 1/2] ASoC: fsl_audmix: remove "model" attribute
From: Viorel Suman @ 2019-04-09 8:35 UTC (permalink / raw)
To: Timur Tabi, Nicolin Chen, Xiubo Li, Fabio Estevam, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, Shawn Guo,
Sascha Hauer, Julia Lawall
Cc: alsa-devel@alsa-project.org, Viorel Suman,
linux-kernel@vger.kernel.org, Viorel Suman, dl-linux-imx,
Pengutronix Kernel Team, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1554798876-20653-1-git-send-email-viorel.suman@nxp.com>
Use "of_device_id.data" to specify the machine driver,
instead of "model" DTS attribute.
Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
---
sound/soc/fsl/fsl_audmix.c | 61 ++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 29 deletions(-)
diff --git a/sound/soc/fsl/fsl_audmix.c b/sound/soc/fsl/fsl_audmix.c
index dabde03..2d10d8b 100644
--- a/sound/soc/fsl/fsl_audmix.c
+++ b/sound/soc/fsl/fsl_audmix.c
@@ -445,61 +445,70 @@ static const struct regmap_config fsl_audmix_regmap_config = {
.cache_type = REGCACHE_FLAT,
};
+static const struct of_device_id fsl_audmix_ids[] = {
+ {
+ .compatible = "fsl,imx8qm-audmix",
+ .data = "imx-audmix",
+ },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, fsl_audmix_ids);
+
static int fsl_audmix_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct fsl_audmix *priv;
struct resource *res;
+ const char *mdrv;
+ const struct of_device_id *of_id;
void __iomem *regs;
int ret;
- const char *sprop;
- priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ of_id = of_match_device(fsl_audmix_ids, dev);
+ if (!of_id || !of_id->data)
+ return -EINVAL;
+
+ mdrv = of_id->data;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
/* Get the addresses */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- regs = devm_ioremap_resource(&pdev->dev, res);
+ regs = devm_ioremap_resource(dev, res);
if (IS_ERR(regs))
return PTR_ERR(regs);
- priv->regmap = devm_regmap_init_mmio_clk(&pdev->dev, "ipg", regs,
+ priv->regmap = devm_regmap_init_mmio_clk(dev, "ipg", regs,
&fsl_audmix_regmap_config);
if (IS_ERR(priv->regmap)) {
- dev_err(&pdev->dev, "failed to init regmap\n");
+ dev_err(dev, "failed to init regmap\n");
return PTR_ERR(priv->regmap);
}
- priv->ipg_clk = devm_clk_get(&pdev->dev, "ipg");
+ priv->ipg_clk = devm_clk_get(dev, "ipg");
if (IS_ERR(priv->ipg_clk)) {
- dev_err(&pdev->dev, "failed to get ipg clock\n");
+ dev_err(dev, "failed to get ipg clock\n");
return PTR_ERR(priv->ipg_clk);
}
platform_set_drvdata(pdev, priv);
- pm_runtime_enable(&pdev->dev);
+ pm_runtime_enable(dev);
- ret = devm_snd_soc_register_component(&pdev->dev, &fsl_audmix_component,
+ ret = devm_snd_soc_register_component(dev, &fsl_audmix_component,
fsl_audmix_dai,
ARRAY_SIZE(fsl_audmix_dai));
if (ret) {
- dev_err(&pdev->dev, "failed to register ASoC DAI\n");
+ dev_err(dev, "failed to register ASoC DAI\n");
return ret;
}
- sprop = of_get_property(pdev->dev.of_node, "model", NULL);
- if (sprop) {
- priv->pdev = platform_device_register_data(&pdev->dev, sprop, 0,
- NULL, 0);
- if (IS_ERR(priv->pdev)) {
- ret = PTR_ERR(priv->pdev);
- dev_err(&pdev->dev,
- "failed to register platform %s: %d\n", sprop,
- ret);
- }
- } else {
- dev_err(&pdev->dev, "[model] attribute missing.\n");
- ret = -EINVAL;
+ priv->pdev = platform_device_register_data(dev, mdrv, 0, NULL, 0);
+ if (IS_ERR(priv->pdev)) {
+ ret = PTR_ERR(priv->pdev);
+ dev_err(dev, "failed to register platform %s: %d\n",
+ mdrv, ret);
}
return ret;
@@ -553,12 +562,6 @@ static const struct dev_pm_ops fsl_audmix_pm = {
pm_runtime_force_resume)
};
-static const struct of_device_id fsl_audmix_ids[] = {
- { .compatible = "fsl,imx8qm-audmix", },
- { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, fsl_audmix_ids);
-
static struct platform_driver fsl_audmix_driver = {
.probe = fsl_audmix_probe,
.remove = fsl_audmix_remove,
--
2.7.4
^ permalink raw reply related
* [PATCH 0/2] ASoC: fsl: audmix: fix two issues
From: Viorel Suman @ 2019-04-09 8:35 UTC (permalink / raw)
To: Timur Tabi, Nicolin Chen, Xiubo Li, Fabio Estevam, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, Shawn Guo,
Sascha Hauer, Julia Lawall
Cc: alsa-devel@alsa-project.org, Viorel Suman,
linux-kernel@vger.kernel.org, Viorel Suman, dl-linux-imx,
Pengutronix Kernel Team, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
The latest audmix patch-set (v5) had the "model" attribute
removed as requested by Nicolin Chen, but looks like (v4)
version of DAI driver reached "for-next" branch - fix this.
Asside of this fix object reference leaks in machine probe reported
by Julia Lawall.
Viorel Suman (2):
ASoC: fsl_audmix: remove "model" attribute
ASoC: imx-audmix: fix object reference leaks in probe
sound/soc/fsl/fsl_audmix.c | 61 ++++++++++++++++++++++++----------------------
sound/soc/fsl/imx-audmix.c | 31 +++++++++--------------
2 files changed, 43 insertions(+), 49 deletions(-)
--
2.7.4
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox