Subject: Re: [v1][PATCH 1/1] KVM: PPC: disable preemption when using hard_irq_disable()

From: Scott Wood <[email-masked]>
Date: Thu, 11 Jul 2013 09:13:36 -0500

On 07/10/2013 10:00:33 PM, tiejun.chen wrote:
> On 07/11/2013 03:15 AM, Scott Wood wrote:
>> On 07/10/2013 01:02:19 AM, Tiejun Chen wrote:
>>> We should ensure the preemption cannot occur while calling =20
>>> get_paca()
>>> insdide hard_irq_disable(), otherwise the paca_struct may be the
>>> wrong one just after. And btw, we may update timing stats in this =20
>>> case.
>>=20
>> The soft-ee mechanism depends on accessing the PACA directly via r13 =20
>> to avoid
>> this.  We probably should be using inline asm to read was_enabled =20
>> rather than
>=20
> Yes.
>=20
>> hoping the compiler doesn't do anything silly.
>=20
> Do you recommend I should directly replace get_paca() with local_paca =20
> inside hard_irq_disable()?
>=20
> #define hard_irq_disable()      do {                    \
>         u8 _was_enabled =3D get_paca()->soft_enabled;     \
>=20
> ->	u8 _was_enabled =3D local_paca->soft_enabled;
>=20
> But is this safe for all scenarios?

get_paca() is just a #define for local_paca.  It won't make a =20
difference, other than to evade the debug check.

In practice, it's unlikely that GCC would do anything other than a load =20
directly from r13, but to be sure we should use inline asm to do the =20
load, just like arch_local_save_flags and arch_local_irq_disable do.

-Scott=