LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Nicolin Chen @ 2014-09-09 21:09 UTC (permalink / raw)
  To: Timur Tabi, Shengjiu Wang
  Cc: alsa-devel, tiwai, Li.Xiubo, lgirdwood, perex, broonie, mpa,
	linuxppc-dev, linux-kernel
In-Reply-To: <540F6506.4060302@tabi.org>

On Tue, Sep 09, 2014 at 03:37:26PM -0500, Timur Tabi wrote:
> >However, my approach doesn't need any check. The open() or pm_resume()
> >can just call clk_prepare_enable() directly. The __clk_enable() will
> >then handle the 'clk == NULL' case:
> 
> Yes, I was thinking the same thing.

Because that's following your suggestion :)

@Shengjiu
Another thing I forgot to mention is we still need a return check for
clk_prepare_enable() which isn't in the current version. And I said
"doesn't need any check" is indicating the pre-check of the call.

Thank you
Nicolin

^ permalink raw reply

* Re: [PATCH 0/2 v6] powerpc/kvm: support to handle sw breakpoint
From: Alexander Graf @ 2014-09-09 21:15 UTC (permalink / raw)
  To: Madhavan Srinivasan, benh, paulus, mpe; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1410282456-11287-1-git-send-email-maddy@linux.vnet.ibm.com>



On 09.09.14 19:07, Madhavan Srinivasan wrote:
> This patchset adds ppc64 server side support for software breakpoint
> and extends the use of illegal instruction as software
> breakpoint across ppc platform.
> 
> Patch 1, adds kernel side support for software breakpoint.
> Design is that, by using an illegal instruction, we trap to
> hypervisor via Emulation Assistance interrupt, where we check
> for the illegal instruction and accordingly we return to Host
> or Guest. Patch also adds support for software breakpoint
> in PR KVM.
> 
> Patch 2,extends the use of illegal instruction as software
> breakpoint instruction across the ppc platform. Patch extends
> booke program interrupt code to support software breakpoint.

Thanks, applied to kvm-ppc-queue.


Alex

^ permalink raw reply

* Re: [PATCH 1/2 v6] powerpc/kvm: support to handle sw breakpoint
From: Alexander Graf @ 2014-09-09 21:12 UTC (permalink / raw)
  To: Madhavan Srinivasan, benh, paulus, mpe; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1410282456-11287-2-git-send-email-maddy@linux.vnet.ibm.com>



On 09.09.14 19:07, Madhavan Srinivasan wrote:
> This patch adds kernel side support for software breakpoint.
> Design is that, by using an illegal instruction, we trap to hypervisor
> via Emulation Assistance interrupt, where we check for the illegal instruction
> and accordingly we return to Host or Guest. Patch also adds support for
> software breakpoint in PR KVM.
> 
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/kvm_ppc.h |  6 ++++++
>  arch/powerpc/kvm/book3s.c          |  3 ++-
>  arch/powerpc/kvm/book3s_hv.c       | 41 ++++++++++++++++++++++++++++++++++----
>  arch/powerpc/kvm/book3s_pr.c       |  3 +++
>  arch/powerpc/kvm/emulate.c         | 15 ++++++++++++++
>  5 files changed, 63 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index fb86a22..dd83c9a 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -38,6 +38,12 @@
>  #include <asm/paca.h>
>  #endif
>  
> +/*
> + * KVMPPC_INST_SW_BREAKPOINT is debug Instruction
> + * for supporting software breakpoint.
> + */
> +#define KVMPPC_INST_SW_BREAKPOINT	0x00dddd00
> +
>  enum emulation_result {
>  	EMULATE_DONE,         /* no further processing */
>  	EMULATE_DO_MMIO,      /* kvm_run filled with MMIO request */
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index dd03f6b..00e9c9f 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -778,7 +778,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
>  int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>  					struct kvm_guest_debug *dbg)
>  {
> -	return -EINVAL;
> +	vcpu->guest_debug = dbg->control;
> +	return 0;
>  }
>  
>  void kvmppc_decrementer_func(unsigned long data)
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 27cced9..000fbec 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -725,6 +725,30 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
>  	return kvmppc_hcall_impl_hv_realmode(cmd);
>  }
>  
> +static int kvmppc_emulate_debug_inst(struct kvm_run *run,
> +					struct kvm_vcpu *vcpu)
> +{
> +	u32 last_inst;
> +
> +	if (kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) !=
> +					EMULATE_DONE) {
> +		/*
> +		 * Fetch failed, so return to guest and
> +		 * try executing it again.
> +		 */
> +		return RESUME_GUEST;
> +	}
> +
> +	if (last_inst == KVMPPC_INST_SW_BREAKPOINT) {
> +		run->exit_reason = KVM_EXIT_DEBUG;
> +		run->debug.arch.address = kvmppc_get_pc(vcpu);
> +		return RESUME_HOST;
> +	} else {
> +		kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> +		return RESUME_GUEST;
> +	}
> +}
> +
>  static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
>  				 struct task_struct *tsk)
>  {
> @@ -807,12 +831,18 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
>  		break;
>  	/*
>  	 * This occurs if the guest executes an illegal instruction.
> -	 * We just generate a program interrupt to the guest, since
> -	 * we don't emulate any guest instructions at this stage.
> +	 * If the guest debug is disabled, generate a program interrupt
> +	 * to the guest. If guest debug is enabled, we need to check
> +	 * whether the instruction is a software breakpoint instruction.
> +	 * Accordingly return to Guest or Host.
>  	 */
>  	case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
> -		kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> -		r = RESUME_GUEST;
> +		if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) {
> +			r = kvmppc_emulate_debug_inst(run, vcpu);
> +		} else {
> +			kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> +			r = RESUME_GUEST;
> +		}
>  		break;
>  	/*
>  	 * This occurs if the guest (kernel or userspace), does something that
> @@ -922,6 +952,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
>  	long int i;
>  
>  	switch (id) {
> +	case KVM_REG_PPC_DEBUG_INST:
> +		*val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT);
> +		break;
>  	case KVM_REG_PPC_HIOR:
>  		*val = get_reg_val(id, 0);
>  		break;
> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
> index faffb27..6d73708 100644
> --- a/arch/powerpc/kvm/book3s_pr.c
> +++ b/arch/powerpc/kvm/book3s_pr.c

Very nice patch set :). The only thing we're missing now is that Book3S
PR does not allow sw breakpoints to arrive in user mode (MSR.PR == 1),
because there we're never going to arrive at kvmppc_emulate_instruction().

But this can come as a follow-up patch.


Alex

^ permalink raw reply

* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Timur Tabi @ 2014-09-09 20:37 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: Shengjiu Wang, alsa-devel, tiwai, linux-kernel, broonie,
	lgirdwood, perex, Li.Xiubo, mpa, linuxppc-dev
In-Reply-To: <20140909202710.GB5224@Asurada>

On 09/09/2014 03:27 PM, Nicolin Chen wrote:
> I guess Mark's comment is merely against the check for clk validation
> because if talking about clk validation, we should check IS_ERR(clk)
> rather than check !=NULL directly.

Ah, that makes sense now.

> However, my approach doesn't need any check. The open() or pm_resume()
> can just call clk_prepare_enable() directly. The __clk_enable() will
> then handle the 'clk == NULL' case:

Yes, I was thinking the same thing.

^ permalink raw reply

* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Nicolin Chen @ 2014-09-09 20:27 UTC (permalink / raw)
  To: Timur Tabi
  Cc: Shengjiu Wang, alsa-devel, tiwai, linux-kernel, broonie,
	lgirdwood, perex, Li.Xiubo, mpa, linuxppc-dev
In-Reply-To: <540F5D29.7040309@tabi.org>

On Tue, Sep 09, 2014 at 03:03:53PM -0500, Timur Tabi wrote:
> On 09/09/2014 02:59 PM, Nicolin Chen wrote:
> >+	/*
> >+	 * Initially mark the clock to NULL for all platforms so that later
> >+	 * clk_prepare_enable() will ignore and return 0 for non-clock cases.
> >+	 */
> >+	ssi_private->clk = NULL;
> 
> According to Mark, NULL is a valid clock, so this should be instead:
> 
> 	ssi_private->clk = PTR_ERR(-EINVAL);
> 
> although that doesn't sit well with me.

I guess Mark's comment is merely against the check for clk validation
because if talking about clk validation, we should check IS_ERR(clk)
rather than check !=NULL directly.

However, my approach doesn't need any check. The open() or pm_resume()
can just call clk_prepare_enable() directly. The __clk_enable() will
then handle the 'clk == NULL' case:

static int __clk_enable(struct clk *clk)
{
	int ret = 0;

	if (!clk)
		return 0;

^ permalink raw reply

* Re: [PATCH V7 04/17] PCI: Take additional IOV BAR alignment in sizing and assigning
From: Bjorn Helgaas @ 2014-09-09 20:09 UTC (permalink / raw)
  To: Wei Yang
  Cc: Benjamin Herrenschmidt, linux-pci@vger.kernel.org, Gavin Shan,
	Mike Qiu, Guo Chao, linuxppc-dev
In-Reply-To: <20140820061402.GA8381@richard>

On Wed, Aug 20, 2014 at 12:14 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
> On Tue, Aug 19, 2014 at 09:08:41PM -0600, Bjorn Helgaas wrote:
>>On Thu, Jul 24, 2014 at 02:22:14PM +0800, Wei Yang wrote:
>>> At resource sizing/assigning stage, resources are divided into two lists,
>>> requested list and additional list, while the alignement of the additional
>>> IOV BAR is not taken into the sizeing and assigning procedure.
>>>
>>> This is reasonable in the original implementation, since IOV BAR's alignment is
>>> mostly the size of a PF BAR alignemt. This means the alignment is already taken
>>> into consideration. While this rule may be violated on some platform.
>>>
>>> This patch take the additional IOV BAR alignment in sizing and assigning stage
>>> explicitly.
>>>
>>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>> ---
>>>  drivers/pci/setup-bus.c |   68 +++++++++++++++++++++++++++++++++++++++++------
>>>  1 file changed, 60 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>>> index a5a63ec..d83681f 100644
>>> --- a/drivers/pci/setup-bus.c
>>> +++ b/drivers/pci/setup-bus.c
>>> @@ -120,6 +120,28 @@ static resource_size_t get_res_add_size(struct list_head *head,
>>>      return 0;
>>>  }
>>>
>>> +static resource_size_t get_res_add_align(struct list_head *head,
>>> +            struct resource *res)
>>> +{
>>> +    struct pci_dev_resource *dev_res;
>>> +
>>> +    list_for_each_entry(dev_res, head, list) {
>>> +            if (dev_res->res == res) {
>>> +                    int idx = res - &dev_res->dev->resource[0];
>>> +
>>> +                    dev_printk(KERN_DEBUG, &dev_res->dev->dev,
>>> +                               "res[%d]=%pR get_res_add_align min_align %llx\n",
>>> +                               idx, dev_res->res,
>>> +                               (unsigned long long)dev_res->min_align);
>>> +
>>> +                    return dev_res->min_align;
>>> +            }
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>
>>I see that you copied the structure of the existing get_res_add_size()
>>here.  But I don't understand *that* function.  It looks basically like
>>this:
>>
>>  resource_size_t get_res_add_size(list, res)
>>  {
>>    list_for_each_entry(dev_res, head, list) {
>>      if (dev_res->res == res)
>>        return dev_res->add_size;
>>    }
>>    return 0;
>>  }
>>
>>and we call it like this:
>>
>>  dev_res->res->end += get_res_add_size(realloc_head, dev_res->res);
>>
>>So we start out with dev_res", pass in dev_res->res, search the
>>realloc_head list to find dev_res again, and return dev_res->add_size.
>>That looks equivalent to just:
>>
>>  dev_res->res->end += dev_res->add_size;
>>
>>It looks like get_res_add_size() merely adds a printk and some complexity.
>>Am I missing something?
>>
>
> Let me try to explain it, if not correct, please let know :-)
>
>   dev_res->res->end += get_res_add_size(realloc_head, dev_res->res);
>
> would be expanded to:
>
>   dev_res->res->end += dev_res_1->add_size;
>
> with the dev_res_1 is another one from dev_res which is stored in realloc_head.

Yep, I see now.

>>I do see that there are other callers where we don't actually start with
>>dev_res, which makes it a little more complicated.  But I think you should
>>either add something like this:
>>
>>  struct pci_dev_resource *res_to_dev_res(list, res)
>>  {
>>    list_for_each_entry(dev_res, head, list) {
>>      if (dev_res->res == res)
>>        return dev_res;
>>    }
>>    return NULL;
>>  }
>>
>
> Ok, we can extract the common part of these two functions.
>
>>which can be used to replace get_res_add_size() and get_res_add_align(), OR
>>figure out whether the dev_res of interest is always one we've just added.
>>If it is, maybe you can just make add_to_list() return the dev_res pointer
>>instead of an errno, and hang onto the pointer.  I'd like that much better
>>if that's possible.
>>
>
> Sorry, I don't get this point.

Don't worry, it didn't make sense.  I was thinking that we knew the
dev_res up front and didn't need to look it up, but that's not the
case.

Sorry it took me so long to respond to this; I'm a bit swamped dealing
with some regressions.

Bjorn

> add_to_list() is used to create the pci_dev_resource list, get_res_add_size()
> and get_res_add_align() is to retrieve the information in the list. I am not
> sure how to leverage add_to_list() in these two functions?
>
>>> +
>>> +
>>>  /* Sort resources by alignment */
>>>  static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head)
>>>  {
>>> @@ -368,8 +390,9 @@ static void __assign_resources_sorted(struct list_head *head,
>>>      LIST_HEAD(save_head);
>>>      LIST_HEAD(local_fail_head);
>>>      struct pci_dev_resource *save_res;
>>> -    struct pci_dev_resource *dev_res, *tmp_res;
>>> +    struct pci_dev_resource *dev_res, *tmp_res, *dev_res2;
>>>      unsigned long fail_type;
>>> +    resource_size_t add_align, align;
>>>
>>>      /* Check if optional add_size is there */
>>>      if (!realloc_head || list_empty(realloc_head))
>>> @@ -384,10 +407,31 @@ static void __assign_resources_sorted(struct list_head *head,
>>>      }
>>>
>>>      /* Update res in head list with add_size in realloc_head list */
>>> -    list_for_each_entry(dev_res, head, list)
>>> +    list_for_each_entry_safe(dev_res, tmp_res, head, list) {
>>>              dev_res->res->end += get_res_add_size(realloc_head,
>>>                                                      dev_res->res);
>>>
>>> +            if (!(dev_res->res->flags & IORESOURCE_STARTALIGN))
>>> +                    continue;
>>> +
>>> +            add_align = get_res_add_align(realloc_head, dev_res->res);
>>> +
>>> +            if (add_align > dev_res->res->start) {
>>> +                    dev_res->res->start = add_align;
>>> +                    dev_res->res->end = add_align +
>>> +                                        resource_size(dev_res->res);
>>> +
>>> +                    list_for_each_entry(dev_res2, head, list) {
>>> +                            align = pci_resource_alignment(dev_res2->dev,
>>> +                                                           dev_res2->res);
>>> +                            if (add_align > align)
>>> +                                    list_move_tail(&dev_res->list,
>>> +                                                   &dev_res2->list);
>>> +                    }
>>> +               }
>>> +
>>> +    }
>>> +
>>>      /* Try updated head list with add_size added */
>>>      assign_requested_resources_sorted(head, &local_fail_head);
>>>
>>> @@ -930,6 +974,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>>>      struct resource *b_res = find_free_bus_resource(bus,
>>>                                      mask | IORESOURCE_PREFETCH, type);
>>>      resource_size_t children_add_size = 0;
>>> +    resource_size_t children_add_align = 0;
>>> +    resource_size_t add_align = 0;
>>>
>>>      if (!b_res)
>>>              return -ENOSPC;
>>> @@ -954,6 +1000,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>>>                      /* put SRIOV requested res to the optional list */
>>>                      if (realloc_head && i >= PCI_IOV_RESOURCES &&
>>>                                      i <= PCI_IOV_RESOURCE_END) {
>>> +                            add_align = max(pci_resource_alignment(dev, r), add_align);
>>>                              r->end = r->start - 1;
>>>                              add_to_list(realloc_head, dev, r, r_size, 0/* don't care */);
>>>                              children_add_size += r_size;
>>> @@ -984,8 +1031,11 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>>>                      if (order > max_order)
>>>                              max_order = order;
>>>
>>> -                    if (realloc_head)
>>> +                    if (realloc_head) {
>>>                              children_add_size += get_res_add_size(realloc_head, r);
>>> +                            children_add_align = get_res_add_align(realloc_head, r);
>>> +                            add_align = max(add_align, children_add_align);
>>> +                    }
>>>              }
>>>      }
>>>
>>> @@ -996,7 +1046,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>>>              add_size = children_add_size;
>>>      size1 = (!realloc_head || (realloc_head && !add_size)) ? size0 :
>>>              calculate_memsize(size, min_size, add_size,
>>> -                            resource_size(b_res), min_align);
>>> +                            resource_size(b_res), max(min_align, add_align));
>>>      if (!size0 && !size1) {
>>>              if (b_res->start || b_res->end)
>>>                      dev_info(&bus->self->dev, "disabling bridge window %pR to %pR (unused)\n",
>>> @@ -1008,10 +1058,12 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>>>      b_res->end = size0 + min_align - 1;
>>>      b_res->flags |= IORESOURCE_STARTALIGN;
>>>      if (size1 > size0 && realloc_head) {
>>> -            add_to_list(realloc_head, bus->self, b_res, size1-size0, min_align);
>>> -            dev_printk(KERN_DEBUG, &bus->self->dev, "bridge window %pR to %pR add_size %llx\n",
>>> -                       b_res, &bus->busn_res,
>>> -                       (unsigned long long)size1-size0);
>>> +            add_to_list(realloc_head, bus->self, b_res, size1-size0,
>>> +                            max(min_align, add_align));
>>> +            dev_printk(KERN_DEBUG, &bus->self->dev, "bridge window "
>>> +                             "%pR to %pR add_size %llx add_align %llx\n", b_res,
>>> +                             &bus->busn_res, (unsigned long long)size1-size0,
>>> +                             max(min_align, add_align));
>>
>>Factor out this "max(min_align, add_align)" thing so we don't have to
>>change these lines.  Bonus points if you can also factor it out of the
>>calculate_memsize() call above.  That one is a pretty complicated ternary
>>expression that should probably be turned into an "if" instead anyway.
>>
>
> Ok, I get your point. Let me make it more easy to read.
>
>>>      }
>>>      return 0;
>>>  }
>>> --
>>> 1.7.9.5
>>>
>
> --
> Richard Yang
> Help you, Help me
>

^ permalink raw reply

* [RFC PATCH] powerpc/numa: add ability to disable and debug topology updates
From: Nishanth Aravamudan @ 2014-09-09 20:09 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Paul Mackerras, linuxppc-dev

We have hit a few customer issues with the topology update code (VPHN
and PRRN). It would be nice to be able to debug the notifications coming
from the hypervisor in both cases to the LPAR, as well as to disable
reacting to the notifications, to narrow down the source of the
problems. Add a basic level of such functionality, similar to the numa=
command-line parameter.

Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
---
This is pretty rough, but has been useful in the field already. I'm not
sure if more information would be useful than this basic amount.

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 5ae8608ca9f5..6e3b9e3a2ab4 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3370,6 +3370,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			e.g. base its process migration decisions on it.
 			Default is on.
 
+	topology_updates= [KNL, PPC, NUMA]
+			Format: {off | debug}
+			Specify if the kernel should ignore (off) or
+			emit more information (debug) when the
+			hypervisor sends NUMA topology updates to an
+			LPAR.
+
 	tp720=		[HW,PS2]
 
 	tpm_suspend_pcr=[HW,TPM]
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index d7737a542fd7..72c5ad313cbe 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1160,6 +1160,28 @@ static int __init early_numa(char *p)
 }
 early_param("numa", early_numa);
 
+static int topology_updates_enabled = 1;
+static int topology_updates_debug = 0;
+
+static int __init early_topology_updates(char *p)
+{
+	if (!p)
+		return 0;
+
+	if (strstr(p, "off")) {
+		printk(KERN_INFO "Disabling topology updates\n");
+		topology_updates_enabled = 0;
+	}
+
+	if (strstr(p, "debug")) {
+		printk(KERN_INFO "Enabling topology updates debug\n");
+		topology_updates_debug = 1;
+	}
+
+	return 0;
+}
+early_param("topology_updates", early_topology_updates);
+
 #ifdef CONFIG_MEMORY_HOTPLUG
 /*
  * Find the node associated with a hot added memory section for
@@ -1546,6 +1568,9 @@ int arch_update_cpu_topology(void)
 	struct device *dev;
 	int weight, new_nid, i = 0;
 
+	if (!topology_updates_enabled)
+		return 0;
+
 	weight = cpumask_weight(&cpu_associativity_changes_mask);
 	if (!weight)
 		return 0;
@@ -1610,6 +1635,25 @@ int arch_update_cpu_topology(void)
 	 *
 	 * And for the similar reason, we will skip all the following updating.
 	 */
+
+	if (topology_updates_debug) {
+		char *buf = kmalloc_array(NR_CPUS*5, sizeof(char), GFP_KERNEL);
+		cpumask_scnprintf(buf, NR_CPUS*5, &updated_cpus);
+		printk(KERN_DEBUG "Topology update for the following CPUs:\n");
+		printk(KERN_DEBUG " %s\n", buf);
+		printk(KERN_DEBUG "cpumask_weight(&updated_cpus)) = %u\n",
+						cpumask_weight(&updated_cpus));
+
+		if (cpumask_weight(&updated_cpus)) {
+			for (ud = &updates[0]; ud; ud = ud->next) {
+				printk(KERN_DEBUG "cpu %d moving from node %d "
+						  "to %d\n", ud->cpu,
+						  ud->old_nid, ud->new_nid);
+			}
+		}
+		kfree(buf);
+	}
+
 	if (!cpumask_weight(&updated_cpus))
 		goto out;
 
@@ -1807,8 +1851,10 @@ static const struct file_operations topology_ops = {
 
 static int topology_update_init(void)
 {
-	start_topology_update();
-	proc_create("powerpc/topology_updates", 0644, NULL, &topology_ops);
+	if (topology_updates_enabled) {
+		start_topology_update();
+		proc_create("powerpc/topology_updates", 0644, NULL, &topology_ops);
+	}
 
 	return 0;
 }

^ permalink raw reply related

* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Timur Tabi @ 2014-09-09 20:03 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: Shengjiu Wang, alsa-devel, tiwai, linux-kernel, broonie,
	lgirdwood, perex, Li.Xiubo, mpa, linuxppc-dev
In-Reply-To: <20140909195928.GA5224@Asurada>

On 09/09/2014 02:59 PM, Nicolin Chen wrote:
> +	/*
> +	 * Initially mark the clock to NULL for all platforms so that later
> +	 * clk_prepare_enable() will ignore and return 0 for non-clock cases.
> +	 */
> +	ssi_private->clk = NULL;

According to Mark, NULL is a valid clock, so this should be instead:

	ssi_private->clk = PTR_ERR(-EINVAL);

although that doesn't sit well with me.

^ permalink raw reply

* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Nicolin Chen @ 2014-09-09 19:59 UTC (permalink / raw)
  To: Timur Tabi
  Cc: Shengjiu Wang, alsa-devel, tiwai, linux-kernel, broonie,
	lgirdwood, perex, Li.Xiubo, mpa, linuxppc-dev
In-Reply-To: <540F5706.1050303@tabi.org>

On Tue, Sep 09, 2014 at 02:37:42PM -0500, Timur Tabi wrote:
> On 09/09/2014 01:38 PM, Nicolin Chen wrote:
> >make sure to have the call for imx only because it seems that
> >the other platforms do not depend on the clock.
> 
> Although I doubt anyone will every add support for clocks to PowerPC "side"
> of this driver, I would prefer to avoid IMX-specific changes. Instead, the
> code should check if a clock is available.  That's why I suggested this
> change:
> 
> -	if (ssi_private->soc->imx)
> +	if (!IS_ERR(ssi_private->clk))

Hmm.... I think the following change may be better?

probe() {
	....
+	/*
+	 * Initially mark the clock to NULL for all platforms so that later
+	 * clk_prepare_enable() will ignore and return 0 for non-clock cases.
+	 */
+	ssi_private->clk = NULL;
	.....
	fsl_ssi_imx_probe();
}

In this way, all platforms, not confined to imx any more, will be able
to call clk_prepare_enable(). Then we don't need an extra platform check
before calling it.

^ permalink raw reply

* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Timur Tabi @ 2014-09-09 19:37 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: Shengjiu Wang, alsa-devel, tiwai, linux-kernel, broonie,
	lgirdwood, perex, Li.Xiubo, mpa, linuxppc-dev
In-Reply-To: <20140909183804.GA6944@Asurada>

On 09/09/2014 01:38 PM, Nicolin Chen wrote:
> make sure to have the call for imx only because it seems that
> the other platforms do not depend on the clock.

Although I doubt anyone will every add support for clocks to PowerPC 
"side" of this driver, I would prefer to avoid IMX-specific changes. 
Instead, the code should check if a clock is available.  That's why I 
suggested this change:

-	if (ssi_private->soc->imx)
+	if (!IS_ERR(ssi_private->clk))

^ permalink raw reply

* [PATCH 3/3] Partial revert of 81c98869faa5 ("kthread: ensure locality of task_struct allocations")
From: Nishanth Aravamudan @ 2014-09-09 19:06 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Han Pingtian, Matt Mackall, David Rientjes, Pekka Enberg,
	Linux Memory Management List, Paul Mackerras, Tejun Heo,
	Joonsoo Kim, linuxppc-dev, Christoph Lameter, Wanpeng Li,
	Anton Blanchard
In-Reply-To: <20140909190514.GE22906@linux.vnet.ibm.com>

After discussions with Tejun, we don't want to spread the use of
cpu_to_mem() (and thus knowledge of allocators/NUMA topology details)
into callers, but would rather ensure the callees correctly handle
memoryless nodes. With the previous patches ("topology: add support for
node_to_mem_node() to determine the fallback node" and "slub: fallback
to node_to_mem_node() node if allocating on memoryless node") adding and
using node_to_mem_node(), we can safely undo part of the change to the
kthread logic from 81c98869faa5.

Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>

diff --git a/kernel/kthread.c b/kernel/kthread.c
index ef483220e855..10e489c448fe 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -369,7 +369,7 @@ struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data),
 {
 	struct task_struct *p;
 
-	p = kthread_create_on_node(threadfn, data, cpu_to_mem(cpu), namefmt,
+	p = kthread_create_on_node(threadfn, data, cpu_to_node(cpu), namefmt,
 				   cpu);
 	if (IS_ERR(p))
 		return p;

^ permalink raw reply related

* [PATCH 2/3] slub: fallback to node_to_mem_node() node if allocating on memoryless node
From: Nishanth Aravamudan @ 2014-09-09 19:05 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Han Pingtian, Matt Mackall, David Rientjes, Pekka Enberg,
	Linux Memory Management List, Paul Mackerras, Tejun Heo,
	Joonsoo Kim, linuxppc-dev, Christoph Lameter, Wanpeng Li,
	Anton Blanchard
In-Reply-To: <20140909190326.GD22906@linux.vnet.ibm.com>

From: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Update the SLUB code to search for partial slabs on the nearest node
with memory in the presence of memoryless nodes. Additionally, do not
consider it to be an ALLOC_NODE_MISMATCH (and deactivate the slab) when
a memoryless-node specified allocation goes off-node.

Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>

---
v1 -> v2 (Nishanth):
  Add commit message
  Clean-up conditions in get_partial()

diff --git a/mm/slub.c b/mm/slub.c
index 3e8afcc07a76..497fdfed2f01 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1699,7 +1699,12 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
 		struct kmem_cache_cpu *c)
 {
 	void *object;
-	int searchnode = (node == NUMA_NO_NODE) ? numa_mem_id() : node;
+	int searchnode = node;
+
+	if (node == NUMA_NO_NODE)
+		searchnode = numa_mem_id();
+	else if (!node_present_pages(node))
+		searchnode = node_to_mem_node(node);
 
 	object = get_partial_node(s, get_node(s, searchnode), c, flags);
 	if (object || node != NUMA_NO_NODE)
@@ -2280,11 +2285,18 @@ static void *__slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
 redo:
 
 	if (unlikely(!node_match(page, node))) {
-		stat(s, ALLOC_NODE_MISMATCH);
-		deactivate_slab(s, page, c->freelist);
-		c->page = NULL;
-		c->freelist = NULL;
-		goto new_slab;
+		int searchnode = node;
+
+		if (node != NUMA_NO_NODE && !node_present_pages(node))
+			searchnode = node_to_mem_node(node);
+
+		if (unlikely(!node_match(page, searchnode))) {
+			stat(s, ALLOC_NODE_MISMATCH);
+			deactivate_slab(s, page, c->freelist);
+			c->page = NULL;
+			c->freelist = NULL;
+			goto new_slab;
+		}
 	}
 
 	/*

^ permalink raw reply related

* [PATCH v3] topology: add support for node_to_mem_node() to determine the fallback node
From: Nishanth Aravamudan @ 2014-09-09 19:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Han Pingtian, Matt Mackall, David Rientjes, Pekka Enberg,
	Linux Memory Management List, Paul Mackerras, Tejun Heo,
	Joonsoo Kim, linuxppc-dev, Christoph Lameter, Wanpeng Li,
	Anton Blanchard
In-Reply-To: <20140909190154.GC22906@linux.vnet.ibm.com>

From: Joonsoo Kim <iamjoonsoo.kim@lge.com>

We need to determine the fallback node in slub allocator if the
allocation target node is memoryless node. Without it, the SLUB wrongly
select the node which has no memory and can't use a partial slab,
because of node mismatch. Introduced function, node_to_mem_node(X), will
return a node Y with memory that has the nearest distance. If X is
memoryless node, it will return nearest distance node, but, if X is
normal node, it will return itself.

We will use this function in following patch to determine the fallback
node.

Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>

---
v2 -> v3 (Nishanth):
  Fix declaration and definition of _node_numa_mem_.
  s/node_numa_mem/node_to_mem_node/ as suggested by David Rientjes.

Andrew, I decided to leave the definition of the variables as-is. I
followed-up with Christoph, but didn't hear back on what he actually
wanted. I think the naming can be changed in a future patch if it's
urgent.

diff --git a/include/linux/topology.h b/include/linux/topology.h
index dda6ee521e74..909b6e43b694 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -119,11 +119,20 @@ static inline int numa_node_id(void)
  * Use the accessor functions set_numa_mem(), numa_mem_id() and cpu_to_mem().
  */
 DECLARE_PER_CPU(int, _numa_mem_);
+extern int _node_numa_mem_[MAX_NUMNODES];
 
 #ifndef set_numa_mem
 static inline void set_numa_mem(int node)
 {
 	this_cpu_write(_numa_mem_, node);
+	_node_numa_mem_[numa_node_id()] = node;
+}
+#endif
+
+#ifndef node_to_mem_node
+static inline int node_to_mem_node(int node)
+{
+	return _node_numa_mem_[node];
 }
 #endif
 
@@ -146,6 +155,7 @@ static inline int cpu_to_mem(int cpu)
 static inline void set_cpu_numa_mem(int cpu, int node)
 {
 	per_cpu(_numa_mem_, cpu) = node;
+	_node_numa_mem_[cpu_to_node(cpu)] = node;
 }
 #endif
 
@@ -159,6 +169,13 @@ static inline int numa_mem_id(void)
 }
 #endif
 
+#ifndef node_to_mem_node
+static inline int node_to_mem_node(int node)
+{
+	return node;
+}
+#endif
+
 #ifndef cpu_to_mem
 static inline int cpu_to_mem(int cpu)
 {
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 18cee0d4c8a2..0883c42936d4 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -85,6 +85,7 @@ EXPORT_PER_CPU_SYMBOL(numa_node);
  */
 DEFINE_PER_CPU(int, _numa_mem_);		/* Kernel "local memory" node */
 EXPORT_PER_CPU_SYMBOL(_numa_mem_);
+int _node_numa_mem_[MAX_NUMNODES];
 #endif
 
 /*

^ permalink raw reply related

* [PATCH 0/3] Improve slab consumption with memoryless nodes
From: Nishanth Aravamudan @ 2014-09-09 19:01 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Han Pingtian, Matt Mackall, David Rientjes, Pekka Enberg,
	Linux Memory Management List, Paul Mackerras, Tejun Heo,
	Joonsoo Kim, linuxppc-dev, Christoph Lameter, Wanpeng Li,
	Anton Blanchard

Anton noticed (http://www.spinics.net/lists/linux-mm/msg67489.html) that
on ppc LPARs with memoryless nodes, a large amount of memory was
consumed by slabs and was marked unreclaimable. He tracked it down to
slab deactivations in the SLUB core when we allocate remotely, leading
to poor efficiency always when memoryless nodes are present.

After much discussion, Joonsoo provided a few patches that help
significantly. They don't resolve the problem altogether:

 - memory hotplug still needs testing, that is when a memoryless node
   becomes memory-ful, we want to dtrt
 - there are other reasons for going off-node than memoryless nodes,
   e.g., fully exhausted local nodes

Neither case is resolved with this series, but I don't think that should
block their acceptance, as they can be explored/resolved with follow-on
patches.

The series consists of:

[1/3] topology: add support for node_to_mem_node() to determine the fallback node
[2/3] slub: fallback to node_to_mem_node() node if allocating on memoryless node

 - Joonsoo's patches to cache the nearest node with memory for each
   NUMA node

[3/3] Partial revert of 81c98869faa5 (""kthread: ensure locality of task_struct allocations")

 - At Tejun's request, keep the knowledge of memoryless node fallback to
   the allocator core.

 include/linux/topology.h | 17 +++++++++++++++++
 kernel/kthread.c         |  2 +-
 mm/page_alloc.c          |  1 +
 mm/slub.c                | 24 ++++++++++++++++++------
 4 files changed, 37 insertions(+), 7 deletions(-)

^ permalink raw reply

* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Nicolin Chen @ 2014-09-09 18:41 UTC (permalink / raw)
  To: Mark Brown
  Cc: Shengjiu Wang, alsa-devel, lgirdwood, tiwai, linux-kernel, timur,
	perex, Li.Xiubo, linuxppc-dev
In-Reply-To: <20140909181516.GI2601@sirena.org.uk>

On Tue, Sep 09, 2014 at 07:15:16PM +0100, Mark Brown wrote:
> On Tue, Sep 09, 2014 at 11:03:10AM -0700, Nicolin Chen wrote:
> > On Tue, Sep 09, 2014 at 12:27:50PM +0100, Mark Brown wrote:
> > > On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote:
> > > > -	ssi_private->clk = devm_clk_get(&pdev->dev, NULL);
> > > > +	ssi_private->clk = devm_clk_get(&pdev->dev, "ipg");
> 
> > > Why is this change being made?  It wasn't mentioned in the commit log
> > > and doesn't seem relevant to moving where the enable and disable are
> > > done which is what the patch is supposed to be doing...
> 
> > I think Shengjiu is trying to keep the clock disabled while SSI's idle.
> > The current driver enables ipg clock anyway even if there's no stream
> > running.
> 
> > Apparently, these should be put into the comment log.
> 
> I got that bit.  However as well as changing where the enable and
> disable take place this is also changing from requesting a clock with a
> NULL to requesting one called "ipg".

Understood. Making one patch do one single change is the rule we should
always follow.

^ permalink raw reply

* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Nicolin Chen @ 2014-09-09 18:38 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: alsa-devel, lgirdwood, tiwai, Li.Xiubo, timur, perex, broonie,
	mpa, linuxppc-dev, linux-kernel
In-Reply-To: <e5a50d8bfde3aea5fcd092118da5aea4072c7e2f.1410254125.git.shengjiu.wang@freescale.com>

On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote:
> @@ -1321,7 +1333,11 @@ static int fsl_ssi_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>  
> -	ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> +	if (ssi_private->soc->imx)
> +		ssi_private->regs = devm_regmap_init_mmio_clk(&pdev->dev,
> +			"ipg", iomem, &fsl_ssi_regconfig);
> +	else
> +		ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,

As Markus mentioned, the key point here is to be compatible with those
non-clock-name platforms.

I think it would be safer to keep the current code while adding an extra
clk_disable_unprepare() at the end of probe() as a common routine. And
meantime, make sure to have the call for imx only because it seems that
the other platforms do not depend on the clock. //a bit guessing here :)

Then we can get a patch like:
open() {
+	clk_prepare_enable();
	....
}

close() {
	....
+	clk_disable_unprepare()
}

probe() {
	clk_get();
	clk_prepare_enable();
	....
	if (xxx)
-		goto err_xx;
+		return ret;
	....
+	clk_disable_unprepare();
	return 0;
-err_xx:
-	clk_disable_unprepare()
}

remove() {
	....
-	clk_disable_unprepare()
}

As long as you make the subject clear as 'Don't enable core/ipg clock
when SSI's idle', I'm sure you can make them within a single patch.

And an alternative way for open() and close() is to put those code into
pm_runtime_resume/suspend() instead (since we might have some internal
code need to be added by using pm_runtime as well), which would make
the further code neater IMO.

Thank you
Nicolin

^ permalink raw reply

* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Mark Brown @ 2014-09-09 18:15 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: Shengjiu Wang, alsa-devel, lgirdwood, tiwai, linux-kernel, timur,
	perex, Li.Xiubo, linuxppc-dev
In-Reply-To: <20140909180309.GA6784@Asurada>

[-- Attachment #1: Type: text/plain, Size: 909 bytes --]

On Tue, Sep 09, 2014 at 11:03:10AM -0700, Nicolin Chen wrote:
> On Tue, Sep 09, 2014 at 12:27:50PM +0100, Mark Brown wrote:
> > On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote:
> > > -	ssi_private->clk = devm_clk_get(&pdev->dev, NULL);
> > > +	ssi_private->clk = devm_clk_get(&pdev->dev, "ipg");

> > Why is this change being made?  It wasn't mentioned in the commit log
> > and doesn't seem relevant to moving where the enable and disable are
> > done which is what the patch is supposed to be doing...

> I think Shengjiu is trying to keep the clock disabled while SSI's idle.
> The current driver enables ipg clock anyway even if there's no stream
> running.

> Apparently, these should be put into the comment log.

I got that bit.  However as well as changing where the enable and
disable take place this is also changing from requesting a clock with a
NULL to requesting one called "ipg".

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply

* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Nicolin Chen @ 2014-09-09 18:03 UTC (permalink / raw)
  To: Mark Brown
  Cc: Shengjiu Wang, alsa-devel, lgirdwood, tiwai, linux-kernel, timur,
	perex, Li.Xiubo, linuxppc-dev
In-Reply-To: <20140909112750.GS2601@sirena.org.uk>

On Tue, Sep 09, 2014 at 12:27:50PM +0100, Mark Brown wrote:
> On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote:
> > -	ssi_private->clk = devm_clk_get(&pdev->dev, NULL);
> > +	ssi_private->clk = devm_clk_get(&pdev->dev, "ipg");
> >  	if (IS_ERR(ssi_private->clk)) {
> >  		ret = PTR_ERR(ssi_private->clk);
> > -		dev_err(&pdev->dev, "could not get clock: %d\n", ret);
> > -		return ret;
> 
> Why is this change being made?  It wasn't mentioned in the commit log
> and doesn't seem relevant to moving where the enable and disable are
> done which is what the patch is supposed to be doing...

I think Shengjiu is trying to keep the clock disabled while SSI's idle.
The current driver enables ipg clock anyway even if there's no stream
running.

Apparently, these should be put into the comment log.

Thank you
Nicolin

^ permalink raw reply

* [PATCH 2/2 v6] powerpc/kvm: common sw breakpoint instr across ppc
From: Madhavan Srinivasan @ 2014-09-09 17:07 UTC (permalink / raw)
  To: agraf, benh, paulus, mpe; +Cc: Madhavan Srinivasan, linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1410282456-11287-1-git-send-email-maddy@linux.vnet.ibm.com>

This patch extends the use of illegal instruction as software
breakpoint instruction across the ppc platform. Patch extends
booke program interrupt code to support software breakpoint.

Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
Patch is only compile tested. Will really help if
someone can try it out and let me know the comments.

 arch/powerpc/include/asm/kvm_booke.h |  2 --
 arch/powerpc/kvm/booke.c             | 19 ++++++++++++++++++-
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_booke.h b/arch/powerpc/include/asm/kvm_booke.h
index f7aa5cc..ab7123a 100644
--- a/arch/powerpc/include/asm/kvm_booke.h
+++ b/arch/powerpc/include/asm/kvm_booke.h
@@ -30,8 +30,6 @@
 #define EHPRIV_OC_SHIFT			11
 /* "ehpriv 1" : ehpriv with OC = 1 is used for debug emulation */
 #define EHPRIV_OC_DEBUG			1
-#define KVMPPC_INST_EHPRIV_DEBUG	(KVMPPC_INST_EHPRIV | \
-					 (EHPRIV_OC_DEBUG << EHPRIV_OC_SHIFT))
 
 static inline void kvmppc_set_gpr(struct kvm_vcpu *vcpu, int num, ulong val)
 {
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index b4c89fa..365e85d 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -870,6 +870,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
 	case BOOKE_INTERRUPT_HV_PRIV:
 		emulated = kvmppc_get_last_inst(vcpu, false, &last_inst);
 		break;
+	case BOOKE_INTERRUPT_PROGRAM:
+		/* SW breakpoints arrive as illegal instructions on HV */
+		if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
+			emulated = kvmppc_get_last_inst(vcpu, false, &last_inst);
+		break;
 	default:
 		break;
 	}
@@ -947,6 +952,18 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
 		break;
 
 	case BOOKE_INTERRUPT_PROGRAM:
+		if ((vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) &&
+			(last_inst == KVMPPC_INST_SW_BREAKPOINT)) {
+			/*
+			 * We are here because of an SW breakpoint instr,
+			 * so lets return to host to handle.
+			 */
+			r = kvmppc_handle_debug(run, vcpu);
+			run->exit_reason = KVM_EXIT_DEBUG;
+			kvmppc_account_exit(vcpu, DEBUG_EXITS);
+			break;
+		}
+
 		if (vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) {
 			/*
 			 * Program traps generated by user-level software must
@@ -1505,7 +1522,7 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
 		val = get_reg_val(reg->id, vcpu->arch.tsr);
 		break;
 	case KVM_REG_PPC_DEBUG_INST:
-		val = get_reg_val(reg->id, KVMPPC_INST_EHPRIV_DEBUG);
+		val = get_reg_val(reg->id, KVMPPC_INST_SW_BREAKPOINT);
 		break;
 	case KVM_REG_PPC_VRSAVE:
 		val = get_reg_val(reg->id, vcpu->arch.vrsave);
-- 
1.7.11.4

^ permalink raw reply related

* [PATCH 0/2 v6] powerpc/kvm: support to handle sw breakpoint
From: Madhavan Srinivasan @ 2014-09-09 17:07 UTC (permalink / raw)
  To: agraf, benh, paulus, mpe; +Cc: Madhavan Srinivasan, linuxppc-dev, kvm-ppc, kvm

This patchset adds ppc64 server side support for software breakpoint
and extends the use of illegal instruction as software
breakpoint across ppc platform.

Patch 1, adds kernel side support for software breakpoint.
Design is that, by using an illegal instruction, we trap to
hypervisor via Emulation Assistance interrupt, where we check
for the illegal instruction and accordingly we return to Host
or Guest. Patch also adds support for software breakpoint
in PR KVM.

Patch 2,extends the use of illegal instruction as software
breakpoint instruction across the ppc platform. Patch extends
booke program interrupt code to support software breakpoint.

Patch 2 is only compile tested. Will really help if
someone can try it out and let me know comments.

Changes v5->v6:
 Fixed checkpatch.pl errors
 Added abort case as a seperate condition block in
  emulation function in book3s_hv
 Added debug active check for instruction emulation
 Modified return value to emulate_fail in else part
  of illegal instruction case in book3s_pr.c
 Removed KVMPPC_INST_EHPRIV_DEBUG instruction
 Moved the debug instruction as a separate block in
   program interrupt code

Changes v4->v5:
 Made changes to code comments and commit messages
 Added debugging active checks for illegal instr comparison
 Added debug instruction check in emulate code
 Extended SW breakpoint to booke

Changes v3->v4:
 Made changes to code comments and removed #define of zero opcode
 Added a new function to handle the debug instruction emulation in book3s_hv
 Rebased the code to latest upstream source.

Changes v2->v3:
 Changed the debug instructions. Using the all zero opcode in the instruction word
  as illegal instruction as mentioned in Power ISA instead of ABS
 Removed reg updated in emulation assist and added a call to
  kvmppc_emulate_instruction for reg update.

Changes v1->v2:

 Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM can also share it.
 Added code to use KVM get one reg infrastructure to get debug opcode.
 Updated emulate.c to include emulation of debug instruction incase of PR_KVM.
 Made changes to commit message.

Madhavan Srinivasan (2):
  powerpc/kvm: support to handle sw breakpoint
  powerpc/kvm: common sw breakpoint instr across ppc

 arch/powerpc/include/asm/kvm_booke.h |  2 --
 arch/powerpc/include/asm/kvm_ppc.h   |  6 ++++++
 arch/powerpc/kvm/book3s.c            |  3 ++-
 arch/powerpc/kvm/book3s_hv.c         | 41 ++++++++++++++++++++++++++++++++----
 arch/powerpc/kvm/book3s_pr.c         |  3 +++
 arch/powerpc/kvm/booke.c             | 19 ++++++++++++++++-
 arch/powerpc/kvm/emulate.c           | 15 +++++++++++++
 7 files changed, 81 insertions(+), 8 deletions(-)

-- 
1.7.11.4

^ permalink raw reply

* [PATCH 1/2 v6] powerpc/kvm: support to handle sw breakpoint
From: Madhavan Srinivasan @ 2014-09-09 17:07 UTC (permalink / raw)
  To: agraf, benh, paulus, mpe; +Cc: Madhavan Srinivasan, linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1410282456-11287-1-git-send-email-maddy@linux.vnet.ibm.com>

This patch adds kernel side support for software breakpoint.
Design is that, by using an illegal instruction, we trap to hypervisor
via Emulation Assistance interrupt, where we check for the illegal instruction
and accordingly we return to Host or Guest. Patch also adds support for
software breakpoint in PR KVM.

Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/kvm_ppc.h |  6 ++++++
 arch/powerpc/kvm/book3s.c          |  3 ++-
 arch/powerpc/kvm/book3s_hv.c       | 41 ++++++++++++++++++++++++++++++++++----
 arch/powerpc/kvm/book3s_pr.c       |  3 +++
 arch/powerpc/kvm/emulate.c         | 15 ++++++++++++++
 5 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index fb86a22..dd83c9a 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -38,6 +38,12 @@
 #include <asm/paca.h>
 #endif
 
+/*
+ * KVMPPC_INST_SW_BREAKPOINT is debug Instruction
+ * for supporting software breakpoint.
+ */
+#define KVMPPC_INST_SW_BREAKPOINT	0x00dddd00
+
 enum emulation_result {
 	EMULATE_DONE,         /* no further processing */
 	EMULATE_DO_MMIO,      /* kvm_run filled with MMIO request */
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index dd03f6b..00e9c9f 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -778,7 +778,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
 int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
 					struct kvm_guest_debug *dbg)
 {
-	return -EINVAL;
+	vcpu->guest_debug = dbg->control;
+	return 0;
 }
 
 void kvmppc_decrementer_func(unsigned long data)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 27cced9..000fbec 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -725,6 +725,30 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
 	return kvmppc_hcall_impl_hv_realmode(cmd);
 }
 
+static int kvmppc_emulate_debug_inst(struct kvm_run *run,
+					struct kvm_vcpu *vcpu)
+{
+	u32 last_inst;
+
+	if (kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) !=
+					EMULATE_DONE) {
+		/*
+		 * Fetch failed, so return to guest and
+		 * try executing it again.
+		 */
+		return RESUME_GUEST;
+	}
+
+	if (last_inst == KVMPPC_INST_SW_BREAKPOINT) {
+		run->exit_reason = KVM_EXIT_DEBUG;
+		run->debug.arch.address = kvmppc_get_pc(vcpu);
+		return RESUME_HOST;
+	} else {
+		kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+		return RESUME_GUEST;
+	}
+}
+
 static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
 				 struct task_struct *tsk)
 {
@@ -807,12 +831,18 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
 		break;
 	/*
 	 * This occurs if the guest executes an illegal instruction.
-	 * We just generate a program interrupt to the guest, since
-	 * we don't emulate any guest instructions at this stage.
+	 * If the guest debug is disabled, generate a program interrupt
+	 * to the guest. If guest debug is enabled, we need to check
+	 * whether the instruction is a software breakpoint instruction.
+	 * Accordingly return to Guest or Host.
 	 */
 	case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
-		kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
-		r = RESUME_GUEST;
+		if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) {
+			r = kvmppc_emulate_debug_inst(run, vcpu);
+		} else {
+			kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+			r = RESUME_GUEST;
+		}
 		break;
 	/*
 	 * This occurs if the guest (kernel or userspace), does something that
@@ -922,6 +952,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
 	long int i;
 
 	switch (id) {
+	case KVM_REG_PPC_DEBUG_INST:
+		*val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT);
+		break;
 	case KVM_REG_PPC_HIOR:
 		*val = get_reg_val(id, 0);
 		break;
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index faffb27..6d73708 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -1319,6 +1319,9 @@ static int kvmppc_get_one_reg_pr(struct kvm_vcpu *vcpu, u64 id,
 	int r = 0;
 
 	switch (id) {
+	case KVM_REG_PPC_DEBUG_INST:
+		*val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT);
+		break;
 	case KVM_REG_PPC_HIOR:
 		*val = get_reg_val(id, to_book3s(vcpu)->hior);
 		break;
diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
index e96b50d..005222b 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -274,6 +274,21 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 		}
 		break;
 
+	case 0:
+		/*
+		 * Instruction with primary opcode 0. Based on PowerISA
+		 * these are illegal instructions.
+		 */
+		if (inst == KVMPPC_INST_SW_BREAKPOINT) {
+			run->exit_reason = KVM_EXIT_DEBUG;
+			run->debug.arch.address = kvmppc_get_pc(vcpu);
+			emulated = EMULATE_EXIT_USER;
+			advance = 0;
+		} else
+			emulated = EMULATE_FAIL;
+
+		break;
+
 	default:
 		emulated = EMULATE_FAIL;
 	}
-- 
1.7.11.4

^ permalink raw reply related

* Re: [PATCH v2 3/5] ARM: mvebu: Change vendor prefix for Intersil Corporation to isil
From: Jason Cooper @ 2014-09-09 16:05 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Mark Rutland, devicetree, Alessandro Zummo, Russell King,
	Pawel Moll, Ian Campbell, Mark Brown, Wolfram Sang, Rob Herring,
	Paul Mackerras, Kumar Gala, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1410167960-554-4-git-send-email-p.zabel@pengutronix.de>

On Mon, Sep 08, 2014 at 11:19:18AM +0200, Philipp Zabel wrote:
> Currently there is a wild mixture of isl, isil, and intersil
> compatibles in the kernel. At this point, changing the vendor
> symbol to the most often used variant, which is equal to the
> NASDAQ symbol, isil, should not hurt.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  arch/arm/boot/dts/armada-370-netgear-rn102.dts | 2 +-
>  arch/arm/boot/dts/armada-370-netgear-rn104.dts | 2 +-
>  arch/arm/boot/dts/armada-xp-netgear-rn2120.dts | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Applied to mvebu/dt

thx,

Jason.

^ permalink raw reply

* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Timur Tabi @ 2014-09-09 15:24 UTC (permalink / raw)
  To: Mark Brown
  Cc: Shengjiu Wang, alsa-devel, tiwai, linux-kernel, lgirdwood, perex,
	nicoleotsuka, Li.Xiubo, linuxppc-dev
In-Reply-To: <20140909152114.GA2601@sirena.org.uk>

On 09/09/2014 10:21 AM, Mark Brown wrote:
>> if (ssi_private->clk)
>> >	clk_prepare_enable(ssi_private->clk);

> Should be a !IS_ERR() - NULL is a valid clock.

In that case, ssi_private->clk needs to be initialized to -EINVAL or 
something, so that the check works on systems that don't have any clocks.

^ permalink raw reply

* Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Mark Brown @ 2014-09-09 15:21 UTC (permalink / raw)
  To: Timur Tabi
  Cc: Shengjiu Wang, alsa-devel, tiwai, linux-kernel, lgirdwood, perex,
	nicoleotsuka, Li.Xiubo, linuxppc-dev
In-Reply-To: <540EFDFF.7050804@tabi.org>

[-- Attachment #1: Type: text/plain, Size: 300 bytes --]

On Tue, Sep 09, 2014 at 08:17:51AM -0500, Timur Tabi wrote:
> Shengjiu Wang wrote:
> >+	if (ssi_private->soc->imx)
> >+		clk_prepare_enable(ssi_private->clk);

> How about this instead?

> if (ssi_private->clk)
> 	clk_prepare_enable(ssi_private->clk);

Should be a !IS_ERR() - NULL is a valid clock.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply

* [PATCH v2] PowerPC: boot: Parse chosen/cmdline-timeout parameter
From: Simon Kågström @ 2014-09-09 14:40 UTC (permalink / raw)
  To: linuxppc-dev, grant.likely, geert, scottwood

A 5 second timeout during boot might be too long, so make it
configurable. Run the loop at least once to let the user stop the boot
by holding a key pressed.

The property is added to the chosen node, e.g.,

	chosen {
		bootargs = "console=ttyUL0 root=/dev/ram0";
		linux,stdout-path = "/plb@0/serial@46000000";
		linux,cmdline-timeout = <100>;
	} ;

Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---

ChangeLog:

v2:
- Rename the property linux,cmdline-timeout (Grant Likely)
- Run the loop at least once to allow (Grant Likely)

 arch/powerpc/boot/main.c   |   11 ++++++++++-
 arch/powerpc/boot/ops.h    |    2 +-
 arch/powerpc/boot/serial.c |    6 +++---
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/boot/main.c b/arch/powerpc/boot/main.c
index a28f021..c1931bb 100644
--- a/arch/powerpc/boot/main.c
+++ b/arch/powerpc/boot/main.c
@@ -144,13 +144,22 @@ static char cmdline[COMMAND_LINE_SIZE]
 
 static void prep_cmdline(void *chosen)
 {
+	unsigned int getline_timeout = 5000;
+	int v;
+	int n;
+
+	/* Wait-for-input time */
+	n = getprop(chosen, "linux,cmdline-timeout", &v, sizeof(v));
+	if (n == sizeof(v))
+		getline_timeout = v;
+
 	if (cmdline[0] == '\0')
 		getprop(chosen, "bootargs", cmdline, COMMAND_LINE_SIZE-1);
 
 	printf("\n\rLinux/PowerPC load: %s", cmdline);
 	/* If possible, edit the command line */
 	if (console_ops.edit_cmdline)
-		console_ops.edit_cmdline(cmdline, COMMAND_LINE_SIZE);
+		console_ops.edit_cmdline(cmdline, COMMAND_LINE_SIZE, getline_timeout);
 	printf("\n\r");
 
 	/* Put the command line back into the devtree for the kernel */
diff --git a/arch/powerpc/boot/ops.h b/arch/powerpc/boot/ops.h
index b3218ce..c42ea70 100644
--- a/arch/powerpc/boot/ops.h
+++ b/arch/powerpc/boot/ops.h
@@ -58,7 +58,7 @@ extern struct dt_ops dt_ops;
 struct console_ops {
 	int	(*open)(void);
 	void	(*write)(const char *buf, int len);
-	void	(*edit_cmdline)(char *buf, int len);
+	void	(*edit_cmdline)(char *buf, int len, unsigned int getline_timeout);
 	void	(*close)(void);
 	void	*data;
 };
diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c
index f2156f0..167ee94 100644
--- a/arch/powerpc/boot/serial.c
+++ b/arch/powerpc/boot/serial.c
@@ -33,7 +33,7 @@ static void serial_write(const char *buf, int len)
 		scdp->putc(*buf++);
 }
 
-static void serial_edit_cmdline(char *buf, int len)
+static void serial_edit_cmdline(char *buf, int len, unsigned int timeout)
 {
 	int timer = 0, count;
 	char ch, *cp;
@@ -44,7 +44,7 @@ static void serial_edit_cmdline(char *buf, int len)
 	cp = &buf[count];
 	count++;
 
-	while (timer++ < 5*1000) {
+	do {
 		if (scdp->tstc()) {
 			while (((ch = scdp->getc()) != '\n') && (ch != '\r')) {
 				/* Test for backspace/delete */
@@ -70,7 +70,7 @@ static void serial_edit_cmdline(char *buf, int len)
 			break;  /* Exit 'timer' loop */
 		}
 		udelay(1000);  /* 1 msec */
-	}
+	} while (timer++ < timeout);
 	*cp = 0;
 }
 
-- 
1.7.9.6

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox