Re: [RFC v8 2/5] memory: Add IOMMUTLBEvent

[Eugenio Perez Martin]

Applying both, thanks!

On Wed, Sep 2, 2020 at 9:54 AM Juan Quintela <quintela@redhat.com> wrote:
>
> Eugenio Pérez <eperezma@redhat.com> wrote:
> > This way we can tell between regulars IOMMURLBEntries (entry of IOMMU
> > hardware) and notifications.
> >
> > In the notifications, we set explicitly if it is a MAPs or an UNMAP,
> > instead of trusting in entry permissions to differenciate them.
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
>
> if you have to respin for whatever other reasons, two suggestions.
>
>
> > @@ -1430,10 +1435,10 @@ static int vtd_dev_to_context_entry(IntelIOMMUState 
> > *s, uint8_t bus_num,
> >      return 0;
> >  }
> >
> > -static int vtd_sync_shadow_page_hook(IOMMUTLBEntry *entry,
> > +static int vtd_sync_shadow_page_hook(IOMMUTLBEvent *event,
> >                                       void *private)
> >  {
> > -    memory_region_notify_iommu((IOMMUMemoryRegion *)private, 0, *entry);
> > +    memory_region_notify_iommu((IOMMUMemoryRegion *)private, 0, *event);
>
> I know that it already was there, but if you respin, you can remove the cast.
>
>
>
> > @@ -1281,12 +1286,12 @@ void memory_region_notify_iommu(IOMMUMemoryRegion 
> > *iommu_mr,
> >   * notifies a specific notifier, not all of them.
> >   *
> >   * @notifier: the notifier to be notified
> > - * @entry: the new entry in the IOMMU translation table.  The entry
> > - *         replaces all old entries for the same virtual I/O address range.
> > - *         Deleted entries have .@perm == 0.
> > + * @event: TLB event with the new entry in the IOMMU translation table.
> > + *         The entry replaces all old entries for the same virtual I/O 
> > address
> > + *         range.
> >   */
> >  void memory_region_notify_iommu_one(IOMMUNotifier *notifier,
> > -                              IOMMUTLBEntry *entry);
> > +                                    IOMMUTLBEvent *event);
>
> I didn't catch the missing of indentation on the previous patch ....
> O:-)
>