From mboxrd@z Thu Jan 1 00:00:00 1970 From: Glauber Costa Subject: Re: [PATCH 3/9] allow intersecting region to be on the boundary. Date: Mon, 22 Sep 2008 10:48:04 -0300 Message-ID: <20080922134804.GB3618@poweredge.glommer> References: <1221840506-22996-1-git-send-email-glommer@redhat.com> <1221840506-22996-4-git-send-email-glommer@redhat.com> <48D541D0.7090701@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, aliguori@us.ibm.com To: Avi Kivity Return-path: Received: from mx2.redhat.com ([66.187.237.31]:42502 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752428AbYIVOIw (ORCPT ); Mon, 22 Sep 2008 10:08:52 -0400 Content-Disposition: inline In-Reply-To: <48D541D0.7090701@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Sat, Sep 20, 2008 at 11:32:48AM -0700, Avi Kivity wrote: > Glauber Costa wrote: >> Signed-off-by: Glauber Costa >> --- >> 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. >