Re: [PATCH RFC 10/17] acpi: Do not use dev->iommu within acpi_iommu_configure()

[Rafael J. Wysocki]

On Fri, Nov 3, 2023 at 5:45 PM Jason Gunthorpe  wrote:
>
> This call chain is using dev->iommu->fwspec to pass around the fwspec
> between the three parts (acpi_iommu_configure(), acpi_iommu_fwspec_init(),
> iommu_probe_device()).
>
> However there is no locking around the accesses to dev->iommu, so this is
> all racy.
>
> Allocate a clean, local, fwspec at the start of acpu_iommu_configure(),
> pass it through all functions on the stack to fill it with data, and
> finally pass it into iommu_probe_device_fwspec() which will load it into
> dev->iommu under a lock.
>
> Signed-off-by: Jason Gunthorpe 

Acked-by: Rafael J. Wysocki 

> ---
>  drivers/acpi/arm64/iort.c | 39 -
>  drivers/acpi/scan.c   | 89 ++-
>  drivers/acpi/viot.c   | 44 ++-
>  drivers/iommu/iommu.c |  5 +--
>  include/acpi/acpi_bus.h   |  8 ++--
>  include/linux/acpi_iort.h |  3 +-
>  include/linux/acpi_viot.h |  5 ++-
>  include/linux/iommu.h |  2 +
>  8 files changed, 97 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 6496ff5a6ba20d..accd01dcfe93f5 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -1218,10 +1218,9 @@ static bool iort_pci_rc_supports_ats(struct 
> acpi_iort_node *node)
> return pci_rc->ats_attribute & ACPI_IORT_ATS_SUPPORTED;
>  }
>
> -static int iort_iommu_xlate(struct device *dev, struct acpi_iort_node *node,
> -   u32 streamid)
> +static int iort_iommu_xlate(struct iommu_fwspec *fwspec, struct device *dev,
> +   struct acpi_iort_node *node, u32 streamid)
>  {
> -   const struct iommu_ops *ops;
> struct fwnode_handle *iort_fwnode;
>
> if (!node)
> @@ -1239,17 +1238,14 @@ static int iort_iommu_xlate(struct device *dev, 
> struct acpi_iort_node *node,
>  * in the kernel or not, defer the IOMMU configuration
>  * or just abort it.
>  */
> -   ops = iommu_ops_from_fwnode(iort_fwnode);
> -   if (!ops)
> -   return iort_iommu_driver_enabled(node->type) ?
> -  -EPROBE_DEFER : -ENODEV;
> -
> -   return acpi_iommu_fwspec_init(dev, streamid, iort_fwnode, ops);
> +   return acpi_iommu_fwspec_init(fwspec, dev, streamid, iort_fwnode,
> + iort_iommu_driver_enabled(node->type));
>  }
>
>  struct iort_pci_alias_info {
> struct device *dev;
> struct acpi_iort_node *node;
> +   struct iommu_fwspec *fwspec;
>  };
>
>  static int iort_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
> @@ -1260,7 +1256,7 @@ static int iort_pci_iommu_init(struct pci_dev *pdev, 
> u16 alias, void *data)
>
> parent = iort_node_map_id(info->node, alias, ,
>   IORT_IOMMU_TYPE);
> -   return iort_iommu_xlate(info->dev, parent, streamid);
> +   return iort_iommu_xlate(info->fwspec, info->dev, parent, streamid);
>  }
>
>  static void iort_named_component_init(struct device *dev,
> @@ -1280,7 +1276,8 @@ static void iort_named_component_init(struct device 
> *dev,
> dev_warn(dev, "Could not add device properties\n");
>  }
>
> -static int iort_nc_iommu_map(struct device *dev, struct acpi_iort_node *node)
> +static int iort_nc_iommu_map(struct iommu_fwspec *fwspec, struct device *dev,
> +struct acpi_iort_node *node)
>  {
> struct acpi_iort_node *parent;
> int err = -ENODEV, i = 0;
> @@ -1293,13 +1290,13 @@ static int iort_nc_iommu_map(struct device *dev, 
> struct acpi_iort_node *node)
>i++);
>
> if (parent)
> -   err = iort_iommu_xlate(dev, parent, streamid);
> +   err = iort_iommu_xlate(fwspec, dev, parent, streamid);
> } while (parent && !err);
>
> return err;
>  }
>
> -static int iort_nc_iommu_map_id(struct device *dev,
> +static int iort_nc_iommu_map_id(struct iommu_fwspec *fwspec, struct device 
> *dev,
> struct acpi_iort_node *node,
> const u32 *in_id)
>  {
> @@ -1308,7 +1305,7 @@ static int iort_nc_iommu_map_id(struct device *dev,
>
> parent = iort_node_map_id(node, *in_id, , IORT_IOMMU_TYPE);
> if (parent)
> -   return iort_iommu_xlate(dev, parent, streamid);
> +   return iort_iommu_xlate(fwspec, dev, parent, streamid);
>
> return -ENODEV;
>  }
> @@ 

Re: [PATCH RFC 04/17] acpi: Do not return struct iommu_ops from acpi_iommu_configure_id()

[Rafael J. Wysocki]

On Fri, Nov 3, 2023 at 5:45 PM Jason Gunthorpe  wrote:
>
> Nothing needs this pointer. Return a normal error code with the usual
> IOMMU semantic that ENODEV means 'there is no IOMMU driver'.
>
> Signed-off-by: Jason Gunthorpe 

Acked-by: Rafael J. Wysocki 

> ---
>  drivers/acpi/scan.c | 24 +++-
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index a6891ad0ceee2c..fbabde001a23a2 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1562,8 +1562,7 @@ static inline const struct iommu_ops 
> *acpi_iommu_fwspec_ops(struct device *dev)
> return fwspec ? fwspec->ops : NULL;
>  }
>
> -static const struct iommu_ops *acpi_iommu_configure_id(struct device *dev,
> -  const u32 *id_in)
> +static int acpi_iommu_configure_id(struct device *dev, const u32 *id_in)
>  {
> int err;
> const struct iommu_ops *ops;
> @@ -1574,7 +1573,7 @@ static const struct iommu_ops 
> *acpi_iommu_configure_id(struct device *dev,
>  */
> ops = acpi_iommu_fwspec_ops(dev);
> if (ops)
> -   return ops;
> +   return 0;
>
> err = iort_iommu_configure_id(dev, id_in);
> if (err && err != -EPROBE_DEFER)
> @@ -1589,12 +1588,14 @@ static const struct iommu_ops 
> *acpi_iommu_configure_id(struct device *dev,
>
> /* Ignore all other errors apart from EPROBE_DEFER */
> if (err == -EPROBE_DEFER) {
> -   return ERR_PTR(err);
> +   return err;
> } else if (err) {
> dev_dbg(dev, "Adding to IOMMU failed: %d\n", err);
> -   return NULL;
> +   return -ENODEV;
> }
> -   return acpi_iommu_fwspec_ops(dev);
> +   if (!acpi_iommu_fwspec_ops(dev))
> +   return -ENODEV;
> +   return 0;
>  }
>
>  #else /* !CONFIG_IOMMU_API */
> @@ -1623,7 +1624,7 @@ static const struct iommu_ops 
> *acpi_iommu_configure_id(struct device *dev,
>  int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr,
>   const u32 *input_id)
>  {
> -   const struct iommu_ops *iommu;
> +   int ret;
>
> if (attr == DEV_DMA_NOT_SUPPORTED) {
> set_dma_ops(dev, _dummy_ops);
> @@ -1632,10 +1633,15 @@ int acpi_dma_configure_id(struct device *dev, enum 
> dev_dma_attr attr,
>
> acpi_arch_dma_setup(dev);
>
> -   iommu = acpi_iommu_configure_id(dev, input_id);
> -   if (PTR_ERR(iommu) == -EPROBE_DEFER)
> +   ret = acpi_iommu_configure_id(dev, input_id);
> +   if (ret == -EPROBE_DEFER)
> return -EPROBE_DEFER;
>
> +   /*
> +* Historically this routine doesn't fail driver probing due to errors
> +* in acpi_iommu_configure()
> +*/
> +
> arch_setup_dma_ops(dev, 0, U64_MAX, attr == DEV_DMA_COHERENT);
>
> return 0;
> --
> 2.42.0
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v6 2/5] ACPI: bus: Add stub acpi_sleep_state_supported() in non-ACPI cases

[Rafael J. Wysocki]

On Fri, Feb 10, 2023 at 10:34 AM Saurabh Sengar
 wrote:
>
> acpi_sleep_state_supported() is defined only when CONFIG_ACPI=y. The
> function is in acpi_bus.h, and acpi_bus.h can only be used in
> CONFIG_ACPI=y cases. Add the stub function to linux/acpi.h to make
> compilation successful for !CONFIG_ACPI cases.
>
> Signed-off-by: Saurabh Sengar 

Acked-by: Rafael J. Wysocki 

and please feel free to toute this patch whichever way is convenient.

Thanks!

> ---
>  include/linux/acpi.h | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index efff750f326d..d331f76b0c19 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1075,6 +1075,11 @@ static inline u32 acpi_osc_ctx_get_cxl_control(struct 
> acpi_osc_context *context)
> return 0;
>  }
>
> +static inline bool acpi_sleep_state_supported(u8 sleep_state)
> +{
> +   return false;
> +}
> +
>  #endif /* !CONFIG_ACPI */
>
>  #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
> --
> 2.34.1
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 6/6] cpuidle: Fix poll_idle() noinstr annotation

[Rafael J. Wysocki]

On Mon, Jan 23, 2023 at 9:58 PM Peter Zijlstra  wrote:
>
> The instrumentation_begin()/end() annotations in poll_idle() were
> complete nonsense. Specifically they caused tracing to happen in the
> middle of noinstr code, resulting in RCU splats.
>
> Now that local_clock() is noinstr, mark up the rest and let it rip.
>
> Fixes: 00717eb8c955 ("cpuidle: Annotate poll_idle()")
> Signed-off-by: Peter Zijlstra (Intel) 
> Reported-by: kernel test robot 
> Link: 
> https://lore.kernel.org/oe-lkp/202301192148.58ece903-oliver.s...@intel.com

Acked-by: Rafael J. Wysocki 

> ---
>  drivers/cpuidle/cpuidle.c|2 +-
>  drivers/cpuidle/poll_state.c |2 --
>  2 files changed, 1 insertion(+), 3 deletions(-)
>
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -426,7 +426,7 @@ void cpuidle_reflect(struct cpuidle_devi
>   * @dev:   the cpuidle device
>   *
>   */
> -u64 cpuidle_poll_time(struct cpuidle_driver *drv,
> +__cpuidle u64 cpuidle_poll_time(struct cpuidle_driver *drv,
>   struct cpuidle_device *dev)
>  {
> int i;
> --- a/drivers/cpuidle/poll_state.c
> +++ b/drivers/cpuidle/poll_state.c
> @@ -15,7 +15,6 @@ static int __cpuidle poll_idle(struct cp
>  {
> u64 time_start;
>
> -   instrumentation_begin();
> time_start = local_clock();
>
> dev->poll_time_limit = false;
> @@ -42,7 +41,6 @@ static int __cpuidle poll_idle(struct cp
> raw_local_irq_disable();
>
> current_clr_polling();
> -   instrumentation_end();
>
> return index;
>  }
>
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 00/44] cpuidle,rcu: Clean up the mess

[Rafael J. Wysocki]

On Mon, Sep 19, 2022 at 12:17 PM Peter Zijlstra  wrote:
>
> Hi All!
>
> At long last, a respin of the cpuidle vs rcu cleanup patches.
>
> v1: https://lkml.kernel.org/r/20220608142723.103523...@infradead.org
>
> These here patches clean up the mess that is cpuidle vs rcuidle.
>
> At the end of the ride there's only on RCU_NONIDLE user left:
>
>   arch/arm64/kernel/suspend.c:RCU_NONIDLE(__cpu_suspend_exit());
>
> and 'one' trace_*_rcuidle() user:
>
>   kernel/trace/trace_preemptirq.c:
> trace_irq_enable_rcuidle(CALLER_ADDR0, CALLER_ADDR1);
>   kernel/trace/trace_preemptirq.c:
> trace_irq_disable_rcuidle(CALLER_ADDR0, CALLER_ADDR1);
>   kernel/trace/trace_preemptirq.c:
> trace_irq_enable_rcuidle(CALLER_ADDR0, caller_addr);
>   kernel/trace/trace_preemptirq.c:
> trace_irq_disable_rcuidle(CALLER_ADDR0, caller_addr);
>   kernel/trace/trace_preemptirq.c:
> trace_preempt_enable_rcuidle(a0, a1);
>   kernel/trace/trace_preemptirq.c:
> trace_preempt_disable_rcuidle(a0, a1);
>
> However this last is all in deprecated code that should be unused for 
> GENERIC_ENTRY.
>
> I've touched a lot of code that I can't test and I might've broken something 
> by
> accident. In particular the whole ARM cpuidle stuff was quite involved.
>
> Please all; have a look where you haven't already.
>
>
> New since v1:
>
>  - rebase on top of Frederic's rcu-context-tracking rename fest
>  - more omap goodness as per the last discusion (thanks Tony!)
>  - removed one more RCU_NONIDLE() from arm64/risc-v perf code
>  - ubsan/kasan fixes
>  - intel_idle module-param for testing
>  - a bunch of extra __always_inline, because compilers are silly.

Acked-by: Rafael J. Wysocki 

for the whole set and let me know if you want me to merge any of these
through cpuidle.

Thanks!

>
> ---
>  arch/alpha/kernel/process.c   |  1 -
>  arch/alpha/kernel/vmlinux.lds.S   |  1 -
>  arch/arc/kernel/process.c |  3 ++
>  arch/arc/kernel/vmlinux.lds.S |  1 -
>  arch/arm/include/asm/vmlinux.lds.h|  1 -
>  arch/arm/kernel/process.c |  1 -
>  arch/arm/kernel/smp.c |  6 +--
>  arch/arm/mach-gemini/board-dt.c   |  3 +-
>  arch/arm/mach-imx/cpuidle-imx6q.c |  4 +-
>  arch/arm/mach-imx/cpuidle-imx6sx.c|  5 ++-
>  arch/arm/mach-omap2/common.h  |  6 ++-
>  arch/arm/mach-omap2/cpuidle34xx.c | 16 +++-
>  arch/arm/mach-omap2/cpuidle44xx.c | 29 +++---
>  arch/arm/mach-omap2/omap-mpuss-lowpower.c | 12 +-
>  arch/arm/mach-omap2/pm.h  |  2 +-
>  arch/arm/mach-omap2/pm24xx.c  | 51 +---
>  arch/arm/mach-omap2/pm34xx.c  | 14 +--
>  arch/arm/mach-omap2/pm44xx.c  |  2 +-
>  arch/arm/mach-omap2/powerdomain.c | 10 ++---
>  arch/arm64/kernel/idle.c  |  1 -
>  arch/arm64/kernel/smp.c   |  4 +-
>  arch/arm64/kernel/vmlinux.lds.S   |  1 -
>  arch/csky/kernel/process.c|  1 -
>  arch/csky/kernel/smp.c|  2 +-
>  arch/csky/kernel/vmlinux.lds.S|  1 -
>  arch/hexagon/kernel/process.c |  1 -
>  arch/hexagon/kernel/vmlinux.lds.S |  1 -
>  arch/ia64/kernel/process.c|  1 +
>  arch/ia64/kernel/vmlinux.lds.S|  1 -
>  arch/loongarch/kernel/idle.c  |  1 +
>  arch/loongarch/kernel/vmlinux.lds.S   |  1 -
>  arch/m68k/kernel/vmlinux-nommu.lds|  1 -
>  arch/m68k/kernel/vmlinux-std.lds  |  1 -
>  arch/m68k/kernel/vmlinux-sun3.lds |  1 -
>  arch/microblaze/kernel/process.c  |  1 -
>  arch/microblaze/kernel/vmlinux.lds.S  |  1 -
>  arch/mips/kernel/idle.c   |  8 ++--
>  arch/mips/kernel/vmlinux.lds.S|  1 -
>  arch/nios2/kernel/process.c   |  1 -
>  arch/nios2/kernel/vmlinux.lds.S   |  1 -
>  arch/openrisc/kernel/process.c|  1 +
>  arch/openrisc/kernel/vmlinux.lds.S|  1 -
>  arch/parisc/kernel/process.c  |  2 -
>  arch/parisc/kernel/vmlinux.lds.S  |  1 -
>  arch/powerpc/kernel/idle.c|  5 +--
>  arch/powerpc/kernel/vmlinux.lds.S |  1 -
>  arch/riscv/kernel/process.c   |  1 -
>  arch/riscv/kernel/vmlinux-xip.lds.S   |  1 -
>  arch/riscv/kernel/vmlinux.lds.S   |  1 -
>  arch/s390/kernel/idle.c   |  1 -
>  arch/s390/kernel/vmlinux.lds.S|  1 -
>  arch/sh/kernel/idle.c 

Re: [PATCH 04/36] cpuidle,intel_idle: Fix CPUIDLE_FLAG_IRQ_ENABLE

[Rafael J. Wysocki]

On Sat, Jul 30, 2022 at 11:48 AM Michel Lespinasse
 wrote:
>
> On Fri, Jul 29, 2022 at 04:59:50PM +0200, Rafael J. Wysocki wrote:
> > On Fri, Jul 29, 2022 at 12:25 PM Michel Lespinasse
> >  wrote:
> > >
> > > On Thu, Jul 28, 2022 at 10:20:53AM -0700, Paul E. McKenney wrote:
> > > > On Mon, Jul 25, 2022 at 12:43:06PM -0700, Michel Lespinasse wrote:
> > > > > On Wed, Jun 08, 2022 at 04:27:27PM +0200, Peter Zijlstra wrote:
> > > > > > Commit c227233ad64c ("intel_idle: enable interrupts before C1 on
> > > > > > Xeons") wrecked intel_idle in two ways:
> > > > > >
> > > > > >  - must not have tracing in idle functions
> > > > > >  - must return with IRQs disabled
> > > > > >
> > > > > > Additionally, it added a branch for no good reason.
> > > > > >
> > > > > > Fixes: c227233ad64c ("intel_idle: enable interrupts before C1 on 
> > > > > > Xeons")
> > > > > > Signed-off-by: Peter Zijlstra (Intel) 
> > > > >
> > > > > After this change was introduced, I am seeing "WARNING: suspicious RCU
> > > > > usage" when booting a kernel with debug options compiled in. Please
> > > > > see the attached dmesg output. The issue starts with commit 
> > > > > 32d4fd5751ea
> > > > > and is still present in v5.19-rc8.
> > > > >
> > > > > I'm not sure, is this too late to fix or revert in v5.19 final ?
> > > >
> > > > I finally got a chance to take a quick look at this.
> > > >
> > > > The rcu_eqs_exit() function is making a lockdep complaint about
> > > > being invoked with interrupts enabled.  This function is called from
> > > > rcu_idle_exit(), which is an expected code path from 
> > > > cpuidle_enter_state()
> > > > via its call to rcu_idle_exit().  Except that rcu_idle_exit() disables
> > > > interrupts before invoking rcu_eqs_exit().
> > > >
> > > > The only other call to rcu_idle_exit() does not disable interrupts,
> > > > but it is via rcu_user_exit(), which would be a very odd choice for
> > > > cpuidle_enter_state().
> > > >
> > > > It seems unlikely, but it might be that it is the use of 
> > > > local_irq_save()
> > > > instead of raw_local_irq_save() within rcu_idle_exit() that is causing
> > > > the trouble.  If this is the case, then the commit shown below would
> > > > help.  Note that this commit removes the warning from lockdep, so it
> > > > is necessary to build the kernel with CONFIG_RCU_EQS_DEBUG=y to enable
> > > > equivalent debugging.
> > > >
> > > > Could you please try your test with the -rce commit shown below applied?
> > >
> > > Thanks for looking into it.
> > >
> > > After checking out Peter's commit 32d4fd5751ea,
> > > cherry picking your commit ed4ae5eff4b3,
> > > and setting CONFIG_RCU_EQS_DEBUG=y in addition of my usual debug config,
> > > I am now seeing this a few seconds into the boot:
> > >
> > > [3.010650] [ cut here ]
> > > [3.010651] WARNING: CPU: 0 PID: 0 at kernel/sched/clock.c:397 
> > > sched_clock_tick+0x27/0x60
> > > [3.010657] Modules linked in:
> > > [3.010660] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
> > > 5.19.0-rc1-test-5-g1be22fea0611 #1
> > > [3.010662] Hardware name: LENOVO 30BFS44D00/1036, BIOS S03KT51A 
> > > 01/17/2022
> > > [3.010663] RIP: 0010:sched_clock_tick+0x27/0x60
> > > [3.010665] Code: 1f 40 00 53 eb 02 5b c3 66 90 8b 05 2f c3 40 01 85 
> > > c0 74 18 65 8b 05 60 88 8f 4e 85 c0 75 0d 65 8b 05 a9 85 8f 4e 85 c0 74 
> > > 02 <0f> 0b e8 e2 6c 89 00 48 c7 c3 40 d5 02 00
> > >  89 c0 48 03 1c c5 c0 98
> > > [3.010667] RSP: :b2803e28 EFLAGS: 00010002
> > > [3.010670] RAX: 0001 RBX: c8ce7fa07060 RCX: 
> > > 0001
> > > [3.010671] RDX:  RSI: b268dd21 RDI: 
> > > b269ab13
> > > [3.010673] RBP: 0001 R08: ffc300d5 R09: 
> > > 0002be80
> > > [3.010674] R10: 03625b53183a R11: a012b802b7a4 R12: 
> > > b2aa9e80
> > > [3.010675] R13: b2aa9e00 R14: 0001 R15: 
> > > 
> > > 

Re: [PATCH 04/36] cpuidle,intel_idle: Fix CPUIDLE_FLAG_IRQ_ENABLE

[Rafael J. Wysocki]

On Fri, Jul 29, 2022 at 12:25 PM Michel Lespinasse
 wrote:
>
> On Thu, Jul 28, 2022 at 10:20:53AM -0700, Paul E. McKenney wrote:
> > On Mon, Jul 25, 2022 at 12:43:06PM -0700, Michel Lespinasse wrote:
> > > On Wed, Jun 08, 2022 at 04:27:27PM +0200, Peter Zijlstra wrote:
> > > > Commit c227233ad64c ("intel_idle: enable interrupts before C1 on
> > > > Xeons") wrecked intel_idle in two ways:
> > > >
> > > >  - must not have tracing in idle functions
> > > >  - must return with IRQs disabled
> > > >
> > > > Additionally, it added a branch for no good reason.
> > > >
> > > > Fixes: c227233ad64c ("intel_idle: enable interrupts before C1 on Xeons")
> > > > Signed-off-by: Peter Zijlstra (Intel) 
> > >
> > > After this change was introduced, I am seeing "WARNING: suspicious RCU
> > > usage" when booting a kernel with debug options compiled in. Please
> > > see the attached dmesg output. The issue starts with commit 32d4fd5751ea
> > > and is still present in v5.19-rc8.
> > >
> > > I'm not sure, is this too late to fix or revert in v5.19 final ?
> >
> > I finally got a chance to take a quick look at this.
> >
> > The rcu_eqs_exit() function is making a lockdep complaint about
> > being invoked with interrupts enabled.  This function is called from
> > rcu_idle_exit(), which is an expected code path from cpuidle_enter_state()
> > via its call to rcu_idle_exit().  Except that rcu_idle_exit() disables
> > interrupts before invoking rcu_eqs_exit().
> >
> > The only other call to rcu_idle_exit() does not disable interrupts,
> > but it is via rcu_user_exit(), which would be a very odd choice for
> > cpuidle_enter_state().
> >
> > It seems unlikely, but it might be that it is the use of local_irq_save()
> > instead of raw_local_irq_save() within rcu_idle_exit() that is causing
> > the trouble.  If this is the case, then the commit shown below would
> > help.  Note that this commit removes the warning from lockdep, so it
> > is necessary to build the kernel with CONFIG_RCU_EQS_DEBUG=y to enable
> > equivalent debugging.
> >
> > Could you please try your test with the -rce commit shown below applied?
>
> Thanks for looking into it.
>
> After checking out Peter's commit 32d4fd5751ea,
> cherry picking your commit ed4ae5eff4b3,
> and setting CONFIG_RCU_EQS_DEBUG=y in addition of my usual debug config,
> I am now seeing this a few seconds into the boot:
>
> [3.010650] [ cut here ]
> [3.010651] WARNING: CPU: 0 PID: 0 at kernel/sched/clock.c:397 
> sched_clock_tick+0x27/0x60
> [3.010657] Modules linked in:
> [3.010660] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
> 5.19.0-rc1-test-5-g1be22fea0611 #1
> [3.010662] Hardware name: LENOVO 30BFS44D00/1036, BIOS S03KT51A 01/17/2022
> [3.010663] RIP: 0010:sched_clock_tick+0x27/0x60
> [3.010665] Code: 1f 40 00 53 eb 02 5b c3 66 90 8b 05 2f c3 40 01 85 c0 74 
> 18 65 8b 05 60 88 8f 4e 85 c0 75 0d 65 8b 05 a9 85 8f 4e 85 c0 74 02 <0f> 0b 
> e8 e2 6c 89 00 48 c7 c3 40 d5 02 00
>  89 c0 48 03 1c c5 c0 98
> [3.010667] RSP: :b2803e28 EFLAGS: 00010002
> [3.010670] RAX: 0001 RBX: c8ce7fa07060 RCX: 
> 0001
> [3.010671] RDX:  RSI: b268dd21 RDI: 
> b269ab13
> [3.010673] RBP: 0001 R08: ffc300d5 R09: 
> 0002be80
> [3.010674] R10: 03625b53183a R11: a012b802b7a4 R12: 
> b2aa9e80
> [3.010675] R13: b2aa9e00 R14: 0001 R15: 
> 
> [3.010677] FS:  () GS:a012b800() 
> knlGS:
> [3.010678] CS:  0010 DS:  ES:  CR0: 80050033
> [3.010680] CR2: a012f81ff000 CR3: 000c99612001 CR4: 
> 003706f0
> [3.010681] DR0:  DR1:  DR2: 
> 
> [3.010682] DR3:  DR6: fffe0ff0 DR7: 
> 0400
> [3.010683] Call Trace:
> [3.010685]  
> [3.010688]  cpuidle_enter_state+0xb7/0x4b0
> [3.010694]  cpuidle_enter+0x29/0x40
> [3.010697]  do_idle+0x1d4/0x210
> [3.010702]  cpu_startup_entry+0x19/0x20
> [3.010704]  rest_init+0x117/0x1a0
> [3.010708]  arch_call_rest_init+0xa/0x10
> [3.010711]  start_kernel+0x6d8/0x6ff
> [3.010716]  secondary_startup_64_no_verify+0xce/0xdb
> [3.010728]  
> [3.010729] irq event stamp: 44179
> [3.010730] hardirqs last  enabled at (44179): [] 
> asm_sysvec_apic_timer_interrupt+0x1b/0x20
> [3.010734] hardirqs last disabled at (44177): [] 
> __do_softirq+0x3f0/0x498
> [3.010736] softirqs last  enabled at (44178): [] 
> __do_softirq+0x332/0x498
> [3.010738] softirqs last disabled at (44171): [] 
> irq_exit_rcu+0xab/0xf0
> [3.010741] ---[ end trace  ]---

Can you please give this patch a go:
https://patchwork.kernel.org/project/linux-pm/patch/Yt/axpfi88new...@e126311.manchester.arm.com/
?
___

Re: [PATCH 31/36] cpuidle,acpi: Make noinstr clean

[Rafael J. Wysocki]

On Wed, Jun 8, 2022 at 4:47 PM Peter Zijlstra  wrote:
>
> vmlinux.o: warning: objtool: io_idle+0xc: call to __inb.isra.0() leaves 
> .noinstr.text section
> vmlinux.o: warning: objtool: acpi_idle_enter+0xfe: call to num_online_cpus() 
> leaves .noinstr.text section
> vmlinux.o: warning: objtool: acpi_idle_enter+0x115: call to 
> acpi_idle_fallback_to_c1.isra.0() leaves .noinstr.text section
>
> Signed-off-by: Peter Zijlstra (Intel) 

Acked-by: Rafael J. Wysocki 

> ---
>  arch/x86/include/asm/shared/io.h |4 ++--
>  drivers/acpi/processor_idle.c|2 +-
>  include/linux/cpumask.h  |4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)
>
> --- a/arch/x86/include/asm/shared/io.h
> +++ b/arch/x86/include/asm/shared/io.h
> @@ -5,13 +5,13 @@
>  #include 
>
>  #define BUILDIO(bwl, bw, type) \
> -static inline void __out##bwl(type value, u16 port)\
> +static __always_inline void __out##bwl(type value, u16 port)   \
>  {  \
> asm volatile("out" #bwl " %" #bw "0, %w1"   \
>  : : "a"(value), "Nd"(port));   \
>  }  \
> \
> -static inline type __in##bwl(u16 port) \
> +static __always_inline type __in##bwl(u16 port)  
>   \
>  {  \
> type value; \
> asm volatile("in" #bwl " %w1, %" #bw "0"\
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -593,7 +593,7 @@ static int acpi_idle_play_dead(struct cp
> return 0;
>  }
>
> -static bool acpi_idle_fallback_to_c1(struct acpi_processor *pr)
> +static __always_inline bool acpi_idle_fallback_to_c1(struct acpi_processor 
> *pr)
>  {
> return IS_ENABLED(CONFIG_HOTPLUG_CPU) && !pr->flags.has_cst &&
> !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED);
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -908,9 +908,9 @@ static inline const struct cpumask *get_
>   * concurrent CPU hotplug operations unless invoked from a cpuhp_lock held
>   * region.
>   */
> -static inline unsigned int num_online_cpus(void)
> +static __always_inline unsigned int num_online_cpus(void)
>  {
> -   return atomic_read(&__num_online_cpus);
> +   return arch_atomic_read(&__num_online_cpus);
>  }
>  #define num_possible_cpus()cpumask_weight(cpu_possible_mask)
>  #define num_present_cpus() cpumask_weight(cpu_present_mask)
>
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 20/36] arch/idle: Change arch_cpu_idle() IRQ behaviour

[Rafael J. Wysocki]

On Wed, Jun 8, 2022 at 4:46 PM Peter Zijlstra  wrote:
>
> Current arch_cpu_idle() is called with IRQs disabled, but will return
> with IRQs enabled.
>
> However, the very first thing the generic code does after calling
> arch_cpu_idle() is raw_local_irq_disable(). This means that
> architectures that can idle with IRQs disabled end up doing a
> pointless 'enable-disable' dance.
>
> Therefore, push this IRQ disabling into the idle function, meaning
> that those architectures can avoid the pointless IRQ state flipping.
>
> Signed-off-by: Peter Zijlstra (Intel) 

Acked-by: Rafael J. Wysocki 

> ---
>  arch/alpha/kernel/process.c  |1 -
>  arch/arc/kernel/process.c|3 +++
>  arch/arm/kernel/process.c|1 -
>  arch/arm/mach-gemini/board-dt.c  |3 ++-
>  arch/arm64/kernel/idle.c |1 -
>  arch/csky/kernel/process.c   |1 -
>  arch/csky/kernel/smp.c   |2 +-
>  arch/hexagon/kernel/process.c|1 -
>  arch/ia64/kernel/process.c   |1 +
>  arch/microblaze/kernel/process.c |1 -
>  arch/mips/kernel/idle.c  |8 +++-
>  arch/nios2/kernel/process.c  |1 -
>  arch/openrisc/kernel/process.c   |1 +
>  arch/parisc/kernel/process.c |2 --
>  arch/powerpc/kernel/idle.c   |5 ++---
>  arch/riscv/kernel/process.c  |1 -
>  arch/s390/kernel/idle.c  |1 -
>  arch/sh/kernel/idle.c|1 +
>  arch/sparc/kernel/leon_pmc.c |4 
>  arch/sparc/kernel/process_32.c   |1 -
>  arch/sparc/kernel/process_64.c   |3 ++-
>  arch/um/kernel/process.c |1 -
>  arch/x86/coco/tdx/tdx.c  |3 +++
>  arch/x86/kernel/process.c|   15 ---
>  arch/xtensa/kernel/process.c |1 +
>  kernel/sched/idle.c  |2 --
>  26 files changed, 28 insertions(+), 37 deletions(-)
>
> --- a/arch/alpha/kernel/process.c
> +++ b/arch/alpha/kernel/process.c
> @@ -57,7 +57,6 @@ EXPORT_SYMBOL(pm_power_off);
>  void arch_cpu_idle(void)
>  {
> wtint(0);
> -   raw_local_irq_enable();
>  }
>
>  void arch_cpu_idle_dead(void)
> --- a/arch/arc/kernel/process.c
> +++ b/arch/arc/kernel/process.c
> @@ -114,6 +114,8 @@ void arch_cpu_idle(void)
> "sleep %0   \n"
> :
> :"I"(arg)); /* can't be "r" has to be embedded const */
> +
> +   raw_local_irq_disable();
>  }
>
>  #else  /* ARC700 */
> @@ -122,6 +124,7 @@ void arch_cpu_idle(void)
>  {
> /* sleep, but enable both set E1/E2 (levels of interrupts) before 
> committing */
> __asm__ __volatile__("sleep 0x3 \n");
> +   raw_local_irq_disable();
>  }
>
>  #endif
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -78,7 +78,6 @@ void arch_cpu_idle(void)
> arm_pm_idle();
> else
> cpu_do_idle();
> -   raw_local_irq_enable();
>  }
>
>  void arch_cpu_idle_prepare(void)
> --- a/arch/arm/mach-gemini/board-dt.c
> +++ b/arch/arm/mach-gemini/board-dt.c
> @@ -42,8 +42,9 @@ static void gemini_idle(void)
>  */
>
> /* FIXME: Enabling interrupts here is racy! */
> -   local_irq_enable();
> +   raw_local_irq_enable();
> cpu_do_idle();
> +   raw_local_irq_disable();
>  }
>
>  static void __init gemini_init_machine(void)
> --- a/arch/arm64/kernel/idle.c
> +++ b/arch/arm64/kernel/idle.c
> @@ -42,5 +42,4 @@ void noinstr arch_cpu_idle(void)
>  * tricks
>  */
> cpu_do_idle();
> -   raw_local_irq_enable();
>  }
> --- a/arch/csky/kernel/process.c
> +++ b/arch/csky/kernel/process.c
> @@ -101,6 +101,5 @@ void arch_cpu_idle(void)
>  #ifdef CONFIG_CPU_PM_STOP
> asm volatile("stop\n");
>  #endif
> -   raw_local_irq_enable();
>  }
>  #endif
> --- a/arch/csky/kernel/smp.c
> +++ b/arch/csky/kernel/smp.c
> @@ -314,7 +314,7 @@ void arch_cpu_idle_dead(void)
> while (!secondary_stack)
> arch_cpu_idle();
>
> -   local_irq_disable();
> +   raw_local_irq_disable();
>
> asm volatile(
> "movsp, %0\n"
> --- a/arch/hexagon/kernel/process.c
> +++ b/arch/hexagon/kernel/process.c
> @@ -44,7 +44,6 @@ void arch_cpu_idle(void)
>  {
> __vmwait();
> /*  interrupts wake us up, but irqs are still disabled */
> -   raw_local_irq_enable();
>  }
>
>  /*
> --- a/arch/ia64/kernel/process.c
> +++ b/arch/ia64/kernel/process.c
> @@ -241,6 +241,7 @@ void arch_cpu_idle(void)
> (*mark_idle)(1

Re: [PATCH 18/36] cpuidle: Annotate poll_idle()

[Rafael J. Wysocki]

On Wed, Jun 8, 2022 at 4:46 PM Peter Zijlstra  wrote:
>
> The __cpuidle functions will become a noinstr class, as such they need
> explicit annotations.
>
> Signed-off-by: Peter Zijlstra (Intel) 

Reviewed-by: Rafael J. Wysocki 

> ---
>  drivers/cpuidle/poll_state.c |6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> --- a/drivers/cpuidle/poll_state.c
> +++ b/drivers/cpuidle/poll_state.c
> @@ -13,7 +13,10 @@
>  static int __cpuidle poll_idle(struct cpuidle_device *dev,
>struct cpuidle_driver *drv, int index)
>  {
> -   u64 time_start = local_clock();
> +   u64 time_start;
> +
> +   instrumentation_begin();
> +   time_start = local_clock();
>
> dev->poll_time_limit = false;
>
> @@ -39,6 +42,7 @@ static int __cpuidle poll_idle(struct cp
> raw_local_irq_disable();
>
> current_clr_polling();
> +   instrumentation_end();
>
> return index;
>  }
>
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 17/36] acpi_idle: Remove tracing

[Rafael J. Wysocki]

On Wed, Jun 8, 2022 at 4:47 PM Peter Zijlstra  wrote:
>
> All the idle routines are called with RCU disabled, as such there must
> not be any tracing inside.
>
> Signed-off-by: Peter Zijlstra (Intel) 

This actually does some additional code duplication cleanup which
would be good to mention in the changelog.  Or even move to a separate
patch for that matter.

Otherwise LGTM.

> ---
>  drivers/acpi/processor_idle.c |   24 +---
>  1 file changed, 13 insertions(+), 11 deletions(-)
>
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -108,8 +108,8 @@ static const struct dmi_system_id proces
>  static void __cpuidle acpi_safe_halt(void)
>  {
> if (!tif_need_resched()) {
> -   safe_halt();
> -   local_irq_disable();
> +   raw_safe_halt();
> +   raw_local_irq_disable();
> }
>  }
>
> @@ -524,16 +524,21 @@ static int acpi_idle_bm_check(void)
> return bm_status;
>  }
>
> -static void wait_for_freeze(void)
> +static __cpuidle void io_idle(unsigned long addr)
>  {
> +   /* IO port based C-state */
> +   inb(addr);
> +
>  #ifdef CONFIG_X86
> /* No delay is needed if we are in guest */
> if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
> return;
>  #endif
> -   /* Dummy wait op - must do something useless after P_LVL2 read
> -  because chipsets cannot guarantee that STPCLK# signal
> -  gets asserted in time to freeze execution properly. */
> +   /*
> +* Dummy wait op - must do something useless after P_LVL2 read
> +* because chipsets cannot guarantee that STPCLK# signal
> +* gets asserted in time to freeze execution properly.
> +*/
> inl(acpi_gbl_FADT.xpm_timer_block.address);
>  }
>
> @@ -553,9 +558,7 @@ static void __cpuidle acpi_idle_do_entry
> } else if (cx->entry_method == ACPI_CSTATE_HALT) {
> acpi_safe_halt();
> } else {
> -   /* IO port based C-state */
> -   inb(cx->address);
> -   wait_for_freeze();
> +   io_idle(cx->address);
> }
>
> perf_lopwr_cb(false);
> @@ -577,8 +580,7 @@ static int acpi_idle_play_dead(struct cp
> if (cx->entry_method == ACPI_CSTATE_HALT)
> safe_halt();
> else if (cx->entry_method == ACPI_CSTATE_SYSTEMIO) {
> -   inb(cx->address);
> -   wait_for_freeze();
> +   io_idle(cx->address);
> } else
> return -ENODEV;
>
>
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 05/36] cpuidle: Move IRQ state validation

[Rafael J. Wysocki]

On Wed, Jun 8, 2022 at 4:47 PM Peter Zijlstra  wrote:
>
> Make cpuidle_enter_state() consistent with the s2idle variant and
> verify ->enter() always returns with interrupts disabled.
>
> Signed-off-by: Peter Zijlstra (Intel) 
> ---
>  drivers/cpuidle/cpuidle.c |   10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -234,7 +234,11 @@ int cpuidle_enter_state(struct cpuidle_d
> stop_critical_timings();
> if (!(target_state->flags & CPUIDLE_FLAG_RCU_IDLE))
> rcu_idle_enter();
> +
> entered_state = target_state->enter(dev, drv, index);
> +   if (WARN_ONCE(!irqs_disabled(), "%ps leaked IRQ state", 
> target_state->enter))

I'm not sure if dumping a call trace here is really useful and
WARN_ON() often gets converted to panic().

I would print an error message with pr_warn_once().

Otherwise LGTM.

> +   raw_local_irq_disable();
> +
> if (!(target_state->flags & CPUIDLE_FLAG_RCU_IDLE))
> rcu_idle_exit();
> start_critical_timings();
> @@ -246,12 +250,8 @@ int cpuidle_enter_state(struct cpuidle_d
> /* The cpu is no longer idle or about to enter idle. */
> sched_idle_set_state(NULL);
>
> -   if (broadcast) {
> -   if (WARN_ON_ONCE(!irqs_disabled()))
> -   local_irq_disable();
> -
> +   if (broadcast)
> tick_broadcast_exit();
> -   }
>
> if (!cpuidle_state_is_coupled(drv, index))
> local_irq_enable();
>
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 03/36] cpuidle/poll: Ensure IRQ state is invariant

[Rafael J. Wysocki]

On Wed, Jun 8, 2022 at 4:47 PM Peter Zijlstra  wrote:
>
> cpuidle_state::enter() methods should be IRQ invariant
>
> Signed-off-by: Peter Zijlstra (Intel) 

Reviewed-by: Rafael J. Wysocki 

> ---
>  drivers/cpuidle/poll_state.c |4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> --- a/drivers/cpuidle/poll_state.c
> +++ b/drivers/cpuidle/poll_state.c
> @@ -17,7 +17,7 @@ static int __cpuidle poll_idle(struct cp
>
> dev->poll_time_limit = false;
>
> -   local_irq_enable();
> +   raw_local_irq_enable();
> if (!current_set_polling_and_test()) {
> unsigned int loop_count = 0;
> u64 limit;
> @@ -36,6 +36,8 @@ static int __cpuidle poll_idle(struct cp
> }
> }
> }
> +   raw_local_irq_disable();
> +
> current_clr_polling();
>
> return index;
>
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 02/36] x86/idle: Replace x86_idle with a static_call

[Rafael J. Wysocki]

On Wed, Jun 8, 2022 at 4:47 PM Peter Zijlstra  wrote:
>
> Typical boot time setup; no need to suffer an indirect call for that.
>
> Signed-off-by: Peter Zijlstra (Intel) 
> Reviewed-by: Frederic Weisbecker 

Reviewed-by: Rafael J. Wysocki 

> ---
>  arch/x86/kernel/process.c |   50 
> +-
>  1 file changed, 28 insertions(+), 22 deletions(-)
>
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -692,7 +693,23 @@ void __switch_to_xtra(struct task_struct
>  unsigned long boot_option_idle_override = IDLE_NO_OVERRIDE;
>  EXPORT_SYMBOL(boot_option_idle_override);
>
> -static void (*x86_idle)(void);
> +/*
> + * We use this if we don't have any better idle routine..
> + */
> +void __cpuidle default_idle(void)
> +{
> +   raw_safe_halt();
> +}
> +#if defined(CONFIG_APM_MODULE) || defined(CONFIG_HALTPOLL_CPUIDLE_MODULE)
> +EXPORT_SYMBOL(default_idle);
> +#endif
> +
> +DEFINE_STATIC_CALL_NULL(x86_idle, default_idle);
> +
> +static bool x86_idle_set(void)
> +{
> +   return !!static_call_query(x86_idle);
> +}
>
>  #ifndef CONFIG_SMP
>  static inline void play_dead(void)
> @@ -715,28 +732,17 @@ void arch_cpu_idle_dead(void)
>  /*
>   * Called from the generic idle code.
>   */
> -void arch_cpu_idle(void)
> -{
> -   x86_idle();
> -}
> -
> -/*
> - * We use this if we don't have any better idle routine..
> - */
> -void __cpuidle default_idle(void)
> +void __cpuidle arch_cpu_idle(void)
>  {
> -   raw_safe_halt();
> +   static_call(x86_idle)();
>  }
> -#if defined(CONFIG_APM_MODULE) || defined(CONFIG_HALTPOLL_CPUIDLE_MODULE)
> -EXPORT_SYMBOL(default_idle);
> -#endif
>
>  #ifdef CONFIG_XEN
>  bool xen_set_default_idle(void)
>  {
> -   bool ret = !!x86_idle;
> +   bool ret = x86_idle_set();
>
> -   x86_idle = default_idle;
> +   static_call_update(x86_idle, default_idle);
>
> return ret;
>  }
> @@ -859,20 +865,20 @@ void select_idle_routine(const struct cp
> if (boot_option_idle_override == IDLE_POLL && smp_num_siblings > 1)
> pr_warn_once("WARNING: polling idle and HT enabled, 
> performance may degrade\n");
>  #endif
> -   if (x86_idle || boot_option_idle_override == IDLE_POLL)
> +   if (x86_idle_set() || boot_option_idle_override == IDLE_POLL)
> return;
>
> if (boot_cpu_has_bug(X86_BUG_AMD_E400)) {
> pr_info("using AMD E400 aware idle routine\n");
> -   x86_idle = amd_e400_idle;
> +   static_call_update(x86_idle, amd_e400_idle);
> } else if (prefer_mwait_c1_over_halt(c)) {
> pr_info("using mwait in idle threads\n");
> -   x86_idle = mwait_idle;
> +   static_call_update(x86_idle, mwait_idle);
> } else if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) {
> pr_info("using TDX aware idle routine\n");
> -   x86_idle = tdx_safe_halt;
> +   static_call_update(x86_idle, tdx_safe_halt);
> } else
> -   x86_idle = default_idle;
> +   static_call_update(x86_idle, default_idle);
>  }
>
>  void amd_e400_c1e_apic_setup(void)
> @@ -925,7 +931,7 @@ static int __init idle_setup(char *str)
>  * To continue to load the CPU idle driver, don't touch
>  * the boot_option_idle_override.
>  */
> -   x86_idle = default_idle;
> +   static_call_update(x86_idle, default_idle);
> boot_option_idle_override = IDLE_HALT;
> } else if (!strcmp(str, "nomwait")) {
> /*
>
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 04/36] cpuidle,intel_idle: Fix CPUIDLE_FLAG_IRQ_ENABLE

[Rafael J. Wysocki]

On Wed, Jun 8, 2022 at 5:48 PM Peter Zijlstra  wrote:
>
> On Wed, Jun 08, 2022 at 05:01:05PM +0200, Rafael J. Wysocki wrote:
> > On Wed, Jun 8, 2022 at 4:47 PM Peter Zijlstra  wrote:
> > >
> > > Commit c227233ad64c ("intel_idle: enable interrupts before C1 on
> > > Xeons") wrecked intel_idle in two ways:
> > >
> > >  - must not have tracing in idle functions
> > >  - must return with IRQs disabled
> > >
> > > Additionally, it added a branch for no good reason.
> > >
> > > Fixes: c227233ad64c ("intel_idle: enable interrupts before C1 on Xeons")
> > > Signed-off-by: Peter Zijlstra (Intel) 
> >
> > Acked-by: Rafael J. Wysocki 
> >
> > And do I think correctly that this can be applied without the rest of
> > the series?
>
> Yeah, I don't think this relies on any of the preceding patches. If you
> want to route this through the pm/fixes tree that's fine.

OK, thanks, applied (and I moved the intel_idle() kerneldoc so it is
next to the function to avoid the docs build warning).
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 04/36] cpuidle,intel_idle: Fix CPUIDLE_FLAG_IRQ_ENABLE

[Rafael J. Wysocki]

On Wed, Jun 8, 2022 at 4:47 PM Peter Zijlstra  wrote:
>
> Commit c227233ad64c ("intel_idle: enable interrupts before C1 on
> Xeons") wrecked intel_idle in two ways:
>
>  - must not have tracing in idle functions
>  - must return with IRQs disabled
>
> Additionally, it added a branch for no good reason.
>
> Fixes: c227233ad64c ("intel_idle: enable interrupts before C1 on Xeons")
> Signed-off-by: Peter Zijlstra (Intel) 

Acked-by: Rafael J. Wysocki 

And do I think correctly that this can be applied without the rest of
the series?

> ---
>  drivers/idle/intel_idle.c |   48 
> +++---
>  1 file changed, 37 insertions(+), 11 deletions(-)
>
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -129,21 +137,37 @@ static unsigned int mwait_substates __in
>   *
>   * Must be called under local_irq_disable().
>   */
> +
> -static __cpuidle int intel_idle(struct cpuidle_device *dev,
> -   struct cpuidle_driver *drv, int index)
> +static __always_inline int __intel_idle(struct cpuidle_device *dev,
> +   struct cpuidle_driver *drv, int index)
>  {
> struct cpuidle_state *state = >states[index];
> unsigned long eax = flg2MWAIT(state->flags);
> unsigned long ecx = 1; /* break on interrupt flag */
>
> -   if (state->flags & CPUIDLE_FLAG_IRQ_ENABLE)
> -   local_irq_enable();
> -
> mwait_idle_with_hints(eax, ecx);
>
> return index;
>  }
>
> +static __cpuidle int intel_idle(struct cpuidle_device *dev,
> +   struct cpuidle_driver *drv, int index)
> +{
> +   return __intel_idle(dev, drv, index);
> +}
> +
> +static __cpuidle int intel_idle_irq(struct cpuidle_device *dev,
> +   struct cpuidle_driver *drv, int index)
> +{
> +   int ret;
> +
> +   raw_local_irq_enable();
> +   ret = __intel_idle(dev, drv, index);
> +   raw_local_irq_disable();
> +
> +   return ret;
> +}
> +
>  /**
>   * intel_idle_s2idle - Ask the processor to enter the given idle state.
>   * @dev: cpuidle device of the target CPU.
> @@ -1801,6 +1824,9 @@ static void __init intel_idle_init_cstat
> /* Structure copy. */
> drv->states[drv->state_count] = cpuidle_state_table[cstate];
>
> +   if (cpuidle_state_table[cstate].flags & 
> CPUIDLE_FLAG_IRQ_ENABLE)
> +   drv->states[drv->state_count].enter = intel_idle_irq;
> +
> if ((disabled_states_mask & BIT(drv->state_count)) ||
> ((icpu->use_acpi || force_use_acpi) &&
>  intel_idle_off_by_default(mwait_hint) &&
>
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v7 01/12] driver: platform: Add helper for safer setting of driver_override

[Rafael J. Wysocki]

On Tue, Apr 19, 2022 at 1:34 PM Krzysztof Kozlowski
 wrote:
>
> Several core drivers and buses expect that driver_override is a
> dynamically allocated memory thus later they can kfree() it.
>
> However such assumption is not documented, there were in the past and
> there are already users setting it to a string literal. This leads to
> kfree() of static memory during device release (e.g. in error paths or
> during unbind):
>
> kernel BUG at ../mm/slub.c:3960!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> ...
> (kfree) from [] (platform_device_release+0x88/0xb4)
> (platform_device_release) from [] (device_release+0x2c/0x90)
> (device_release) from [] (kobject_put+0xec/0x20c)
> (kobject_put) from [] (exynos5_clk_probe+0x154/0x18c)
> (exynos5_clk_probe) from [] (platform_drv_probe+0x6c/0xa4)
> (platform_drv_probe) from [] (really_probe+0x280/0x414)
> (really_probe) from [] (driver_probe_device+0x78/0x1c4)
> (driver_probe_device) from [] (bus_for_each_drv+0x74/0xb8)
> (bus_for_each_drv) from [] (__device_attach+0xd4/0x16c)
> (__device_attach) from [] (bus_probe_device+0x88/0x90)
> (bus_probe_device) from [] (device_add+0x3dc/0x62c)
> (device_add) from [] (of_platform_device_create_pdata+0x94/0xbc)
> (of_platform_device_create_pdata) from [] 
> (of_platform_bus_create+0x1a8/0x4fc)
> (of_platform_bus_create) from [] 
> (of_platform_bus_create+0x20c/0x4fc)
> (of_platform_bus_create) from [] 
> (of_platform_populate+0x84/0x118)
> (of_platform_populate) from [] 
> (of_platform_default_populate_init+0xa0/0xb8)
> (of_platform_default_populate_init) from [] 
> (do_one_initcall+0x8c/0x404)
>
> Provide a helper which clearly documents the usage of driver_override.
> This will allow later to reuse the helper and reduce the amount of
> duplicated code.
>
> Convert the platform driver to use a new helper and make the
> driver_override field const char (it is not modified by the core).
>
> Signed-off-by: Krzysztof Kozlowski 
> ---
>  drivers/base/driver.c   | 69 +
>  drivers/base/platform.c | 28 ++---
>  include/linux/device/driver.h   |  2 +
>  include/linux/platform_device.h |  6 ++-
>  4 files changed, 80 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index 8c0d33e182fd..1b9d47b10bd0 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -30,6 +30,75 @@ static struct device *next_device(struct klist_iter *i)
> return dev;
>  }
>
> +/**
> + * driver_set_override() - Helper to set or clear driver override.
> + * @dev: Device to change
> + * @override: Address of string to change (e.g. >driver_override);
> + *The contents will be freed and hold newly allocated override.

I would stick to one-line description here and possibly expand them in
the body of the comment.

Regardless, I think that the series is an improvement, so please feel
free to add

Reviewed-by: Rafael J. Wysocki 

to this patch and

Acked-by: Rafael J. Wysocki 

to the other patches in the series.

> + * @s: NUL-terminated string, new driver name to force a match, pass empty
> + * string to clear it ("" or "\n", where the latter is only for sysfs
> + * interface).
> + * @len: length of @s
> + *
> + * Helper to set or clear driver override in a device, intended for the cases
> + * when the driver_override field is allocated by driver/bus code.
> + *
> + * Returns: 0 on success or a negative error code on failure.
> + */
> +int driver_set_override(struct device *dev, const char **override,
> +   const char *s, size_t len)
> +{
> +   const char *new, *old;
> +   char *cp;
> +
> +   if (!override || !s)
> +   return -EINVAL;
> +
> +   /*
> +* The stored value will be used in sysfs show callback 
> (sysfs_emit()),
> +* which has a length limit of PAGE_SIZE and adds a trailing newline.
> +* Thus we can store one character less to avoid truncation during 
> sysfs
> +* show.
> +*/
> +   if (len >= (PAGE_SIZE - 1))
> +   return -EINVAL;
> +
> +   if (!len) {
> +   /* Empty string passed - clear override */
> +   device_lock(dev);
> +   old = *override;
> +   *override = NULL;
> +   device_unlock(dev);
> +   kfree(old);
> +
> +   return 0;
> +   }
> +
> +   cp = strnchr(s, len, '\n');
> +   if (cp)
> +   len = cp - s;
> +
> +   new = kstrndup(s, len, GFP_KERNEL

Re: [PATCH 02/17] Use memberof(T, m) instead of explicit NULL dereference

[Rafael J. Wysocki]

On Fri, Nov 19, 2021 at 12:37 PM Alejandro Colomar
 wrote:
>
> Signed-off-by: Alejandro Colomar 
> Cc: Ajit Khaparde 
> Cc: Andrew Morton 
> Cc: Andy Shevchenko 
> Cc: Arnd Bergmann 
> Cc: Bjorn Andersson 
> Cc: Borislav Petkov 
> Cc: Corey Minyard 
> Cc: Chris Mason 
> Cc: Christian Brauner 
> Cc: David Sterba 
> Cc: Jani Nikula 
> Cc: Jason Wang 
> Cc: Jitendra Bhivare 
> Cc: John Hubbard 
> Cc: John S. Gruber 
> Cc: Jonathan Cameron 
> Cc: Joonas Lahtinen 
> Cc: Josef Bacik 
> Cc: Kees Cook 
> Cc: Ketan Mukadam 
> Cc: Len Brown 
> Cc: "Michael S. Tsirkin" 
> Cc: Miguel Ojeda 
> Cc: Mike Rapoport 
> Cc: Nick Desaulniers 
> Cc: "Rafael J. Wysocki" 
> Cc: Rasmus Villemoes 
> Cc: Rodrigo Vivi 
> Cc: Russell King 
> Cc: Somnath Kotur 
> Cc: Sriharsha Basavapatna 
> Cc: Subbu Seetharaman 
> Cc: 
> Cc: 
> Cc: 
> Cc: 
> Cc: 
> Cc: 
> Cc: 
> ---
>  arch/x86/include/asm/bootparam_utils.h  |  3 ++-
>  arch/x86/kernel/signal_compat.c |  5 +++--
>  drivers/gpu/drm/i915/i915_utils.h   |  5 ++---
>  drivers/gpu/drm/i915/intel_runtime_pm.h |  2 +-
>  drivers/net/ethernet/emulex/benet/be.h  |  7 ---
>  drivers/net/ethernet/i825xx/ether1.c|  7 +--
>  drivers/scsi/be2iscsi/be.h  |  7 ---
>  drivers/scsi/be2iscsi/be_cmds.h |  5 -
>  fs/btrfs/ctree.h|  5 +++--
>  include/acpi/actypes.h  |  4 +++-

The change in actypes.h would need to be submitted to the upstream
ACPICA project via https://github.com/acpica/acpica/

Thanks!

>  include/linux/container_of.h|  6 +++---
>  include/linux/virtio_config.h   | 14 +++---
>  12 files changed, 41 insertions(+), 29 deletions(-)
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 1/6] driver core: Move the "authorized" attribute from USB/Thunderbolt to core

[Rafael J. Wysocki]

On Thu, Sep 30, 2021 at 6:59 AM Dan Williams  wrote:
>
> On Wed, Sep 29, 2021 at 7:39 PM Kuppuswamy, Sathyanarayanan
>  wrote:
> >
> >
> >
> > On 9/29/21 6:55 PM, Dan Williams wrote:
> > >> Also, you ignored the usb_[de]authorize_interface() functions and
> > >> their friends.
> > > Ugh, yes.
> >
> > I did not change it because I am not sure about the interface vs device
> > dependency.
> >
>
> This is was the rationale for has_probe_authorization flag. USB
> performs authorization of child devices based on the authorization
> state of the parent interface.
>
> > I think following change should work.
> >
> > diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> > index f57b5a7a90ca..84969732d09c 100644
> > --- a/drivers/usb/core/driver.c
> > +++ b/drivers/usb/core/driver.c
> > @@ -334,7 +334,7 @@ static int usb_probe_interface(struct device *dev)
> > if (udev->dev.authorized == false) {
> > dev_err(>dev, "Device is not authorized for usage\n");
> > return error;
> > -   } else if (intf->authorized == 0) {
> > +   } else if (intf->dev.authorized == 0) {
>
> == false

Or even (!intf->dev.authorized).

> > dev_err(>dev, "Interface %d is not authorized for 
> > usage\n",
> > intf->altsetting->desc.bInterfaceNumber);
> > return error;
> > @@ -546,7 +546,7 @@ int usb_driver_claim_interface(struct usb_driver 
> > *driver,
> > return -EBUSY;
> >
> > /* reject claim if interface is not authorized */
> > -   if (!iface->authorized)
> > +   if (!iface->dev.authorized)
>
> I'd do == false to keep it consistent with other conversions.

But this looks odd, FWIW.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC 0/3] cpuidle: add poll_source API and virtio vq polling

[Rafael J. Wysocki]

On Mon, Jul 26, 2021 at 6:04 PM Stefan Hajnoczi  wrote:
>
> On Mon, Jul 26, 2021 at 05:47:19PM +0200, Rafael J. Wysocki wrote:
> > On Mon, Jul 26, 2021 at 5:17 PM Stefan Hajnoczi  wrote:
> > >
> > > On Thu, Jul 22, 2021 at 05:04:57PM +0800, Jason Wang wrote:
> > > >
> > > > 在 2021/7/21 下午5:41, Stefan Hajnoczi 写道:
> > > > > On Wed, Jul 21, 2021 at 11:29:55AM +0800, Jason Wang wrote:
> > > > > > 在 2021/7/14 上午12:19, Stefan Hajnoczi 写道:
> > > > > > > These patches are not polished yet but I would like request 
> > > > > > > feedback on this
> > > > > > > approach and share performance results with you.
> > > > > > >
> > > > > > > Idle CPUs tentatively enter a busy wait loop before halting when 
> > > > > > > the cpuidle
> > > > > > > haltpoll driver is enabled inside a virtual machine. This reduces 
> > > > > > > wakeup
> > > > > > > latency for events that occur soon after the vCPU becomes idle.
> > > > > > >
> > > > > > > This patch series extends the cpuidle busy wait loop with the new 
> > > > > > > poll_source
> > > > > > > API so drivers can participate in polling. Such polling-aware 
> > > > > > > drivers disable
> > > > > > > their device's irq during the busy wait loop to avoid the cost of 
> > > > > > > interrupts.
> > > > > > > This reduces latency further than regular cpuidle haltpoll, which 
> > > > > > > still relies
> > > > > > > on irqs.
> > > > > > >
> > > > > > > Virtio drivers are modified to use the poll_source API so all 
> > > > > > > virtio device
> > > > > > > types get this feature. The following virtio-blk fio benchmark 
> > > > > > > results show the
> > > > > > > improvement:
> > > > > > >
> > > > > > >IOPS (numjobs=4, iodepth=1, 4 virtqueues)
> > > > > > >  before   poll_source  io_poll
> > > > > > > 4k randread167102  186049 (+11%)  186654 (+11%)
> > > > > > > 4k randwrite   162204  181214 (+11%)  181850 (+12%)
> > > > > > > 4k randrw  159520  177071 (+11%)  177928 (+11%)
> > > > > > >
> > > > > > > The comparison against io_poll shows that cpuidle poll_source 
> > > > > > > achieves
> > > > > > > equivalent performance to the block layer's io_poll feature 
> > > > > > > (which I
> > > > > > > implemented in a separate patch series [1]).
> > > > > > >
> > > > > > > The advantage of poll_source is that applications do not need to 
> > > > > > > explicitly set
> > > > > > > the RWF_HIPRI I/O request flag. The poll_source approach is 
> > > > > > > attractive because
> > > > > > > few applications actually use RWF_HIPRI and it takes advantage of 
> > > > > > > CPU cycles we
> > > > > > > would have spent in cpuidle haltpoll anyway.
> > > > > > >
> > > > > > > The current series does not improve virtio-net. I haven't 
> > > > > > > investigated deeply,
> > > > > > > but it is possible that NAPI and poll_source do not combine. See 
> > > > > > > the final
> > > > > > > patch for a starting point on making the two work together.
> > > > > > >
> > > > > > > I have not tried this on bare metal but it might help there too. 
> > > > > > > The cost of
> > > > > > > disabling a device's irq must be less than the savings from 
> > > > > > > avoiding irq
> > > > > > > handling for this optimization to make sense.
> > > > > > >
> > > > > > > [1] 
> > > > > > > https://lore.kernel.org/linux-block/20210520141305.355961-1-stefa...@redhat.com/
> > > > > >
> > > > > > Hi Stefan:
> > > > > >
> > > > > > Some questions:
> > > > > >
> > > > > > 1) What's the advantages of introducing polling at virtio level 
> > > &

Re: [RFC 0/3] cpuidle: add poll_source API and virtio vq polling

[Rafael J. Wysocki]

On Mon, Jul 26, 2021 at 5:17 PM Stefan Hajnoczi  wrote:
>
> On Thu, Jul 22, 2021 at 05:04:57PM +0800, Jason Wang wrote:
> >
> > 在 2021/7/21 下午5:41, Stefan Hajnoczi 写道:
> > > On Wed, Jul 21, 2021 at 11:29:55AM +0800, Jason Wang wrote:
> > > > 在 2021/7/14 上午12:19, Stefan Hajnoczi 写道:
> > > > > These patches are not polished yet but I would like request feedback 
> > > > > on this
> > > > > approach and share performance results with you.
> > > > >
> > > > > Idle CPUs tentatively enter a busy wait loop before halting when the 
> > > > > cpuidle
> > > > > haltpoll driver is enabled inside a virtual machine. This reduces 
> > > > > wakeup
> > > > > latency for events that occur soon after the vCPU becomes idle.
> > > > >
> > > > > This patch series extends the cpuidle busy wait loop with the new 
> > > > > poll_source
> > > > > API so drivers can participate in polling. Such polling-aware drivers 
> > > > > disable
> > > > > their device's irq during the busy wait loop to avoid the cost of 
> > > > > interrupts.
> > > > > This reduces latency further than regular cpuidle haltpoll, which 
> > > > > still relies
> > > > > on irqs.
> > > > >
> > > > > Virtio drivers are modified to use the poll_source API so all virtio 
> > > > > device
> > > > > types get this feature. The following virtio-blk fio benchmark 
> > > > > results show the
> > > > > improvement:
> > > > >
> > > > >IOPS (numjobs=4, iodepth=1, 4 virtqueues)
> > > > >  before   poll_source  io_poll
> > > > > 4k randread167102  186049 (+11%)  186654 (+11%)
> > > > > 4k randwrite   162204  181214 (+11%)  181850 (+12%)
> > > > > 4k randrw  159520  177071 (+11%)  177928 (+11%)
> > > > >
> > > > > The comparison against io_poll shows that cpuidle poll_source achieves
> > > > > equivalent performance to the block layer's io_poll feature (which I
> > > > > implemented in a separate patch series [1]).
> > > > >
> > > > > The advantage of poll_source is that applications do not need to 
> > > > > explicitly set
> > > > > the RWF_HIPRI I/O request flag. The poll_source approach is 
> > > > > attractive because
> > > > > few applications actually use RWF_HIPRI and it takes advantage of CPU 
> > > > > cycles we
> > > > > would have spent in cpuidle haltpoll anyway.
> > > > >
> > > > > The current series does not improve virtio-net. I haven't 
> > > > > investigated deeply,
> > > > > but it is possible that NAPI and poll_source do not combine. See the 
> > > > > final
> > > > > patch for a starting point on making the two work together.
> > > > >
> > > > > I have not tried this on bare metal but it might help there too. The 
> > > > > cost of
> > > > > disabling a device's irq must be less than the savings from avoiding 
> > > > > irq
> > > > > handling for this optimization to make sense.
> > > > >
> > > > > [1] 
> > > > > https://lore.kernel.org/linux-block/20210520141305.355961-1-stefa...@redhat.com/
> > > >
> > > > Hi Stefan:
> > > >
> > > > Some questions:
> > > >
> > > > 1) What's the advantages of introducing polling at virtio level instead 
> > > > of
> > > > doing it at each subsystems? Polling in virtio level may only work well 
> > > > if
> > > > all (or most) of the devices are virtio
> > > I'm not sure I understand the question. cpuidle haltpoll benefits all
> > > devices today, except it incurs interrupt latency. The poll_source API
> > > eliminates the interrupt latency for drivers that can disable device
> > > interrupts cheaply.
> > >
> > > This patch adds poll_source to core virtio code so that all virtio
> > > drivers get this feature for free. No driver-specific changes are
> > > needed.
> > >
> > > If you mean networking, block layer, etc by "subsystems" then there's
> > > nothing those subsystems can do to help. Whether poll_source can be used
> > > depends on the specific driver, not the subsystem. If you consider
> > > drivers/virtio/ a subsystem, then that's exactly what the patch series
> > > is doing.
> >
> >
> > I meant, if we choose to use idle poll, we have some several choices:
> >
> > 1) bus level (e.g the virtio)
> > 2) subsystem level (e.g the networking and block)
> >
> > I'm not sure which one is better.
>
> This API is intended to be driver- or bus-level. I don't think
> subsystems can do very much since they don't know the hardware
> capabilities (cheap interrupt disabling) and in most cases there's no
> advantage of plumbing it through subsystems when drivers can call the
> API directly.
>
> > > > 2) What's the advantages of using cpuidle instead of using a thread (and
> > > > leverage the scheduler)?
> > > In order to combine with the existing cpuidle infrastructure. No new
> > > polling loop is introduced and no additional CPU cycles are spent on
> > > polling.
> > >
> > > If cpuidle itself is converted to threads then poll_source would
> > > automatically operate in a thread too, but this patch series doesn't
> > > change how the core cpuidle code works.
> >
> >
> > So networking 

Re: [PATCH v2 4/4] bus: Make remove callback return void

[Rafael J. Wysocki]

On Tue, Jul 6, 2021 at 5:53 PM Uwe Kleine-König
 wrote:
>
> The driver core ignores the return value of this callback because there
> is only little it can do when a device disappears.
>
> This is the final bit of a long lasting cleanup quest where several
> buses were converted to also return void from their remove callback.
> Additionally some resource leaks were fixed that were caused by drivers
> returning an error code in the expectation that the driver won't go
> away.
>
> With struct bus_type::remove returning void it's prevented that newly
> implemented buses return an ignored error code and so don't anticipate
> wrong expectations for driver authors.
>
> Acked-by: Russell King (Oracle)  (For ARM, Amba 
> and related parts)
> Acked-by: Mark Brown 
> Acked-by: Chen-Yu Tsai  (for drivers/bus/sunxi-rsb.c)
> Acked-by: Pali Rohár 
> Acked-by: Mauro Carvalho Chehab  (for drivers/media)
> Acked-by: Hans de Goede  (For drivers/platform)
> Acked-by: Alexandre Belloni 
> Acked-By: Vinod Koul 
> Acked-by: Juergen Gross  (For Xen)
> Acked-by: Lee Jones  (For drivers/mfd)
> Acked-by: Johannes Thumshirn  (For drivers/mcb)
> Acked-by: Johan Hovold 
> Acked-by: Srinivas Kandagatla  (For 
> drivers/slimbus)
> Acked-by: Kirti Wankhede  (For drivers/vfio)
> Acked-by: Maximilian Luz 
> Acked-by: Heikki Krogerus  (For ulpi and 
> typec)
> Acked-by: Samuel Iglesias Gonsálvez  (For ipack)
> Reviewed-by: Tom Rix  (For fpga)
> Acked-by: Geoff Levand  (For ps3)
> Signed-off-by: Uwe Kleine-König 

For the ACPI part:

Acked-by: Rafael J. Wysocki 

> ---
>
>  arch/arm/common/locomo.c  | 3 +--
>  arch/arm/common/sa.c  | 4 +---
>  arch/arm/mach-rpc/ecard.c | 4 +---
>  arch/mips/sgi-ip22/ip22-gio.c | 3 +--
>  arch/parisc/kernel/drivers.c  | 5 ++---
>  arch/powerpc/platforms/ps3/system-bus.c   | 3 +--
>  arch/powerpc/platforms/pseries/ibmebus.c  | 3 +--
>  arch/powerpc/platforms/pseries/vio.c  | 3 +--
>  drivers/acpi/bus.c| 3 +--
>  drivers/amba/bus.c| 4 +---
>  drivers/base/auxiliary.c  | 4 +---
>  drivers/base/isa.c| 4 +---
>  drivers/base/platform.c   | 4 +---
>  drivers/bcma/main.c   | 6 ++
>  drivers/bus/sunxi-rsb.c   | 4 +---
>  drivers/cxl/core.c| 3 +--
>  drivers/dax/bus.c | 4 +---
>  drivers/dma/idxd/sysfs.c  | 4 +---
>  drivers/firewire/core-device.c| 4 +---
>  drivers/firmware/arm_scmi/bus.c   | 4 +---
>  drivers/firmware/google/coreboot_table.c  | 4 +---
>  drivers/fpga/dfl.c| 4 +---
>  drivers/hid/hid-core.c| 4 +---
>  drivers/hid/intel-ish-hid/ishtp/bus.c | 4 +---
>  drivers/hv/vmbus_drv.c| 5 +
>  drivers/hwtracing/intel_th/core.c | 4 +---
>  drivers/i2c/i2c-core-base.c   | 5 +
>  drivers/i3c/master.c  | 4 +---
>  drivers/input/gameport/gameport.c | 3 +--
>  drivers/input/serio/serio.c   | 3 +--
>  drivers/ipack/ipack.c | 4 +---
>  drivers/macintosh/macio_asic.c| 4 +---
>  drivers/mcb/mcb-core.c| 4 +---
>  drivers/media/pci/bt8xx/bttv-gpio.c   | 3 +--
>  drivers/memstick/core/memstick.c  | 3 +--
>  drivers/mfd/mcp-core.c| 3 +--
>  drivers/misc/mei/bus.c| 4 +---
>  drivers/misc/tifm_core.c  | 3 +--
>  drivers/mmc/core/bus.c| 4 +---
>  drivers/mmc/core/sdio_bus.c   | 4 +---
>  drivers/net/netdevsim/bus.c   | 3 +--
>  drivers/ntb/core.c| 4 +---
>  drivers/ntb/ntb_transport.c   | 4 +---
>  drivers/nvdimm/bus.c  | 3 +--
>  drivers/pci/endpoint/pci-epf-core.c   | 4 +---
>  drivers/pci/pci-driver.c  | 3 +--
>  drivers/pcmcia/ds.c   | 4 +---
>  drivers/platform/surface/aggregator/bus.c | 4 +---
>  drivers/platform/x86/wmi.c| 4 +---
>  drivers/pnp/driver.c  | 3 +--
>  drivers/rapidio/rio-driver.c  | 4 +---
>  drivers/rpmsg/rpmsg_core.c| 4 +---
>  drivers/s390/cio/ccwgroup.c   | 4 +---
>  drivers/s390/cio/css.c| 4 +---
>  drivers/s390/cio/device.c | 4 +---
>  drivers/s390/cio/scm.c| 4 +---
>  drivers/s390/crypto/ap_bus.c  | 4 +---
>  drivers/scsi/scsi_debug.c | 3 +--
>  drivers/siox/siox-co

Re: [PATCH v5 3/5] ACPI: Add driver for the VIOT table

[Rafael J. Wysocki]

On Fri, Jun 18, 2021 at 5:33 PM Jean-Philippe Brucker
 wrote:
>
> The ACPI Virtual I/O Translation Table describes topology of
> para-virtual platforms, similarly to vendor tables DMAR, IVRS and IORT.
> For now it describes the relation between virtio-iommu and the endpoints
> it manages.
>
> Three steps are needed to configure DMA of endpoints:
>
> (1) acpi_viot_init(): parse the VIOT table, find or create the fwnode
> associated to each vIOMMU device. This needs to happen after
> acpi_scan_init(), because it relies on the struct device and their
> fwnode to be available.
>
> (2) When probing the vIOMMU device, the driver registers its IOMMU ops
> within the IOMMU subsystem. This step doesn't require any
> intervention from the VIOT driver.
>
> (3) viot_iommu_configure(): before binding the endpoint to a driver,
> find the associated IOMMU ops. Register them, along with the
> endpoint ID, into the device's iommu_fwspec.
>
> If step (3) happens before step (2), it is deferred until the IOMMU is
> initialized, then retried.
>
> Tested-by: Eric Auger 
> Reviewed-by: Eric Auger 
> Signed-off-by: Jean-Philippe Brucker 

>From the general ACPI perspective

Acked-by: Rafael J. Wysocki 

and I'm assuming that it will be routed through a different tree.

> ---
>  drivers/acpi/Kconfig  |   3 +
>  drivers/iommu/Kconfig |   1 +
>  drivers/acpi/Makefile |   2 +
>  include/linux/acpi_viot.h |  19 ++
>  drivers/acpi/bus.c|   2 +
>  drivers/acpi/scan.c   |   3 +
>  drivers/acpi/viot.c   | 366 ++
>  MAINTAINERS   |   8 +
>  8 files changed, 404 insertions(+)
>  create mode 100644 include/linux/acpi_viot.h
>  create mode 100644 drivers/acpi/viot.c
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index eedec61e3476..3758c6940ed7 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -526,6 +526,9 @@ endif
>
>  source "drivers/acpi/pmic/Kconfig"
>
> +config ACPI_VIOT
> +   bool
> +
>  endif  # ACPI
>
>  config X86_PM_TIMER
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 1f111b399bca..aff8a4830dd1 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -403,6 +403,7 @@ config VIRTIO_IOMMU
> depends on ARM64
> select IOMMU_API
> select INTERVAL_TREE
> +   select ACPI_VIOT if ACPI
> help
>   Para-virtualised IOMMU driver with virtio.
>
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 700b41adf2db..a6e644c48987 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -118,3 +118,5 @@ video-objs  += acpi_video.o video_detect.o
>  obj-y  += dptf/
>
>  obj-$(CONFIG_ARM64)+= arm64/
> +
> +obj-$(CONFIG_ACPI_VIOT)+= viot.o
> diff --git a/include/linux/acpi_viot.h b/include/linux/acpi_viot.h
> new file mode 100644
> index ..1eb8ee5b0e5f
> --- /dev/null
> +++ b/include/linux/acpi_viot.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __ACPI_VIOT_H__
> +#define __ACPI_VIOT_H__
> +
> +#include 
> +
> +#ifdef CONFIG_ACPI_VIOT
> +void __init acpi_viot_init(void);
> +int viot_iommu_configure(struct device *dev);
> +#else
> +static inline void acpi_viot_init(void) {}
> +static inline int viot_iommu_configure(struct device *dev)
> +{
> +   return -ENODEV;
> +}
> +#endif
> +
> +#endif /* __ACPI_VIOT_H__ */
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index a4bd673934c0..d6f4e2f06fdb 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -27,6 +27,7 @@
>  #include 
>  #endif
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1334,6 +1335,7 @@ static int __init acpi_init(void)
> acpi_wakeup_device_init();
> acpi_debugger_init();
> acpi_setup_sb_notify_handler();
> +   acpi_viot_init();
> return 0;
>  }
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 2a2e690040e9..3e2bb04ab528 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -9,6 +9,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1556,6 +1557,8 @@ static const struct iommu_ops 
> *acpi_iommu_configure_id(struct device *dev,
> return ops;
>
> err = iort_iommu_configure_id(dev, id_in);
> +   if (err && err != -EPROBE_DEFER)
> +   err = viot_iommu_configure(dev);
>
> /*
>

Re: [PATCH v4 3/6] ACPI: Add driver for the VIOT table

[Rafael J. Wysocki]

On Thu, Jun 10, 2021 at 10:03 AM Jean-Philippe Brucker
 wrote:
>
> The ACPI Virtual I/O Translation Table describes topology of
> para-virtual platforms, similarly to vendor tables DMAR, IVRS and IORT.
> For now it describes the relation between virtio-iommu and the endpoints
> it manages.
>
> Three steps are needed to configure DMA of endpoints:
>
> (1) acpi_viot_init(): parse the VIOT table, find or create the fwnode
> associated to each vIOMMU device.
>
> (2) When probing the vIOMMU device, the driver registers its IOMMU ops
> within the IOMMU subsystem. This step doesn't require any
> intervention from the VIOT driver.
>
> (3) viot_iommu_configure(): before binding the endpoint to a driver,
> find the associated IOMMU ops. Register them, along with the
> endpoint ID, into the device's iommu_fwspec.
>
> If step (3) happens before step (2), it is deferred until the IOMMU is
> initialized, then retried.
>
> Signed-off-by: Jean-Philippe Brucker 
> ---
>  drivers/acpi/Kconfig  |   3 +
>  drivers/iommu/Kconfig |   1 +
>  drivers/acpi/Makefile |   2 +
>  include/linux/acpi_viot.h |  19 ++
>  drivers/acpi/bus.c|   2 +
>  drivers/acpi/scan.c   |   3 +
>  drivers/acpi/viot.c   | 364 ++
>  MAINTAINERS   |   8 +
>  8 files changed, 402 insertions(+)
>  create mode 100644 include/linux/acpi_viot.h
>  create mode 100644 drivers/acpi/viot.c
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index eedec61e3476..3758c6940ed7 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -526,6 +526,9 @@ endif
>
>  source "drivers/acpi/pmic/Kconfig"
>
> +config ACPI_VIOT
> +   bool
> +
>  endif  # ACPI
>
>  config X86_PM_TIMER
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 1f111b399bca..aff8a4830dd1 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -403,6 +403,7 @@ config VIRTIO_IOMMU
> depends on ARM64
> select IOMMU_API
> select INTERVAL_TREE
> +   select ACPI_VIOT if ACPI
> help
>   Para-virtualised IOMMU driver with virtio.
>
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 700b41adf2db..a6e644c48987 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -118,3 +118,5 @@ video-objs  += acpi_video.o video_detect.o
>  obj-y  += dptf/
>
>  obj-$(CONFIG_ARM64)+= arm64/
> +
> +obj-$(CONFIG_ACPI_VIOT)+= viot.o
> diff --git a/include/linux/acpi_viot.h b/include/linux/acpi_viot.h
> new file mode 100644
> index ..1eb8ee5b0e5f
> --- /dev/null
> +++ b/include/linux/acpi_viot.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __ACPI_VIOT_H__
> +#define __ACPI_VIOT_H__
> +
> +#include 
> +
> +#ifdef CONFIG_ACPI_VIOT
> +void __init acpi_viot_init(void);
> +int viot_iommu_configure(struct device *dev);
> +#else
> +static inline void acpi_viot_init(void) {}
> +static inline int viot_iommu_configure(struct device *dev)
> +{
> +   return -ENODEV;
> +}
> +#endif
> +
> +#endif /* __ACPI_VIOT_H__ */
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index be7da23fad76..b835ca702ff0 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -27,6 +27,7 @@
>  #include 
>  #endif
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1339,6 +1340,7 @@ static int __init acpi_init(void)
> pci_mmcfg_late_init();
> acpi_iort_init();
> acpi_scan_init();
> +   acpi_viot_init();

Is there a specific reason why to call it right here?

In particular, does it need to be called after acpi_scan_init()?  And
does it need to be called before the subsequent functions?  If so,
then why?

> acpi_ec_init();
> acpi_debugfs_init();
> acpi_sleep_proc_init();
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 0c53c8533300..4fa684fdfda8 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -9,6 +9,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1556,6 +1557,8 @@ static const struct iommu_ops 
> *acpi_iommu_configure_id(struct device *dev,
> return ops;
>
> err = iort_iommu_configure_id(dev, id_in);
> +   if (err && err != -EPROBE_DEFER)
> +   err = viot_iommu_configure(dev);
>
> /*
>  * If we have reason to believe the IOMMU driver missed the initial
> diff --git a/drivers/acpi/viot.c b/drivers/acpi/viot.c
> new file mode 100644
> index ..892cd9fa7b6d
> --- /dev/null
> +++ b/drivers/acpi/viot.c
> @@ -0,0 +1,364 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Virtual I/O topology
> + *
> + * The Virtual I/O Translation Table (VIOT) describes the topology of
> + * para-virtual IOMMUs and the endpoints they manage. The OS uses it to
> + * initialize devices in the right order, 

Re: [PATCH v1 09/12] ACPI: memhotplug: use a single static memory group for a single memory device

[Rafael J. Wysocki]

On Mon, Jun 7, 2021 at 9:55 PM David Hildenbrand  wrote:
>
> Let's group all memory we add for a single memory device - we want a
> single node for that (which also seems to be the sane thing to do).
>
> We won't care for now about memory that was already added to the system
> (e.g., via e820) -- usually *all* memory of a memory device was already
> added and we'll fail acpi_memory_enable_device().
>
> Signed-off-by: David Hildenbrand 

Acked-by: Rafael J. Wysocki 

> ---
>  drivers/acpi/acpi_memhotplug.c | 35 +-
>  1 file changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index eb4faf7c5cad..0c7b062c0e5d 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -54,6 +54,7 @@ struct acpi_memory_info {
>  struct acpi_memory_device {
> struct acpi_device *device;
> struct list_head res_list;
> +   int mgid;
>  };
>
>  static acpi_status
> @@ -171,10 +172,31 @@ static int acpi_memory_enable_device(struct 
> acpi_memory_device *mem_device)
> acpi_handle handle = mem_device->device->handle;
> int result, num_enabled = 0;
> struct acpi_memory_info *info;
> -   mhp_t mhp_flags = MHP_NONE;
> -   int node;
> +   mhp_t mhp_flags = MHP_NID_IS_MGID;
> +   u64 total_length = 0;
> +   int node, mgid;
>
> node = acpi_get_node(handle);
> +
> +   list_for_each_entry(info, _device->res_list, list) {
> +   if (!info->length)
> +   continue;
> +   /* We want a single node for the whole memory group */
> +   if (node < 0)
> +   node = memory_add_physaddr_to_nid(info->start_addr);
> +   total_length += info->length;
> +   }
> +
> +   if (!total_length) {
> +   dev_err(_device->device->dev, "device is empty\n");
> +   return -EINVAL;
> +   }
> +
> +   mgid = register_static_memory_group(node, PFN_UP(total_length));
> +   if (mgid < 0)
> +   return mgid;
> +   mem_device->mgid = mgid;
> +
> /*
>  * Tell the VM there is more memory here...
>  * Note: Assume that this function returns zero on success
> @@ -188,12 +210,10 @@ static int acpi_memory_enable_device(struct 
> acpi_memory_device *mem_device)
>  */
> if (!info->length)
> continue;
> -   if (node < 0)
> -   node = memory_add_physaddr_to_nid(info->start_addr);
>
> if (mhp_supports_memmap_on_memory(info->length))
> mhp_flags |= MHP_MEMMAP_ON_MEMORY;
> -   result = __add_memory(node, info->start_addr, info->length,
> +   result = __add_memory(mgid, info->start_addr, info->length,
>   mhp_flags);
>
> /*
> @@ -253,6 +273,10 @@ static void acpi_memory_device_free(struct 
> acpi_memory_device *mem_device)
> if (!mem_device)
> return;
>
> +   /* In case we succeeded adding *some* memory, unregistering fails. */
> +   if (mem_device->mgid >= 0)
> +   unregister_memory_group(mem_device->mgid);
> +
> acpi_memory_free_device_resources(mem_device);
> mem_device->device->driver_data = NULL;
> kfree(mem_device);
> @@ -273,6 +297,7 @@ static int acpi_memory_device_add(struct acpi_device 
> *device,
>
> INIT_LIST_HEAD(_device->res_list);
> mem_device->device = device;
> +   mem_device->mgid = -1;
> sprintf(acpi_device_name(device), "%s", ACPI_MEMORY_DEVICE_NAME);
> sprintf(acpi_device_class(device), "%s", ACPI_MEMORY_DEVICE_CLASS);
> device->driver_data = mem_device;
> --
> 2.31.1
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v1 08/12] ACPI: memhotplug: memory resources cannot be enabled yet

[Rafael J. Wysocki]

On Mon, Jun 7, 2021 at 9:55 PM David Hildenbrand  wrote:
>
> We allocate + initialize everything from scratch. In case enabling the
> device fails, we free all memory resourcs.
>
> Signed-off-by: David Hildenbrand 

Acked-by: Rafael J. Wysocki 

> ---
>  drivers/acpi/acpi_memhotplug.c | 4 
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index 1d01d9414c40..eb4faf7c5cad 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -182,10 +182,6 @@ static int acpi_memory_enable_device(struct 
> acpi_memory_device *mem_device)
>  * (i.e. memory-hot-remove function)
>  */
> list_for_each_entry(info, _device->res_list, list) {
> -   if (info->enabled) { /* just sanity check...*/
> -   num_enabled++;
> -   continue;
> -   }
> /*
>  * If the memory block size is zero, please ignore it.
>  * Don't try to do the following memory hotplug flowchart.
> --
> 2.31.1
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 2/6] ACPI: Move IOMMU setup code out of IORT

[Rafael J. Wysocki]

On Fri, Apr 23, 2021 at 1:57 PM Jean-Philippe Brucker
 wrote:
>
> Some of the IOMMU setup code in IORT is fairly generic and can be reused
> by VIOT. Extract it from IORT.

Except that iort_iommu_configure_id() is not really generic AFAICS.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 3/6] ACPI: Add driver for the VIOT table

[Rafael J. Wysocki]

On Fri, Apr 23, 2021 at 1:57 PM Jean-Philippe Brucker
 wrote:
>
> The ACPI Virtual I/O Translation Table describes topology of
> para-virtual platforms, similarly to vendor tables DMAR, IVRS and IORT.
> For now it describes the relation between virtio-iommu and the endpoints
> it manages.
>
> Three steps are needed to configure DMA of endpoints:
>
> (1) acpi_viot_init(): parse the VIOT table, find or create the fwnode
> associated to each vIOMMU device.
>
> (2) When probing the vIOMMU device, the driver registers its IOMMU ops
> within the IOMMU subsystem. This step doesn't require any
> intervention from the VIOT driver.
>
> (3) viot_iommu_configure(): before binding the endpoint to a driver,
> find the associated IOMMU ops. Register them, along with the
> endpoint ID, into the device's iommu_fwspec.
>
> If step (3) happens before step (2), it is deferred until the IOMMU is
> initialized, then retried.
>
> Signed-off-by: Jean-Philippe Brucker 
> ---
>  drivers/acpi/Kconfig  |   3 +
>  drivers/iommu/Kconfig |   1 +
>  drivers/acpi/Makefile |   2 +
>  include/linux/acpi_viot.h |  19 +++
>  drivers/acpi/bus.c|   2 +
>  drivers/acpi/scan.c   |   3 +
>  drivers/acpi/viot.c   | 350 ++
>  MAINTAINERS   |   8 +
>  8 files changed, 388 insertions(+)
>  create mode 100644 include/linux/acpi_viot.h
>  create mode 100644 drivers/acpi/viot.c
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index eedec61e3476..3758c6940ed7 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -526,6 +526,9 @@ endif
>
>  source "drivers/acpi/pmic/Kconfig"
>
> +config ACPI_VIOT
> +   bool
> +
>  endif  # ACPI
>
>  config X86_PM_TIMER
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 1f111b399bca..aff8a4830dd1 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -403,6 +403,7 @@ config VIRTIO_IOMMU
> depends on ARM64
> select IOMMU_API
> select INTERVAL_TREE
> +   select ACPI_VIOT if ACPI
> help
>   Para-virtualised IOMMU driver with virtio.
>
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 700b41adf2db..a6e644c48987 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -118,3 +118,5 @@ video-objs  += acpi_video.o video_detect.o
>  obj-y  += dptf/
>
>  obj-$(CONFIG_ARM64)+= arm64/
> +
> +obj-$(CONFIG_ACPI_VIOT)+= viot.o
> diff --git a/include/linux/acpi_viot.h b/include/linux/acpi_viot.h
> new file mode 100644
> index ..1eb8ee5b0e5f
> --- /dev/null
> +++ b/include/linux/acpi_viot.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __ACPI_VIOT_H__
> +#define __ACPI_VIOT_H__
> +
> +#include 
> +
> +#ifdef CONFIG_ACPI_VIOT
> +void __init acpi_viot_init(void);
> +int viot_iommu_configure(struct device *dev);
> +#else
> +static inline void acpi_viot_init(void) {}
> +static inline int viot_iommu_configure(struct device *dev)
> +{
> +   return -ENODEV;
> +}
> +#endif
> +
> +#endif /* __ACPI_VIOT_H__ */
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index be7da23fad76..b835ca702ff0 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -27,6 +27,7 @@
>  #include 
>  #endif
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1339,6 +1340,7 @@ static int __init acpi_init(void)
> pci_mmcfg_late_init();
> acpi_iort_init();
> acpi_scan_init();
> +   acpi_viot_init();
> acpi_ec_init();
> acpi_debugfs_init();
> acpi_sleep_proc_init();
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 5924421075f6..4db43c822ee7 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -9,6 +9,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1554,6 +1555,8 @@ static const struct iommu_ops 
> *acpi_iommu_configure_id(struct device *dev,
> return ops;
>
> err = iort_iommu_configure_id(dev, id_in);
> +   if (err && err != -EPROBE_DEFER)
> +   err = viot_iommu_configure(dev);
>
> /*
>  * If we have reason to believe the IOMMU driver missed the initial
> diff --git a/drivers/acpi/viot.c b/drivers/acpi/viot.c
> new file mode 100644
> index ..710e5a5eac70
> --- /dev/null
> +++ b/drivers/acpi/viot.c
> @@ -0,0 +1,350 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Virtual I/O topology

In the first place, more information on what this is all about, please.

What it does and how it is used.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v6 08/16] ACPI / NUMA: add a stub function for node_to_pxm()

[Rafael J. Wysocki]

On Thu, Feb 4, 2021 at 7:41 PM Wei Liu  wrote:
>
> On Wed, Feb 03, 2021 at 03:04:27PM +, Wei Liu wrote:
> > There is already a stub function for pxm_to_node but conversion to the
> > other direction is missing.
> >
> > It will be used by Microsoft Hypervisor code later.
> >
> > Signed-off-by: Wei Liu 
>
> Hi ACPI maintainers, if you're happy with this patch I can take it via
> the hyperv-next tree, given the issue is discovered when pxm_to_node is
> called in our code.

Yes, you can.

Thanks!

>
> > ---
> > v6: new
> > ---
> >  include/acpi/acpi_numa.h | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/include/acpi/acpi_numa.h b/include/acpi/acpi_numa.h
> > index a4c6ef809e27..40a91ce87e04 100644
> > --- a/include/acpi/acpi_numa.h
> > +++ b/include/acpi/acpi_numa.h
> > @@ -30,6 +30,10 @@ static inline int pxm_to_node(int pxm)
> >  {
> >   return 0;
> >  }
> > +static inline int node_to_pxm(int node)
> > +{
> > + return 0;
> > +}
> >  #endif   /* CONFIG_ACPI_NUMA */
> >
> >  #ifdef CONFIG_ACPI_HMAT
> > --
> > 2.20.1
> >
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Rafael J. Wysocki]

On Mon, Nov 23, 2020 at 4:58 PM James Bottomley
 wrote:
>
> On Mon, 2020-11-23 at 15:19 +0100, Miguel Ojeda wrote:
> > On Sun, Nov 22, 2020 at 11:36 PM James Bottomley
> >  wrote:

[cut]

> >
> > Maintainers routinely review 1-line trivial patches, not to mention
> > internal API changes, etc.
>
> We're also complaining about the inability to recruit maintainers:
>
> https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/
>
> And burn out:
>
> http://antirez.com/news/129

Right.

> The whole crux of your argument seems to be maintainers' time isn't
> important so we should accept all trivial patches ... I'm pushing back
> on that assumption in two places, firstly the valulessness of the time
> and secondly that all trivial patches are valuable.
>
> > If some company does not want to pay for that, that's fine, but they
> > don't get to be maintainers and claim `Supported`.
>
> What I'm actually trying to articulate is a way of measuring value of
> the patch vs cost ... it has nothing really to do with who foots the
> actual bill.
>
> One thesis I'm actually starting to formulate is that this continual
> devaluing of maintainers is why we have so much difficulty keeping and
> recruiting them.

Absolutely.

This is just one of the factors involved, but a significant one IMV.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC]: mm,power: introduce MADV_WIPEONSUSPEND

[Rafael J. Wysocki]

On Fri, Jul 3, 2020 at 1:30 PM Michal Hocko  wrote:
>
> On Fri 03-07-20 10:34:09, Catangiu, Adrian Costin wrote:
> > This patch adds logic to the kernel power code to zero out contents of
> > all MADV_WIPEONSUSPEND VMAs present in the system during its transition
> > to any suspend state equal or greater/deeper than Suspend-to-memory,
> > known as S3.
>
> How does the application learn that its memory got wiped? S2disk is an
> async operation and it can happen at any time during the task execution.
> So how does the application work to prevent from corrupted state - e.g.
> when suspended between two memory loads?

This doesn't affect hibernation AFAICS, but system suspend
(suspend-to-RAM or suspend-to-idle, or standby) is async too.

I guess this calls for an interface to notify user space (that opted
in to receive such notifications) on system-wide suspend start and
finish.

Thanks!
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC v4 01/13] ACPI: NUMA: export pxm_to_node

[Rafael J. Wysocki]

On Fri, Dec 13, 2019 at 10:41 AM David Hildenbrand  wrote:
>
> On 12.12.19 22:43, Rafael J. Wysocki wrote:
> > On Thursday, December 12, 2019 6:11:25 PM CET David Hildenbrand wrote:
> >> Will be needed by virtio-mem to identify the node from a pxm.
> >>
> >> Cc: "Rafael J. Wysocki" 
> >> Cc: Len Brown 
> >> Cc: linux-a...@vger.kernel.org
> >> Signed-off-by: David Hildenbrand 
> >> ---
> >>  drivers/acpi/numa/srat.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
> >> index eadbf90e65d1..d5847fa7ac69 100644
> >> --- a/drivers/acpi/numa/srat.c
> >> +++ b/drivers/acpi/numa/srat.c
> >> @@ -35,6 +35,7 @@ int pxm_to_node(int pxm)
> >>  return NUMA_NO_NODE;
> >>  return pxm_to_node_map[pxm];
> >>  }
> >> +EXPORT_SYMBOL(pxm_to_node);
> >>
> >>  int node_to_pxm(int node)
> >>  {
> >>
> >
> > This is fine by me FWIW.
>
> Can I count that as an Acked-by and carry it along? Thanks!

Yes, please.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 10/10] docs: fix broken documentation links

[Rafael J. Wysocki]

On Mon, May 20, 2019 at 4:48 PM Mauro Carvalho Chehab
 wrote:
>
> Mostly due to x86 and acpi conversion, several documentation
> links are still pointing to the old file. Fix them.
>
> Signed-off-by: Mauro Carvalho Chehab 

For the ACPI part:

Acked-by: Rafael J. Wysocki 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/35] treewide trivial patches converting pr_warning to pr_warn

[Rafael J. Wysocki]

On Fri, Feb 17, 2017 at 8:11 AM, Joe Perches  wrote:
> There are ~4300 uses of pr_warn and ~250 uses of the older
> pr_warning in the kernel source tree.
>
> Make the use of pr_warn consistent across all kernel files.
>
> This excludes all files in tools/ as there is a separate
> define pr_warning for that directory tree and pr_warn is
> not used in tools/.
>
> Done with 'sed s/\bpr_warning\b/pr_warn/' and some emacsing.

Sorry about asking if that has been asked already.

Wouldn't it be slightly less intrusive to simply redefined
pr_warning() as a synonym for pr_warn()?

Thanks,
Rafael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[Announcement] Linux Plumbers ACPI/PM, PCI Microconference

[Rafael J. Wysocki]

Hi All,

The original announcement didn't go to linux-pm, so again:

On Tuesday, July 16, 2013 08:21:26 PM Myron Stowe wrote:
 Linux Plumbers has approved an ACPI/PM, PCI microconference. The
 overview page is here:
 
 http://wiki.linuxplumbersconf.org/2013:pci_subsystem
 
 We would like to start receiving volunteers for presenting topics of
 interest.  There is a lot of activity in these subsystems so please
 respond by submitting presentation or discussion proposals that you
 would be willing to cover for consideration.  You should also feel
 free to submit ideas as proposals that others could cover.  The
 instructions for submitting ideas should be at:
 
 http://www.linuxplumbersconf.org/2013/submitting-topic/

If anyone who's going to attend the Linux Plumbers Conf this year is interested
in any particular topic for discussion during the ACPI/PM+PCI uconf, please
submit a formal topic proposal at:

http://www.linuxplumbersconf.org/2013/ocw/events/LPC2013/proposals/new

If you submit a topic, that doesn't mean you'll be expected to present anything
(if accepted).  In fact, we are mostly interested in discussion topics and we'd
like to know what is important to people.

That said if you have a (short) presentation to make, please submit it too.

We'll appreciate all submissions very much.

Thanks,
Rafael

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Announcement] Linux Plumbers ACPI/PM, PCI Microconference

[Rafael J. Wysocki]

On Wednesday, July 31, 2013 10:35:05 AM Shuah Khan wrote:
 On Wed, Jul 31, 2013 at 5:40 AM, Rafael J. Wysocki r...@sisk.pl wrote:
  Hi All,
 
  The original announcement didn't go to linux-pm, so again:
 
  On Tuesday, July 16, 2013 08:21:26 PM Myron Stowe wrote:
  Linux Plumbers has approved an ACPI/PM, PCI microconference. The
  overview page is here:
 
  http://wiki.linuxplumbersconf.org/2013:pci_subsystem
 
  We would like to start receiving volunteers for presenting topics of
  interest.  There is a lot of activity in these subsystems so please
  respond by submitting presentation or discussion proposals that you
  would be willing to cover for consideration.  You should also feel
  free to submit ideas as proposals that others could cover.  The
  instructions for submitting ideas should be at:
 
  http://www.linuxplumbersconf.org/2013/submitting-topic/
 
  If anyone who's going to attend the Linux Plumbers Conf this year is 
  interested
  in any particular topic for discussion during the ACPI/PM+PCI uconf, please
  submit a formal topic proposal at:
 
  http://www.linuxplumbersconf.org/2013/ocw/events/LPC2013/proposals/new
 
  If you submit a topic, that doesn't mean you'll be expected to present 
  anything
  (if accepted).  In fact, we are mostly interested in discussion topics and 
  we'd
  like to know what is important to people.
 
  That said if you have a (short) presentation to make, please submit it too.
 
  We'll appreciate all submissions very much.
 
 
 What I have mind is a discussion topic for the Mini-conference, which
 in-line with what you are asking for.
 I will add it to the web.

Thank you!

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: Linux Plumbers ACPI/PM, PCI Microconference

[Rafael J. Wysocki]

On Wednesday, July 17, 2013 08:31:55 AM Shuah Khan wrote:
 Myron,
 
 On Tue, Jul 16, 2013 at 8:21 PM, Myron Stowe myron.st...@gmail.com wrote:
 
 
  Shuah - You brought up the idea about Converting drivers from Legacy
  PM ops to dev_pm_ops; would you like to present what you have
  done/encountered so far?
 
 
 Awesome. Yes, I would like to present what I have done so far and I do
 have a couple of things that could benefit from a face to face
 discussion which would help me make progress on the rest of the work
 that needs to get done.

Care to sumbit a formal proposal through the LPC web page?

Rafael

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v4 01/12] virtio: pci: switch to new PM API

[Rafael J. Wysocki]

On Wednesday, December 07, 2011, Amit Shah wrote:
 Hi Rafael,
 
 On (Tue) 06 Dec 2011 [23:12:36], Rafael J. Wysocki wrote:
  Hi,
  
  On Tuesday, December 06, 2011, Amit Shah wrote:
   The older PM API doesn't have a way to get notifications on hibernate
   events.  Switch to the newer one that gives us those notifications.
   
   Signed-off-by: Amit Shah amit.s...@redhat.com
   ---
drivers/virtio/virtio_pci.c |   16 
1 files changed, 12 insertions(+), 4 deletions(-)
   
   diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
   index 03d1984..23e1532 100644
   --- a/drivers/virtio/virtio_pci.c
   +++ b/drivers/virtio/virtio_pci.c
   @@ -708,19 +708,28 @@ static void __devexit virtio_pci_remove(struct 
   pci_dev *pci_dev)
}

#ifdef CONFIG_PM
   -static int virtio_pci_suspend(struct pci_dev *pci_dev, pm_message_t 
   state)
   +static int virtio_pci_suspend(struct device *dev)
{
   + struct pci_dev *pci_dev = to_pci_dev(dev);
   +
 pci_save_state(pci_dev);
 pci_set_power_state(pci_dev, PCI_D3hot);
 return 0;
}

   -static int virtio_pci_resume(struct pci_dev *pci_dev)
   +static int virtio_pci_resume(struct device *dev)
{
   + struct pci_dev *pci_dev = to_pci_dev(dev);
   +
 pci_restore_state(pci_dev);
 pci_set_power_state(pci_dev, PCI_D0);
 return 0;
}
   +
   +static const struct dev_pm_ops virtio_pci_pm_ops = {
   + .suspend = virtio_pci_suspend,
   + .resume  = virtio_pci_resume,
   +};
#endif
  
  You seem to have forgotten about hibernation callbacks.
 
 This patch just moves to the new API keeping everything else the same.
 The hibernation callbacks come in patch 2.

OK, but then hibernation will be broken between the two patches, so perhaps
it's better to merge them into one?

   Please use
  one the macros defined in include/linux/pm.h if you want to use the same
  callback routines for hibernation.
 
 No, they're different functions, so I don't use the maros.

OK

Thanks,
Rafael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v4 01/12] virtio: pci: switch to new PM API

[Rafael J. Wysocki]

On Wednesday, December 07, 2011, Amit Shah wrote:
 On (Wed) 07 Dec 2011 [10:48:24], Rafael J. Wysocki wrote:
  On Wednesday, December 07, 2011, Amit Shah wrote:
   Hi Rafael,
   
   On (Tue) 06 Dec 2011 [23:12:36], Rafael J. Wysocki wrote:
Hi,

On Tuesday, December 06, 2011, Amit Shah wrote:
 The older PM API doesn't have a way to get notifications on hibernate
 events.  Switch to the newer one that gives us those notifications.
 
 Signed-off-by: Amit Shah amit.s...@redhat.com
 ---
  drivers/virtio/virtio_pci.c |   16 
  1 files changed, 12 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
 index 03d1984..23e1532 100644
 --- a/drivers/virtio/virtio_pci.c
 +++ b/drivers/virtio/virtio_pci.c
 @@ -708,19 +708,28 @@ static void __devexit virtio_pci_remove(struct 
 pci_dev *pci_dev)
  }
  
  #ifdef CONFIG_PM
 -static int virtio_pci_suspend(struct pci_dev *pci_dev, pm_message_t 
 state)
 +static int virtio_pci_suspend(struct device *dev)
  {
 + struct pci_dev *pci_dev = to_pci_dev(dev);
 +
   pci_save_state(pci_dev);
   pci_set_power_state(pci_dev, PCI_D3hot);
   return 0;
  }
  
 -static int virtio_pci_resume(struct pci_dev *pci_dev)
 +static int virtio_pci_resume(struct device *dev)
  {
 + struct pci_dev *pci_dev = to_pci_dev(dev);
 +
   pci_restore_state(pci_dev);
   pci_set_power_state(pci_dev, PCI_D0);
   return 0;
  }
 +
 +static const struct dev_pm_ops virtio_pci_pm_ops = {
 + .suspend = virtio_pci_suspend,
 + .resume  = virtio_pci_resume,
 +};
  #endif

You seem to have forgotten about hibernation callbacks.
   
   This patch just moves to the new API keeping everything else the same.
   The hibernation callbacks come in patch 2.
  
  OK, but then hibernation will be broken between the two patches, so perhaps
  it's better to merge them into one?
 
 The hibernation support doesn't exist now.  It's added in the later
 patches of the series (3 onwards).

OK then, sorry for the noise.

Rafael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v4 01/12] virtio: pci: switch to new PM API

[Rafael J. Wysocki]

Hi,

On Tuesday, December 06, 2011, Amit Shah wrote:
 The older PM API doesn't have a way to get notifications on hibernate
 events.  Switch to the newer one that gives us those notifications.
 
 Signed-off-by: Amit Shah amit.s...@redhat.com
 ---
  drivers/virtio/virtio_pci.c |   16 
  1 files changed, 12 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
 index 03d1984..23e1532 100644
 --- a/drivers/virtio/virtio_pci.c
 +++ b/drivers/virtio/virtio_pci.c
 @@ -708,19 +708,28 @@ static void __devexit virtio_pci_remove(struct pci_dev 
 *pci_dev)
  }
  
  #ifdef CONFIG_PM
 -static int virtio_pci_suspend(struct pci_dev *pci_dev, pm_message_t state)
 +static int virtio_pci_suspend(struct device *dev)
  {
 + struct pci_dev *pci_dev = to_pci_dev(dev);
 +
   pci_save_state(pci_dev);
   pci_set_power_state(pci_dev, PCI_D3hot);
   return 0;
  }
  
 -static int virtio_pci_resume(struct pci_dev *pci_dev)
 +static int virtio_pci_resume(struct device *dev)
  {
 + struct pci_dev *pci_dev = to_pci_dev(dev);
 +
   pci_restore_state(pci_dev);
   pci_set_power_state(pci_dev, PCI_D0);
   return 0;
  }
 +
 +static const struct dev_pm_ops virtio_pci_pm_ops = {
 + .suspend = virtio_pci_suspend,
 + .resume  = virtio_pci_resume,
 +};
  #endif

You seem to have forgotten about hibernation callbacks.  Please use
one the macros defined in include/linux/pm.h if you want to use the same
callback routines for hibernation.

  static struct pci_driver virtio_pci_driver = {
 @@ -729,8 +738,7 @@ static struct pci_driver virtio_pci_driver = {
   .probe  = virtio_pci_probe,
   .remove = __devexit_p(virtio_pci_remove),
  #ifdef CONFIG_PM
 - .suspend= virtio_pci_suspend,
 - .resume = virtio_pci_resume,
 + .driver.pm  = virtio_pci_pm_ops,
  #endif
  };

Thanks,
Rafael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: 2.6.29 git, resume from ram broken on thinkpad

[Rafael J. Wysocki]

On Thursday 02 April 2009, Chris Wright wrote:
 * Rafael J. Wysocki (r...@sisk.pl) wrote:
  Sorry for the misunderstanding, I thought the breakage might be introduced
  between 15f7176eb1cccec0a332541285ee752b935c1c85 and
  0a0c5168df270a50e3518e4f12bddb31f8f5f38f, so I thought it would be a good
  idea to verify if 0a0c5168df270a50e3518e4f12bddb31f8f5f38f fails too.
 
 Ah, sure.  It fails too (both test_suspend=mem and regular suspend/resume).

Having looked at the commit the Arek's bisect turned up I don't think it's
likely to have caused this problem to appear.

It seems that the regression had been introduced before the PM and PCI updates
went it, so I bet it's one of the x86 changes.  Ingo, are there any commits
obviously worth testing?

Rafael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: 2.6.29 git, resume from ram broken on thinkpad

[Rafael J. Wysocki]

On Thursday 02 April 2009, Ingo Molnar wrote:
 
 * Rafael J. Wysocki r...@sisk.pl wrote:
 
  On Thursday 02 April 2009, Chris Wright wrote:
   * Rafael J. Wysocki (r...@sisk.pl) wrote:
Sorry for the misunderstanding, I thought the breakage might be 
introduced
between 15f7176eb1cccec0a332541285ee752b935c1c85 and
0a0c5168df270a50e3518e4f12bddb31f8f5f38f, so I thought it would be a 
good
idea to verify if 0a0c5168df270a50e3518e4f12bddb31f8f5f38f fails too.
   
   Ah, sure.  It fails too (both test_suspend=mem and regular 
   suspend/resume).
  
  Having looked at the commit the Arek's bisect turned up I don't 
  think it's likely to have caused this problem to appear.
  
  It seems that the regression had been introduced before the PM and 
  PCI updates went it, so I bet it's one of the x86 changes.  Ingo, 
  are there any commits obviously worth testing?
 
 i lost context - a list/range of commits to check would be nice.

So far we know that d54b3538b0bfb31351d02d1669d4a978d2abfc5f is good
(according to Arek) and 0a0c5168df270a50e3518e4f12bddb31f8f5f38f (that doesn't
introduce functional changes) is already bad (according to Chris).

Thanks,
Rafael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: 2.6.29 git, resume from ram broken on thinkpad

[Rafael J. Wysocki]

On Wednesday 01 April 2009, Arkadiusz Miskiewicz wrote:
 
 Hi,
 
 After some recent merge to Linus tree resume from ram on my thinkpad t400 
 stopped working. My latest test was done on today Linus git master tree.
 
 I'm closing lid, it suspends according to moon led. When I open lid I see 
 that LCD backlight is on (not sure if it turns on when I open lid or maybe it 
 never turns off), black screen with cursor blinking in upper corner but... 
 moon led is on all the time. 
 
 Now two things happen after, 1) lcd goes off, moon led still on; it looks 
 like 
 itsuspend done again or 2) lcd stays on, moon led still on, no further 
 changes.
 
 I cannot wake up from this second like-suspend thing, I have to power off 
 notebook and power on.
 
 I started bisecting with such data:
 2d25ee36c84d5b2d6be8bfaf80256ecad69a06ca - bad
 d54b3538b0bfb31351d02d1669d4a978d2abfc5f - good
 
 ended with bisect log/bad commit below. (I'm not sure if can fully trust the 
 this bisect anyway).
 
 Note that the hang happens when t400 in integrated mode (intel) and also in 
 discrete mode (ati), so looks GPU unrelated.
 
 Any ideas what's going on?

This may be caused by the recent PM changes.  Can you please test if commit
8efb8c76fcdccf5050c0ea059dac392789baaff2 is fine?

Rafael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: 2.6.29 git, resume from ram broken on thinkpad

[Rafael J. Wysocki]

On Thursday 02 April 2009, Chris Wright wrote:
 * Rafael J. Wysocki (r...@sisk.pl) wrote:
  On Wednesday 01 April 2009, Chris Wright wrote:
   * Rafael J. Wysocki (r...@sisk.pl) wrote:
This may be caused by the recent PM changes.  Can you please test if 
commit
8efb8c76fcdccf5050c0ea059dac392789baaff2 is fine?
   
   I just tested on my t400, it's not[1].  See same symptoms as Arkadiusz.
   Seems as if it responds to initial apci event, I see some disk activity,
   then nothing (as if it tried to wakeup, then went back to sleep).
   
   [1] 15f7176eb1cccec0a332541285ee752b935c1c85 bad
   fae3e7fba4c664b3a15f2cf15ac439e8d754afc2 good(ish) (display stays 
   blank)
  
  Hmm, can you also test commit 0a0c5168df270a50e3518e4f12bddb31f8f5f38f, 
  please?
 
 Well, that's already included in 15f7176eb1cccec0a332541285ee752b935c1c85

Sorry for the misunderstanding, I thought the breakage might be introduced
between 15f7176eb1cccec0a332541285ee752b935c1c85 and
0a0c5168df270a50e3518e4f12bddb31f8f5f38f, so I thought it would be a good
idea to verify if 0a0c5168df270a50e3518e4f12bddb31f8f5f38f fails too.

Thanks,
Rafael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: [PATCH 4/8] unify paravirt parts of system.h

[Rafael J. Wysocki]

On Monday, 17 of December 2007, Pavel Machek wrote:
 On Mon 2007-12-17 01:27:29, Rafael J. Wysocki wrote:
  On Saturday, 15 of December 2007, Ingo Molnar wrote:
   
   * Pavel Machek [EMAIL PROTECTED] wrote:
   
 Linux never uses that register. The only user is suspend 
 save/restore, but that' bogus because it wasn't ever initialized by 
 Linux in the first place. It could be probably all safely removed.

It probably is safe to remove... but we currently support '2.8.95 
kernel loads/resumes 2.6.24 image'... which would break if 2.8 uses 
cr8.

So please keep it if it is not a big problem.
   
   hm, so __save_processor_state() is in essence an ABI? Could you please 
   also send a patch that documents this prominently, in the structure 
   itself?
  
  Hmm, I'm not sure if it really is an ABI part.  It doesn't communicate 
  anything
  outside of the kernel in which it is defined.
 
 Well, it is not application binary interface, but it is
 kernel-to-kernel binary interface...

Hm, rather a kernel-to-itself interface. ;-)

Rafael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization