[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/2] util/vfio-helpers: Rework the IOVA allocator to avoid
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH v2 2/2] util/vfio-helpers: Rework the IOVA allocator to avoid IOVA reserved regions |
Date: |
Wed, 30 Sep 2020 10:18:28 +0100 |
On Tue, Sep 29, 2020 at 09:44:48PM +0200, Auger Eric wrote:
> Hi Stefan,
>
> On 9/29/20 5:59 PM, Stefan Hajnoczi wrote:
> > On Tue, Sep 29, 2020 at 10:55:50AM +0200, Eric Auger wrote:
> >> diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
> >> index ba0ee6e21c..71145970f3 100644
> >> --- a/util/vfio-helpers.c
> >> +++ b/util/vfio-helpers.c
> >> @@ -667,6 +667,50 @@ static bool qemu_vfio_verify_mappings(QEMUVFIOState
> >> *s)
> >> return true;
> >> }
> >>
> >> +static int
> >> +qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
> >> +{
> >> + int i;
> >> +
> >> + for (i = 0; i < s->nb_iova_ranges; i++) {
> >> + if (s->usable_iova_ranges[i].end < s->low_water_mark) {
> >> + continue;
> >> + }
> >> + s->low_water_mark =
> >> + MAX(s->low_water_mark, s->usable_iova_ranges[i].start);
> >> +
> >> + if (s->usable_iova_ranges[i].end - s->low_water_mark + 1 >= size
> >> ||
> >> + s->usable_iova_ranges[i].end - s->low_water_mark + 1 == 0) {
> >
> > I don't understand the == 0 case. It seems like we are allocating an
> > IOVA beyond usable_iova_ranges[i].end?>
> It is meant to handle the case were low_water_mark = 0 and
> s->usable_iova_ranges[0].end = ULLONG_MAX (I know it cannot exist at the
> moment but may happen in the future) where we get an overflow. Given the
> if (s->usable_iova_ranges[i].end < s->low_water_mark) {
> continue;
> }
> I think this prevents us from allocating beyond
> usable_iova_ranges[i].end or do I miss something?
Yes, you are right. Here are the constraints:
e >= l
j = max(l, s)
e - j + 1 < s
e - j + 1 == 0
Assume l >= s so we can replace j with l:
e >= l
e - l + 1 < s
e - l + 1 == 0
The case I'm worried about is when the iova range cannot fit s bytes.
The last condition is only true when e = l - 1, but this violates the
first condition e >= l.
So the problem scenario cannot occur.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
signature.asc
Description: PGP signature