date:20180223

[Nouveau] [PATCH 4.14 148/159] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses

2018-02-23 Thread Greg Kroah-Hartman

4.14-stable review patch.  If anyone has any objections, please let me know.

--

From: Karol Herbst 


[ Upstream commit 6d60ce384d1d5ca32b595244db4077a419acc687 ]

If something calls ioremap() with an address not aligned to PAGE_SIZE, the
returned address might be not aligned as well. This led to a probe
registered on exactly the returned address, but the entire page was armed
for mmiotracing.

On calling iounmap() the address passed to unregister_kmmio_probe() was
PAGE_SIZE aligned by the caller leading to a complete freeze of the
machine.

We should always page align addresses while (un)registerung mappings,
because the mmiotracer works on top of pages, not mappings. We still keep
track of the probes based on their real addresses and lengths though,
because the mmiotrace still needs to know what are mapped memory regions.

Also move the call to mmiotrace_iounmap() prior page aligning the address,
so that all probes are unregistered properly, otherwise the kernel ends up
failing memory allocations randomly after disabling the mmiotracer.

Tested-by: Lyude 
Signed-off-by: Karol Herbst 
Acked-by: Pekka Paalanen 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Steven Rostedt 
Cc: Thomas Gleixner 
Cc: nouveau@lists.freedesktop.org
Link: http://lkml.kernel.org/r/20171127075139.4928-1-kher...@redhat.com
Signed-off-by: Ingo Molnar 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/x86/mm/ioremap.c |4 ++--
 arch/x86/mm/kmmio.c   |   12 +++-
 2 files changed, 9 insertions(+), 7 deletions(-)

--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -349,11 +349,11 @@ void iounmap(volatile void __iomem *addr
return;
}
 
+   mmiotrace_iounmap(addr);
+
addr = (volatile void __iomem *)
(PAGE_MASK & (unsigned long __force)addr);
 
-   mmiotrace_iounmap(addr);
-
/* Use the vm area unlocked, assuming the caller
   ensures there isn't another iounmap for the same address
   in parallel. Reuse of the virtual address is prevented by
--- a/arch/x86/mm/kmmio.c
+++ b/arch/x86/mm/kmmio.c
@@ -435,17 +435,18 @@ int register_kmmio_probe(struct kmmio_pr
unsigned long flags;
int ret = 0;
unsigned long size = 0;
+   unsigned long addr = p->addr & PAGE_MASK;
const unsigned long size_lim = p->len + (p->addr & ~PAGE_MASK);
unsigned int l;
pte_t *pte;
 
spin_lock_irqsave(_lock, flags);
-   if (get_kmmio_probe(p->addr)) {
+   if (get_kmmio_probe(addr)) {
ret = -EEXIST;
goto out;
}
 
-   pte = lookup_address(p->addr, );
+   pte = lookup_address(addr, );
if (!pte) {
ret = -EINVAL;
goto out;
@@ -454,7 +455,7 @@ int register_kmmio_probe(struct kmmio_pr
kmmio_count++;
list_add_rcu(>list, _probes);
while (size < size_lim) {
-   if (add_kmmio_fault_page(p->addr + size))
+   if (add_kmmio_fault_page(addr + size))
pr_err("Unable to set page fault.\n");
size += page_level_size(l);
}
@@ -528,19 +529,20 @@ void unregister_kmmio_probe(struct kmmio
 {
unsigned long flags;
unsigned long size = 0;
+   unsigned long addr = p->addr & PAGE_MASK;
const unsigned long size_lim = p->len + (p->addr & ~PAGE_MASK);
struct kmmio_fault_page *release_list = NULL;
struct kmmio_delayed_release *drelease;
unsigned int l;
pte_t *pte;
 
-   pte = lookup_address(p->addr, );
+   pte = lookup_address(addr, );
if (!pte)
return;
 
spin_lock_irqsave(_lock, flags);
while (size < size_lim) {
-   release_kmmio_fault_page(p->addr + size, _list);
+   release_kmmio_fault_page(addr + size, _list);
size += page_level_size(l);
}
list_del_rcu(>list);


___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [PATCH 4.9 083/145] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses

2018-02-23 Thread Greg Kroah-Hartman

4.9-stable review patch.  If anyone has any objections, please let me know.

--

From: Karol Herbst 


[ Upstream commit 6d60ce384d1d5ca32b595244db4077a419acc687 ]

If something calls ioremap() with an address not aligned to PAGE_SIZE, the
returned address might be not aligned as well. This led to a probe
registered on exactly the returned address, but the entire page was armed
for mmiotracing.

On calling iounmap() the address passed to unregister_kmmio_probe() was
PAGE_SIZE aligned by the caller leading to a complete freeze of the
machine.

We should always page align addresses while (un)registerung mappings,
because the mmiotracer works on top of pages, not mappings. We still keep
track of the probes based on their real addresses and lengths though,
because the mmiotrace still needs to know what are mapped memory regions.

Also move the call to mmiotrace_iounmap() prior page aligning the address,
so that all probes are unregistered properly, otherwise the kernel ends up
failing memory allocations randomly after disabling the mmiotracer.

Tested-by: Lyude 
Signed-off-by: Karol Herbst 
Acked-by: Pekka Paalanen 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Steven Rostedt 
Cc: Thomas Gleixner 
Cc: nouveau@lists.freedesktop.org
Link: http://lkml.kernel.org/r/20171127075139.4928-1-kher...@redhat.com
Signed-off-by: Ingo Molnar 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/x86/mm/ioremap.c |4 ++--
 arch/x86/mm/kmmio.c   |   12 +++-
 2 files changed, 9 insertions(+), 7 deletions(-)

--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -347,11 +347,11 @@ void iounmap(volatile void __iomem *addr
(void __force *)addr < phys_to_virt(ISA_END_ADDRESS))
return;
 
+   mmiotrace_iounmap(addr);
+
addr = (volatile void __iomem *)
(PAGE_MASK & (unsigned long __force)addr);
 
-   mmiotrace_iounmap(addr);
-
/* Use the vm area unlocked, assuming the caller
   ensures there isn't another iounmap for the same address
   in parallel. Reuse of the virtual address is prevented by
--- a/arch/x86/mm/kmmio.c
+++ b/arch/x86/mm/kmmio.c
@@ -434,17 +434,18 @@ int register_kmmio_probe(struct kmmio_pr
unsigned long flags;
int ret = 0;
unsigned long size = 0;
+   unsigned long addr = p->addr & PAGE_MASK;
const unsigned long size_lim = p->len + (p->addr & ~PAGE_MASK);
unsigned int l;
pte_t *pte;
 
spin_lock_irqsave(_lock, flags);
-   if (get_kmmio_probe(p->addr)) {
+   if (get_kmmio_probe(addr)) {
ret = -EEXIST;
goto out;
}
 
-   pte = lookup_address(p->addr, );
+   pte = lookup_address(addr, );
if (!pte) {
ret = -EINVAL;
goto out;
@@ -453,7 +454,7 @@ int register_kmmio_probe(struct kmmio_pr
kmmio_count++;
list_add_rcu(>list, _probes);
while (size < size_lim) {
-   if (add_kmmio_fault_page(p->addr + size))
+   if (add_kmmio_fault_page(addr + size))
pr_err("Unable to set page fault.\n");
size += page_level_size(l);
}
@@ -527,19 +528,20 @@ void unregister_kmmio_probe(struct kmmio
 {
unsigned long flags;
unsigned long size = 0;
+   unsigned long addr = p->addr & PAGE_MASK;
const unsigned long size_lim = p->len + (p->addr & ~PAGE_MASK);
struct kmmio_fault_page *release_list = NULL;
struct kmmio_delayed_release *drelease;
unsigned int l;
pte_t *pte;
 
-   pte = lookup_address(p->addr, );
+   pte = lookup_address(addr, );
if (!pte)
return;
 
spin_lock_irqsave(_lock, flags);
while (size < size_lim) {
-   release_kmmio_fault_page(p->addr + size, _list);
+   release_kmmio_fault_page(addr + size, _list);
size += page_level_size(l);
}
list_del_rcu(>list);


___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [PATCH 4.4 059/193] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses

2018-02-23 Thread Greg Kroah-Hartman

4.4-stable review patch.  If anyone has any objections, please let me know.

--

From: Karol Herbst 


[ Upstream commit 6d60ce384d1d5ca32b595244db4077a419acc687 ]

If something calls ioremap() with an address not aligned to PAGE_SIZE, the
returned address might be not aligned as well. This led to a probe
registered on exactly the returned address, but the entire page was armed
for mmiotracing.

On calling iounmap() the address passed to unregister_kmmio_probe() was
PAGE_SIZE aligned by the caller leading to a complete freeze of the
machine.

We should always page align addresses while (un)registerung mappings,
because the mmiotracer works on top of pages, not mappings. We still keep
track of the probes based on their real addresses and lengths though,
because the mmiotrace still needs to know what are mapped memory regions.

Also move the call to mmiotrace_iounmap() prior page aligning the address,
so that all probes are unregistered properly, otherwise the kernel ends up
failing memory allocations randomly after disabling the mmiotracer.

Tested-by: Lyude 
Signed-off-by: Karol Herbst 
Acked-by: Pekka Paalanen 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Steven Rostedt 
Cc: Thomas Gleixner 
Cc: nouveau@lists.freedesktop.org
Link: http://lkml.kernel.org/r/20171127075139.4928-1-kher...@redhat.com
Signed-off-by: Ingo Molnar 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/x86/mm/ioremap.c |4 ++--
 arch/x86/mm/kmmio.c   |   12 +++-
 2 files changed, 9 insertions(+), 7 deletions(-)

--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -348,11 +348,11 @@ void iounmap(volatile void __iomem *addr
(void __force *)addr < phys_to_virt(ISA_END_ADDRESS))
return;
 
+   mmiotrace_iounmap(addr);
+
addr = (volatile void __iomem *)
(PAGE_MASK & (unsigned long __force)addr);
 
-   mmiotrace_iounmap(addr);
-
/* Use the vm area unlocked, assuming the caller
   ensures there isn't another iounmap for the same address
   in parallel. Reuse of the virtual address is prevented by
--- a/arch/x86/mm/kmmio.c
+++ b/arch/x86/mm/kmmio.c
@@ -434,17 +434,18 @@ int register_kmmio_probe(struct kmmio_pr
unsigned long flags;
int ret = 0;
unsigned long size = 0;
+   unsigned long addr = p->addr & PAGE_MASK;
const unsigned long size_lim = p->len + (p->addr & ~PAGE_MASK);
unsigned int l;
pte_t *pte;
 
spin_lock_irqsave(_lock, flags);
-   if (get_kmmio_probe(p->addr)) {
+   if (get_kmmio_probe(addr)) {
ret = -EEXIST;
goto out;
}
 
-   pte = lookup_address(p->addr, );
+   pte = lookup_address(addr, );
if (!pte) {
ret = -EINVAL;
goto out;
@@ -453,7 +454,7 @@ int register_kmmio_probe(struct kmmio_pr
kmmio_count++;
list_add_rcu(>list, _probes);
while (size < size_lim) {
-   if (add_kmmio_fault_page(p->addr + size))
+   if (add_kmmio_fault_page(addr + size))
pr_err("Unable to set page fault.\n");
size += page_level_size(l);
}
@@ -527,19 +528,20 @@ void unregister_kmmio_probe(struct kmmio
 {
unsigned long flags;
unsigned long size = 0;
+   unsigned long addr = p->addr & PAGE_MASK;
const unsigned long size_lim = p->len + (p->addr & ~PAGE_MASK);
struct kmmio_fault_page *release_list = NULL;
struct kmmio_delayed_release *drelease;
unsigned int l;
pte_t *pte;
 
-   pte = lookup_address(p->addr, );
+   pte = lookup_address(addr, );
if (!pte)
return;
 
spin_lock_irqsave(_lock, flags);
while (size < size_lim) {
-   release_kmmio_fault_page(p->addr + size, _list);
+   release_kmmio_fault_page(addr + size, _list);
size += page_level_size(l);
}
list_del_rcu(>list);


___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [PATCH 3.18 54/58] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses

2018-02-23 Thread Greg Kroah-Hartman

3.18-stable review patch.  If anyone has any objections, please let me know.

--

From: Karol Herbst 


[ Upstream commit 6d60ce384d1d5ca32b595244db4077a419acc687 ]

If something calls ioremap() with an address not aligned to PAGE_SIZE, the
returned address might be not aligned as well. This led to a probe
registered on exactly the returned address, but the entire page was armed
for mmiotracing.

On calling iounmap() the address passed to unregister_kmmio_probe() was
PAGE_SIZE aligned by the caller leading to a complete freeze of the
machine.

We should always page align addresses while (un)registerung mappings,
because the mmiotracer works on top of pages, not mappings. We still keep
track of the probes based on their real addresses and lengths though,
because the mmiotrace still needs to know what are mapped memory regions.

Also move the call to mmiotrace_iounmap() prior page aligning the address,
so that all probes are unregistered properly, otherwise the kernel ends up
failing memory allocations randomly after disabling the mmiotracer.

Tested-by: Lyude 
Signed-off-by: Karol Herbst 
Acked-by: Pekka Paalanen 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Steven Rostedt 
Cc: Thomas Gleixner 
Cc: nouveau@lists.freedesktop.org
Link: http://lkml.kernel.org/r/20171127075139.4928-1-kher...@redhat.com
Signed-off-by: Ingo Molnar 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/x86/mm/ioremap.c |4 ++--
 arch/x86/mm/kmmio.c   |   12 +++-
 2 files changed, 9 insertions(+), 7 deletions(-)

--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -296,11 +296,11 @@ void iounmap(volatile void __iomem *addr
(void __force *)addr < phys_to_virt(ISA_END_ADDRESS))
return;
 
+   mmiotrace_iounmap(addr);
+
addr = (volatile void __iomem *)
(PAGE_MASK & (unsigned long __force)addr);
 
-   mmiotrace_iounmap(addr);
-
/* Use the vm area unlocked, assuming the caller
   ensures there isn't another iounmap for the same address
   in parallel. Reuse of the virtual address is prevented by
--- a/arch/x86/mm/kmmio.c
+++ b/arch/x86/mm/kmmio.c
@@ -434,17 +434,18 @@ int register_kmmio_probe(struct kmmio_pr
unsigned long flags;
int ret = 0;
unsigned long size = 0;
+   unsigned long addr = p->addr & PAGE_MASK;
const unsigned long size_lim = p->len + (p->addr & ~PAGE_MASK);
unsigned int l;
pte_t *pte;
 
spin_lock_irqsave(_lock, flags);
-   if (get_kmmio_probe(p->addr)) {
+   if (get_kmmio_probe(addr)) {
ret = -EEXIST;
goto out;
}
 
-   pte = lookup_address(p->addr, );
+   pte = lookup_address(addr, );
if (!pte) {
ret = -EINVAL;
goto out;
@@ -453,7 +454,7 @@ int register_kmmio_probe(struct kmmio_pr
kmmio_count++;
list_add_rcu(>list, _probes);
while (size < size_lim) {
-   if (add_kmmio_fault_page(p->addr + size))
+   if (add_kmmio_fault_page(addr + size))
pr_err("Unable to set page fault.\n");
size += page_level_size(l);
}
@@ -527,19 +528,20 @@ void unregister_kmmio_probe(struct kmmio
 {
unsigned long flags;
unsigned long size = 0;
+   unsigned long addr = p->addr & PAGE_MASK;
const unsigned long size_lim = p->len + (p->addr & ~PAGE_MASK);
struct kmmio_fault_page *release_list = NULL;
struct kmmio_delayed_release *drelease;
unsigned int l;
pte_t *pte;
 
-   pte = lookup_address(p->addr, );
+   pte = lookup_address(addr, );
if (!pte)
return;
 
spin_lock_irqsave(_lock, flags);
while (size < size_lim) {
-   release_kmmio_fault_page(p->addr + size, _list);
+   release_kmmio_fault_page(addr + size, _list);
size += page_level_size(l);
}
list_del_rcu(>list);


___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH 5/7] vga_switcheroo: Use device link for HDA controller

2018-02-23 Thread Bjorn Helgaas

[+cc George]

On Fri, Feb 23, 2018 at 10:51:47AM +0100, Lukas Wunner wrote:
> On Tue, Feb 20, 2018 at 04:20:59PM -0600, Bjorn Helgaas wrote:
> > On Sun, Feb 18, 2018 at 09:38:32AM +0100, Lukas Wunner wrote:
> > > The device link is added in a PCI quirk rather than in hda_intel.c.
> > > It is therefore legal for the GPU to runtime suspend to D3cold even if
> > > the HDA controller is not bound to a driver or if CONFIG_SND_HDA_INTEL
> > > is not enabled, for accesses to the HDA controller will cause the GPU to
> > > wake up regardless if they're occurring outside of hda_intel.c (think
> > > config space readout via sysfs).
> > 
> > I guess this GPU wakeup happens *if* the path accessing the HDA
> > controller calls pm_runtime_get_sync() or similar, right?
> 
> Right.
> 
> > We do have
> > that in the sysfs config accessors via pci_config_pm_runtime_get(),
> > but not in the sysfs mmap paths.  I think that's a latent PCI core
> > defect independent of this series.
> 
> Yes, there may be a few places where the parent of the device is
> erroneously not runtime resumed when sysfs files are accessed.
> I've never used the sysfs mmap feature, so never witnessed issues
> with it.
> 
> > We also don't have that in PCI core config accessors.  That maybe
> > doesn't matter because the core probably shouldn't be touching devices
> > after enumeration (although there might be holes there for things like
> > ASPM where a sysfs file can cause reconfiguration of several devices).
> 
> I assume with PCI core config accessors you're referring to
> pci_read_config_word() and friends?  Those are arguably hotpaths
> where we wouldn't want the overhead to check runtime PM status
> everytime.  They might also be called from atomic context, I'm
> not sure, and the runtime PM callbacks may sleep by default
> (unless pm_runtime_irq_safe() was called).

Yes, I was thinking of pci_read_config_word(), etc.  They're used by
the core during enumeration and occasionally by drivers, and both
cases already pay attention to PM.  I think we have a few holes in the
runtime sysfs area, but I think it's reasonable to deal with those in
the callers.

So I don't think those need to actually *do* anything with PM status,
although it might be interesting to add some (optional) checking
there.  George Cherian is turning up some issues in this area because
he has a root port that reports errors with an exception instead of
silently synthesizing all ones data like most hardware does [1].

Bjorn

[1] 
https://lkml.kernel.org/r/1517554846-16703-1-git-send-email-george.cher...@cavium.com
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH 5/7] vga_switcheroo: Use device link for HDA controller

2018-02-23 Thread Lukas Wunner

On Tue, Feb 20, 2018 at 04:20:59PM -0600, Bjorn Helgaas wrote:
> On Sun, Feb 18, 2018 at 09:38:32AM +0100, Lukas Wunner wrote:
> > The device link is added in a PCI quirk rather than in hda_intel.c.
> > It is therefore legal for the GPU to runtime suspend to D3cold even if
> > the HDA controller is not bound to a driver or if CONFIG_SND_HDA_INTEL
> > is not enabled, for accesses to the HDA controller will cause the GPU to
> > wake up regardless if they're occurring outside of hda_intel.c (think
> > config space readout via sysfs).
> 
> I guess this GPU wakeup happens *if* the path accessing the HDA
> controller calls pm_runtime_get_sync() or similar, right?

Right.

> We do have
> that in the sysfs config accessors via pci_config_pm_runtime_get(),
> but not in the sysfs mmap paths.  I think that's a latent PCI core
> defect independent of this series.

Yes, there may be a few places where the parent of the device is
erroneously not runtime resumed when sysfs files are accessed.
I've never used the sysfs mmap feature, so never witnessed issues
with it.

> We also don't have that in PCI core config accessors.  That maybe
> doesn't matter because the core probably shouldn't be touching devices
> after enumeration (although there might be holes there for things like
> ASPM where a sysfs file can cause reconfiguration of several devices).

I assume with PCI core config accessors you're referring to
pci_read_config_word() and friends?  Those are arguably hotpaths
where we wouldn't want the overhead to check runtime PM status
everytime.  They might also be called from atomic context, I'm
not sure, and the runtime PM callbacks may sleep by default
(unless pm_runtime_irq_safe() was called).

> > +static void quirk_gpu_hda(struct pci_dev *hda)
> > +{
> > +   struct pci_dev *gpu = NULL;
> > +
> > +   if (PCI_FUNC(hda->devfn) != 1)
> > +   return;
> > +
> > +   gpu = pci_get_domain_bus_and_slot(pci_domain_nr(hda->bus),
> 
> Unnecessary initialization.

Thanks for spotting this, it's a remnant of an earlier version of the
patch which called pci_dev_put(gpu) in the (PCI_FUNC(hda->devfn) != 1)
case.

Best regards,

Lukas
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [PATCH 4.14 148/159] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses

[Nouveau] [PATCH 4.9 083/145] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses

[Nouveau] [PATCH 4.4 059/193] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses

[Nouveau] [PATCH 3.18 54/58] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses

Re: [Nouveau] [PATCH 5/7] vga_switcheroo: Use device link for HDA controller

Re: [Nouveau] [PATCH 5/7] vga_switcheroo: Use device link for HDA controller

6 matches

Site Navigation

Mail list logo

Footer information