On Tue, Oct 05, 2021 at 11:52:37AM +0200, Eugenio Perez Martin wrote:
> On Tue, Oct 5, 2021 at 10:15 AM Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:
> >
> > On Tue, Oct 05, 2021 at 10:01:30AM +0200, Eugenio Pérez wrote:
> > > Abstract this operation, that will be reused when validating the region
> > > against the iova range that the device supports.
> > >
> > > Signed-off-by: Eugenio Pérez <eperezma@xxxxxxxxxx>
> >
> > Note that as defined end is actually 1 byte beyond end of section.
> > As such it can e.g. overflow if cast to u64.
> > So be careful to use int128 ops with it.
>
> You are right, but this is only the result of extracting "llend"
> calculation in its own function, since it is going to be used a third
> time in the next commit. This next commit contains a mistake because
> of this, as you pointed out.
>
> Since "last" would be a very misleading name, do you think we could
> give a better name / type to it?
>
> > Also - document?
>
> It will be documented with that ("It returns one byte beyond end of
> section" or similar) too.
>
> Thanks!
that's how c++ containers work so maybe it's not too bad as long
as we document this carefully.
> >
> > > ---
> > > hw/virtio/vhost-vdpa.c | 18 +++++++++++-------
> > > 1 file changed, 11 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > > index ea1aa71ad8..a1de6c7c9c 100644
> > > --- a/hw/virtio/vhost-vdpa.c
> > > +++ b/hw/virtio/vhost-vdpa.c
> > > @@ -24,6 +24,15 @@
> > > #include "trace.h"
> > > #include "qemu-common.h"
> > >
> > > +static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section)
> > > +{
> > > + Int128 llend = int128_make64(section->offset_within_address_space);
> > > + llend = int128_add(llend, section->size);
> > > + llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK));
> > > +
> > > + return llend;
> > > +}
> > > +
> > > static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section)
> > > {
> > > return (!memory_region_is_ram(section->mr) &&
> > > @@ -160,10 +169,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener,
> > > }
> > >
> > > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
> > > - llend = int128_make64(section->offset_within_address_space);
> > > - llend = int128_add(llend, section->size);
> > > - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK));
> > > -
> > > + llend = vhost_vdpa_section_end(section);
> > > if (int128_ge(int128_make64(iova), llend)) {
> > > return;
> > > }
> > > @@ -221,9 +227,7 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener,
> > > }
> > >
> > > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
> > > - llend = int128_make64(section->offset_within_address_space);
> > > - llend = int128_add(llend, section->size);
> > > - llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK));
> > > + llend = vhost_vdpa_section_end(section);
> > >
> > > trace_vhost_vdpa_listener_region_del(v, iova, int128_get64(llend));
> > >
> > > --
> > > 2.27.0
> >
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
