Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Atish Patra <atish.patra@wdc.com>
Subject: Re: [PATCH v2 2/3] RISC-V: Use Linux logical cpu number instead of
 hartid
To:     Christoph Hellwig <hch@infradead.org>
Cc:     "palmer@sifive.com" <palmer@sifive.com>,
        "linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "anup@brainfault.org" <anup@brainfault.org>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "damein@vger.kernel.org" <damein@vger.kernel.org>
References: <1535445370-19004-1-git-send-email-atish.patra@wdc.com>
 <1535445370-19004-3-git-send-email-atish.patra@wdc.com>
 <20180831061104.GB19292@infradead.org>
Message-ID: <ec0819d5-9dc6-6b25-47a9-cd9d9e5099a8@wdc.com>
Date:   Tue, 4 Sep 2018 13:35:10 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:52.0)
 Gecko/20100101 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <20180831061104.GB19292@infradead.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 8/30/18 11:11 PM, Christoph Hellwig wrote:
>> -#define flush_tlb_all() sbi_remote_sfence_vma(NULL, 0, -1)
>> +static inline void remote_sfence_vma(struct cpumask *cmask, unsigned long start,
>> +				     unsigned long size)
>> +{
>> +	struct cpumask hmask;
>> +
>> +	riscv_cpuid_to_hartid_mask(cmask, &hmask);
>> +	sbi_remote_sfence_vma(hmask.bits, start, size);
>> +}
>> +
>> +#define flush_tlb_all() remote_sfence_vma(NULL, 0, -1)
> 
> flush_tlb_all passed NULL to sbi_remote_sfence_vma before, so this
> changes what we pass.  I think we should keep the existing behavior.
> 

sure. How about this ?


--- a/arch/riscv/include/asm/tlbflush.h
+++ b/arch/riscv/include/asm/tlbflush.h
@@ -55,8 +55,13 @@ static inline void remote_sfence_vma(struct cpumask 
*cmask, unsigned long start,
  {
         struct cpumask hmask;

-       riscv_cpuid_to_hartid_mask(cmask, &hmask);
-       sbi_remote_sfence_vma(hmask.bits, start, size);
+       if (cmask == NULL) {
+               sbi_remote_sfence_vma(NULL, start, size);
+       } else {
+               cpumask_clear(&hmask);
+               riscv_cpuid_to_hartid_mask(cmask, &hmask);
+               sbi_remote_sfence_vma(hmask.bits, start, size);
+       }
  }

>> @@ -93,10 +94,11 @@ static inline void plic_toggle(int ctxid, int hwirq, int enable)
>>   static inline void plic_irq_toggle(struct irq_data *d, int enable)
>>   {
>>   	int cpu;
>> +	struct plic_handler *handler;
>>   
>>   	writel(enable, plic_regs + PRIORITY_BASE + d->hwirq * PRIORITY_PER_ID);
>>   	for_each_cpu(cpu, irq_data_get_affinity_mask(d)) {
>> -		struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu);
>> +		handler = per_cpu_ptr(&plic_handlers, cpu);
> 
> This looks like a spurious change.
> 

I think I did this to avoid possible compiler warnings. Will revert it.

Regards,
Atish