On 08/03/2011 06:24 AM, Michael S. Tsirkin wrote:
> On Wed, Aug 03, 2011 at 02:48:05PM +0300, Avi Kivity wrote:
>> On 08/01/2011 08:44 PM, David Ahern wrote:
>>> qemu-kvm.git as of:
>>>
>>> commit dacdc4b10bafbb21120e1c24a9665444768ef999
>>> Merge: 7b69d4f 0af4922
>>> Author: Avi Kivity<avi@xxxxxxxxxx>
>>> Date: Sun Jul 31 11:42:26 2011 +0300
>>>
>>> Merge branch 'upstream-merge' into next
>>>
>>> is aborting with the error:
>>>
>>> qemu-kvm: qemu-kvm.git/hw/vhost.c:123: vhost_dev_unassign_memory:
>>> Assertion `to>= 0' failed.
>>> Aborted
>>>
>>
>> It's a bug in vhost:
>>
>> /* Assign/unassign. Keep an unsorted array of non-overlapping
>> * memory regions in dev->mem. */
>> static void vhost_dev_unassign_memory(struct vhost_dev *dev,
>> uint64_t start_addr,
>> uint64_t size)
>> {
>> int from, to, n = dev->mem->nregions;
>> /* Track overlapping/split regions for sanity checking. */
>> int overlap_start = 0, overlap_end = 0, overlap_middle = 0, split = 0;
>>
>> for (from = 0, to = 0; from < n; ++from, ++to) {
>> struct vhost_memory_region *reg = dev->mem->regions + to;
>> uint64_t reglast;
>> uint64_t memlast;
>> uint64_t change;
>>
>> /* clone old region */
>> if (to != from) {
>> memcpy(reg, dev->mem->regions + from, sizeof *reg);
>> }
>>
>> /* No overlap is simple */
>> if (!ranges_overlap(reg->guest_phys_addr, reg->memory_size,
>> start_addr, size)) {
>> continue;
>> }
>>
>> /* Split only happens if supplied region
>> * is in the middle of an existing one. Thus it can not
>> * overlap with any other existing region. */
>> assert(!split);
>>
>> reglast = range_get_last(reg->guest_phys_addr, reg->memory_size);
>> memlast = range_get_last(start_addr, size);
>>
>> /* Remove whole region */
>> if (start_addr <= reg->guest_phys_addr && memlast >= reglast) {
>> --dev->mem->nregions;
>> --to;
>> assert(to >= 0);
>> ++overlap_middle;
>> continue;
>> }
>>
>>
>> We're removing the first region, and 'to' goes negative. Michael?
>
> Yes, that assert is wrong.
>
> --->
> Subject: vhost: remove an incorrect assert
>
> The 'to' can go negative when the first region gets removed
> (it gets incremented by to 0 immediately afterward), which
> makes the assertion fail. Nothing breaks if
> to < 0 here so just remove the assert.
>
> Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
>
> ----
>
> diff --git a/hw/vhost.c b/hw/vhost.c
> index c3d8821..19e7255 100644
> --- a/hw/vhost.c
> +++ b/hw/vhost.c
> @@ -120,7 +120,6 @@ static void vhost_dev_unassign_memory(struct vhost_dev *dev,
> if (start_addr <= reg->guest_phys_addr && memlast >= reglast) {
> --dev->mem->nregions;
> --to;
> - assert(to >= 0);
> ++overlap_middle;
> continue;
> }
>
Removing the assert appears to work fine.
Tested-by: David Ahern <daahern@xxxxxxxxx>
David
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
