* [PATCH 1/2] powerpc: bpf: Use correct mask while accessing the VLAN tag
From: Denis Kirjanov @ 2014-06-24 9:59 UTC (permalink / raw)
To: netdev; +Cc: Denis Kirjanov, linuxppc-dev
Use the proper mask which is 0xefff
---
arch/powerpc/net/bpf_jit_comp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 6dcdade..af0ed4d 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -393,7 +393,7 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 *image,
PPC_LHZ_OFFS(r_A, r_skb, offsetof(struct sk_buff,
vlan_tci));
if (code == (BPF_ANC | SKF_AD_VLAN_TAG))
- PPC_ANDI(r_A, r_A, VLAN_VID_MASK);
+ PPC_ANDI(r_A, r_A, ~VLAN_TAG_PRESENT);
else
PPC_ANDI(r_A, r_A, VLAN_TAG_PRESENT);
break;
--
2.0.0
^ permalink raw reply related
* [PATCH 2/2] powerpc: bpf: Fix the broken LD_VLAN_TAG_PRESENT test
From: Denis Kirjanov @ 2014-06-24 9:59 UTC (permalink / raw)
To: netdev; +Cc: Denis Kirjanov, linuxppc-dev
In-Reply-To: <1403603969-24436-1-git-send-email-kda@linux-powerpc.org>
We have to return the boolean here if the tag presents
or not, not jusr ORing the TCI with the mask which results to:
[ 709.412097] test_bpf: #18 LD_VLAN_TAG_PRESENT
[ 709.412245] ret 4096 != 1
[ 709.412332] ret 4096 != 1
[ 709.412333] FAIL (2 times)
---
arch/powerpc/net/bpf_jit_comp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index af0ed4d..a3d8f58 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -394,8 +394,10 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 *image,
vlan_tci));
if (code == (BPF_ANC | SKF_AD_VLAN_TAG))
PPC_ANDI(r_A, r_A, ~VLAN_TAG_PRESENT);
- else
+ else {
PPC_ANDI(r_A, r_A, VLAN_TAG_PRESENT);
+ PPC_SRWI(r_A, r_A, 12);
+ }
break;
case BPF_ANC | SKF_AD_QUEUE:
BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff,
--
2.0.0
^ permalink raw reply related
* Re: [PATCH] vfio: Fix endianness handling for emulated BARs
From: Alexander Graf @ 2014-06-24 10:41 UTC (permalink / raw)
To: Alexey Kardashevskiy, Benjamin Herrenschmidt, Alex Williamson
Cc: Nikunj A Dadhania, linuxppc-dev, linux-kernel, kvm
In-Reply-To: <53A94EBD.101@ozlabs.ru>
On 24.06.14 12:11, Alexey Kardashevskiy wrote:
> On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote:
>> On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote:
>>
>>> Working on big endian being an accident may be a matter of perspective
>> :-)
>>
>>> The comment remains that this patch doesn't actually fix anything except
>>> the overhead on big endian systems doing redundant byte swapping and
>>> maybe the philosophy that vfio regions are little endian.
>> Yes, that works by accident because technically VFIO is a transport and
>> thus shouldn't perform any endian swapping of any sort, which remains
>> the responsibility of the end driver which is the only one to know
>> whether a given BAR location is a a register or some streaming data
>> and in the former case whether it's LE or BE (some PCI devices are BE
>> even ! :-)
>>
>> But yes, in the end, it works with the dual "cancelling" swaps and the
>> overhead of those swaps is probably drowned in the noise of the syscall
>> overhead.
>>
>>> I'm still not a fan of iowrite vs iowritebe, there must be something we
>>> can use that doesn't have an implicit swap.
>> Sadly there isn't ... In the old day we didn't even have the "be"
>> variant and readl/writel style accessors still don't have them either
>> for all archs.
>>
>> There is __raw_readl/writel but here the semantics are much more than
>> just "don't swap", they also don't have memory barriers (which means
>> they are essentially useless to most drivers unless those are platform
>> specific drivers which know exactly what they are doing, or in the rare
>> cases such as accessing a framebuffer which we know never have side
>> effects).
>>
>>> Calling it iowrite*_native is also an abuse of the namespace.
>>
>>> Next thing we know some common code
>>> will legitimately use that name.
>> I might make sense to those definitions into a common header. There have
>> been a handful of cases in the past that wanted that sort of "native
>> byte order" MMIOs iirc (though don't ask me for examples, I can't really
>> remember).
>>
>>> If we do need to define an alias
>>> (which I'd like to avoid) it should be something like vfio_iowrite32.
>
> Ping?
>
> We need to make a decision whether to move those xxx_native() helpers
> somewhere (where?) or leave the patch as is (as we figured out that
> iowriteXX functions implement barriers and we cannot just use raw
> accessors) and fix commit log to explain everything.
Is there actually any difference in generated code with this patch
applied and without? I would hope that iowrite..() is inlined and
cancels out the cpu_to_le..() calls that are also inlined?
Alex
^ permalink raw reply
* Re: [PATCH v4] KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule
From: Alexander Graf @ 2014-06-24 12:12 UTC (permalink / raw)
To: Mihai Caraman, kvm-ppc; +Cc: Scott Wood, linuxppc-dev, kvm
In-Reply-To: <1403075722-11151-1-git-send-email-mihai.caraman@freescale.com>
On 18.06.14 09:15, Mihai Caraman wrote:
> On vcpu schedule, the condition checked for tlb pollution is too loose.
> The tlb entries of a vcpu become polluted (vs stale) only when a different
> vcpu within the same logical partition runs in-between. Optimize the tlb
> invalidation condition keeping last_vcpu per logical partition id.
>
> With the new invalidation condition, a guest shows 4% performance improvement
> on P5020DS while running a memory stress application with the cpu oversubscribed,
> the other guest running a cpu intensive workload.
>
> Guest - old invalidation condition
> real 3.89
> user 3.87
> sys 0.01
>
> Guest - enhanced invalidation condition
> real 3.75
> user 3.73
> sys 0.01
>
> Host
> real 3.70
> user 1.85
> sys 0.00
>
> The memory stress application accesses 4KB pages backed by 75% of available
> TLB0 entries:
>
> char foo[ENTRIES][4096] __attribute__ ((aligned (4096)));
>
> int main()
> {
> char bar;
> int i, j;
>
> for (i = 0; i < ITERATIONS; i++)
> for (j = 0; j < ENTRIES; j++)
> bar = foo[j][0];
>
> return 0;
> }
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> Cc: Scott Wood <scottwood@freescale.com>
Thanks, applied to kvm-ppc-queue.
Alex
^ permalink raw reply
* Re: [PATCH] vfio: Fix endianness handling for emulated BARs
From: Alexey Kardashevskiy @ 2014-06-24 12:50 UTC (permalink / raw)
To: Alexander Graf, Benjamin Herrenschmidt, Alex Williamson
Cc: Nikunj A Dadhania, linuxppc-dev, linux-kernel, kvm
In-Reply-To: <53A955F5.6050801@suse.de>
On 06/24/2014 08:41 PM, Alexander Graf wrote:
>
> On 24.06.14 12:11, Alexey Kardashevskiy wrote:
>> On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote:
>>> On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote:
>>>
>>>> Working on big endian being an accident may be a matter of perspective
>>> :-)
>>>
>>>> The comment remains that this patch doesn't actually fix anything except
>>>> the overhead on big endian systems doing redundant byte swapping and
>>>> maybe the philosophy that vfio regions are little endian.
>>> Yes, that works by accident because technically VFIO is a transport and
>>> thus shouldn't perform any endian swapping of any sort, which remains
>>> the responsibility of the end driver which is the only one to know
>>> whether a given BAR location is a a register or some streaming data
>>> and in the former case whether it's LE or BE (some PCI devices are BE
>>> even ! :-)
>>>
>>> But yes, in the end, it works with the dual "cancelling" swaps and the
>>> overhead of those swaps is probably drowned in the noise of the syscall
>>> overhead.
>>>
>>>> I'm still not a fan of iowrite vs iowritebe, there must be something we
>>>> can use that doesn't have an implicit swap.
>>> Sadly there isn't ... In the old day we didn't even have the "be"
>>> variant and readl/writel style accessors still don't have them either
>>> for all archs.
>>>
>>> There is __raw_readl/writel but here the semantics are much more than
>>> just "don't swap", they also don't have memory barriers (which means
>>> they are essentially useless to most drivers unless those are platform
>>> specific drivers which know exactly what they are doing, or in the rare
>>> cases such as accessing a framebuffer which we know never have side
>>> effects).
>>>
>>>> Calling it iowrite*_native is also an abuse of the namespace.
>>>
>>>> Next thing we know some common code
>>>> will legitimately use that name.
>>> I might make sense to those definitions into a common header. There have
>>> been a handful of cases in the past that wanted that sort of "native
>>> byte order" MMIOs iirc (though don't ask me for examples, I can't really
>>> remember).
>>>
>>>> If we do need to define an alias
>>>> (which I'd like to avoid) it should be something like vfio_iowrite32.
>>
>> Ping?
>>
>> We need to make a decision whether to move those xxx_native() helpers
>> somewhere (where?) or leave the patch as is (as we figured out that
>> iowriteXX functions implement barriers and we cannot just use raw
>> accessors) and fix commit log to explain everything.
>
> Is there actually any difference in generated code with this patch applied
> and without? I would hope that iowrite..() is inlined and cancels out the
> cpu_to_le..() calls that are also inlined?
iowrite32 is a non-inline function so conversions take place so are the
others. And sorry but I fail to see why this matters. We are not trying to
accelerate things, we are removing redundant operations which confuse
people who read the code.
--
Alexey
^ permalink raw reply
* Re: [PATCH] vfio: Fix endianness handling for emulated BARs
From: Alexander Graf @ 2014-06-24 12:52 UTC (permalink / raw)
To: Alexey Kardashevskiy, Benjamin Herrenschmidt, Alex Williamson
Cc: Nikunj A Dadhania, linuxppc-dev, linux-kernel, kvm
In-Reply-To: <53A9741B.1040500@ozlabs.ru>
On 24.06.14 14:50, Alexey Kardashevskiy wrote:
> On 06/24/2014 08:41 PM, Alexander Graf wrote:
>> On 24.06.14 12:11, Alexey Kardashevskiy wrote:
>>> On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote:
>>>> On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote:
>>>>
>>>>> Working on big endian being an accident may be a matter of perspective
>>>> :-)
>>>>
>>>>> The comment remains that this patch doesn't actually fix anything except
>>>>> the overhead on big endian systems doing redundant byte swapping and
>>>>> maybe the philosophy that vfio regions are little endian.
>>>> Yes, that works by accident because technically VFIO is a transport and
>>>> thus shouldn't perform any endian swapping of any sort, which remains
>>>> the responsibility of the end driver which is the only one to know
>>>> whether a given BAR location is a a register or some streaming data
>>>> and in the former case whether it's LE or BE (some PCI devices are BE
>>>> even ! :-)
>>>>
>>>> But yes, in the end, it works with the dual "cancelling" swaps and the
>>>> overhead of those swaps is probably drowned in the noise of the syscall
>>>> overhead.
>>>>
>>>>> I'm still not a fan of iowrite vs iowritebe, there must be something we
>>>>> can use that doesn't have an implicit swap.
>>>> Sadly there isn't ... In the old day we didn't even have the "be"
>>>> variant and readl/writel style accessors still don't have them either
>>>> for all archs.
>>>>
>>>> There is __raw_readl/writel but here the semantics are much more than
>>>> just "don't swap", they also don't have memory barriers (which means
>>>> they are essentially useless to most drivers unless those are platform
>>>> specific drivers which know exactly what they are doing, or in the rare
>>>> cases such as accessing a framebuffer which we know never have side
>>>> effects).
>>>>
>>>>> Calling it iowrite*_native is also an abuse of the namespace.
>>>>> Next thing we know some common code
>>>>> will legitimately use that name.
>>>> I might make sense to those definitions into a common header. There have
>>>> been a handful of cases in the past that wanted that sort of "native
>>>> byte order" MMIOs iirc (though don't ask me for examples, I can't really
>>>> remember).
>>>>
>>>>> If we do need to define an alias
>>>>> (which I'd like to avoid) it should be something like vfio_iowrite32.
>>> Ping?
>>>
>>> We need to make a decision whether to move those xxx_native() helpers
>>> somewhere (where?) or leave the patch as is (as we figured out that
>>> iowriteXX functions implement barriers and we cannot just use raw
>>> accessors) and fix commit log to explain everything.
>> Is there actually any difference in generated code with this patch applied
>> and without? I would hope that iowrite..() is inlined and cancels out the
>> cpu_to_le..() calls that are also inlined?
> iowrite32 is a non-inline function so conversions take place so are the
> others. And sorry but I fail to see why this matters. We are not trying to
> accelerate things, we are removing redundant operations which confuse
> people who read the code.
The confusion depends on where you're coming from. If you happen to know
that "iowrite32" writes in LE, then the LE conversion makes a lot of sense.
I don't have a strong feeling either way though and will let Alex decide
on the path forward :).
Alex
^ permalink raw reply
* Re: [PATCH] vfio: Fix endianness handling for emulated BARs
From: Alexey Kardashevskiy @ 2014-06-24 13:01 UTC (permalink / raw)
To: Alexander Graf, Benjamin Herrenschmidt, Alex Williamson
Cc: Nikunj A Dadhania, linuxppc-dev, linux-kernel, kvm
In-Reply-To: <53A97486.4070604@suse.de>
On 06/24/2014 10:52 PM, Alexander Graf wrote:
>
> On 24.06.14 14:50, Alexey Kardashevskiy wrote:
>> On 06/24/2014 08:41 PM, Alexander Graf wrote:
>>> On 24.06.14 12:11, Alexey Kardashevskiy wrote:
>>>> On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote:
>>>>> On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote:
>>>>>
>>>>>> Working on big endian being an accident may be a matter of perspective
>>>>> :-)
>>>>>
>>>>>> The comment remains that this patch doesn't actually fix anything except
>>>>>> the overhead on big endian systems doing redundant byte swapping and
>>>>>> maybe the philosophy that vfio regions are little endian.
>>>>> Yes, that works by accident because technically VFIO is a transport and
>>>>> thus shouldn't perform any endian swapping of any sort, which remains
>>>>> the responsibility of the end driver which is the only one to know
>>>>> whether a given BAR location is a a register or some streaming data
>>>>> and in the former case whether it's LE or BE (some PCI devices are BE
>>>>> even ! :-)
>>>>>
>>>>> But yes, in the end, it works with the dual "cancelling" swaps and the
>>>>> overhead of those swaps is probably drowned in the noise of the syscall
>>>>> overhead.
>>>>>
>>>>>> I'm still not a fan of iowrite vs iowritebe, there must be something we
>>>>>> can use that doesn't have an implicit swap.
>>>>> Sadly there isn't ... In the old day we didn't even have the "be"
>>>>> variant and readl/writel style accessors still don't have them either
>>>>> for all archs.
>>>>>
>>>>> There is __raw_readl/writel but here the semantics are much more than
>>>>> just "don't swap", they also don't have memory barriers (which means
>>>>> they are essentially useless to most drivers unless those are platform
>>>>> specific drivers which know exactly what they are doing, or in the rare
>>>>> cases such as accessing a framebuffer which we know never have side
>>>>> effects).
>>>>>
>>>>>> Calling it iowrite*_native is also an abuse of the namespace.
>>>>>> Next thing we know some common code
>>>>>> will legitimately use that name.
>>>>> I might make sense to those definitions into a common header. There have
>>>>> been a handful of cases in the past that wanted that sort of "native
>>>>> byte order" MMIOs iirc (though don't ask me for examples, I can't really
>>>>> remember).
>>>>>
>>>>>> If we do need to define an alias
>>>>>> (which I'd like to avoid) it should be something like vfio_iowrite32.
>>>> Ping?
>>>>
>>>> We need to make a decision whether to move those xxx_native() helpers
>>>> somewhere (where?) or leave the patch as is (as we figured out that
>>>> iowriteXX functions implement barriers and we cannot just use raw
>>>> accessors) and fix commit log to explain everything.
>>> Is there actually any difference in generated code with this patch applied
>>> and without? I would hope that iowrite..() is inlined and cancels out the
>>> cpu_to_le..() calls that are also inlined?
>> iowrite32 is a non-inline function so conversions take place so are the
>> others. And sorry but I fail to see why this matters. We are not trying to
>> accelerate things, we are removing redundant operations which confuse
>> people who read the code.
>
> The confusion depends on where you're coming from. If you happen to know
> that "iowrite32" writes in LE, then the LE conversion makes a lot of sense.
It was like this (and this is just confusing):
iowrite32(le32_to_cpu(val), io + off);
What would make sense (according to you and I would understand this) is this:
iowrite32(cpu_to_le32(val), io + off);
Or I missed your point, did I?
> I don't have a strong feeling either way though and will let Alex decide on
> the path forward :)
It would probably help if you picked the side :)
--
Alexey
^ permalink raw reply
* Re: [PATCH] vfio: Fix endianness handling for emulated BARs
From: Alexander Graf @ 2014-06-24 13:22 UTC (permalink / raw)
To: Alexey Kardashevskiy, Benjamin Herrenschmidt, Alex Williamson
Cc: Nikunj A Dadhania, linuxppc-dev, linux-kernel, kvm
In-Reply-To: <53A976B7.3070709@ozlabs.ru>
On 24.06.14 15:01, Alexey Kardashevskiy wrote:
> On 06/24/2014 10:52 PM, Alexander Graf wrote:
>> On 24.06.14 14:50, Alexey Kardashevskiy wrote:
>>> On 06/24/2014 08:41 PM, Alexander Graf wrote:
>>>> On 24.06.14 12:11, Alexey Kardashevskiy wrote:
>>>>> On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote:
>>>>>> On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote:
>>>>>>
>>>>>>> Working on big endian being an accident may be a matter of perspective
>>>>>> :-)
>>>>>>
>>>>>>> The comment remains that this patch doesn't actually fix anything except
>>>>>>> the overhead on big endian systems doing redundant byte swapping and
>>>>>>> maybe the philosophy that vfio regions are little endian.
>>>>>> Yes, that works by accident because technically VFIO is a transport and
>>>>>> thus shouldn't perform any endian swapping of any sort, which remains
>>>>>> the responsibility of the end driver which is the only one to know
>>>>>> whether a given BAR location is a a register or some streaming data
>>>>>> and in the former case whether it's LE or BE (some PCI devices are BE
>>>>>> even ! :-)
>>>>>>
>>>>>> But yes, in the end, it works with the dual "cancelling" swaps and the
>>>>>> overhead of those swaps is probably drowned in the noise of the syscall
>>>>>> overhead.
>>>>>>
>>>>>>> I'm still not a fan of iowrite vs iowritebe, there must be something we
>>>>>>> can use that doesn't have an implicit swap.
>>>>>> Sadly there isn't ... In the old day we didn't even have the "be"
>>>>>> variant and readl/writel style accessors still don't have them either
>>>>>> for all archs.
>>>>>>
>>>>>> There is __raw_readl/writel but here the semantics are much more than
>>>>>> just "don't swap", they also don't have memory barriers (which means
>>>>>> they are essentially useless to most drivers unless those are platform
>>>>>> specific drivers which know exactly what they are doing, or in the rare
>>>>>> cases such as accessing a framebuffer which we know never have side
>>>>>> effects).
>>>>>>
>>>>>>> Calling it iowrite*_native is also an abuse of the namespace.
>>>>>>> Next thing we know some common code
>>>>>>> will legitimately use that name.
>>>>>> I might make sense to those definitions into a common header. There have
>>>>>> been a handful of cases in the past that wanted that sort of "native
>>>>>> byte order" MMIOs iirc (though don't ask me for examples, I can't really
>>>>>> remember).
>>>>>>
>>>>>>> If we do need to define an alias
>>>>>>> (which I'd like to avoid) it should be something like vfio_iowrite32.
>>>>> Ping?
>>>>>
>>>>> We need to make a decision whether to move those xxx_native() helpers
>>>>> somewhere (where?) or leave the patch as is (as we figured out that
>>>>> iowriteXX functions implement barriers and we cannot just use raw
>>>>> accessors) and fix commit log to explain everything.
>>>> Is there actually any difference in generated code with this patch applied
>>>> and without? I would hope that iowrite..() is inlined and cancels out the
>>>> cpu_to_le..() calls that are also inlined?
>>> iowrite32 is a non-inline function so conversions take place so are the
>>> others. And sorry but I fail to see why this matters. We are not trying to
>>> accelerate things, we are removing redundant operations which confuse
>>> people who read the code.
>> The confusion depends on where you're coming from. If you happen to know
>> that "iowrite32" writes in LE, then the LE conversion makes a lot of sense.
> It was like this (and this is just confusing):
>
> iowrite32(le32_to_cpu(val), io + off);
>
> What would make sense (according to you and I would understand this) is this:
>
> iowrite32(cpu_to_le32(val), io + off);
>
>
> Or I missed your point, did I?
No, you didn't miss it. I think for people who know how iowrite32()
works the above is obvious. I find the fact that iowrite32() writes in
LE always pretty scary though ;).
So IMHO we should either create new, generic iowrite helpers that don't
do any endian swapping at all or do iowrite32(cpu_to_le32(val)) calls.
Alex
^ permalink raw reply
* Re: [PATCH] KVM: PPC: Book3E: Unlock mmu_lock when setting caching atttribute
From: Alexander Graf @ 2014-06-24 13:39 UTC (permalink / raw)
To: Mihai Caraman, kvm-ppc; +Cc: Bharat Bhushan, linuxppc-dev, kvm
In-Reply-To: <1403106305-11565-1-git-send-email-mihai.caraman@freescale.com>
On 18.06.14 17:45, Mihai Caraman wrote:
> The patch 08c9a188d0d0fc0f0c5e17d89a06bb59c493110f
> kvm: powerpc: use caching attributes as per linux pte
> do not handle properly the error case, letting mmu_lock locked. The lock
> will further generate a RCU stall from kvmppc_e500_emul_tlbwe() caller.
>
> In case of an error go to out label.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> Cc: Bharat Bhushan <bharat.bhushan@freescale.com>
Thanks, applied to for-3.16.
Alex
^ permalink raw reply
* Re: [PATCH] vfio: Fix endianness handling for emulated BARs
From: Alex Williamson @ 2014-06-24 14:21 UTC (permalink / raw)
To: Alexander Graf
Cc: kvm, Nikunj A Dadhania, Alexey Kardashevskiy, linux-kernel,
linuxppc-dev
In-Reply-To: <53A97BA9.4020702@suse.de>
On Tue, 2014-06-24 at 15:22 +0200, Alexander Graf wrote:
> On 24.06.14 15:01, Alexey Kardashevskiy wrote:
> > On 06/24/2014 10:52 PM, Alexander Graf wrote:
> >> On 24.06.14 14:50, Alexey Kardashevskiy wrote:
> >>> On 06/24/2014 08:41 PM, Alexander Graf wrote:
> >>>> On 24.06.14 12:11, Alexey Kardashevskiy wrote:
> >>>>> On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote:
> >>>>>> On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote:
> >>>>>>
> >>>>>>> Working on big endian being an accident may be a matter of perspective
> >>>>>> :-)
> >>>>>>
> >>>>>>> The comment remains that this patch doesn't actually fix anything except
> >>>>>>> the overhead on big endian systems doing redundant byte swapping and
> >>>>>>> maybe the philosophy that vfio regions are little endian.
> >>>>>> Yes, that works by accident because technically VFIO is a transport and
> >>>>>> thus shouldn't perform any endian swapping of any sort, which remains
> >>>>>> the responsibility of the end driver which is the only one to know
> >>>>>> whether a given BAR location is a a register or some streaming data
> >>>>>> and in the former case whether it's LE or BE (some PCI devices are BE
> >>>>>> even ! :-)
> >>>>>>
> >>>>>> But yes, in the end, it works with the dual "cancelling" swaps and the
> >>>>>> overhead of those swaps is probably drowned in the noise of the syscall
> >>>>>> overhead.
> >>>>>>
> >>>>>>> I'm still not a fan of iowrite vs iowritebe, there must be something we
> >>>>>>> can use that doesn't have an implicit swap.
> >>>>>> Sadly there isn't ... In the old day we didn't even have the "be"
> >>>>>> variant and readl/writel style accessors still don't have them either
> >>>>>> for all archs.
> >>>>>>
> >>>>>> There is __raw_readl/writel but here the semantics are much more than
> >>>>>> just "don't swap", they also don't have memory barriers (which means
> >>>>>> they are essentially useless to most drivers unless those are platform
> >>>>>> specific drivers which know exactly what they are doing, or in the rare
> >>>>>> cases such as accessing a framebuffer which we know never have side
> >>>>>> effects).
> >>>>>>
> >>>>>>> Calling it iowrite*_native is also an abuse of the namespace.
> >>>>>>> Next thing we know some common code
> >>>>>>> will legitimately use that name.
> >>>>>> I might make sense to those definitions into a common header. There have
> >>>>>> been a handful of cases in the past that wanted that sort of "native
> >>>>>> byte order" MMIOs iirc (though don't ask me for examples, I can't really
> >>>>>> remember).
> >>>>>>
> >>>>>>> If we do need to define an alias
> >>>>>>> (which I'd like to avoid) it should be something like vfio_iowrite32.
> >>>>> Ping?
> >>>>>
> >>>>> We need to make a decision whether to move those xxx_native() helpers
> >>>>> somewhere (where?) or leave the patch as is (as we figured out that
> >>>>> iowriteXX functions implement barriers and we cannot just use raw
> >>>>> accessors) and fix commit log to explain everything.
> >>>> Is there actually any difference in generated code with this patch applied
> >>>> and without? I would hope that iowrite..() is inlined and cancels out the
> >>>> cpu_to_le..() calls that are also inlined?
> >>> iowrite32 is a non-inline function so conversions take place so are the
> >>> others. And sorry but I fail to see why this matters. We are not trying to
> >>> accelerate things, we are removing redundant operations which confuse
> >>> people who read the code.
> >> The confusion depends on where you're coming from. If you happen to know
> >> that "iowrite32" writes in LE, then the LE conversion makes a lot of sense.
> > It was like this (and this is just confusing):
> >
> > iowrite32(le32_to_cpu(val), io + off);
> >
> > What would make sense (according to you and I would understand this) is this:
> >
> > iowrite32(cpu_to_le32(val), io + off);
> >
> >
> > Or I missed your point, did I?
>
> No, you didn't miss it. I think for people who know how iowrite32()
> works the above is obvious. I find the fact that iowrite32() writes in
> LE always pretty scary though ;).
>
> So IMHO we should either create new, generic iowrite helpers that don't
> do any endian swapping at all or do iowrite32(cpu_to_le32(val)) calls.
I'm one of those people for whom iowrite32(le32_to_cpu(val)) makes sense
and keeps the byte order consistent regardless of the platform, while
iowrite32(val) or iowrite32be(val) makes me scratch my head and try to
remember that the byte swaps are a nop on the given platforms. As Ben
noted, a native, no-swap ioread/write doesn't exist, but perhaps should.
I'd prefer an attempt be made to make it exist before adding
vfio-specific macros. vfio is arguably doing the right thing here given
the functions available. Thanks,
Alex
^ permalink raw reply
* Re: [PATCH 2/2] powerpc: bpf: Fix the broken LD_VLAN_TAG_PRESENT test
From: Sergei Shtylyov @ 2014-06-24 14:23 UTC (permalink / raw)
To: Denis Kirjanov, netdev; +Cc: linuxppc-dev
In-Reply-To: <1403603969-24436-2-git-send-email-kda@linux-powerpc.org>
Hello.
On 06/24/2014 01:59 PM, Denis Kirjanov wrote:
> We have to return the boolean here if the tag presents
> or not, not jusr ORing the TCI with the mask which results to:
> [ 709.412097] test_bpf: #18 LD_VLAN_TAG_PRESENT
> [ 709.412245] ret 4096 != 1
> [ 709.412332] ret 4096 != 1
> [ 709.412333] FAIL (2 times)
You need to sign off on the patch, else it won't be applied.
> ---
> arch/powerpc/net/bpf_jit_comp.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
> diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
> index af0ed4d..a3d8f58 100644
> --- a/arch/powerpc/net/bpf_jit_comp.c
> +++ b/arch/powerpc/net/bpf_jit_comp.c
> @@ -394,8 +394,10 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 *image,
> vlan_tci));
> if (code == (BPF_ANC | SKF_AD_VLAN_TAG))
> PPC_ANDI(r_A, r_A, ~VLAN_TAG_PRESENT);
> - else
> + else {
All arms of the *if* statement should have {} if one branch has {}.
> PPC_ANDI(r_A, r_A, VLAN_TAG_PRESENT);
> + PPC_SRWI(r_A, r_A, 12);
> + }
> break;
> case BPF_ANC | SKF_AD_QUEUE:
> BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff,
WBR, Sergei
^ permalink raw reply
* [PATCH 0/3] iommu/fsl: Fixes for the PAMU driver.
From: Varun Sethi @ 2014-06-24 13:57 UTC (permalink / raw)
To: iommu, joro, linux-kernel, linuxppc-dev, alex.williamson; +Cc: Varun Sethi
This patch set contains fixes for the PAMU driver.
The patches are based on 3.16-rc1.
Varun Sethi (3):
Fix PAMU window size check.
Fix the device domain attach condition.
Fix the error condition during iommu group creation.
drivers/iommu/fsl_pamu.c | 8 ++++----
drivers/iommu/fsl_pamu_domain.c | 19 +++++++++----------
2 files changed, 13 insertions(+), 14 deletions(-)
--
1.7.9.5
^ permalink raw reply
* Re: [PATCH] vfio: Fix endianness handling for emulated BARs
From: Alexey Kardashevskiy @ 2014-06-24 14:33 UTC (permalink / raw)
To: Alex Williamson, Alexander Graf
Cc: Nikunj A Dadhania, linuxppc-dev, linux-kernel, kvm
In-Reply-To: <1403619685.16078.15.camel@ul30vt.home>
On 06/25/2014 12:21 AM, Alex Williamson wrote:
> On Tue, 2014-06-24 at 15:22 +0200, Alexander Graf wrote:
>> On 24.06.14 15:01, Alexey Kardashevskiy wrote:
>>> On 06/24/2014 10:52 PM, Alexander Graf wrote:
>>>> On 24.06.14 14:50, Alexey Kardashevskiy wrote:
>>>>> On 06/24/2014 08:41 PM, Alexander Graf wrote:
>>>>>> On 24.06.14 12:11, Alexey Kardashevskiy wrote:
>>>>>>> On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote:
>>>>>>>> On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote:
>>>>>>>>
>>>>>>>>> Working on big endian being an accident may be a matter of perspective
>>>>>>>> :-)
>>>>>>>>
>>>>>>>>> The comment remains that this patch doesn't actually fix anything except
>>>>>>>>> the overhead on big endian systems doing redundant byte swapping and
>>>>>>>>> maybe the philosophy that vfio regions are little endian.
>>>>>>>> Yes, that works by accident because technically VFIO is a transport and
>>>>>>>> thus shouldn't perform any endian swapping of any sort, which remains
>>>>>>>> the responsibility of the end driver which is the only one to know
>>>>>>>> whether a given BAR location is a a register or some streaming data
>>>>>>>> and in the former case whether it's LE or BE (some PCI devices are BE
>>>>>>>> even ! :-)
>>>>>>>>
>>>>>>>> But yes, in the end, it works with the dual "cancelling" swaps and the
>>>>>>>> overhead of those swaps is probably drowned in the noise of the syscall
>>>>>>>> overhead.
>>>>>>>>
>>>>>>>>> I'm still not a fan of iowrite vs iowritebe, there must be something we
>>>>>>>>> can use that doesn't have an implicit swap.
>>>>>>>> Sadly there isn't ... In the old day we didn't even have the "be"
>>>>>>>> variant and readl/writel style accessors still don't have them either
>>>>>>>> for all archs.
>>>>>>>>
>>>>>>>> There is __raw_readl/writel but here the semantics are much more than
>>>>>>>> just "don't swap", they also don't have memory barriers (which means
>>>>>>>> they are essentially useless to most drivers unless those are platform
>>>>>>>> specific drivers which know exactly what they are doing, or in the rare
>>>>>>>> cases such as accessing a framebuffer which we know never have side
>>>>>>>> effects).
>>>>>>>>
>>>>>>>>> Calling it iowrite*_native is also an abuse of the namespace.
>>>>>>>>> Next thing we know some common code
>>>>>>>>> will legitimately use that name.
>>>>>>>> I might make sense to those definitions into a common header. There have
>>>>>>>> been a handful of cases in the past that wanted that sort of "native
>>>>>>>> byte order" MMIOs iirc (though don't ask me for examples, I can't really
>>>>>>>> remember).
>>>>>>>>
>>>>>>>>> If we do need to define an alias
>>>>>>>>> (which I'd like to avoid) it should be something like vfio_iowrite32.
>>>>>>> Ping?
>>>>>>>
>>>>>>> We need to make a decision whether to move those xxx_native() helpers
>>>>>>> somewhere (where?) or leave the patch as is (as we figured out that
>>>>>>> iowriteXX functions implement barriers and we cannot just use raw
>>>>>>> accessors) and fix commit log to explain everything.
>>>>>> Is there actually any difference in generated code with this patch applied
>>>>>> and without? I would hope that iowrite..() is inlined and cancels out the
>>>>>> cpu_to_le..() calls that are also inlined?
>>>>> iowrite32 is a non-inline function so conversions take place so are the
>>>>> others. And sorry but I fail to see why this matters. We are not trying to
>>>>> accelerate things, we are removing redundant operations which confuse
>>>>> people who read the code.
>>>> The confusion depends on where you're coming from. If you happen to know
>>>> that "iowrite32" writes in LE, then the LE conversion makes a lot of sense.
>>> It was like this (and this is just confusing):
>>>
>>> iowrite32(le32_to_cpu(val), io + off);
>>>
>>> What would make sense (according to you and I would understand this) is this:
>>>
>>> iowrite32(cpu_to_le32(val), io + off);
>>>
>>>
>>> Or I missed your point, did I?
>>
>> No, you didn't miss it. I think for people who know how iowrite32()
>> works the above is obvious. I find the fact that iowrite32() writes in
>> LE always pretty scary though ;).
>>
>> So IMHO we should either create new, generic iowrite helpers that don't
>> do any endian swapping at all or do iowrite32(cpu_to_le32(val)) calls.
>
> I'm one of those people for whom iowrite32(le32_to_cpu(val)) makes sense
I do not understand why @val is considered LE here and need to be converted
to CPU. Really. I truly believe it should be cpu_to_le32().
> and keeps the byte order consistent regardless of the platform, while
> iowrite32(val) or iowrite32be(val) makes me scratch my head and try to
> remember that the byte swaps are a nop on the given platforms. As Ben
> noted, a native, no-swap ioread/write doesn't exist, but perhaps should.
> I'd prefer an attempt be made to make it exist before adding
> vfio-specific macros. vfio is arguably doing the right thing here given
> the functions available. Thanks,
--
Alexey
^ permalink raw reply
* RE: [PATCH] vfio: Fix endianness handling for emulated BARs
From: David Laight @ 2014-06-24 14:40 UTC (permalink / raw)
To: 'Alexey Kardashevskiy', Alex Williamson, Alexander Graf
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
Nikunj A Dadhania, kvm@vger.kernel.org
In-Reply-To: <53A98C38.8020205@ozlabs.ru>
RnJvbTogQWxleGV5IEthcmRhc2hldnNraXkNCi4uLg0KPiA+PiBTbyBJTUhPIHdlIHNob3VsZCBl
aXRoZXIgY3JlYXRlIG5ldywgZ2VuZXJpYyBpb3dyaXRlIGhlbHBlcnMgdGhhdCBkb24ndA0KPiA+
PiBkbyBhbnkgZW5kaWFuIHN3YXBwaW5nIGF0IGFsbCBvciBkbyBpb3dyaXRlMzIoY3B1X3RvX2xl
MzIodmFsKSkgY2FsbHMuDQo+ID4NCj4gPiBJJ20gb25lIG9mIHRob3NlIHBlb3BsZSBmb3Igd2hv
bSBpb3dyaXRlMzIobGUzMl90b19jcHUodmFsKSkgbWFrZXMgc2Vuc2UNCj4gDQo+IA0KPiBJIGRv
IG5vdCB1bmRlcnN0YW5kIHdoeSBAdmFsIGlzIGNvbnNpZGVyZWQgTEUgaGVyZSBhbmQgbmVlZCB0
byBiZSBjb252ZXJ0ZWQNCj4gdG8gQ1BVLiBSZWFsbHkuIEkgdHJ1bHkgYmVsaWV2ZSBpdCBzaG91
bGQgYmUgY3B1X3RvX2xlMzIoKS4NCg0KVGhpbmsgYWJvdXQgaXQgY2FyZWZ1bGx5Li4uDQoNCkFw
cGFyZW50bHkgaW93cml0ZTMyKCkgaXMgd3JpdGluZyBhICdjcHUnIHZhbHVlIG91dCAnbGUnLg0K
U28gaWYgeW91IGhhdmUgYSAnbGUnIHZhbHVlIHlvdSBuZWVkIHRvIGNvbnZlcnQgaXQgdG8gJ2Nw
dScgZmlyc3QuDQoNCkkgcmVhbGx5IHdvbid0IGFzayBob3cgJ2JlJyBwcGMgbWFuYWdlZCB0byBn
ZXQgJ2xlJyBvbi1jaGlwIHBlcmlwaGVyYWxzLg0KDQoJRGF2aWQNCg0K
^ permalink raw reply
* Re: [PATCH] vfio: Fix endianness handling for emulated BARs
From: Alex Williamson @ 2014-06-24 14:43 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: kvm, Nikunj A Dadhania, Alexander Graf, linux-kernel,
linuxppc-dev
In-Reply-To: <53A98C38.8020205@ozlabs.ru>
On Wed, 2014-06-25 at 00:33 +1000, Alexey Kardashevskiy wrote:
> On 06/25/2014 12:21 AM, Alex Williamson wrote:
> > On Tue, 2014-06-24 at 15:22 +0200, Alexander Graf wrote:
> >> On 24.06.14 15:01, Alexey Kardashevskiy wrote:
> >>> On 06/24/2014 10:52 PM, Alexander Graf wrote:
> >>>> On 24.06.14 14:50, Alexey Kardashevskiy wrote:
> >>>>> On 06/24/2014 08:41 PM, Alexander Graf wrote:
> >>>>>> On 24.06.14 12:11, Alexey Kardashevskiy wrote:
> >>>>>>> On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote:
> >>>>>>>> On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote:
> >>>>>>>>
> >>>>>>>>> Working on big endian being an accident may be a matter of perspective
> >>>>>>>> :-)
> >>>>>>>>
> >>>>>>>>> The comment remains that this patch doesn't actually fix anything except
> >>>>>>>>> the overhead on big endian systems doing redundant byte swapping and
> >>>>>>>>> maybe the philosophy that vfio regions are little endian.
> >>>>>>>> Yes, that works by accident because technically VFIO is a transport and
> >>>>>>>> thus shouldn't perform any endian swapping of any sort, which remains
> >>>>>>>> the responsibility of the end driver which is the only one to know
> >>>>>>>> whether a given BAR location is a a register or some streaming data
> >>>>>>>> and in the former case whether it's LE or BE (some PCI devices are BE
> >>>>>>>> even ! :-)
> >>>>>>>>
> >>>>>>>> But yes, in the end, it works with the dual "cancelling" swaps and the
> >>>>>>>> overhead of those swaps is probably drowned in the noise of the syscall
> >>>>>>>> overhead.
> >>>>>>>>
> >>>>>>>>> I'm still not a fan of iowrite vs iowritebe, there must be something we
> >>>>>>>>> can use that doesn't have an implicit swap.
> >>>>>>>> Sadly there isn't ... In the old day we didn't even have the "be"
> >>>>>>>> variant and readl/writel style accessors still don't have them either
> >>>>>>>> for all archs.
> >>>>>>>>
> >>>>>>>> There is __raw_readl/writel but here the semantics are much more than
> >>>>>>>> just "don't swap", they also don't have memory barriers (which means
> >>>>>>>> they are essentially useless to most drivers unless those are platform
> >>>>>>>> specific drivers which know exactly what they are doing, or in the rare
> >>>>>>>> cases such as accessing a framebuffer which we know never have side
> >>>>>>>> effects).
> >>>>>>>>
> >>>>>>>>> Calling it iowrite*_native is also an abuse of the namespace.
> >>>>>>>>> Next thing we know some common code
> >>>>>>>>> will legitimately use that name.
> >>>>>>>> I might make sense to those definitions into a common header. There have
> >>>>>>>> been a handful of cases in the past that wanted that sort of "native
> >>>>>>>> byte order" MMIOs iirc (though don't ask me for examples, I can't really
> >>>>>>>> remember).
> >>>>>>>>
> >>>>>>>>> If we do need to define an alias
> >>>>>>>>> (which I'd like to avoid) it should be something like vfio_iowrite32.
> >>>>>>> Ping?
> >>>>>>>
> >>>>>>> We need to make a decision whether to move those xxx_native() helpers
> >>>>>>> somewhere (where?) or leave the patch as is (as we figured out that
> >>>>>>> iowriteXX functions implement barriers and we cannot just use raw
> >>>>>>> accessors) and fix commit log to explain everything.
> >>>>>> Is there actually any difference in generated code with this patch applied
> >>>>>> and without? I would hope that iowrite..() is inlined and cancels out the
> >>>>>> cpu_to_le..() calls that are also inlined?
> >>>>> iowrite32 is a non-inline function so conversions take place so are the
> >>>>> others. And sorry but I fail to see why this matters. We are not trying to
> >>>>> accelerate things, we are removing redundant operations which confuse
> >>>>> people who read the code.
> >>>> The confusion depends on where you're coming from. If you happen to know
> >>>> that "iowrite32" writes in LE, then the LE conversion makes a lot of sense.
> >>> It was like this (and this is just confusing):
> >>>
> >>> iowrite32(le32_to_cpu(val), io + off);
> >>>
> >>> What would make sense (according to you and I would understand this) is this:
> >>>
> >>> iowrite32(cpu_to_le32(val), io + off);
> >>>
> >>>
> >>> Or I missed your point, did I?
> >>
> >> No, you didn't miss it. I think for people who know how iowrite32()
> >> works the above is obvious. I find the fact that iowrite32() writes in
> >> LE always pretty scary though ;).
> >>
> >> So IMHO we should either create new, generic iowrite helpers that don't
> >> do any endian swapping at all or do iowrite32(cpu_to_le32(val)) calls.
> >
> > I'm one of those people for whom iowrite32(le32_to_cpu(val)) makes sense
>
>
> I do not understand why @val is considered LE here and need to be converted
> to CPU. Really. I truly believe it should be cpu_to_le32().
Because iowrite32 is defined to take a cpu byte order value and write it
as little endian.
> > and keeps the byte order consistent regardless of the platform, while
> > iowrite32(val) or iowrite32be(val) makes me scratch my head and try to
> > remember that the byte swaps are a nop on the given platforms. As Ben
> > noted, a native, no-swap ioread/write doesn't exist, but perhaps should.
> > I'd prefer an attempt be made to make it exist before adding
> > vfio-specific macros. vfio is arguably doing the right thing here given
> > the functions available. Thanks,
>
>
^ permalink raw reply
* Re: [PATCH 2/2] powerpc: bpf: Fix the broken LD_VLAN_TAG_PRESENT test
From: Denis Kirjanov @ 2014-06-24 14:52 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: netdev, linuxppc-dev
In-Reply-To: <53A989CA.9030405@cogentembedded.com>
On 6/24/14, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
> On 06/24/2014 01:59 PM, Denis Kirjanov wrote:
>
>> We have to return the boolean here if the tag presents
>> or not, not jusr ORing the TCI with the mask which results to:
>
>> [ 709.412097] test_bpf: #18 LD_VLAN_TAG_PRESENT
>> [ 709.412245] ret 4096 != 1
>> [ 709.412332] ret 4096 != 1
>> [ 709.412333] FAIL (2 times)
>
> You need to sign off on the patch, else it won't be applied.
Oh, right. Moreover I've made a mistake in the description (we're ANDing)
Thanks!
>> ---
>> arch/powerpc/net/bpf_jit_comp.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>> diff --git a/arch/powerpc/net/bpf_jit_comp.c
>> b/arch/powerpc/net/bpf_jit_comp.c
>> index af0ed4d..a3d8f58 100644
>> --- a/arch/powerpc/net/bpf_jit_comp.c
>> +++ b/arch/powerpc/net/bpf_jit_comp.c
>> @@ -394,8 +394,10 @@ static int bpf_jit_build_body(struct sk_filter *fp,
>> u32 *image,
>> vlan_tci));
>> if (code == (BPF_ANC | SKF_AD_VLAN_TAG))
>> PPC_ANDI(r_A, r_A, ~VLAN_TAG_PRESENT);
>> - else
>> + else {
>
> All arms of the *if* statement should have {} if one branch has {}.
>
>> PPC_ANDI(r_A, r_A, VLAN_TAG_PRESENT);
>> + PPC_SRWI(r_A, r_A, 12);
>> + }
>> break;
>> case BPF_ANC | SKF_AD_QUEUE:
>> BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff,
>
> WBR, Sergei
>
>
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: bpf: Use correct mask while accessing the VLAN tag
From: Alexei Starovoitov @ 2014-06-24 15:29 UTC (permalink / raw)
To: Denis Kirjanov; +Cc: netdev@vger.kernel.org, linuxppc-dev
In-Reply-To: <1403603969-24436-1-git-send-email-kda@linux-powerpc.org>
On Tue, Jun 24, 2014 at 2:59 AM, Denis Kirjanov <kda@linux-powerpc.org> wrote:
> Use the proper mask which is 0xefff
sob is missing.
also please expand the commit message a bit, otherwise it's too cryptic for
folks who don't know bpf details.
> ---
> arch/powerpc/net/bpf_jit_comp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
> index 6dcdade..af0ed4d 100644
> --- a/arch/powerpc/net/bpf_jit_comp.c
> +++ b/arch/powerpc/net/bpf_jit_comp.c
> @@ -393,7 +393,7 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 *image,
> PPC_LHZ_OFFS(r_A, r_skb, offsetof(struct sk_buff,
> vlan_tci));
> if (code == (BPF_ANC | SKF_AD_VLAN_TAG))
> - PPC_ANDI(r_A, r_A, VLAN_VID_MASK);
> + PPC_ANDI(r_A, r_A, ~VLAN_TAG_PRESENT);
> else
> PPC_ANDI(r_A, r_A, VLAN_TAG_PRESENT);
> break;
> --
> 2.0.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/3] iommu/fsl: Fix PAMU window size check.
From: Varun Sethi @ 2014-06-24 13:57 UTC (permalink / raw)
To: iommu, joro, linux-kernel, linuxppc-dev, alex.williamson; +Cc: Varun Sethi
In-Reply-To: <1403618237-26248-1-git-send-email-Varun.Sethi@freescale.com>
is_power_of_2 requires an unsigned long parameter which would
lead to truncation of 64 bit values on 32 bit architectures.
__ffs also expects an unsigned long parameter thus won't work
for 64 bit values on 32 bit architectures.
Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
---
drivers/iommu/fsl_pamu.c | 8 ++++----
drivers/iommu/fsl_pamu_domain.c | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index b99dd88..bb446d7 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -170,10 +170,10 @@ int pamu_disable_liodn(int liodn)
static unsigned int map_addrspace_size_to_wse(phys_addr_t addrspace_size)
{
/* Bug if not a power of 2 */
- BUG_ON(!is_power_of_2(addrspace_size));
+ BUG_ON((addrspace_size & (addrspace_size - 1)));
/* window size is 2^(WSE+1) bytes */
- return __ffs(addrspace_size) - 1;
+ return fls64(addrspace_size) - 2;
}
/* Derive the PAACE window count encoding for the subwindow count */
@@ -351,7 +351,7 @@ int pamu_config_ppaace(int liodn, phys_addr_t win_addr, phys_addr_t win_size,
struct paace *ppaace;
unsigned long fspi;
- if (!is_power_of_2(win_size) || win_size < PAMU_PAGE_SIZE) {
+ if ((win_size & (win_size - 1)) || win_size < PAMU_PAGE_SIZE) {
pr_debug("window size too small or not a power of two %llx\n", win_size);
return -EINVAL;
}
@@ -464,7 +464,7 @@ int pamu_config_spaace(int liodn, u32 subwin_cnt, u32 subwin,
return -ENOENT;
}
- if (!is_power_of_2(subwin_size) || subwin_size < PAMU_PAGE_SIZE) {
+ if ((subwin_size & (subwin_size - 1)) || subwin_size < PAMU_PAGE_SIZE) {
pr_debug("subwindow size out of range, or not a power of 2\n");
return -EINVAL;
}
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 93072ba..3dd0b8e 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -301,7 +301,7 @@ static int check_size(u64 size, dma_addr_t iova)
* Size must be a power of two and at least be equal
* to PAMU page size.
*/
- if (!is_power_of_2(size) || size < PAMU_PAGE_SIZE) {
+ if ((size & (size - 1)) || size < PAMU_PAGE_SIZE) {
pr_debug("%s: size too small or not a power of two\n", __func__);
return -EINVAL;
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] vfio: Fix endianness handling for emulated BARs
From: Alexey Kardashevskiy @ 2014-06-24 16:26 UTC (permalink / raw)
To: Alex Williamson
Cc: kvm, Nikunj A Dadhania, Alexander Graf, linux-kernel,
linuxppc-dev
In-Reply-To: <1403620984.16078.18.camel@ul30vt.home>
On 06/25/2014 12:43 AM, Alex Williamson wrote:
> On Wed, 2014-06-25 at 00:33 +1000, Alexey Kardashevskiy wrote:
>> On 06/25/2014 12:21 AM, Alex Williamson wrote:
>>> On Tue, 2014-06-24 at 15:22 +0200, Alexander Graf wrote:
>>>> On 24.06.14 15:01, Alexey Kardashevskiy wrote:
>>>>> On 06/24/2014 10:52 PM, Alexander Graf wrote:
>>>>>> On 24.06.14 14:50, Alexey Kardashevskiy wrote:
>>>>>>> On 06/24/2014 08:41 PM, Alexander Graf wrote:
>>>>>>>> On 24.06.14 12:11, Alexey Kardashevskiy wrote:
>>>>>>>>> On 06/21/2014 09:12 AM, Benjamin Herrenschmidt wrote:
>>>>>>>>>> On Thu, 2014-06-19 at 21:21 -0600, Alex Williamson wrote:
>>>>>>>>>>
>>>>>>>>>>> Working on big endian being an accident may be a matter of perspective
>>>>>>>>>> :-)
>>>>>>>>>>
>>>>>>>>>>> The comment remains that this patch doesn't actually fix anything except
>>>>>>>>>>> the overhead on big endian systems doing redundant byte swapping and
>>>>>>>>>>> maybe the philosophy that vfio regions are little endian.
>>>>>>>>>> Yes, that works by accident because technically VFIO is a transport and
>>>>>>>>>> thus shouldn't perform any endian swapping of any sort, which remains
>>>>>>>>>> the responsibility of the end driver which is the only one to know
>>>>>>>>>> whether a given BAR location is a a register or some streaming data
>>>>>>>>>> and in the former case whether it's LE or BE (some PCI devices are BE
>>>>>>>>>> even ! :-)
>>>>>>>>>>
>>>>>>>>>> But yes, in the end, it works with the dual "cancelling" swaps and the
>>>>>>>>>> overhead of those swaps is probably drowned in the noise of the syscall
>>>>>>>>>> overhead.
>>>>>>>>>>
>>>>>>>>>>> I'm still not a fan of iowrite vs iowritebe, there must be something we
>>>>>>>>>>> can use that doesn't have an implicit swap.
>>>>>>>>>> Sadly there isn't ... In the old day we didn't even have the "be"
>>>>>>>>>> variant and readl/writel style accessors still don't have them either
>>>>>>>>>> for all archs.
>>>>>>>>>>
>>>>>>>>>> There is __raw_readl/writel but here the semantics are much more than
>>>>>>>>>> just "don't swap", they also don't have memory barriers (which means
>>>>>>>>>> they are essentially useless to most drivers unless those are platform
>>>>>>>>>> specific drivers which know exactly what they are doing, or in the rare
>>>>>>>>>> cases such as accessing a framebuffer which we know never have side
>>>>>>>>>> effects).
>>>>>>>>>>
>>>>>>>>>>> Calling it iowrite*_native is also an abuse of the namespace.
>>>>>>>>>>> Next thing we know some common code
>>>>>>>>>>> will legitimately use that name.
>>>>>>>>>> I might make sense to those definitions into a common header. There have
>>>>>>>>>> been a handful of cases in the past that wanted that sort of "native
>>>>>>>>>> byte order" MMIOs iirc (though don't ask me for examples, I can't really
>>>>>>>>>> remember).
>>>>>>>>>>
>>>>>>>>>>> If we do need to define an alias
>>>>>>>>>>> (which I'd like to avoid) it should be something like vfio_iowrite32.
>>>>>>>>> Ping?
>>>>>>>>>
>>>>>>>>> We need to make a decision whether to move those xxx_native() helpers
>>>>>>>>> somewhere (where?) or leave the patch as is (as we figured out that
>>>>>>>>> iowriteXX functions implement barriers and we cannot just use raw
>>>>>>>>> accessors) and fix commit log to explain everything.
>>>>>>>> Is there actually any difference in generated code with this patch applied
>>>>>>>> and without? I would hope that iowrite..() is inlined and cancels out the
>>>>>>>> cpu_to_le..() calls that are also inlined?
>>>>>>> iowrite32 is a non-inline function so conversions take place so are the
>>>>>>> others. And sorry but I fail to see why this matters. We are not trying to
>>>>>>> accelerate things, we are removing redundant operations which confuse
>>>>>>> people who read the code.
>>>>>> The confusion depends on where you're coming from. If you happen to know
>>>>>> that "iowrite32" writes in LE, then the LE conversion makes a lot of sense.
>>>>> It was like this (and this is just confusing):
>>>>>
>>>>> iowrite32(le32_to_cpu(val), io + off);
>>>>>
>>>>> What would make sense (according to you and I would understand this) is this:
>>>>>
>>>>> iowrite32(cpu_to_le32(val), io + off);
>>>>>
>>>>>
>>>>> Or I missed your point, did I?
>>>>
>>>> No, you didn't miss it. I think for people who know how iowrite32()
>>>> works the above is obvious. I find the fact that iowrite32() writes in
>>>> LE always pretty scary though ;).
>>>>
>>>> So IMHO we should either create new, generic iowrite helpers that don't
>>>> do any endian swapping at all or do iowrite32(cpu_to_le32(val)) calls.
>>>
>>> I'm one of those people for whom iowrite32(le32_to_cpu(val)) makes sense
>>
>>
>> I do not understand why @val is considered LE here and need to be converted
>> to CPU. Really. I truly believe it should be cpu_to_le32().
>
> Because iowrite32 is defined to take a cpu byte order value and write it
> as little endian.
Ok, then neither le32_to_cpu() nor cpu_to_le32() should be there at all, if
we are talking about not scratching anyone's head :)
>>> and keeps the byte order consistent regardless of the platform, while
>>> iowrite32(val) or iowrite32be(val) makes me scratch my head and try to
>>> remember that the byte swaps are a nop on the given platforms. As Ben
>>> noted, a native, no-swap ioread/write doesn't exist, but perhaps should.
>>> I'd prefer an attempt be made to make it exist before adding
>>> vfio-specific macros. vfio is arguably doing the right thing here given
>>> the functions available. Thanks,
I do not mind to make that atempt but what exactly would make sense here?
Try moving macros to include/asm-generic/io.h? Something else? Thanks.
--
Alexey
^ permalink raw reply
* Re: [PATCH v2 1/2] flexcan: add err_irq handler for flexcan
From: Scott Wood @ 2014-06-24 17:34 UTC (permalink / raw)
To: Zhao Qiang-B45475
Cc: mkl@pengutronix.de, linuxppc-dev@lists.ozlabs.org,
wg@grandegger.com, linux-can@vger.kernel.org
In-Reply-To: <fc0a7150924249768aee8e4361c654a1@BLUPR03MB341.namprd03.prod.outlook.com>
On Mon, 2014-06-23 at 01:20 -0500, Zhao Qiang-B45475 wrote:
> On Sat, 2014-06-21 at 12:19, Wood Scott wrote:
>
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Saturday, June 21, 2014 12:19 AM
> > To: Zhao Qiang-B45475
> > Cc: linuxppc-dev@lists.ozlabs.org; linux-can@vger.kernel.org;
> > wg@grandegger.com; mkl@pengutronix.de; Wood Scott-B07421
> > Subject: Re: [PATCH v2 1/2] flexcan: add err_irq handler for flexcan
> >
> > On Fri, 2014-06-20 at 10:01 +0800, Zhao Qiang wrote:
> > > when flexcan is not physically linked, command 'cantest' will trigger
> > > an err_irq, add err_irq handler for it.
> > >
> > > Signed-off-by: Zhao Qiang <B45475@freescale.com>
> > > ---
> > > Changes for v2:
> > > - use a space instead of tab
> > > - use flexcan_poll_state instead of print
> > >
> > > drivers/net/can/flexcan.c | 31 ++++++++++++++++++++++++++++++-
> > > 1 file changed, 30 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > > index f425ec2..7432ba4 100644
> > > --- a/drivers/net/can/flexcan.c
> > > +++ b/drivers/net/can/flexcan.c
> > > @@ -208,6 +208,7 @@ struct flexcan_priv {
> > > void __iomem *base;
> > > u32 reg_esr;
> > > u32 reg_ctrl_default;
> > > + unsigned int err_irq;
> >
> > Why unsigned?
> Err_irq is from 0.
So? irqs are plain "int" almost everywhere in the kernel.
-Scott
^ permalink raw reply
* [PATCH 3/3] iommu/fsl: Fix the error condition during iommu group
From: Varun Sethi @ 2014-06-24 13:57 UTC (permalink / raw)
To: iommu, joro, linux-kernel, linuxppc-dev, alex.williamson; +Cc: Varun Sethi
In-Reply-To: <1403618237-26248-1-git-send-email-Varun.Sethi@freescale.com>
Earlier PTR_ERR was being returned even if group was set to null.
Now, we explicitly set an ERR_PTR value in case the group pointer is
NULL.
Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
---
drivers/iommu/fsl_pamu_domain.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 54060d1..af47648 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -1037,12 +1037,15 @@ root_bus:
group = get_shared_pci_device_group(pdev);
}
+ if (!group)
+ group = ERR_PTR(-ENODEV);
+
return group;
}
static int fsl_pamu_add_device(struct device *dev)
{
- struct iommu_group *group = NULL;
+ struct iommu_group *group = ERR_PTR(-ENODEV);
struct pci_dev *pdev;
const u32 *prop;
int ret, len;
@@ -1065,7 +1068,7 @@ static int fsl_pamu_add_device(struct device *dev)
group = get_device_iommu_group(dev);
}
- if (!group || IS_ERR(group))
+ if (IS_ERR(group))
return PTR_ERR(group);
ret = iommu_group_add_device(group, dev);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] spi: include "int ret" with macro
From: Scott Wood @ 2014-06-24 18:55 UTC (permalink / raw)
To: Zhao Qiang; +Cc: B07421, linuxppc-dev
In-Reply-To: <1403596509-12411-1-git-send-email-B45475@freescale.com>
On Tue, 2014-06-24 at 15:55 +0800, Zhao Qiang wrote:
> ret is unused when CONFIG_FSL_SOC defined,
> so include it with "#ifndef CONFIG_FSL_SOC".
>
> Signed-off-by: Zhao Qiang <B45475@freescale.com>
> ---
> drivers/spi/spi-fsl-lib.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
This needs to be sent to the SPI list and maintainer.
-Scott
^ permalink raw reply
* Re: OF_DYNAMIC node lifecycle
From: Nathan Fontenot @ 2014-06-24 20:07 UTC (permalink / raw)
To: Grant Likely, Pantelis Antoniou
Cc: devicetree, Steven Rostedt, linuxppc-dev, Tyrel Datwyler,
Thomas Gleixner
In-Reply-To: <20140623145844.DA6A3C40AE5@trevor.secretlab.ca>
On 06/23/2014 09:58 AM, Grant Likely wrote:
> On Thu, 19 Jun 2014 11:33:20 +0300, Pantelis Antoniou <pantelis.antoniou@konsulko.com> wrote:
>> Hi Grant,
>>
>> CCing Thomas Gleixner & Steven Rostedt, since they might have a few
>> ideas...
>>
>> On Jun 18, 2014, at 11:07 PM, Grant Likely wrote:
>>
>>> Hi Nathan and Tyrel,
>>>
>>> I'm looking into lifecycle issues on nodes modified by OF_DYNAMIC, and
>>> I'm hoping you can help me. Right now, pseries seems to be the only
>>> user of OF_DYNAMIC, but making OF_DYNAMIC work has a huge impact on
>>> the entire kernel because it requires all DT code to manage reference
>>> counting with iterating over nodes. Most users simply get it wrong.
>>> Pantelis did some investigation and found that the reference counts on
>>> a running kernel are all over the place. I have my doubts that any
>>> code really gets it right.
>>>
>>> The problem is that users need to know when it is appropriate to call
>>> of_node_get()/of_node_put(). All list traversals that exit early need
>>> an extra call to of_node_put(), and code that is searching for a node
>>> in the tree and holding a reference to it needs to call of_node_get().
>>>
>>
>> In hindsight it appears that drivers just can't get the lifecycle right.
>> So we need to simplify things.
>>
>>> I've got a few pseries questions:
>>> - What are the changes being requested by pseries firmware? Is it only
>>> CPUs and memory nodes, or does it manipulate things all over the tree?
>>> - How frequent are the changes? How many changes would be likely over
>>> the runtime of the system?
>>> - Are you able to verify that removed nodes are actually able to be
>>> freed correctly? Do you have any testcases for node removal?
>>>
>>> I'm thinking very seriously about changing the locking semantics of DT
>>> code entirely so that most users never have to worry about
>>> of_node_get/put at all. If the DT code is switched to use rcu
>>> primitives for tree iteration (which also means making DT code use
>>> list_head, something I'm already investigating), then instead of
>>> trying to figure out of_node_get/put rules, callers could use
>>> rcu_read_lock()/rcu_read_unlock() to protect the region that is
>>> searching over nodes, and only call of_node_get() if the node pointer
>>> is needed outside the rcu read-side lock.
>>>
>>> I'd really like to be rid of the node reference counting entirely, but
>>> I can't figure out a way of doing that safely, so I'd settle for
>>> making it a lot easier to get correct.
>>>
>>
>> Since we're going about changing things, how about that devtree_lock?
>
> I believe rcu would pretty much eliminate the devtree_lock entirely. All
> modifiers would need to grab a mutex to ensure there is only one writer
> at any given time, but readers would have free reign to parse the tree
> however they like.
>
> DT writers would have to follow some strict rules about how to handle
> nodes that are removed (ie. don't modify or of_node_put() them until
> after rcu is syncronized), but the number of writers is very small and
> we have control of all of them.
>
>> We're using a raw_spinlock and we're always taking the lock with
>> interrupts disabled.
>>
>> If we're going to make DT changes frequently during normal runtime
>> and not only during boot time, those are bad for any kind of real-time
>> performance.
>>
>> So the question is, do we really have code that access the live tree
>> during atomic sections? Is that something we want? Enforcing this
>> will make our lives easier, and we'll get the change to replace
>> that spinlock with a mutex.
>
> Yes, I believe the powerpc CPU hotplug code accesses the DT in atomic
> sections. I cannot put my finger on the exact code however. Nathan might
> know better. But, if I'm right, the whole problem goes away with RCU.
I went back through the cpu hotplug code. we do update the DT during cpu
hotplug but I don't see it happening during atomic sections.
The code is in arch/powerpc/platforms/pseries/dlpar.c
-Nathan
>
> The design with RCU is to switch struct device_node and struct property
> to use list_head and/or hlist_head with the _rcu accessors. They allow
> items to be removed from a list without syncronizing with readers. Right
> now we have two lists that need to be modified; the allnodes list and
> the sibling list. I *think* it will be fine for the two list removals to
> be non-atomic (there will be a brief period where the node can be found
> on one list, but not the other) because it is a transient state already
> accounted for in rcu read-side critical region.
>
> That said, I've also got a design to remove the allnodes list entirely
> and only work with the sibling list. I need to prototype this.
>
> We'll also need a transition plan to move to RCU. I think the existing
> iterators can be modified to do the rcu locking in-line, but still require
> the of_node_get/put stuff (basically, so existing code continue to works
> unchanged). Then we can add _rcu versions that drop the need for
> of_node_get/put(). When everything is converted, the old iterators can
> be dropped.
>
> g.
>
^ permalink raw reply
* Re: OF_DYNAMIC node lifecycle
From: Nathan Fontenot @ 2014-06-24 20:10 UTC (permalink / raw)
To: Grant Likely, Tyrel Datwyler
Cc: devicetree@vger.kernel.org, Pantelis Antoniou, linuxppc-dev
In-Reply-To: <20140623144806.1348EC40A60@trevor.secretlab.ca>
On 06/23/2014 09:48 AM, Grant Likely wrote:
> On Thu, 19 Jun 2014 10:26:15 -0500, Nathan Fontenot <nfont@austin.ibm.com> wrote:
>> On 06/18/2014 03:07 PM, Grant Likely wrote:
>>> Hi Nathan and Tyrel,
>>>
>>> I'm looking into lifecycle issues on nodes modified by OF_DYNAMIC, and
>>> I'm hoping you can help me. Right now, pseries seems to be the only
>>> user of OF_DYNAMIC, but making OF_DYNAMIC work has a huge impact on
>>> the entire kernel because it requires all DT code to manage reference
>>> counting with iterating over nodes. Most users simply get it wrong.
>>> Pantelis did some investigation and found that the reference counts on
>>> a running kernel are all over the place. I have my doubts that any
>>> code really gets it right.
>>>
>>> The problem is that users need to know when it is appropriate to call
>>> of_node_get()/of_node_put(). All list traversals that exit early need
>>> an extra call to of_node_put(), and code that is searching for a node
>>> in the tree and holding a reference to it needs to call of_node_get().
>>>
>>> I've got a few pseries questions:
>>> - What are the changes being requested by pseries firmware? Is it only
>>> CPUs and memory nodes, or does it manipulate things all over the tree?
>>
>> The short answer, everything.
>
> :-)
>
>> For pseries the two big actions that can change the device tree are
>> adding/removing resources and partition migration.
>>
>> The most frequent updates to the device tree happen during resource
>> (cpu, memory, and pci/phb) add and remove. During this process we add
>> and remove the node and its properties from the device tree.
>> - For memory on newer systems this just involves updating the
>> ibm,dynamic-reconfiguration-memory/ibm,dynamic-memory property. Older
>> firmware levels add and remove the memroy@XXX nodes and their properties.
>> - For cpus the cpus/PowerPC,POWERXXXX nodes and its properties are added
>> or removed
>> - For pci/phb the pci@XXXXX nodes and properties are added/removed.
>>
>> The less frequent operation of live partition migration (and suspend/resume)
>> can update just about anything in the device tree. When this occurs and the
>> systems starts after being migrated (or waking up after a suspend) we make
>> a call to firmware to get updates to the device tree for the new hardware
>> we are running on.
>>
>>> - How frequent are the changes? How many changes would be likely over
>>> the runtime of the system?
>>
>> This can happen frequently.
>
> Thanks, that is exactly the information that I want. I'm not so much
> concerned with the addition or removal of nodes/properties, which is
> actually pretty easy to handle. It is the lifecycle of allocations on
> dynamic nodes that causes heartburn.
>
>>> - Are you able to verify that removed nodes are actually able to be
>>> freed correctly? Do you have any testcases for node removal?
>>
>> I have always tested this by doing resource add/remove, usually cpu and memory
>> since it is the easiest.
>
> Is that just testing the functionality, or do you have tests that check
> if the memory gets freed?
In general it's just functionality testing.
>
>>> I'm thinking very seriously about changing the locking semantics of DT
>>> code entirely so that most users never have to worry about
>>> of_node_get/put at all. If the DT code is switched to use rcu
>>> primitives for tree iteration (which also means making DT code use
>>> list_head, something I'm already investigating), then instead of
>>> trying to figure out of_node_get/put rules, callers could use
>>> rcu_read_lock()/rcu_read_unlock() to protect the region that is
>>> searching over nodes, and only call of_node_get() if the node pointer
>>> is needed outside the rcu read-side lock.
>>>
>>
>> This sounds good. I like just taking the rcu lock around accessing the DT.
>> Do we have many places where DT node pointers are held that require
>> keeping the of_node_get/put calls? If this did exist perhaps we could
>> update those places to look up the DT node every time instead of
>> holding on to the pointer. We could just get rid of the reference counting
>> altogether then.
>
> There are a few, but I would be happy to restrict reference counting to
> only those locations. Most places will decode the DT data, and then
> throw away the reference. We /might/ even be able to do rcu_lock/unlock
> around the entire probe path which would make it transparent to all
> device drivers.
>
>>> I'd really like to be rid of the node reference counting entirely, but
>>> I can't figure out a way of doing that safely, so I'd settle for
>>> making it a lot easier to get correct.
>>>
>>
>> heh! I have often thought about adding reference counting to device tree
>> properties.
>
> You horrible, horrible man.
Yes. I are evil :)
After looking again the work needed to add reference counts to properties
would be huge. The few properties I am concerned with are specific to powerpc
so perhaps just adding an arch specific lock around updating those
properties would work.
-Nathan
>
>> I don't really want to but there are some properties that can
>> get updated frequently (namely the one mentioned above for memory) that
>> can also get pretty big, especially on systems with a lot of memory. We
>> never free the memory for old versions of a device tree property. This is
>> a pretty minor issue though and probably best suited for a separate
>> discussion after resolving this.
>
> We might be able to do some in-place modification of properties if the
> size of the property doesn't change. That still leaves some nasty
> lifecycle issues that need to be resolved though. It would require
> swapping back and forth between memory for an old copy of the property
> and a new one. Yes, this should be a separate discussion.
>
>>
>> Other than pseries, who else does dynamic device tree updating? Are we the
>> only ones?
>
> Right now you're the only ones. Pantelis has a series that adds bulk
> changes to the device tree which are also removable (called overlays). I
> also have a GSoC student working on the selftest code which will
> dynamically add testcase data to the tree.
>
> g.
>
^ permalink raw reply
* [PATCH 2/3] iommu/fsl: Fix the device domain attach condition.
From: Varun Sethi @ 2014-06-24 13:57 UTC (permalink / raw)
To: iommu, joro, linux-kernel, linuxppc-dev, alex.williamson; +Cc: Varun Sethi
In-Reply-To: <1403618237-26248-1-git-send-email-Varun.Sethi@freescale.com>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox