LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] of/mdio: fix fixed link bus name
From: Baruch Siach @ 2012-02-27 12:48 UTC (permalink / raw)
  To: Grant Likely, Rob Herring
  Cc: netdev, devicetree-discuss, linuxppc-dev, Baruch Siach

Since 9e6c643b (phy/fixed: use an unique MDIO bus name) the name of the fixed
PHY bus is "fixed-0". Teach of_phy_connect_fixed_link() the new name.

Tested on a P1020RDB PowerPC system.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/of/of_mdio.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 980c079..483c0ad 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -182,7 +182,7 @@ struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
 	if (!phy_id || sz < sizeof(*phy_id))
 		return NULL;
 
-	sprintf(bus_id, PHY_ID_FMT, "0", be32_to_cpu(phy_id[0]));
+	sprintf(bus_id, PHY_ID_FMT, "fixed-0", be32_to_cpu(phy_id[0]));
 
 	phy = phy_connect(dev, bus_id, hndlr, 0, iface);
 	return IS_ERR(phy) ? NULL : phy;
-- 
1.7.9

^ permalink raw reply related

* Re: [PATCH 1/2] powerpc: Move GE GPIO and PIC drivers
From: Martyn Welch @ 2012-02-27 13:57 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Wim Van Sebroeck, linuxppc-dev, linux-kernel
In-Reply-To: <1330299464.20389.58.camel@pasglop>

On 26/02/12 23:37, Benjamin Herrenschmidt wrote:
> On Tue, 2012-02-07 at 11:28 +0000, Martyn Welch wrote:
>> Move the GE GPIO and PIC drivers to allow these to be used by non-86xx
>> boards.
> 
> Hi, Sorry for the late review...
> 

No problem, thanks for the review!

>> Signed-off-by: Martyn Welch <martyn.welch@ge.com>
>> ---
>>  arch/powerpc/platforms/86xx/Kconfig      |    3 +
>>  arch/powerpc/platforms/86xx/Makefile     |    7 +-
>>  arch/powerpc/platforms/86xx/gef_gpio.c   |  171 --------------------
>>  arch/powerpc/platforms/86xx/gef_pic.c    |  252 ------------------------------
>>  arch/powerpc/platforms/86xx/gef_pic.h    |   11 --
>>  arch/powerpc/platforms/86xx/gef_ppc9a.c  |    3 +-
>>  arch/powerpc/platforms/86xx/gef_sbc310.c |    3 +-
>>  arch/powerpc/platforms/86xx/gef_sbc610.c |    3 +-
>>  arch/powerpc/platforms/Kconfig           |    7 +
>>  arch/powerpc/platforms/Makefile          |    3 +
>>  arch/powerpc/platforms/ge_gpio.c         |  171 ++++++++++++++++++++
>>  arch/powerpc/platforms/ge_pic.c          |  252 ++++++++++++++++++++++++++++++
>>  arch/powerpc/platforms/ge_pic.h          |   11 ++
> 
> So I don't like having files showing up there. In fact, I want to move
> the only other one here, it's not the right place for it
> (fsl_uli1575.c).
> 

This patch (or one like it) has been around for a while now. Kumar wanted me
to put them here rather than sysdev[1], but I'm easy either way.

> Please contemplate using arch/powerpc/sysdev instead. Maybe make a
> subdir in there (geip or something like that ?)
> 

I'd rather avoid "geip" (we seem to have a habit of renaming divisions), would
"ge" be acceptable?

> Also, use git mv so that the file moves appear as such in the history,
> this will make review easier by clearly separating the move from actual
> changes to the files.
> 

Hmm, thought I'd done that. Will try again.

Martyn

[1] http://old.nabble.com/GE-GPIO-and-PIC-support.-td27212938.html

-- 
Martyn Welch (Lead Software Engineer)  | Registered in England and Wales
GE Intelligent Platforms               | (3828642) at 100 Barbirolli Square
T +44(0)1327322748                     | Manchester, M2 3AB
E martyn.welch@ge.com                  | VAT:GB 927559189

^ permalink raw reply

* Re: [PATCH] of/mdio: fix fixed link bus name
From: Florian Fainelli @ 2012-02-27 14:46 UTC (permalink / raw)
  To: Baruch Siach; +Cc: netdev, devicetree-discuss, linuxppc-dev, Rob Herring
In-Reply-To: <8ebe6a3f11343c392a7353581d5f648980206e8e.1330346765.git.baruch@tkos.co.il>



Le 02/27/12 13:48, Baruch Siach a écrit :
> Since 9e6c643b (phy/fixed: use an unique MDIO bus name) the name of the fixed
> PHY bus is "fixed-0". Teach of_phy_connect_fixed_link() the new name.
>
> Tested on a P1020RDB PowerPC system.
>
> Signed-off-by: Baruch Siach<baruch@tkos.co.il>

Acked-by: Florian Fainelli <florian@openwrt.org>

> ---
>   drivers/of/of_mdio.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 980c079..483c0ad 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -182,7 +182,7 @@ struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
>   	if (!phy_id || sz<  sizeof(*phy_id))
>   		return NULL;
>
> -	sprintf(bus_id, PHY_ID_FMT, "0", be32_to_cpu(phy_id[0]));
> +	sprintf(bus_id, PHY_ID_FMT, "fixed-0", be32_to_cpu(phy_id[0]));
>
>   	phy = phy_connect(dev, bus_id, hndlr, 0, iface);
>   	return IS_ERR(phy) ? NULL : phy;

^ permalink raw reply

* Re: [PATCH 1/2] powerpc/e500: make load_up_spe a normal fuction
From: Tabi Timur-B04825 @ 2012-02-27 15:50 UTC (permalink / raw)
  To: Yin Olivia-R63875
  Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org,
	kvm-ppc@vger.kernel.org
In-Reply-To: <1330340388-30296-1-git-send-email-hong-hua.yin@freescale.com>

On Mon, Feb 27, 2012 at 4:59 AM, Olivia Yin <hong-hua.yin@freescale.com> wr=
ote:
> So that we can call it in kernel.

And why would we want that?

--=20
Timur Tabi
Linux kernel developer at Freescale=

^ permalink raw reply

* Re: [PATCH] of/mdio: fix fixed link bus name
From: Rob Herring @ 2012-02-27 15:59 UTC (permalink / raw)
  To: Baruch Siach; +Cc: netdev, devicetree-discuss, linuxppc-dev
In-Reply-To: <8ebe6a3f11343c392a7353581d5f648980206e8e.1330346765.git.baruch@tkos.co.il>

On 02/27/2012 06:48 AM, Baruch Siach wrote:
> Since 9e6c643b (phy/fixed: use an unique MDIO bus name) the name of the fixed
> PHY bus is "fixed-0". Teach of_phy_connect_fixed_link() the new name.
> 

Applied for 3.3.

Rob

> Tested on a P1020RDB PowerPC system.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  drivers/of/of_mdio.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 980c079..483c0ad 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -182,7 +182,7 @@ struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
>  	if (!phy_id || sz < sizeof(*phy_id))
>  		return NULL;
>  
> -	sprintf(bus_id, PHY_ID_FMT, "0", be32_to_cpu(phy_id[0]));
> +	sprintf(bus_id, PHY_ID_FMT, "fixed-0", be32_to_cpu(phy_id[0]));
>  
>  	phy = phy_connect(dev, bus_id, hndlr, 0, iface);
>  	return IS_ERR(phy) ? NULL : phy;

^ permalink raw reply

* RE: [PATCH 24/37] KVM: PPC: booke: rework rescheduling checks
From: Bhushan Bharat-R65777 @ 2012-02-27 16:34 UTC (permalink / raw)
  To: Alexander Graf, kvm-ppc@vger.kernel.org
  Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org,
	kvm@vger.kernel.org
In-Reply-To: <1330093591-19523-25-git-send-email-agraf@suse.de>



> -----Original Message-----
> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On Beh=
alf Of
> Alexander Graf
> Sent: Friday, February 24, 2012 7:56 PM
> To: kvm-ppc@vger.kernel.org
> Cc: kvm@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421
> Subject: [PATCH 24/37] KVM: PPC: booke: rework rescheduling checks
>=20
> Instead of checking whether we should reschedule only when we exited due =
to an
> interrupt, let's always check before entering the guest back again. This =
gets
> the target more in line with the other archs.
>=20
> Also while at it, generalize the whole thing so that eventually we could =
have a
> single kvmppc_prepare_to_enter function for all ppc targets that does sig=
nal and
> reschedule checking for us.
>=20
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/powerpc/include/asm/kvm_ppc.h |    2 +-
>  arch/powerpc/kvm/book3s.c          |    4 ++-
>  arch/powerpc/kvm/booke.c           |   70 ++++++++++++++++++++++++------=
-----
>  3 files changed, 52 insertions(+), 24 deletions(-)
>=20
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h
> b/arch/powerpc/include/asm/kvm_ppc.h
> index e709975..7f0a3da 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -95,7 +95,7 @@ extern int kvmppc_core_vcpu_translate(struct kvm_vcpu *=
vcpu,
> extern void kvmppc_core_vcpu_load(struct kvm_vcpu *vcpu, int cpu);  exter=
n void
> kvmppc_core_vcpu_put(struct kvm_vcpu *vcpu);
>=20
> -extern void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu);
> +extern int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu);
>  extern int kvmppc_core_pending_dec(struct kvm_vcpu *vcpu);  extern void
> kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags);  extern vo=
id
> kvmppc_core_queue_dec(struct kvm_vcpu *vcpu); diff --git
> a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c index 7d54f4e..c8=
ead7b
> 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -258,7 +258,7 @@ static bool clear_irqprio(struct kvm_vcpu *vcpu, unsi=
gned
> int priority)
>  	return true;
>  }
>=20
> -void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
> +int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>  {
>  	unsigned long *pending =3D &vcpu->arch.pending_exceptions;
>  	unsigned long old_pending =3D vcpu->arch.pending_exceptions; @@ -283,6
> +283,8 @@ void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>=20
>  	/* Tell the guest about our interrupt status */
>  	kvmppc_update_int_pending(vcpu, *pending, old_pending);
> +
> +	return 0;
>  }
>=20
>  pfn_t kvmppc_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn) diff --git
> a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 9979be1..3fce=
c2c
> 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -439,8 +439,9 @@ static void kvmppc_core_check_exceptions(struct kvm_v=
cpu
> *vcpu)  }
>=20
>  /* Check pending exceptions and deliver one, if possible. */ -void
> kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
> +int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>  {
> +	int r =3D 0;
>  	WARN_ON_ONCE(!irqs_disabled());
>=20
>  	kvmppc_core_check_exceptions(vcpu);
> @@ -451,8 +452,44 @@ void kvmppc_core_prepare_to_enter(struct kvm_vcpu *v=
cpu)
>  		local_irq_disable();
>=20
>  		kvmppc_set_exit_type(vcpu, EMULATED_MTMSRWE_EXITS);
> -		kvmppc_core_check_exceptions(vcpu);
> +		r =3D 1;
>  	};
> +
> +	return r;
> +}
> +
> +/*
> + * Common checks before entering the guest world.  Call with interrupts
> + * disabled.
> + *
> + * returns !0 if a signal is pending and check_signal is true  */
> +static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu, bool
> +check_signal) {
> +	int r =3D 0;
> +
> +	WARN_ON_ONCE(!irqs_disabled());
> +	while (true) {
> +		if (need_resched()) {
> +			local_irq_enable();
> +			cond_resched();
> +			local_irq_disable();
> +			continue;
> +		}
> +
> +		if (kvmppc_core_prepare_to_enter(vcpu)) {

kvmppc_prepare_to_enter() is called even on heavyweight_exit. Should not th=
is be called only on lightweight_exit?

Thanks
-Bharat

> +			/* interrupts got enabled in between, so we
> +			   are back at square 1 */
> +			continue;
> +		}
> +
> +		if (check_signal && signal_pending(current))
> +			r =3D 1;
> +
> +		break;
> +	}
> +
> +	return r;
>  }
>=20
>  int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) @@ -=
470,10
> +507,7 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *v=
cpu)
>  	}
>=20
>  	local_irq_disable();
> -
> -	kvmppc_core_prepare_to_enter(vcpu);
> -
> -	if (signal_pending(current)) {
> +	if (kvmppc_prepare_to_enter(vcpu, true)) {
>  		kvm_run->exit_reason =3D KVM_EXIT_INTR;
>  		ret =3D -EINTR;
>  		goto out;
> @@ -598,25 +632,21 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
> kvm_vcpu *vcpu,
>=20
>  	switch (exit_nr) {
>  	case BOOKE_INTERRUPT_MACHINE_CHECK:
> -		kvm_resched(vcpu);
>  		r =3D RESUME_GUEST;
>  		break;
>=20
>  	case BOOKE_INTERRUPT_EXTERNAL:
>  		kvmppc_account_exit(vcpu, EXT_INTR_EXITS);
> -		kvm_resched(vcpu);
>  		r =3D RESUME_GUEST;
>  		break;
>=20
>  	case BOOKE_INTERRUPT_DECREMENTER:
>  		kvmppc_account_exit(vcpu, DEC_EXITS);
> -		kvm_resched(vcpu);
>  		r =3D RESUME_GUEST;
>  		break;
>=20
>  	case BOOKE_INTERRUPT_DOORBELL:
>  		kvmppc_account_exit(vcpu, DBELL_EXITS);
> -		kvm_resched(vcpu);
>  		r =3D RESUME_GUEST;
>  		break;
>=20
> @@ -865,19 +895,15 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
> kvm_vcpu *vcpu,
>  		BUG();
>  	}
>=20
> +	/*
> +	 * To avoid clobbering exit_reason, only check for signals if we
> +	 * aren't already exiting to userspace for some other reason.
> +	 */
>  	local_irq_disable();
> -
> -	kvmppc_core_prepare_to_enter(vcpu);
> -
> -	if (!(r & RESUME_HOST)) {
> -		/* To avoid clobbering exit_reason, only check for signals if
> -		 * we aren't already exiting to userspace for some other
> -		 * reason. */
> -		if (signal_pending(current)) {
> -			run->exit_reason =3D KVM_EXIT_INTR;
> -			r =3D (-EINTR << 2) | RESUME_HOST | (r & RESUME_FLAG_NV);
> -			kvmppc_account_exit(vcpu, SIGNAL_EXITS);
> -		}
> +	if (kvmppc_prepare_to_enter(vcpu, !(r & RESUME_HOST))) {
> +		run->exit_reason =3D KVM_EXIT_INTR;
> +		r =3D (-EINTR << 2) | RESUME_HOST | (r & RESUME_FLAG_NV);
> +		kvmppc_account_exit(vcpu, SIGNAL_EXITS);
>  	}
>=20
>  	return r;
> --
> 1.6.0.2
>=20
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in the bod=
y of a
> message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 24/37] KVM: PPC: booke: rework rescheduling checks
From: Alexander Graf @ 2012-02-27 17:33 UTC (permalink / raw)
  To: Bhushan Bharat-R65777
  Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org,
	kvm@vger.kernel.org, kvm-ppc@vger.kernel.org
In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D141083@039-SN2MPN1-023.039d.mgd.msft.net>

On 02/27/2012 05:34 PM, Bhushan Bharat-R65777 wrote:
>
>> +}
>> +
>> +/*
>> + * Common checks before entering the guest world.  Call with interrupts
>> + * disabled.
>> + *
>> + * returns !0 if a signal is pending and check_signal is true  */
>> +static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu, bool
>> +check_signal) {
>> +	int r = 0;
>> +
>> +	WARN_ON_ONCE(!irqs_disabled());
>> +	while (true) {
>> +		if (need_resched()) {
>> +			local_irq_enable();
>> +			cond_resched();
>> +			local_irq_disable();
>> +			continue;
>> +		}
>> +
>> +		if (kvmppc_core_prepare_to_enter(vcpu)) {
> kvmppc_prepare_to_enter() is called even on heavyweight_exit. Should not this be called only on lightweight_exit?

Yeah, we don't need to call it when exiting anyways. That's a functional 
change though, which this patch is trying not to introduce. So we should 
rather do that as a patch on top.

Alex

^ permalink raw reply

* Re: [PATCH] powerpc: icswx: fix race condition where threads do not get their ACOP register updated in time.
From: Jimi Xenidis @ 2012-02-27 17:56 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Anton Blanchard
In-Reply-To: <1330300068.20389.63.camel@pasglop>


On Feb 26, 2012, at 5:47 PM, Benjamin Herrenschmidt wrote:

>=20
>>=20
>> +	/*
>> +	 * We could be here because another thread has enabled acop
>> +	 * but the ACOP register has yet to be updated.
>> +	 *
>> +	 * This should have been taken care of by the IPI to sync all
>> +	 * the threads (see smp_call_function(sync_cop, mm, 1)), but
>> +	 * that could take forever if there are a significant amount
>> +	 * of threads.
>> +	 *
>> +	 * Given the number of threads on some of these systems,
>> +	 * perhaps this is the best way to sync ACOP rather than whack
>> +	 * every thread with an IPI.
>> +	 */
>=20
> This is actually pretty standard stuff... If it was me I would make it
> all lazy and avoid the IPI completely but it doesn't necessarily hurt
> that much.

I'm happy to get rid of the IPI completely if Anton (or someone else) =
agrees to test on his end.
If not, do you want me to reduce the comment?


> In any case the "recovery" is indeed needed and you should
> probably also remove the pr_debug, it's really just spam.

ack

>=20
>> +	if (acop_copro_type_bit(ct) && current->active_mm->context.acop) =
{
>=20
> Shouldn't that be "&" ? In fact, gcc would even warn so either make
> it acop_check_copro(acop, ct) or do a (x & y) !=3D 0

Ach!! nice catch!
-jx

>=20
> Cheers,
> Ben.
>=20
>> +		pr_debug("%s[%d]: Spurrious ACOP Fault, CT: %d, bit: =
0x%llx "
>> +			"SPR: 0x%lx, mm->acop: 0x%lx\n",
>> +			current->comm, current->pid,
>> +			ct, acop_copro_type_bit(ct), mfspr(SPRN_ACOP),
>> +			current->active_mm->context.acop);
>> +
>> +		sync_cop(current->active_mm);
>> +		return 0;
>> +	}
>> +
>> +	/* check for alternate policy */
>> 	if (!acop_use_cop(ct))
>> 		return 0;
>>=20
>> 	/* at this point the CT is unknown to the system */
>> -	pr_warn("%s[%d]: Coprocessor %d is unavailable",
>> +	pr_warn("%s[%d]: Coprocessor %d is unavailable\n",
>> 		current->comm, current->pid, ct);
>>=20
>> 	/* get inst if we don't already have it */
>> diff --git a/arch/powerpc/mm/icswx.h b/arch/powerpc/mm/icswx.h
>> index 42176bd..6dedc08 100644
>> --- a/arch/powerpc/mm/icswx.h
>> +++ b/arch/powerpc/mm/icswx.h
>> @@ -59,4 +59,10 @@ extern void free_cop_pid(int free_pid);
>>=20
>> extern int acop_handle_fault(struct pt_regs *regs, unsigned long =
address,
>> 			     unsigned long error_code);
>> +
>> +static inline u64 acop_copro_type_bit(unsigned int type)
>> +{
>> +	return 1ULL << (63 - type);
>> +}
>> +
>> #endif /* !_ARCH_POWERPC_MM_ICSWX_H_ */
>=20
>=20

^ permalink raw reply

* Re: [PATCH 24/37] KVM: PPC: booke: rework rescheduling checks
From: Alexander Graf @ 2012-02-27 18:23 UTC (permalink / raw)
  To: Bhushan Bharat-R65777
  Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org,
	kvm@vger.kernel.org, kvm-ppc@vger.kernel.org
In-Reply-To: <4F4BBE55.6020204@suse.de>

On 02/27/2012 06:33 PM, Alexander Graf wrote:
> On 02/27/2012 05:34 PM, Bhushan Bharat-R65777 wrote:
>>
>>> +}
>>> +
>>> +/*
>>> + * Common checks before entering the guest world.  Call with 
>>> interrupts
>>> + * disabled.
>>> + *
>>> + * returns !0 if a signal is pending and check_signal is true  */
>>> +static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu, bool
>>> +check_signal) {
>>> +    int r = 0;
>>> +
>>> +    WARN_ON_ONCE(!irqs_disabled());
>>> +    while (true) {
>>> +        if (need_resched()) {
>>> +            local_irq_enable();
>>> +            cond_resched();
>>> +            local_irq_disable();
>>> +            continue;
>>> +        }
>>> +
>>> +        if (kvmppc_core_prepare_to_enter(vcpu)) {
>> kvmppc_prepare_to_enter() is called even on heavyweight_exit. Should 
>> not this be called only on lightweight_exit?
>
> Yeah, we don't need to call it when exiting anyways. That's a 
> functional change though, which this patch is trying not to introduce. 
> So we should rather do that as a patch on top.

So how about this (warning! broken whitespace)?


diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 7a16b56..616aa2d 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -464,7 +464,7 @@ int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
   *
   * returns !0 if a signal is pending and check_signal is true
   */
-static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu, bool 
check_signal)
+static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
  {
         int r = 0;

@@ -483,7 +483,7 @@ static int kvmppc_prepare_to_enter(struct kvm_vcpu 
*vcpu, bool check_signal)
                         continue;
                 }

-               if (check_signal && signal_pending(current))
+               if (signal_pending(current))
                         r = 1;

                 break;
@@ -507,7 +507,7 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct 
kvm_vcpu *vcpu)
         }

         local_irq_disable();
-       if (kvmppc_prepare_to_enter(vcpu, true)) {
+       if (kvmppc_prepare_to_enter(vcpu)) {
                 kvm_run->exit_reason = KVM_EXIT_INTR;
                 ret = -EINTR;
                 goto out;
@@ -941,13 +941,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
          * To avoid clobbering exit_reason, only check for signals if we
          * aren't already exiting to userspace for some other reason.
          */
-       local_irq_disable();
-       if (kvmppc_prepare_to_enter(vcpu, !(r & RESUME_HOST))) {
-               run->exit_reason = KVM_EXIT_INTR;
-               r = (-EINTR << 2) | RESUME_HOST | (r & RESUME_FLAG_NV);
-               kvmppc_account_exit(vcpu, SIGNAL_EXITS);
+       if (!(r & RESUME_HOST)) {
+               local_irq_disable();
+               if (kvmppc_prepare_to_enter(vcpu)) {
+                       run->exit_reason = KVM_EXIT_INTR;
+                       r = (-EINTR << 2) | RESUME_HOST | (r & 
RESUME_FLAG_NV);
+                       kvmppc_account_exit(vcpu, SIGNAL_EXITS);
+               }
         }

+out:
         return r;
  }

^ permalink raw reply related

* RE: [PATCH 24/37] KVM: PPC: booke: rework rescheduling checks
From: Bhushan Bharat-R65777 @ 2012-02-27 18:29 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org,
	kvm@vger.kernel.org, kvm-ppc@vger.kernel.org
In-Reply-To: <4F4BCA1F.5040905@suse.de>



> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Monday, February 27, 2012 11:53 PM
> To: Bhushan Bharat-R65777
> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; linuxppc-dev@lists.ozla=
bs.org;
> Wood Scott-B07421
> Subject: Re: [PATCH 24/37] KVM: PPC: booke: rework rescheduling checks
>=20
> On 02/27/2012 06:33 PM, Alexander Graf wrote:
> > On 02/27/2012 05:34 PM, Bhushan Bharat-R65777 wrote:
> >>
> >>> +}
> >>> +
> >>> +/*
> >>> + * Common checks before entering the guest world.  Call with
> >>> interrupts
> >>> + * disabled.
> >>> + *
> >>> + * returns !0 if a signal is pending and check_signal is true  */
> >>> +static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu, bool
> >>> +check_signal) {
> >>> +    int r =3D 0;
> >>> +
> >>> +    WARN_ON_ONCE(!irqs_disabled());
> >>> +    while (true) {
> >>> +        if (need_resched()) {
> >>> +            local_irq_enable();
> >>> +            cond_resched();
> >>> +            local_irq_disable();
> >>> +            continue;
> >>> +        }
> >>> +
> >>> +        if (kvmppc_core_prepare_to_enter(vcpu)) {
> >> kvmppc_prepare_to_enter() is called even on heavyweight_exit. Should
> >> not this be called only on lightweight_exit?
> >
> > Yeah, we don't need to call it when exiting anyways. That's a
> > functional change though, which this patch is trying not to introduce.
> > So we should rather do that as a patch on top.
>=20
> So how about this (warning! broken whitespace)?
>=20
>=20
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
> 7a16b56..616aa2d 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -464,7 +464,7 @@ int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcp=
u)
>    *
>    * returns !0 if a signal is pending and check_signal is true
>    */
> -static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu, bool
> check_signal)
> +static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
>   {
>          int r =3D 0;
>=20
> @@ -483,7 +483,7 @@ static int kvmppc_prepare_to_enter(struct kvm_vcpu *v=
cpu,
> bool check_signal)
>                          continue;
>                  }
>=20
> -               if (check_signal && signal_pending(current))
> +               if (signal_pending(current))
>                          r =3D 1;
>=20
>                  break;
> @@ -507,7 +507,7 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct k=
vm_vcpu
> *vcpu)
>          }
>=20
>          local_irq_disable();
> -       if (kvmppc_prepare_to_enter(vcpu, true)) {
> +       if (kvmppc_prepare_to_enter(vcpu)) {
>                  kvm_run->exit_reason =3D KVM_EXIT_INTR;
>                  ret =3D -EINTR;
>                  goto out;
> @@ -941,13 +941,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
> kvm_vcpu *vcpu,
>           * To avoid clobbering exit_reason, only check for signals if we
>           * aren't already exiting to userspace for some other reason.
>           */
> -       local_irq_disable();
> -       if (kvmppc_prepare_to_enter(vcpu, !(r & RESUME_HOST))) {
> -               run->exit_reason =3D KVM_EXIT_INTR;
> -               r =3D (-EINTR << 2) | RESUME_HOST | (r & RESUME_FLAG_NV);
> -               kvmppc_account_exit(vcpu, SIGNAL_EXITS);
> +       if (!(r & RESUME_HOST)) {
> +               local_irq_disable();
> +               if (kvmppc_prepare_to_enter(vcpu)) {
> +                       run->exit_reason =3D KVM_EXIT_INTR;
> +                       r =3D (-EINTR << 2) | RESUME_HOST | (r &
> RESUME_FLAG_NV);
> +                       kvmppc_account_exit(vcpu, SIGNAL_EXITS);
> +               }
>          }
>=20
> +out:

Why?
Otherwise looks ok to me.

Thanks
-Bharat

>          return r;
>   }
>=20
>=20

^ permalink raw reply

* [PATCH] powerpc/prom: remove limit on maximum size of properties
From: Nishanth Aravamudan @ 2012-02-27 18:55 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Nishanth Aravamudan, Anton Blanchard, Paul Mackerras,
	linuxppc-dev, Robert Jennings
In-Reply-To: <1330298534.20389.53.camel@pasglop>

Hi Ben,

On 27.02.2012 [10:22:14 +1100], Benjamin Herrenschmidt wrote:
> On Fri, 2012-02-24 at 16:23 -0800, Nishanth Aravamudan wrote:
> > On a 16TB system (using AMS/CMO), I get:
> > 
> > WARNING: ignoring large property [/ibm,dynamic-reconfiguration-memory] ibm,dynamic-memory length 0x000000000017ffec
> > 
> > and significantly less memory is thus shown to the partition. As far as
> > I can tell, the constant used is arbitrary, but bump it up to 2MB, which
> > covers the above property (approximately 1.5MB).
> > 
> > With this patch, the kernel does see all of the system memory on the
> > 16TB system.
> 
> Why not go all the way to either removing the limit, or setting it to
> something much bigger ? That's just asking to break again when we get an
> even bigger system.

Fair point -- sorry, was just trying to get something out the door that
I had tested, since I have limited access to the hardware in question.

> The limit was originally set because of Apple machines carrying ROM
> images in the device-tree, at a time where we were much more memory
> constrained than we are now.
> 
> But even then, it never represented such a large gain and in the end,
> was probably not -that- useful.
> 
> I'd say bump it to something really large like 16M or remove the limit
> alltogether.

So, in terms of removing the limit altogether, is that a literal
statement? I don't know this code particularly well, but only see
references to the constant, at least in prom_init.c:

powerpc/prom: remove limit on maximum size of properties
    
On a 16TB system (using AMS/CMO), I get:
    
WARNING: ignoring large property [/ibm,dynamic-reconfiguration-memory] ibm,dynamic-memory length 0x000000000017ffec
    
and significantly less memory is thus shown to the partition. As far as
I can tell, the constant used is arbitrary. Ben Herrenschmidt provided
additional background that
    
> The limit was originally set because of Apple machines carrying ROM
> images in the device-tree, at a time where we were much more memory
> constrained than we are now.
    
and that it is likely not very useful any longer.
    
Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
    
---
With this patch, the kernel should see all of the system memory on the
16TB system, but I'm unable to test due to lack of hardware access. I'm
working on getting access again, but not confident that will happen
soon.

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index eca626e..e2d5990 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -48,14 +48,6 @@
 #include <linux/linux_logo.h>
 
 /*
- * Properties whose value is longer than this get excluded from our
- * copy of the device tree. This value does need to be big enough to
- * ensure that we don't lose things like the interrupt-map property
- * on a PCI-PCI bridge.
- */
-#define MAX_PROPERTY_LENGTH	(1UL * 1024 * 1024)
-
-/*
  * Eventually bump that one up
  */
 #define DEVTREE_CHUNK_SIZE	0x100000
@@ -2273,13 +2265,6 @@ static void __init scan_dt_build_struct(phandle node, unsigned long *mem_start,
 		/* sanity checks */
 		if (l == PROM_ERROR)
 			continue;
-		if (l > MAX_PROPERTY_LENGTH) {
-			prom_printf("WARNING: ignoring large property ");
-			/* It seems OF doesn't null-terminate the path :-( */
-			prom_printf("[%s] ", path);
-			prom_printf("%s length 0x%x\n", RELOC(pname), l);
-			continue;
-		}
 
 		/* push property head */
 		dt_push_token(OF_DT_PROP, mem_start, mem_end);

-- 
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center

^ permalink raw reply related

* Re: [PATCH 1/2] powerpc/e500: make load_up_spe a normal fuction
From: Scott Wood @ 2012-02-27 19:19 UTC (permalink / raw)
  To: Olivia Yin; +Cc: Liu Yu, linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1330340388-30296-1-git-send-email-hong-hua.yin@freescale.com>

On 02/27/2012 04:59 AM, Olivia Yin wrote:
> So that we can call it in kernel.
> 
> Signed-off-by: Liu Yu <yu.liu@freescale.com>

Explain why we want this, and point out that this makes it similar to
load_up_fpu.

> ---
>  arch/powerpc/kernel/head_fsl_booke.S |   23 ++++++-----------------
>  1 files changed, 6 insertions(+), 17 deletions(-)

When posting a patch authored by someone else, more or less unchanged,
you should put a From: line in the body of the e-mail.

git send-email will do this automatically if you preserve the authorship
in the git commit.

Also, you should add your own Signed-off-by.

-Scott

^ permalink raw reply

* Re: [PATCH 24/37] KVM: PPC: booke: rework rescheduling checks
From: Scott Wood @ 2012-02-27 19:28 UTC (permalink / raw)
  To: Alexander Graf; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1330093591-19523-25-git-send-email-agraf@suse.de>

On 02/24/2012 08:26 AM, Alexander Graf wrote:
> -void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
> +int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>  {
>  	unsigned long *pending = &vcpu->arch.pending_exceptions;
>  	unsigned long old_pending = vcpu->arch.pending_exceptions;
> @@ -283,6 +283,8 @@ void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>  
>  	/* Tell the guest about our interrupt status */
>  	kvmppc_update_int_pending(vcpu, *pending, old_pending);
> +
> +	return 0;
>  }
>  
>  pfn_t kvmppc_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn)
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 9979be1..3fcec2c 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -439,8 +439,9 @@ static void kvmppc_core_check_exceptions(struct kvm_vcpu *vcpu)
>  }
>  
>  /* Check pending exceptions and deliver one, if possible. */
> -void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
> +int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>  {
> +	int r = 0;
>  	WARN_ON_ONCE(!irqs_disabled());
>  
>  	kvmppc_core_check_exceptions(vcpu);
> @@ -451,8 +452,44 @@ void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>  		local_irq_disable();
>  
>  		kvmppc_set_exit_type(vcpu, EMULATED_MTMSRWE_EXITS);
> -		kvmppc_core_check_exceptions(vcpu);
> +		r = 1;
>  	};
> +
> +	return r;
> +}
> +
> +/*
> + * Common checks before entering the guest world.  Call with interrupts
> + * disabled.
> + *
> + * returns !0 if a signal is pending and check_signal is true
> + */
> +static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu, bool check_signal)
> +{
> +	int r = 0;
> +
> +	WARN_ON_ONCE(!irqs_disabled());
> +	while (true) {
> +		if (need_resched()) {
> +			local_irq_enable();
> +			cond_resched();
> +			local_irq_disable();
> +			continue;
> +		}
> +
> +		if (kvmppc_core_prepare_to_enter(vcpu)) {
> +			/* interrupts got enabled in between, so we
> +			   are back at square 1 */
> +			continue;
> +		}
> +
> +
> +		if (check_signal && signal_pending(current))
> +			r = 1;

If there is a signal pending and MSR[WE] is set, we'll loop forever
without reaching this check.

-Scott

^ permalink raw reply

* Re: [PATCH 36/37] KVM: PPC: booke: expose guest registers on irq reinject
From: Scott Wood @ 2012-02-27 19:45 UTC (permalink / raw)
  To: Alexander Graf; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <810950A7-CD5A-46A5-865B-1209A5532DCC@suse.de>

On 02/26/2012 05:59 AM, Alexander Graf wrote:
> 
> On 25.02.2012, at 00:40, Scott Wood wrote:
> 
>> On 02/24/2012 08:26 AM, Alexander Graf wrote:
>>> +static void kvmppc_fill_pt_regs(struct kvm_vcpu *vcpu, struct pt_regs *regs)
>>> {
>>> -	int r = RESUME_HOST;
>>> +	int i;
>>>
>>> -	/* update before a new last_exit_type is rewritten */
>>> -	kvmppc_update_timing_stats(vcpu);
>>> +	for (i = 0; i < 32; i++)
>>> +		regs->gpr[i] = kvmppc_get_gpr(vcpu, i);
>>> +	regs->nip = vcpu->arch.pc;
>>> +	regs->msr = vcpu->arch.shared->msr;
>>> +	regs->ctr = vcpu->arch.ctr;
>>> +	regs->link = vcpu->arch.lr;
>>> +	regs->xer = kvmppc_get_xer(vcpu);
>>> +	regs->ccr = kvmppc_get_cr(vcpu);
>>> +	regs->dar = get_guest_dear(vcpu);
>>> +	regs->dsisr = get_guest_esr(vcpu);
>>> +}
>>
>> How much overhead does this add to every interrupt?  Can't we keep this
>> to the minimum that perf cares about?
> 
> I would rather not make assumptions on what perf cares about - maybe we want to one day implement "perf kvm" and then perf could rely on pretty much anything in there.

In that case I think we should be populating a real pt_regs from the
start, as in my original patchset.

I only agreed to take it out because I thought the set of things we'd
copy would be minimal.  This seems like a lot of overhead.

I'm not familiar with "perf kvm", but if it's kvm-specific surely the
KVM code should know/dictate what it can rely on?  Or maybe there can be
a debug option that enables full pt_regs (similar to exit timing)?

Could we just set regs to NULL when the debug option isn't enabled?

>>> +static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu,
>>> +				     unsigned int exit_nr)
>>> +{
>>> +	struct pt_regs regs = *current->thread.regs;
>>>
>>> +	kvmppc_fill_pt_regs(vcpu, &regs);
>>
>> Why are you copying out of current->thread.regs?  That's old junk data,
>> set by some previous exception and possibly overwritten since.
> 
> Because it gives us good default values for anything we don't set. Do you have other recommendations?

It does not give good default values for anything.  It is junk,
unallocated memory, overwritten by who knows what.  Same as the memory
you're copying to.

To avoid garbage in fields we don't set, fill it with zeroes first.

-Scott

^ permalink raw reply

* [PATCH net-next] ibm emac: delete module references; phy.c only supported as built-in
From: Paul Gortmaker @ 2012-02-27 19:47 UTC (permalink / raw)
  To: netdev; +Cc: linuxppc-dev, Paul Gortmaker

The Makefile has it as "ibm_emac-y := mal.o core.o phy.o" so there is
no way this can be built modular, so remove all references to module
support.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

diff --git a/drivers/net/ethernet/ibm/emac/phy.c b/drivers/net/ethernet/ibm/emac/phy.c
index d3b9d10..fb96387 100644
--- a/drivers/net/ethernet/ibm/emac/phy.c
+++ b/drivers/net/ethernet/ibm/emac/phy.c
@@ -11,13 +11,15 @@
  * Copyright 2007 Benjamin Herrenschmidt, IBM Corp.
  *                <benh@kernel.crashing.org>
  *
+ * Originally listed as MODULE_LICENSE("GPL") in the code, but that
+ * was removed, since this code is only supported as non-modular.
+ *
  * Based on the arch/ppc version of the driver:
  *
  * (c) 2003, Benjamin Herrenscmidt (benh@kernel.crashing.org)
  * (c) 2004-2005, Eugene Surovegin <ebs@ebshome.net>
  *
  */
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/netdevice.h>
@@ -537,5 +539,3 @@ int emac_mii_phy_probe(struct mii_phy *phy, int address)
 
 	return 0;
 }
-
-MODULE_LICENSE("GPL");
-- 
1.7.9.1

^ permalink raw reply related

* Re: [PATCH v6 4/4] KVM: PPC: epapr: Update other hypercall invoking
From: Scott Wood @ 2012-02-27 19:50 UTC (permalink / raw)
  To: Liu Yu; +Cc: linuxppc-dev, B07421, agraf, kvm-ppc, kvm
In-Reply-To: <1329988942-19610-4-git-send-email-yu.liu@freescale.com>

On 02/23/2012 03:22 AM, Liu Yu wrote:
> diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig
> index 2dcdbc9..99ebdde 100644
> --- a/drivers/virt/Kconfig
> +++ b/drivers/virt/Kconfig
> @@ -15,6 +15,7 @@ if VIRT_DRIVERS
>  config FSL_HV_MANAGER
>  	tristate "Freescale hypervisor management driver"
>  	depends on FSL_SOC
> +	select EPAPR_PARAVIRT
>  	help
>            The Freescale hypervisor management driver provides several services
>  	  to drivers and applications related to the Freescale hypervisor:

What about the byte channel driver, and possibly others?

Grep for the hypercalls to make sure you got everything that uses this.

-Scott

^ permalink raw reply

* Re: [PATCH net-next] ibm emac: delete module references; phy.c only supported as built-in
From: David Miller @ 2012-02-27 19:57 UTC (permalink / raw)
  To: paul.gortmaker; +Cc: netdev, linuxppc-dev
In-Reply-To: <1330372024-30974-1-git-send-email-paul.gortmaker@windriver.com>

From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Mon, 27 Feb 2012 14:47:04 -0500

> The Makefile has it as "ibm_emac-y := mal.o core.o phy.o" so there is
> no way this can be built modular, so remove all references to module
> support.

That doesn't mean it's only buildable statically.

"ibm_emacs-y :=" is merely a way to tell make what objects go into ibm_emac.{o,ko}

IBM_EMAC is tristate

^ permalink raw reply

* Re: [PATCH net-next] ibm emac: delete module references; phy.c only supported as built-in
From: Ben Hutchings @ 2012-02-27 19:56 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: netdev, linuxppc-dev
In-Reply-To: <1330372024-30974-1-git-send-email-paul.gortmaker@windriver.com>

On Mon, 2012-02-27 at 14:47 -0500, Paul Gortmaker wrote:
> The Makefile has it as "ibm_emac-y := mal.o core.o phy.o" so there is
> no way this can be built modular, so remove all references to module
> support.

No, that's nonsense.  You need to look at whether ibm_emac.o can be
added to obj-m (which it can).

Ben.

> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> 
> diff --git a/drivers/net/ethernet/ibm/emac/phy.c b/drivers/net/ethernet/ibm/emac/phy.c
> index d3b9d10..fb96387 100644
> --- a/drivers/net/ethernet/ibm/emac/phy.c
> +++ b/drivers/net/ethernet/ibm/emac/phy.c
> @@ -11,13 +11,15 @@
>   * Copyright 2007 Benjamin Herrenschmidt, IBM Corp.
>   *                <benh@kernel.crashing.org>
>   *
> + * Originally listed as MODULE_LICENSE("GPL") in the code, but that
> + * was removed, since this code is only supported as non-modular.
> + *
>   * Based on the arch/ppc version of the driver:
>   *
>   * (c) 2003, Benjamin Herrenscmidt (benh@kernel.crashing.org)
>   * (c) 2004-2005, Eugene Surovegin <ebs@ebshome.net>
>   *
>   */
> -#include <linux/module.h>
>  #include <linux/kernel.h>
>  #include <linux/types.h>
>  #include <linux/netdevice.h>
> @@ -537,5 +539,3 @@ int emac_mii_phy_probe(struct mii_phy *phy, int address)
>  
>  	return 0;
>  }
> -
> -MODULE_LICENSE("GPL");

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH net-next] ibm emac: delete module references; phy.c only supported as built-in
From: Paul Gortmaker @ 2012-02-27 20:49 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linuxppc-dev
In-Reply-To: <20120227.145753.1111293673513137413.davem@davemloft.net>

[Re: [PATCH net-next] ibm emac: delete module references; phy.c only supported as built-in] On 27/02/2012 (Mon 14:57) David Miller wrote:

> From: Paul Gortmaker <paul.gortmaker@windriver.com>
> Date: Mon, 27 Feb 2012 14:47:04 -0500
> 
> > The Makefile has it as "ibm_emac-y := mal.o core.o phy.o" so there is
> > no way this can be built modular, so remove all references to module
> > support.
> 
> That doesn't mean it's only buildable statically.
> 
> "ibm_emacs-y :=" is merely a way to tell make what objects go into ibm_emac.{o,ko}
> 
> IBM_EMAC is tristate

Argh, yes.  Sorry for the noise.  Since phy.c doesn't call any modular
functionality directly, the patch "worked" and I didn't catch the
error in my reasoning.

Paul.

^ permalink raw reply

* Re: [PATCH v6 1/4] KVM: PPC: epapr: Factor out the epapr init
From: Scott Wood @ 2012-02-27 20:52 UTC (permalink / raw)
  To: Liu Yu; +Cc: linuxppc-dev, B07421, agraf, kvm-ppc, kvm
In-Reply-To: <1329988942-19610-1-git-send-email-yu.liu@freescale.com>

On 02/23/2012 03:22 AM, Liu Yu wrote:
> +static int __init epapr_paravirt_init(void)
> +{
> +	struct device_node *hyper_node;
> +	const u32 *insts;
> +	int len, i;
> +
> +	hyper_node = of_find_node_by_path("/hypervisor");
> +	if (!hyper_node)
> +		return -ENODEV;
> +
> +	insts = of_get_property(hyper_node, "hcall-instructions", &len);
> +	if (!insts)
> +		return 0;

-ENODEV here too.

> +	if (!(len % 4) && len <= (4 * 4)) {
> +		for (i = 0; i < (len / 4); i++)
> +			patch_instruction(epapr_hypercall_start + i, insts[i]);
> +
> +		epapr_paravirt_enabled = true;
> +	} else {
> +		printk(KERN_WARNING
> +		       "ePAPR paravirt: hcall-instructions format error\n");
> +	}

Do this:

if (error) {
	print error
	return error code
}

continue with function

Not this:

if (!error) {
	continue with function
} else {
	report the error from several lines back
}

> @@ -33,6 +34,14 @@ config KVM_GUEST
>  
>  	  In case of doubt, say Y
>  
> +config EPAPR_PARAVIRT
> +	bool "ePAPR para-virtualization support"
> +	default n
> +	help
> +	  Used to enalbe ePAPR complied para-virtualization support for guest.
> +
> +	  In case of doubt, say Y

s/Used to enalbe/Enable/

s/complied/compliant/ (or just s/complied//)

-Scott

^ permalink raw reply

* Re: [PATCH 1/2] powerpc: Move GE GPIO and PIC drivers
From: Benjamin Herrenschmidt @ 2012-02-27 21:32 UTC (permalink / raw)
  To: Martyn Welch; +Cc: Wim Van Sebroeck, linuxppc-dev, linux-kernel
In-Reply-To: <4F4B8BB2.1060004@ge.com>

On Mon, 2012-02-27 at 13:57 +0000, Martyn Welch wrote:

> This patch (or one like it) has been around for a while now. Kumar wanted me
> to put them here rather than sysdev[1], but I'm easy either way.

Ah well, I disagree with Kumar here :-) One thing you can do is put all
your platforms files including the drivers in platforms/ge, that works
too. What I don't want is to have stray files at the top-level of
platforms.

> > Also, use git mv so that the file moves appear as such in the history,
> > this will make review easier by clearly separating the move from actual
> > changes to the files.
> > 
> 
> Hmm, thought I'd done that. Will try again.

Could be the way you exported the patch, I don't remember the git option
off the top of my head but there's a way to make it show the moves as
such.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 09/24] PCI, powerpc: Register busn_res for root buses
From: Bjorn Helgaas @ 2012-02-27 22:44 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: linux-arch, Tony Luck, linuxppc-dev, linux-kernel,
	Dominik Brodowski, Paul Mackerras, Jesse Barnes, linux-pci,
	Andrew Morton, Linus Torvalds
In-Reply-To: <CAE9FiQXx6wXaes1pbpM7SAHzndgr=N2D2g6LEWsB2_pf8+mgSA@mail.gmail.com>

On Sat, Feb 25, 2012 at 12:47 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Feb 24, 2012 at 2:24 PM, Jesse Barnes <jbarnes@virtuousgeek.org> =
wrote:
>> On Thu, 23 Feb 2012 12:51:30 -0800
>> Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> 2) We already have a way to add resources to a root bus: the
>>> pci_add_resource() used to add I/O port and MMIO apertures. =A0I think
>>> it'd be a lot simpler to just use that same interface for the bus
>>> number aperture, e.g.,
>>>
>>> =A0 =A0 pci_add_resource(&resources, hose->io_space);
>>> =A0 =A0 pci_add_resource(&resources, hose->mem_space);
>>> =A0 =A0 pci_add_resource(&resources, hose->busnr_space);
>>> =A0 =A0 bus =3D pci_scan_root_bus(dev, next_busno, pci_ops, sysdata, &r=
esources);
>>>
>>> This is actually a bit redundant, since "next_busno" should be the
>>> same as hose->busnr_space->start. =A0So if we adopted this approach, we
>>> might want to eventually drop the "next_busno" argument.
>>
>> Yeah that would be nice, the call would certainly make more sense that
>> way.
>
> no, I don't think so.
>
> using pci_add_resource will need to create dummy resource abut bus range.

I don't see the problem here.  The bus number aperture is a
fundamental property of a host bridge, and any firmware or native
bridge driver that tells you about a bridge but doesn't tell you the
bus number aperture is just broken.

Every arch already has struct resources for the MMIO and IO port
regions available on the bus.  ACPI already has a resource for the bus
number range.  It makes sense to me that the arch should supply a bus
number resource.

Conceptually, it's just like the MMIO and IO resources, so it makes
sense to me that the code for bus numbers should look like the code
for MMIO and IO ports.

> there is lots of pci_scan_root_bus(), =A0and those user does not bus end
> yet before scan.
> so could just hide pci_insert_busn_res in pci_scan_root_bus, and
> update busn_res end there.

pci_scan_child_bus() does NOT tell you the end of the bus number
aperture, and we shouldn't pretend that it does.  It might give you a
lower bound on the end of the aperture (as long as you're willing to
trust the current PCI config and you don't change anything).

> other arch like x86, ia64, powerpc, sparc, will insert exact bus range
> between pci_create_root_bus and
> pci_scan_child_bus, will not need to update busn_res end.
>
> please check v7 of this patchset.
>
> =A0 =A0 =A0 =A0git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linu=
x-yinghai.git
> for-pci-busn-alloc

I looked at your git tree, but I can't tell whether what's there is v7
or not and it's too much trouble to try to figure it out.

Bjorn

^ permalink raw reply

* Re: linux-next: build failure after merge of the final tree
From: Benjamin Herrenschmidt @ 2012-02-27 23:30 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Bjorn Helgaas, linux-next, ppc-dev, linux-kernel, Jesse Barnes
In-Reply-To: <1330334360.11728.3.camel@pasglop>

On Mon, 2012-02-27 at 20:19 +1100, Benjamin Herrenschmidt wrote:
> On Mon, 2012-02-27 at 17:37 +1100, Stephen Rothwell wrote:
> >         pci_add_resource_offset(resources, res,
> > -                       (resource_size_t) hose->io_base_virt - _IO_BASE);
> > +                       (resource_size_t)(unsigned long)hose->io_base_virt - _IO_BASE);
> 
> We have to be careful here as we do want sign extension to happen (yeah
> it's odd, but it's the way we do IOs on ppc32 :-) Maybe I should change
> it one day).
> 
> So we probably want to do:
> 
> 	 (resource_size_t)(long long)(hose->io_base_virt - _IO_BASE)

Oops ... that was meant to read (long) not (long long)... Any ways, I
more or less convinced myself that even without the sign extension it
would still work, since the IO port number is eventually cast to an
unsigned int by the accessors, so as long as the low 32-bits are correct
(and they'll be with or without the sign extension), we should be fine.
It's just that something trying to print the resource might end up
displaying garbage in the top bits.

Cheers,
Ben.


> Basically, IO resources are relative to _IO_BASE which on ppc32 is
> basically the virtual address where we map the first PHB IO space.
> 
> Subsequent PHB mappings can end up below _IO_BASE, leading to negative
> resource values for IO BARs on those busses. It all works fine because
> even an unsigned addition will do the right thing as long as the value
> is fully sign extended.
> 
> Cheers,
> Ben.

^ permalink raw reply

* Re: [PATCH 14/21] Introduce EEH device
From: Gavin Shan @ 2012-02-28  1:13 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: linuxppc-dev
In-Reply-To: <20120224234610.922da1a3a2523ee5b6319390@canb.auug.org.au>

> Hi Gavin,
> 
> On Fri, 24 Feb 2012 17:38:11 +0800 Gavin Shan <shangw@linux.vnet.ibm.com> wrote:
> >
> > +#define EEH_DEV_TO_OF_NODE(edev)	(edev->dn)
> > +#define EEH_DEV_TO_PCI_DEV(edev)	(edev->pdev)
> > +#define OF_NODE_TO_EEH_DEV(dn)		((struct eeh_dev *)(dn->edev))
> > +#define PCI_DEV_TO_EEH_DEV(pdev)	((struct eeh_dev *)(pdev->dev.archdata.edev))
> 
> You need to put parentheses around the use of macro arguments ... or,
> even better, make these into "static inline" accessor functions (with
> lower case names).
> 

Thanks for your comments, Stephen. I'd like to make "static inline"
functions for them in next revision ;-)

Thanks,
Gavin

> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> http://www.canb.auug.org.au/~sfr/

^ permalink raw reply

* Re: [PATCH 20/21] Introduce struct eeh_stats for EEH
From: Gavin Shan @ 2012-02-28  1:19 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: linuxppc-dev
In-Reply-To: <20120225000141.73c0de577597e675a0fb5021@canb.auug.org.au>

> Hi Gavin,
> 
> On Fri, 24 Feb 2012 17:38:17 +0800 Gavin Shan <shangw@linux.vnet.ibm.com> wrote:
> >
> > diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> > index 1310971..226c9a5 100644
> > --- a/arch/powerpc/include/asm/eeh.h
> > +++ b/arch/powerpc/include/asm/eeh.h
> > @@ -98,6 +98,21 @@ struct eeh_ops {
> >  	int (*configure_bridge)(struct device_node *dn);
> >  };
> >  
> > +/*
> > + * The struct is used to maintain the EEH global statistic
> > + * information. Besides, the EEH global statistics will be
> > + * exported to user space through procfs
> > + */
> > +struct eeh_stats {
> > +	unsigned long no_device;	/* PCI device not found		*/
> > +	unsigned long no_dn;		/* OF node not found		*/
> > +	unsigned long no_cfg_addr;	/* Config address not found	*/
> > +	unsigned long ignored_check;	/* EEH check skipped		*/
> > +	unsigned long total_mmio_ffs;	/* Total EEH checks		*/
> > +	unsigned long false_positives;	/* Unnecessary EEH checks	*/
> > +	unsigned long slot_resets;	/* PE reset			*/
> > +};
> 
> If this is used in only one place, there is not much point in putting it
> in a header file.
> 

Thanks, Stephen. I'll move it to eeh.c in next revision.

Thanks,
Gavin

> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> http://www.canb.auug.org.au/~sfr/

^ permalink raw reply


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