Re: [GIT PULL] Early batch of KVM changes for 4.3 merge window
On 09/02/2015 01:03 AM, Paolo Bonzini wrote: diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index fb16a8ea3dee..3c745f3abde8 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -3309,13 +3309,13 @@ walk_shadow_page_get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) walk_shadow_page_lockless_begin(vcpu); - for (shadow_walk_init(, vcpu, addr), root = iterator.level; + for (shadow_walk_init(, vcpu, addr), +leaf = root = iterator.level; shadow_walk_okay(); __shadow_walk_next(, spte)) { - leaf = iterator.level; spte = mmu_spte_get_lockless(iterator.sptep); - sptes[leaf - 1] = spte; + sptes[--leaf] = spte; if (!is_shadow_present_pte(spte)) break; @@ -3329,7 +3329,7 @@ walk_shadow_page_get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) if (reserved) { pr_err("%s: detect reserved bits on spte, addr 0x%llx, dump hierarchy:\n", __func__, addr); - while (root >= leaf) { + while (root > leaf) { pr_err("-- spte 0x%llx level %d.\n", sptes[root - 1], root); root--; But honestly I haven't even compiled it yet. Xiao, what do you think? It looks good to me! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Early batch of KVM changes for 4.3 merge window
On 01/09/2015 07:45, Xiao Guangrong wrote: > > > Actually i triggered this warning in my another box and posted a patch > to fix it which can be found at: > http://lkml.iu.edu/hypermail/linux/kernel/1508.2/02771.html > I guess Paolo is currently busy with KVM forum so the patch has not been > reviewed yet. Currently I'm busy with the Dolomites, actually. I'll send a fix together with the PPC+ARM pull request. Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Early batch of KVM changes for 4.3 merge window
On 01/09/2015 02:47, Linus Torvalds wrote: > Hmm: > > On Fri, Aug 14, 2015 at 4:57 PM, Paolo Bonzini wrote: >> >> Xiao Guangrong (9): >> KVM: MMU: fully check zero bits for sptes > > The above commit causes an annoying new compiler warning. > > The warning is bogus ("variable 'leaf' possibly uninitialized"), > because the use of the variable is protected by the 'bool reserved' > flag, but gcc is apparently not smart enough to understand that. Unfortunately it doesn't reproduce on all compiler versions. Something like this should do it: diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index fb16a8ea3dee..3c745f3abde8 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -3309,13 +3309,13 @@ walk_shadow_page_get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) walk_shadow_page_lockless_begin(vcpu); - for (shadow_walk_init(, vcpu, addr), root = iterator.level; + for (shadow_walk_init(, vcpu, addr), +leaf = root = iterator.level; shadow_walk_okay(); __shadow_walk_next(, spte)) { - leaf = iterator.level; spte = mmu_spte_get_lockless(iterator.sptep); - sptes[leaf - 1] = spte; + sptes[--leaf] = spte; if (!is_shadow_present_pte(spte)) break; @@ -3329,7 +3329,7 @@ walk_shadow_page_get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) if (reserved) { pr_err("%s: detect reserved bits on spte, addr 0x%llx, dump hierarchy:\n", __func__, addr); - while (root >= leaf) { + while (root > leaf) { pr_err("-- spte 0x%llx level %d.\n", sptes[root - 1], root); root--; But honestly I haven't even compiled it yet. Xiao, what do you think? Paolo > Since bogus warnings cause people to possibly ignore the *real* > warnings, this should be fixed. Maybe the code should get rid of that > 'reserved' flag, and instead initialize "leaf" to zero, and use that > as the flag instead (since zero isn't a valid level)? That would > actually avoid an extra variable, and would get rid of the warning. > > Hmm? > > Linus > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Early batch of KVM changes for 4.3 merge window
On 01/09/2015 07:45, Xiao Guangrong wrote: > > > Actually i triggered this warning in my another box and posted a patch > to fix it which can be found at: > http://lkml.iu.edu/hypermail/linux/kernel/1508.2/02771.html > I guess Paolo is currently busy with KVM forum so the patch has not been > reviewed yet. Currently I'm busy with the Dolomites, actually. I'll send a fix together with the PPC+ARM pull request. Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Early batch of KVM changes for 4.3 merge window
On 01/09/2015 02:47, Linus Torvalds wrote: > Hmm: > > On Fri, Aug 14, 2015 at 4:57 PM, Paolo Bonziniwrote: >> >> Xiao Guangrong (9): >> KVM: MMU: fully check zero bits for sptes > > The above commit causes an annoying new compiler warning. > > The warning is bogus ("variable 'leaf' possibly uninitialized"), > because the use of the variable is protected by the 'bool reserved' > flag, but gcc is apparently not smart enough to understand that. Unfortunately it doesn't reproduce on all compiler versions. Something like this should do it: diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index fb16a8ea3dee..3c745f3abde8 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -3309,13 +3309,13 @@ walk_shadow_page_get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) walk_shadow_page_lockless_begin(vcpu); - for (shadow_walk_init(, vcpu, addr), root = iterator.level; + for (shadow_walk_init(, vcpu, addr), +leaf = root = iterator.level; shadow_walk_okay(); __shadow_walk_next(, spte)) { - leaf = iterator.level; spte = mmu_spte_get_lockless(iterator.sptep); - sptes[leaf - 1] = spte; + sptes[--leaf] = spte; if (!is_shadow_present_pte(spte)) break; @@ -3329,7 +3329,7 @@ walk_shadow_page_get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) if (reserved) { pr_err("%s: detect reserved bits on spte, addr 0x%llx, dump hierarchy:\n", __func__, addr); - while (root >= leaf) { + while (root > leaf) { pr_err("-- spte 0x%llx level %d.\n", sptes[root - 1], root); root--; But honestly I haven't even compiled it yet. Xiao, what do you think? Paolo > Since bogus warnings cause people to possibly ignore the *real* > warnings, this should be fixed. Maybe the code should get rid of that > 'reserved' flag, and instead initialize "leaf" to zero, and use that > as the flag instead (since zero isn't a valid level)? That would > actually avoid an extra variable, and would get rid of the warning. > > Hmm? > > Linus > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Early batch of KVM changes for 4.3 merge window
On 09/02/2015 01:03 AM, Paolo Bonzini wrote: diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index fb16a8ea3dee..3c745f3abde8 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -3309,13 +3309,13 @@ walk_shadow_page_get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) walk_shadow_page_lockless_begin(vcpu); - for (shadow_walk_init(, vcpu, addr), root = iterator.level; + for (shadow_walk_init(, vcpu, addr), +leaf = root = iterator.level; shadow_walk_okay(); __shadow_walk_next(, spte)) { - leaf = iterator.level; spte = mmu_spte_get_lockless(iterator.sptep); - sptes[leaf - 1] = spte; + sptes[--leaf] = spte; if (!is_shadow_present_pte(spte)) break; @@ -3329,7 +3329,7 @@ walk_shadow_page_get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) if (reserved) { pr_err("%s: detect reserved bits on spte, addr 0x%llx, dump hierarchy:\n", __func__, addr); - while (root >= leaf) { + while (root > leaf) { pr_err("-- spte 0x%llx level %d.\n", sptes[root - 1], root); root--; But honestly I haven't even compiled it yet. Xiao, what do you think? It looks good to me! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Early batch of KVM changes for 4.3 merge window
Linus, I am sorry for the annoyance. On 09/01/2015 08:47 AM, Linus Torvalds wrote: Hmm: On Fri, Aug 14, 2015 at 4:57 PM, Paolo Bonzini wrote: Xiao Guangrong (9): KVM: MMU: fully check zero bits for sptes The above commit causes an annoying new compiler warning. The warning is bogus ("variable 'leaf' possibly uninitialized"), because the use of the variable is protected by the 'bool reserved' flag, but gcc is apparently not smart enough to understand that. Since bogus warnings cause people to possibly ignore the *real* warnings, this should be fixed. Maybe the code should get rid of that 'reserved' flag, and instead initialize "leaf" to zero, and use that as the flag instead (since zero isn't a valid level)? That would actually avoid an extra variable, and would get rid of the warning. The logic in that code is: if 'reserved' is true, print out the info in spte[root - leaf]. I am afraid it's not good to use 'leaf' both for the array index and reserved indicator. Or if i missed something please let me know. Actually i triggered this warning in my another box and posted a patch to fix it which can be found at: http://lkml.iu.edu/hypermail/linux/kernel/1508.2/02771.html I guess Paolo is currently busy with KVM forum so the patch has not been reviewed yet. The patch simply initialized 'leaf' to the highest value to stop printing the info, but as you noticed this is no real problem in the code just stop GCC's complaint. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Early batch of KVM changes for 4.3 merge window
Hmm: On Fri, Aug 14, 2015 at 4:57 PM, Paolo Bonzini wrote: > > Xiao Guangrong (9): > KVM: MMU: fully check zero bits for sptes The above commit causes an annoying new compiler warning. The warning is bogus ("variable 'leaf' possibly uninitialized"), because the use of the variable is protected by the 'bool reserved' flag, but gcc is apparently not smart enough to understand that. Since bogus warnings cause people to possibly ignore the *real* warnings, this should be fixed. Maybe the code should get rid of that 'reserved' flag, and instead initialize "leaf" to zero, and use that as the flag instead (since zero isn't a valid level)? That would actually avoid an extra variable, and would get rid of the warning. Hmm? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Early batch of KVM changes for 4.3 merge window
Hmm: On Fri, Aug 14, 2015 at 4:57 PM, Paolo Bonziniwrote: > > Xiao Guangrong (9): > KVM: MMU: fully check zero bits for sptes The above commit causes an annoying new compiler warning. The warning is bogus ("variable 'leaf' possibly uninitialized"), because the use of the variable is protected by the 'bool reserved' flag, but gcc is apparently not smart enough to understand that. Since bogus warnings cause people to possibly ignore the *real* warnings, this should be fixed. Maybe the code should get rid of that 'reserved' flag, and instead initialize "leaf" to zero, and use that as the flag instead (since zero isn't a valid level)? That would actually avoid an extra variable, and would get rid of the warning. Hmm? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Early batch of KVM changes for 4.3 merge window
Linus, I am sorry for the annoyance. On 09/01/2015 08:47 AM, Linus Torvalds wrote: Hmm: On Fri, Aug 14, 2015 at 4:57 PM, Paolo Bonziniwrote: Xiao Guangrong (9): KVM: MMU: fully check zero bits for sptes The above commit causes an annoying new compiler warning. The warning is bogus ("variable 'leaf' possibly uninitialized"), because the use of the variable is protected by the 'bool reserved' flag, but gcc is apparently not smart enough to understand that. Since bogus warnings cause people to possibly ignore the *real* warnings, this should be fixed. Maybe the code should get rid of that 'reserved' flag, and instead initialize "leaf" to zero, and use that as the flag instead (since zero isn't a valid level)? That would actually avoid an extra variable, and would get rid of the warning. The logic in that code is: if 'reserved' is true, print out the info in spte[root - leaf]. I am afraid it's not good to use 'leaf' both for the array index and reserved indicator. Or if i missed something please let me know. Actually i triggered this warning in my another box and posted a patch to fix it which can be found at: http://lkml.iu.edu/hypermail/linux/kernel/1508.2/02771.html I guess Paolo is currently busy with KVM forum so the patch has not been reviewed yet. The patch simply initialized 'leaf' to the highest value to stop printing the info, but as you noticed this is no real problem in the code just stop GCC's complaint. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/