On Wed, Apr 19, 2023 at 5:35 AM Maxime Coquelin
<maxime.coque...@redhat.com> wrote:
>
> Hi Mike,
>
> On 4/17/23 21:39, Mike Pattrick wrote:
> > Hi Maxime,
> >
> > On Fri, Mar 31, 2023 at 11:43 AM Maxime Coquelin
> > <maxime.coque...@redhat.com> wrote:
> >>
> >> This patch introduces a helper to check whether two IOTLB
> >> entries share a page.
> >>
> >> Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com>
> >> ---
> >> lib/vhost/iotlb.c | 25 ++++++++++++++++++++-----
> >> 1 file changed, 20 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/lib/vhost/iotlb.c b/lib/vhost/iotlb.c
> >> index e8f1cb661e..d919f74704 100644
> >> --- a/lib/vhost/iotlb.c
> >> +++ b/lib/vhost/iotlb.c
> >> @@ -23,6 +23,23 @@ struct vhost_iotlb_entry {
> >>
> >> #define IOTLB_CACHE_SIZE 2048
> >>
> >> +static bool
> >> +vhost_user_iotlb_share_page(struct vhost_iotlb_entry *a, struct
> >> vhost_iotlb_entry *b,
> >> + uint64_t align)
> >> +{
> >> + uint64_t a_end, b_start;
> >> +
> >> + if (a == NULL || b == NULL)
> >> + return false;
> >> +
> >> + /* Assumes entry a lower than entry b */
> >> + RTE_ASSERT(a->uaddr < b->uaddr);
> >> + a_end = RTE_ALIGN_CEIL(a->uaddr + a->size, align);
> >> + b_start = RTE_ALIGN_FLOOR(b->uaddr, align);
> >> +
> >> + return a_end > b_start;
> >
> > This may be a very minor detail, but it might improve readability if
> > the a_end calculation used addr + size - 1 and the return conditional
> > used >=.
>
> That's actually how I initially implemented it, but discussing with
> David we agreed it would be better that way for consistency with
> vhost_user_iotlb_clear_dump().
I can get behind consistency.
Acked-by: Mike Pattrick <m...@redhat.com>
>
> Regards,
> Maxime
>
> >
> > Cheers,
> > M
> >
> >
> >> +}
> >> +
> >> static void
> >> vhost_user_iotlb_set_dump(struct virtio_net *dev, struct
> >> vhost_iotlb_entry *node)
> >> {
> >> @@ -37,16 +54,14 @@ static void
> >> vhost_user_iotlb_clear_dump(struct virtio_net *dev, struct
> >> vhost_iotlb_entry *node,
> >> struct vhost_iotlb_entry *prev, struct vhost_iotlb_entry
> >> *next)
> >> {
> >> - uint64_t align, mask;
> >> + uint64_t align;
> >>
> >> align = hua_to_alignment(dev->mem, (void
> >> *)(uintptr_t)node->uaddr);
> >> - mask = ~(align - 1);
> >>
> >> /* Don't disable coredump if the previous node is in the same
> >> page */
> >> - if (prev == NULL || (node->uaddr & mask) != ((prev->uaddr +
> >> prev->size - 1) & mask)) {
> >> + if (!vhost_user_iotlb_share_page(prev, node, align)) {
> >> /* Don't disable coredump if the next node is in the same
> >> page */
> >> - if (next == NULL ||
> >> - ((node->uaddr + node->size - 1) & mask) !=
> >> (next->uaddr & mask))
> >> + if (!vhost_user_iotlb_share_page(node, next, align))
> >> mem_set_dump((void *)(uintptr_t)node->uaddr,
> >> node->size, false, align);
> >> }
> >> }
> >> --
> >> 2.39.2
> >>
> >
>
