Re: [PATCH v2 0/2] Enabling MSI for Microblaze
On Fri, Oct 25, 2019 at 08:10:36AM +0200, Michal Simek wrote: > Hi, > > these two patches come from discussion with Christoph, Bjorn, Palmer and > Waiman. The first patch was suggestion by Christoph here > https://lore.kernel.org/linux-riscv/20191008154604.ga7...@infradead.org/ > The second part was discussed > https://lore.kernel.org/linux-pci/mhng-5d9bcb53-225e-441f-86cc-b335624b3e7c@palmer-si-x1e/ > and > https://lore.kernel.org/linux-pci/20191017181937.7004-1-pal...@sifive.com/ > > Thanks, > Michal > > Changes in v2: > - Fix typo in commit message s/expect/except/ - Reported-by: Masahiro > > Michal Simek (1): > asm-generic: Make msi.h a mandatory include/asm header > > Palmer Dabbelt (1): > pci: Default to PCI_MSI_IRQ_DOMAIN > > arch/arc/include/asm/Kbuild | 1 - > arch/arm/include/asm/Kbuild | 1 - > arch/arm64/include/asm/Kbuild | 1 - > arch/mips/include/asm/Kbuild| 1 - > arch/powerpc/include/asm/Kbuild | 1 - > arch/riscv/include/asm/Kbuild | 1 - > arch/sparc/include/asm/Kbuild | 1 - > drivers/pci/Kconfig | 2 +- > include/asm-generic/Kbuild | 1 + > 9 files changed, 2 insertions(+), 8 deletions(-) I applied these to pci/msi for v5.5, thanks! ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH V7] mm/debug: Add tests validating architecture page table helpers
On 10/25/2019 02:22 PM, Christophe Leroy wrote:
>
>
> Le 25/10/2019 à 10:24, Anshuman Khandual a écrit :
>>
>>
>> On 10/25/2019 12:41 PM, Christophe Leroy wrote:
>>>
>>>
>>> Le 25/10/2019 à 07:52, Qian Cai a écrit :
> On Oct 24, 2019, at 11:45 PM, Anshuman Khandual
> wrote:
>
> Nothing specific. But just tested this with x86 defconfig with relevant
> configs
> which are required for this test. Not sure if it involved W=1.
No, it will not. It needs to run like,
make W=1 -j 64 2>/tmp/warns
>>>
>>> Are we talking about this peace of code ?
>>>
>>> +static unsigned long __init get_random_vaddr(void)
>>> +{
>>> + unsigned long random_vaddr, random_pages, total_user_pages;
>>> +
>>> + total_user_pages = (TASK_SIZE - FIRST_USER_ADDRESS) / PAGE_SIZE;
>>> +
>>> + random_pages = get_random_long() % total_user_pages;
>>> + random_vaddr = FIRST_USER_ADDRESS + random_pages * PAGE_SIZE;
>>> +
>>> + WARN_ON((random_vaddr > TASK_SIZE) ||
>>> + (random_vaddr < FIRST_USER_ADDRESS));
>>> + return random_vaddr;
>>> +}
>>> +
>>>
>>> ramdom_vaddr is unsigned,
>>> random_pages is unsigned and lower than total_user_pages
>>>
>>> So the max value random_vaddr can get is FIRST_USER_ADDRESS + ((TASK_SIZE -
>>> FIRST_USER_ADDRESS - 1) / PAGE_SIZE) * PAGE_SIZE = TASK_SIZE - 1
>>> And the min value random_vaddr can get is FIRST_USER_ADDRESS (that's when
>>> random_pages = 0)
>>
>> That's right.
>>
>>>
>>> So the WARN_ON() is just unneeded, isn't it ?
>>
>> It is just a sanity check on possible vaddr values before it's corresponding
>> page table mappings could be created. If it's worth to drop this in favor of
>> avoiding these unwanted warning messages on x86, will go ahead with it as it
>> is not super important.
>>
>
> But you are checking what ? That the compiler does calculation correctly or
> what ?
IIRC, probably this was for later if and when the vaddr calculation becomes
dependent on other factors rather than this simple arithmetic involving start
and end of process address space on a platform.
> As mentionned just above, based on the calculation done, what you are testing
> cannot happen, so I'm having a hard time understanding what kind of sanity
> check it can be.
You are right.
>
> Can you give an exemple of a situation which could trigger the warning ?
I was mistaken. We dont need those checks for now, hence will drop them next
time.
>
> Christophe
>
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH V7] mm/debug: Add tests validating architecture page table helpers
Le 25/10/2019 à 10:24, Anshuman Khandual a écrit :
On 10/25/2019 12:41 PM, Christophe Leroy wrote:
Le 25/10/2019 à 07:52, Qian Cai a écrit :
On Oct 24, 2019, at 11:45 PM, Anshuman Khandual
wrote:
Nothing specific. But just tested this with x86 defconfig with relevant configs
which are required for this test. Not sure if it involved W=1.
No, it will not. It needs to run like,
make W=1 -j 64 2>/tmp/warns
Are we talking about this peace of code ?
+static unsigned long __init get_random_vaddr(void)
+{
+ unsigned long random_vaddr, random_pages, total_user_pages;
+
+ total_user_pages = (TASK_SIZE - FIRST_USER_ADDRESS) / PAGE_SIZE;
+
+ random_pages = get_random_long() % total_user_pages;
+ random_vaddr = FIRST_USER_ADDRESS + random_pages * PAGE_SIZE;
+
+ WARN_ON((random_vaddr > TASK_SIZE) ||
+ (random_vaddr < FIRST_USER_ADDRESS));
+ return random_vaddr;
+}
+
ramdom_vaddr is unsigned,
random_pages is unsigned and lower than total_user_pages
So the max value random_vaddr can get is FIRST_USER_ADDRESS + ((TASK_SIZE -
FIRST_USER_ADDRESS - 1) / PAGE_SIZE) * PAGE_SIZE = TASK_SIZE - 1
And the min value random_vaddr can get is FIRST_USER_ADDRESS (that's when
random_pages = 0)
That's right.
So the WARN_ON() is just unneeded, isn't it ?
It is just a sanity check on possible vaddr values before it's corresponding
page table mappings could be created. If it's worth to drop this in favor of
avoiding these unwanted warning messages on x86, will go ahead with it as it
is not super important.
But you are checking what ? That the compiler does calculation correctly
or what ?
As mentionned just above, based on the calculation done, what you are
testing cannot happen, so I'm having a hard time understanding what kind
of sanity check it can be.
Can you give an exemple of a situation which could trigger the warning ?
Christophe
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH V7] mm/debug: Add tests validating architecture page table helpers
On 10/25/2019 12:41 PM, Christophe Leroy wrote:
>
>
> Le 25/10/2019 à 07:52, Qian Cai a écrit :
>>
>>
>>> On Oct 24, 2019, at 11:45 PM, Anshuman Khandual
>>> wrote:
>>>
>>> Nothing specific. But just tested this with x86 defconfig with relevant
>>> configs
>>> which are required for this test. Not sure if it involved W=1.
>>
>> No, it will not. It needs to run like,
>>
>> make W=1 -j 64 2>/tmp/warns
>>
>
> Are we talking about this peace of code ?
>
> +static unsigned long __init get_random_vaddr(void)
> +{
> + unsigned long random_vaddr, random_pages, total_user_pages;
> +
> + total_user_pages = (TASK_SIZE - FIRST_USER_ADDRESS) / PAGE_SIZE;
> +
> + random_pages = get_random_long() % total_user_pages;
> + random_vaddr = FIRST_USER_ADDRESS + random_pages * PAGE_SIZE;
> +
> + WARN_ON((random_vaddr > TASK_SIZE) ||
> + (random_vaddr < FIRST_USER_ADDRESS));
> + return random_vaddr;
> +}
> +
>
> ramdom_vaddr is unsigned,
> random_pages is unsigned and lower than total_user_pages
>
> So the max value random_vaddr can get is FIRST_USER_ADDRESS + ((TASK_SIZE -
> FIRST_USER_ADDRESS - 1) / PAGE_SIZE) * PAGE_SIZE = TASK_SIZE - 1
> And the min value random_vaddr can get is FIRST_USER_ADDRESS (that's when
> random_pages = 0)
That's right.
>
> So the WARN_ON() is just unneeded, isn't it ?
It is just a sanity check on possible vaddr values before it's corresponding
page table mappings could be created. If it's worth to drop this in favor of
avoiding these unwanted warning messages on x86, will go ahead with it as it
is not super important.
>
> Christophe
>
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH V7] mm/debug: Add tests validating architecture page table helpers
On 10/25/2019 11:22 AM, Qian Cai wrote: > > >> On Oct 24, 2019, at 11:45 PM, Anshuman Khandual >> wrote: >> >> Nothing specific. But just tested this with x86 defconfig with relevant >> configs >> which are required for this test. Not sure if it involved W=1. > > No, it will not. It needs to run like, > > make W=1 -j 64 2>/tmp/warns Ahh, so we explicitly ask for it. Unfortunately compiler still flags it as an warning. Just wondering why this is still a problem if the second condition for an OR expression is always false. Because evaluation still needs to be performed for the first condition anyways, before arriving at the result. DESCEND objtool CALLscripts/atomic/check-atomics.sh CALLscripts/checksyscalls.sh CHK include/generated/compile.h CC mm/debug_vm_pgtable.o In file included from ./arch/x86/include/asm/bug.h:83:0, from ./include/linux/bug.h:5, from ./include/linux/mmdebug.h:5, from ./include/linux/gfp.h:5, from mm/debug_vm_pgtable.c:13: mm/debug_vm_pgtable.c: In function ‘get_random_vaddr’: mm/debug_vm_pgtable.c:314:17: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits] (random_vaddr < FIRST_USER_ADDRESS)); ^ ./include/asm-generic/bug.h:113:25: note: in definition of macro ‘WARN_ON’ int __ret_warn_on = !!(condition);\ ^ As you mentioned GCC is quite stubborn here. Anyways, lets keep it unchanged. ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 1/2] asm-generic: Make msi.h a mandatory include/asm header
On 24. 10. 19 16:44, Masahiro Yamada wrote: > On Thu, Oct 24, 2019 at 7:13 PM Michal Simek wrote: >> >> msi.h is generic for all architectures expect of x86 which has own version. > > Maybe a typo? "except" unfortunately yes. > > > Anyway, the code looks good to me. > > Reviewed-by: Masahiro Yamada I have sent v2. Who should be taking these patches? Thanks, Michal ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v2 1/2] asm-generic: Make msi.h a mandatory include/asm header
msi.h is generic for all architectures except of x86 which has own version. Enabling MSI by including msi.h to architecture Kbuild is just additional step which doesn't need to be done. The patch was created based on request to enable MSI for Microblaze. Suggested-by: Christoph Hellwig Signed-off-by: Michal Simek Acked-by: Waiman Long Acked-by: Paul Walmsley # arch/riscv Tested-by: Paul Walmsley # build only, rv32/rv64 Reviewed-by: Masahiro Yamada --- Changes in v2: - Fix typo in commit message s/expect/except/ - Reported-by: Masahiro https://lore.kernel.org/linux-riscv/20191008154604.ga7...@infradead.org/ --- arch/arc/include/asm/Kbuild | 1 - arch/arm/include/asm/Kbuild | 1 - arch/arm64/include/asm/Kbuild | 1 - arch/mips/include/asm/Kbuild| 1 - arch/powerpc/include/asm/Kbuild | 1 - arch/riscv/include/asm/Kbuild | 1 - arch/sparc/include/asm/Kbuild | 1 - include/asm-generic/Kbuild | 1 + 8 files changed, 1 insertion(+), 7 deletions(-) diff --git a/arch/arc/include/asm/Kbuild b/arch/arc/include/asm/Kbuild index 393d4f5e1450..1b505694691e 100644 --- a/arch/arc/include/asm/Kbuild +++ b/arch/arc/include/asm/Kbuild @@ -17,7 +17,6 @@ generic-y += local64.h generic-y += mcs_spinlock.h generic-y += mm-arch-hooks.h generic-y += mmiowb.h -generic-y += msi.h generic-y += parport.h generic-y += percpu.h generic-y += preempt.h diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild index 68ca86f85eb7..fa579b23b4df 100644 --- a/arch/arm/include/asm/Kbuild +++ b/arch/arm/include/asm/Kbuild @@ -12,7 +12,6 @@ generic-y += local.h generic-y += local64.h generic-y += mm-arch-hooks.h generic-y += mmiowb.h -generic-y += msi.h generic-y += parport.h generic-y += preempt.h generic-y += seccomp.h diff --git a/arch/arm64/include/asm/Kbuild b/arch/arm64/include/asm/Kbuild index 98a5405c8558..bd23f87d6c55 100644 --- a/arch/arm64/include/asm/Kbuild +++ b/arch/arm64/include/asm/Kbuild @@ -16,7 +16,6 @@ generic-y += local64.h generic-y += mcs_spinlock.h generic-y += mm-arch-hooks.h generic-y += mmiowb.h -generic-y += msi.h generic-y += qrwlock.h generic-y += qspinlock.h generic-y += serial.h diff --git a/arch/mips/include/asm/Kbuild b/arch/mips/include/asm/Kbuild index c8b595c60910..61b0fc2026e6 100644 --- a/arch/mips/include/asm/Kbuild +++ b/arch/mips/include/asm/Kbuild @@ -13,7 +13,6 @@ generic-y += irq_work.h generic-y += local64.h generic-y += mcs_spinlock.h generic-y += mm-arch-hooks.h -generic-y += msi.h generic-y += parport.h generic-y += percpu.h generic-y += preempt.h diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild index 64870c7be4a3..17726f2e46de 100644 --- a/arch/powerpc/include/asm/Kbuild +++ b/arch/powerpc/include/asm/Kbuild @@ -10,4 +10,3 @@ generic-y += local64.h generic-y += mcs_spinlock.h generic-y += preempt.h generic-y += vtime.h -generic-y += msi.h diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild index 16970f246860..1efaeddf1e4b 100644 --- a/arch/riscv/include/asm/Kbuild +++ b/arch/riscv/include/asm/Kbuild @@ -22,7 +22,6 @@ generic-y += kvm_para.h generic-y += local.h generic-y += local64.h generic-y += mm-arch-hooks.h -generic-y += msi.h generic-y += percpu.h generic-y += preempt.h generic-y += sections.h diff --git a/arch/sparc/include/asm/Kbuild b/arch/sparc/include/asm/Kbuild index b6212164847b..62de2eb2773d 100644 --- a/arch/sparc/include/asm/Kbuild +++ b/arch/sparc/include/asm/Kbuild @@ -18,7 +18,6 @@ generic-y += mcs_spinlock.h generic-y += mm-arch-hooks.h generic-y += mmiowb.h generic-y += module.h -generic-y += msi.h generic-y += preempt.h generic-y += serial.h generic-y += trace_clock.h diff --git a/include/asm-generic/Kbuild b/include/asm-generic/Kbuild index adff14fcb8e4..ddfee1bd9dc1 100644 --- a/include/asm-generic/Kbuild +++ b/include/asm-generic/Kbuild @@ -4,4 +4,5 @@ # (This file is not included when SRCARCH=um since UML borrows several # asm headers from the host architecutre.) +mandatory-y += msi.h mandatory-y += simd.h -- 2.17.1 ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v2 0/2] Enabling MSI for Microblaze
Hi, these two patches come from discussion with Christoph, Bjorn, Palmer and Waiman. The first patch was suggestion by Christoph here https://lore.kernel.org/linux-riscv/20191008154604.ga7...@infradead.org/ The second part was discussed https://lore.kernel.org/linux-pci/mhng-5d9bcb53-225e-441f-86cc-b335624b3e7c@palmer-si-x1e/ and https://lore.kernel.org/linux-pci/20191017181937.7004-1-pal...@sifive.com/ Thanks, Michal Changes in v2: - Fix typo in commit message s/expect/except/ - Reported-by: Masahiro Michal Simek (1): asm-generic: Make msi.h a mandatory include/asm header Palmer Dabbelt (1): pci: Default to PCI_MSI_IRQ_DOMAIN arch/arc/include/asm/Kbuild | 1 - arch/arm/include/asm/Kbuild | 1 - arch/arm64/include/asm/Kbuild | 1 - arch/mips/include/asm/Kbuild| 1 - arch/powerpc/include/asm/Kbuild | 1 - arch/riscv/include/asm/Kbuild | 1 - arch/sparc/include/asm/Kbuild | 1 - drivers/pci/Kconfig | 2 +- include/asm-generic/Kbuild | 1 + 9 files changed, 2 insertions(+), 8 deletions(-) -- 2.17.1 ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
