Re: [PATCH 3/9] allow intersecting region to be on the boundary.

[Glauber Costa <glommer@redhat.com>]

On Sat, Sep 20, 2008 at 11:32:48AM -0700, Avi Kivity wrote:
> Glauber Costa wrote:
>> Signed-off-by: Glauber Costa <glommer@redhat.com>
>> ---
>>  libkvm/libkvm.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
>> index e768e44..fa65c30 100644
>> --- a/libkvm/libkvm.c
>> +++ b/libkvm/libkvm.c
>> @@ -130,8 +130,8 @@ int get_intersecting_slot(unsigned long phys_addr)
>>  	int i;
>>   	for (i = 0; i < KVM_MAX_NUM_MEM_REGIONS ; ++i)
>> -		if (slots[i].len && slots[i].phys_addr < phys_addr &&
>> -		    (slots[i].phys_addr + slots[i].len) > phys_addr)
>> +		if (slots[i].len && slots[i].phys_addr <= phys_addr &&
>> +		    (slots[i].phys_addr + slots[i].len) >= phys_addr)
>>  			return i;
>>  	return -1;
>>   
> consider
>
>  slots[i].phys_addr = 0
>  slots[i].len = 1
>  phys_addr = 1
>
> with the new calculation, i (well, not me personally) will be considered  
> an intersecting slot.
>
> Not that I (me this time) can understand how you can calculate interval  
> intersection without the entire interval.
would you be fine with checking only the left interval?

But to be honest, look at:

    r = kvm_is_containing_region(kvm_context, start_addr, size);
    if (r)
        return;

    [ sip ]

    r = kvm_is_intersecting_mem(kvm_context, start_addr);
    if (r) {
        printf("Ignoring intersecting memory %llx (%lx)\n", start_addr, size);

We don't really do anything, which is the same action as the containing region case.
So maybe we should just merge the two checks, and do the same thing (nothing) on both?

>
> -- 
> I have a truly marvellous patch that fixes the bug which this
> signature is too narrow to contain.
>