Linux MIPS Architecture development
 help / color / mirror / Atom feed
From: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
To: David Daney <ddaney.cavm@gmail.com>
Cc: <linux-mips@linux-mips.org>, <ralf@linux-mips.org>,
	David Daney <david.daney@cavium.com>
Subject: Re: [PATCH] MIPS: Fix C0_Pagegrain[IEC] support.
Date: Fri, 19 Dec 2014 17:39:50 -0800	[thread overview]
Message-ID: <5494D366.6060400@imgtec.com> (raw)
In-Reply-To: <5494D04F.4030701@gmail.com>

On 12/19/2014 05:26 PM, David Daney wrote:
> On 12/19/2014 05:21 PM, Leonid Yegoshin wrote:
>> On 12/19/2014 05:15 PM, David Daney wrote:
>>> From: David Daney <david.daney@cavium.com>
>>>
>>> If we are generating TLB exception expecting separate vectors, we must
>>> enable the feature.
>>>
>>> Cc: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
>>> Signed-off-by: David Daney <david.daney@cavium.com>
>>> ---
>>>
>>> Very lightly tested, but it seems to make my XI and RI tests work on
>>> OCTEON II CPUs, which have the C0_Pagegrain[IEC] bit.
>>>
>>>   arch/mips/mm/tlb-r4k.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/mips/mm/tlb-r4k.c b/arch/mips/mm/tlb-r4k.c
>>> index e90b2e8..30639a6 100644
>>> --- a/arch/mips/mm/tlb-r4k.c
>>> +++ b/arch/mips/mm/tlb-r4k.c
>>> @@ -489,6 +489,8 @@ static void r4k_tlb_configure(void)
>>>   #ifdef CONFIG_64BIT
>>>           pg |= PG_ELPA;
>>>   #endif
>>> +        if (cpu_has_rixiex)
>>> +            pg |= PG_IEC;
>>>           write_c0_pagegrain(pg);
>>>       }
>> David, I think it is still better to use set_c0_pagegrain() because
>> PageGrain has a lot of RW bits now and clear all of them may be not 
>> good.
>
> IMHO all the code that sets PageGrain should be in this function. We 
> should calculate all the bits here that should be set, and set them.
>
> The whole reason that we have this mess, is that we were setting the 
> bits at different code sites, and clobbering them in others.
>
> If *all* the PageGrain logic is in one place, we won't have this problem.
>
> If you think this patch is incorrect, then we should revert the other 
> two and take our time to carefully do something that is correct.
>
> David Daney
>
No, I don't see this patch as incorrect. It is just about coding 
assumptions but I don't think it is serious, so you variant is OK.

WARNING: multiple messages have this Message-ID (diff)
From: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
To: David Daney <ddaney.cavm@gmail.com>
Cc: linux-mips@linux-mips.org, ralf@linux-mips.org,
	David Daney <david.daney@cavium.com>
Subject: Re: [PATCH] MIPS: Fix C0_Pagegrain[IEC] support.
Date: Fri, 19 Dec 2014 17:39:50 -0800	[thread overview]
Message-ID: <5494D366.6060400@imgtec.com> (raw)
Message-ID: <20141220013950.sUEMQGVRhoVQxzvxFMKyE7MWpn37Ziv11QIvsTtElW0@z> (raw)
In-Reply-To: <5494D04F.4030701@gmail.com>

On 12/19/2014 05:26 PM, David Daney wrote:
> On 12/19/2014 05:21 PM, Leonid Yegoshin wrote:
>> On 12/19/2014 05:15 PM, David Daney wrote:
>>> From: David Daney <david.daney@cavium.com>
>>>
>>> If we are generating TLB exception expecting separate vectors, we must
>>> enable the feature.
>>>
>>> Cc: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
>>> Signed-off-by: David Daney <david.daney@cavium.com>
>>> ---
>>>
>>> Very lightly tested, but it seems to make my XI and RI tests work on
>>> OCTEON II CPUs, which have the C0_Pagegrain[IEC] bit.
>>>
>>>   arch/mips/mm/tlb-r4k.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/mips/mm/tlb-r4k.c b/arch/mips/mm/tlb-r4k.c
>>> index e90b2e8..30639a6 100644
>>> --- a/arch/mips/mm/tlb-r4k.c
>>> +++ b/arch/mips/mm/tlb-r4k.c
>>> @@ -489,6 +489,8 @@ static void r4k_tlb_configure(void)
>>>   #ifdef CONFIG_64BIT
>>>           pg |= PG_ELPA;
>>>   #endif
>>> +        if (cpu_has_rixiex)
>>> +            pg |= PG_IEC;
>>>           write_c0_pagegrain(pg);
>>>       }
>> David, I think it is still better to use set_c0_pagegrain() because
>> PageGrain has a lot of RW bits now and clear all of them may be not 
>> good.
>
> IMHO all the code that sets PageGrain should be in this function. We 
> should calculate all the bits here that should be set, and set them.
>
> The whole reason that we have this mess, is that we were setting the 
> bits at different code sites, and clobbering them in others.
>
> If *all* the PageGrain logic is in one place, we won't have this problem.
>
> If you think this patch is incorrect, then we should revert the other 
> two and take our time to carefully do something that is correct.
>
> David Daney
>
No, I don't see this patch as incorrect. It is just about coding 
assumptions but I don't think it is serious, so you variant is OK.

  reply	other threads:[~2014-12-20  1:40 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-20  1:15 [PATCH] MIPS: Fix C0_Pagegrain[IEC] support David Daney
2014-12-20  1:18 ` David Daney
2014-12-20  1:18   ` David Daney
2014-12-20  1:21 ` Leonid Yegoshin
2014-12-20  1:21   ` Leonid Yegoshin
2014-12-20  1:26   ` David Daney
2014-12-20  1:39     ` Leonid Yegoshin [this message]
2014-12-20  1:39       ` Leonid Yegoshin
  -- strict thread matches above, loose matches on Subject: below --
2015-01-06 18:42 David Daney
2015-01-06 18:42 ` David Daney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5494D366.6060400@imgtec.com \
    --to=leonid.yegoshin@imgtec.com \
    --cc=david.daney@cavium.com \
    --cc=ddaney.cavm@gmail.com \
    --cc=linux-mips@linux-mips.org \
    --cc=ralf@linux-mips.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox