Always false condition in rmap_add

Always false condition in rmap_add

[Prasad Joshi]

Hello All,

I have question on code of rmap_add

Here is the code of the function

 613 static int rmap_add(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
 614 {

624     rmapp = gfn_to_rmap(vcpu->kvm, gfn, sp->role.level);
 625     if (!*rmapp) {
 626         rmap_printk("rmap_add: %p %llx 0->1\n", spte, *spte);
 627         *rmapp = (unsigned long)spte;
 628     } else if (!(*rmapp & 1)) {
 629         rmap_printk("rmap_add: %p %llx 1->many\n", spte, *spte);
 630         desc = mmu_alloc_rmap_desc(vcpu);
 631         desc->sptes[0] = (u64 *)*rmapp;
 632         desc->sptes[1] = spte;
 633         *rmapp = (unsigned long)desc | 1;
 634         ++count;
 635     } else {

The line 628 checks whether the last bit of the rmapp is 1.
If it is one then line 633 assigns a new value to rmapp with and sets
the last bit to 1.

But the line 633 is the only place that sets rmapp's last bit is set
to 1. IMHO the condition on line 628 would never be true.

Please let me know if I am wrong.

Thanks and Regards,
Prasad

Re: Always false condition in rmap_add

[Joerg Roedel]

On Thu, Nov 11, 2010 at 06:38:47PM +0000, Prasad Joshi wrote:
> Hello All,
> 
> I have question on code of rmap_add
> 
> Here is the code of the function
> 
>  613 static int rmap_add(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
>  614 {
> 
> 624     rmapp = gfn_to_rmap(vcpu->kvm, gfn, sp->role.level);
>  625     if (!*rmapp) {
>  626         rmap_printk("rmap_add: %p %llx 0->1\n", spte, *spte);
>  627         *rmapp = (unsigned long)spte;
>  628     } else if (!(*rmapp & 1)) {
>  629         rmap_printk("rmap_add: %p %llx 1->many\n", spte, *spte);
>  630         desc = mmu_alloc_rmap_desc(vcpu);
>  631         desc->sptes[0] = (u64 *)*rmapp;
>  632         desc->sptes[1] = spte;
>  633         *rmapp = (unsigned long)desc | 1;
>  634         ++count;
>  635     } else {
> 
> The line 628 checks whether the last bit of the rmapp is 1.
> If it is one then line 633 assigns a new value to rmapp with and sets
> the last bit to 1.

No, line 628 checks if the last bit is not 1, so the check works.

	Joerg