RE: [PATCH v1 20/22] intel_iommu: propagate PASID-based iotlb invalidation to host
> From: Peter Xu
> Sent: Thursday, March 26, 2020 9:23 PM
> To: Liu, Yi L
> Subject: Re: [PATCH v1 20/22] intel_iommu: propagate PASID-based iotlb
> invalidation to host
>
> On Thu, Mar 26, 2020 at 09:02:48AM -0400, Peter Xu wrote:
>
> [...]
>
> > > > > > +static inline bool vtd_pasid_cache_valid(
> > > > > > + VTDPASIDAddressSpace *vtd_pasid_as) {
> > > > > > +return vtd_pasid_as->iommu_state &&
> > ^
> >
> > > > >
> > > > > This check can be dropped because always true?
> > > > >
> > > > > If you agree with both the changes, please add:
> > > > >
> > > > > Reviewed-by: Peter Xu
> > > >
> > > > I think the code should ensure all the pasid_as in hash table is
> > > > valid. And we can since all the operations are under protection of
> > > > iommu_lock.
> > > >
> > > Peter,
> > >
> > > I think my reply was wrong. pasid_as in has table may be stale since
> > > the per pasid_as cache_gen may be not identical with the cache_gen
> > > in iommu_state. e.g. vtd_pasid_cache_reset() only increases the
> > > cache_gen in iommu_state. So there will be pasid_as in hash table
> > > which has cached pasid entry but its cache_gen is not equal to the
> > > one in iommu_state. For such pasid_as, we should treat it as stale.
> > > So I guess the vtd_pasid_cache_valid() is still necessary.
> >
> > I guess you misread my comment. :)
> >
> > I was saying the "vtd_pasid_as->iommu_state" check is not needed,
> > because iommu_state was always set if the address space is created.
> > vtd_pasid_cache_valid() is needed.
> >
> > Also, please double confirm that vtd_pasid_cache_reset() should drop
> > all the address spaces (as I think it should), not "only increase the
> > cache_gen". IMHO you should only increase the cache_gen in the PSI
> > hook (vtd_pasid_cache_psi()) only.
>
> Sorry, I mean GSI (vtd_pasid_cache_gsi), not PSI.
Got it.. Really confused me. :-)
Regards,
Yi Liu
RE: [PATCH v1 20/22] intel_iommu: propagate PASID-based iotlb invalidation to host
> From: Peter Xu
> Sent: Thursday, March 26, 2020 9:03 PM
> To: Liu, Yi L
> Subject: Re: [PATCH v1 20/22] intel_iommu: propagate PASID-based iotlb
> invalidation to host
>
> On Thu, Mar 26, 2020 at 05:41:39AM +, Liu, Yi L wrote:
> > > From: Liu, Yi L
> > > Sent: Wednesday, March 25, 2020 9:22 PM
> > > To: 'Peter Xu'
> > > Subject: RE: [PATCH v1 20/22] intel_iommu: propagate PASID-based
> > > iotlb invalidation to host
> > >
> > > > From: Peter Xu
> > > > Sent: Wednesday, March 25, 2020 2:34 AM
> > > > To: Liu, Yi L
> > > > Subject: Re: [PATCH v1 20/22] intel_iommu: propagate PASID-based
> > > > iotlb invalidation to host
> > > >
> > > > On Sun, Mar 22, 2020 at 05:36:17AM -0700, Liu Yi L wrote:
> > > > > This patch propagates PASID-based iotlb invalidation to host.
> > > > >
> > > > > Intel VT-d 3.0 supports nested translation in PASID granular.
> > > > > Guest SVA support could be implemented by configuring nested
> > > > > translation on specific PASID. This is also known as dual stage
> > > > > DMA translation.
> > > > >
> > > > > Under such configuration, guest owns the GVA->GPA translation
> > > > > which is configured as first level page table in host side for a
> > > > > specific pasid, and host owns GPA->HPA translation. As guest
> > > > > owns first level translation table, piotlb invalidation should
> > > > > be propagated to host since host IOMMU will cache first level
> > > > > page table related mappings during DMA address translation.
> > > > >
> > > > > This patch traps the guest PASID-based iotlb flush and propagate
> > > > > it to host.
> > > > >
> > > > > Cc: Kevin Tian
> > > > > Cc: Jacob Pan
> > > > > Cc: Peter Xu
> > > > > Cc: Yi Sun
> > > > > Cc: Paolo Bonzini
> > > > > Cc: Richard Henderson
> > > > > Cc: Eduardo Habkost
> > > > > Signed-off-by: Liu Yi L
> > > > > ---
> > > > > hw/i386/intel_iommu.c | 139
> > > > +
> > > > > hw/i386/intel_iommu_internal.h | 7 +++
> > > > > 2 files changed, 146 insertions(+)
> > > > >
> > > > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index
> > > > > b9ac07d..10d314d 100644
> > > > > --- a/hw/i386/intel_iommu.c
> > > > > +++ b/hw/i386/intel_iommu.c
> > > > > @@ -3134,15 +3134,154 @@ static bool
> > > > vtd_process_pasid_desc(IntelIOMMUState *s,
> > > > > return (ret == 0) ? true : false; }
> > > > >
> > > > > +/**
> > > > > + * Caller of this function should hold iommu_lock.
> > > > > + */
> > > > > +static void vtd_invalidate_piotlb(IntelIOMMUState *s,
> > > > > + VTDBus *vtd_bus,
> > > > > + int devfn,
> > > > > + DualIOMMUStage1Cache
> > > > > +*stage1_cache) {
> > > > > +VTDHostIOMMUContext *vtd_dev_icx;
> > > > > +HostIOMMUContext *host_icx;
> > > > > +
> > > > > +vtd_dev_icx = vtd_bus->dev_icx[devfn];
> > > > > +if (!vtd_dev_icx) {
> > > > > +goto out;
> > > > > +}
> > > > > +host_icx = vtd_dev_icx->host_icx;
> > > > > +if (!host_icx) {
> > > > > +goto out;
> > > > > +}
> > > > > +if (host_iommu_ctx_flush_stage1_cache(host_icx, stage1_cache)) {
> > > > > +error_report("Cache flush failed");
> > > >
> > > > I think this should not easily be triggered by the guest, but just
> > > > in case... Let's use
> > > > error_report_once() to be safe.
> > >
> > > Agreed.
> > >
> > > > > +}
> > > > > +out:
> > > > > +return;
> > > > > +}
> > > > > +
> > > > > +static inline bool vtd_pasid_cache_valid(
> > > > > + VTDPASIDAddressSpace *vtd_pasid_as) {
> > > > > +return vtd_pasid_as->iommu_state &&
Re: [PATCH v1 20/22] intel_iommu: propagate PASID-based iotlb invalidation to host
On Thu, Mar 26, 2020 at 09:02:48AM -0400, Peter Xu wrote:
[...]
> > > > > +static inline bool vtd_pasid_cache_valid(
> > > > > + VTDPASIDAddressSpace *vtd_pasid_as) {
> > > > > +return vtd_pasid_as->iommu_state &&
> ^
>
> > > >
> > > > This check can be dropped because always true?
> > > >
> > > > If you agree with both the changes, please add:
> > > >
> > > > Reviewed-by: Peter Xu
> > >
> > > I think the code should ensure all the pasid_as in hash table is valid.
> > > And we can
> > > since all the operations are under protection of iommu_lock.
> > >
> > Peter,
> >
> > I think my reply was wrong. pasid_as in has table may be stale since
> > the per pasid_as cache_gen may be not identical with the cache_gen
> > in iommu_state. e.g. vtd_pasid_cache_reset() only increases the
> > cache_gen in iommu_state. So there will be pasid_as in hash table
> > which has cached pasid entry but its cache_gen is not equal to the
> > one in iommu_state. For such pasid_as, we should treat it as stale.
> > So I guess the vtd_pasid_cache_valid() is still necessary.
>
> I guess you misread my comment. :)
>
> I was saying the "vtd_pasid_as->iommu_state" check is not needed,
> because iommu_state was always set if the address space is created.
> vtd_pasid_cache_valid() is needed.
>
> Also, please double confirm that vtd_pasid_cache_reset() should drop
> all the address spaces (as I think it should), not "only increase the
> cache_gen". IMHO you should only increase the cache_gen in the PSI
> hook (vtd_pasid_cache_psi()) only.
Sorry, I mean GSI (vtd_pasid_cache_gsi), not PSI.
--
Peter Xu
Re: [PATCH v1 20/22] intel_iommu: propagate PASID-based iotlb invalidation to host
On Thu, Mar 26, 2020 at 05:41:39AM +, Liu, Yi L wrote:
> > From: Liu, Yi L
> > Sent: Wednesday, March 25, 2020 9:22 PM
> > To: 'Peter Xu'
> > Subject: RE: [PATCH v1 20/22] intel_iommu: propagate PASID-based iotlb
> > invalidation to host
> >
> > > From: Peter Xu
> > > Sent: Wednesday, March 25, 2020 2:34 AM
> > > To: Liu, Yi L
> > > Subject: Re: [PATCH v1 20/22] intel_iommu: propagate PASID-based iotlb
> > > invalidation to host
> > >
> > > On Sun, Mar 22, 2020 at 05:36:17AM -0700, Liu Yi L wrote:
> > > > This patch propagates PASID-based iotlb invalidation to host.
> > > >
> > > > Intel VT-d 3.0 supports nested translation in PASID granular.
> > > > Guest SVA support could be implemented by configuring nested
> > > > translation on specific PASID. This is also known as dual stage DMA
> > > > translation.
> > > >
> > > > Under such configuration, guest owns the GVA->GPA translation which
> > > > is configured as first level page table in host side for a specific
> > > > pasid, and host owns GPA->HPA translation. As guest owns first level
> > > > translation table, piotlb invalidation should be propagated to host
> > > > since host IOMMU will cache first level page table related mappings
> > > > during DMA address translation.
> > > >
> > > > This patch traps the guest PASID-based iotlb flush and propagate it
> > > > to host.
> > > >
> > > > Cc: Kevin Tian
> > > > Cc: Jacob Pan
> > > > Cc: Peter Xu
> > > > Cc: Yi Sun
> > > > Cc: Paolo Bonzini
> > > > Cc: Richard Henderson
> > > > Cc: Eduardo Habkost
> > > > Signed-off-by: Liu Yi L
> > > > ---
> > > > hw/i386/intel_iommu.c | 139
> > > +
> > > > hw/i386/intel_iommu_internal.h | 7 +++
> > > > 2 files changed, 146 insertions(+)
> > > >
> > > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index
> > > > b9ac07d..10d314d 100644
> > > > --- a/hw/i386/intel_iommu.c
> > > > +++ b/hw/i386/intel_iommu.c
> > > > @@ -3134,15 +3134,154 @@ static bool
> > > vtd_process_pasid_desc(IntelIOMMUState *s,
> > > > return (ret == 0) ? true : false; }
> > > >
> > > > +/**
> > > > + * Caller of this function should hold iommu_lock.
> > > > + */
> > > > +static void vtd_invalidate_piotlb(IntelIOMMUState *s,
> > > > + VTDBus *vtd_bus,
> > > > + int devfn,
> > > > + DualIOMMUStage1Cache
> > > > +*stage1_cache) {
> > > > +VTDHostIOMMUContext *vtd_dev_icx;
> > > > +HostIOMMUContext *host_icx;
> > > > +
> > > > +vtd_dev_icx = vtd_bus->dev_icx[devfn];
> > > > +if (!vtd_dev_icx) {
> > > > +goto out;
> > > > +}
> > > > +host_icx = vtd_dev_icx->host_icx;
> > > > +if (!host_icx) {
> > > > +goto out;
> > > > +}
> > > > +if (host_iommu_ctx_flush_stage1_cache(host_icx, stage1_cache)) {
> > > > +error_report("Cache flush failed");
> > >
> > > I think this should not easily be triggered by the guest, but just in
> > > case... Let's use
> > > error_report_once() to be safe.
> >
> > Agreed.
> >
> > > > +}
> > > > +out:
> > > > +return;
> > > > +}
> > > > +
> > > > +static inline bool vtd_pasid_cache_valid(
> > > > + VTDPASIDAddressSpace *vtd_pasid_as) {
> > > > +return vtd_pasid_as->iommu_state &&
^
> > >
> > > This check can be dropped because always true?
> > >
> > > If you agree with both the changes, please add:
> > >
> > > Reviewed-by: Peter Xu
> >
> > I think the code should ensure all the pasid_as in hash table is valid. And
> > we can
> > since all the operations are under protection of iommu_lock.
> >
> Peter,
>
> I think my reply was wrong. pasid_as in has table may be stale since
> the per pasid_as cache_gen may be not identical with the cache_gen
> in iommu_state. e.g. vtd_pasid_cache_reset() only increases the
> cache_gen in iommu_state. So there will be pasid_as in hash table
> which has cached pasid entry but its cache_gen is not equal to the
> one in iommu_state. For such pasid_as, we should treat it as stale.
> So I guess the vtd_pasid_cache_valid() is still necessary.
I guess you misread my comment. :)
I was saying the "vtd_pasid_as->iommu_state" check is not needed,
because iommu_state was always set if the address space is created.
vtd_pasid_cache_valid() is needed.
Also, please double confirm that vtd_pasid_cache_reset() should drop
all the address spaces (as I think it should), not "only increase the
cache_gen". IMHO you should only increase the cache_gen in the PSI
hook (vtd_pasid_cache_psi()) only.
Thanks,
--
Peter Xu
RE: [PATCH v1 20/22] intel_iommu: propagate PASID-based iotlb invalidation to host
> From: Liu, Yi L
> Sent: Wednesday, March 25, 2020 9:22 PM
> To: 'Peter Xu'
> Subject: RE: [PATCH v1 20/22] intel_iommu: propagate PASID-based iotlb
> invalidation to host
>
> > From: Peter Xu
> > Sent: Wednesday, March 25, 2020 2:34 AM
> > To: Liu, Yi L
> > Subject: Re: [PATCH v1 20/22] intel_iommu: propagate PASID-based iotlb
> > invalidation to host
> >
> > On Sun, Mar 22, 2020 at 05:36:17AM -0700, Liu Yi L wrote:
> > > This patch propagates PASID-based iotlb invalidation to host.
> > >
> > > Intel VT-d 3.0 supports nested translation in PASID granular.
> > > Guest SVA support could be implemented by configuring nested
> > > translation on specific PASID. This is also known as dual stage DMA
> > > translation.
> > >
> > > Under such configuration, guest owns the GVA->GPA translation which
> > > is configured as first level page table in host side for a specific
> > > pasid, and host owns GPA->HPA translation. As guest owns first level
> > > translation table, piotlb invalidation should be propagated to host
> > > since host IOMMU will cache first level page table related mappings
> > > during DMA address translation.
> > >
> > > This patch traps the guest PASID-based iotlb flush and propagate it
> > > to host.
> > >
> > > Cc: Kevin Tian
> > > Cc: Jacob Pan
> > > Cc: Peter Xu
> > > Cc: Yi Sun
> > > Cc: Paolo Bonzini
> > > Cc: Richard Henderson
> > > Cc: Eduardo Habkost
> > > Signed-off-by: Liu Yi L
> > > ---
> > > hw/i386/intel_iommu.c | 139
> > +
> > > hw/i386/intel_iommu_internal.h | 7 +++
> > > 2 files changed, 146 insertions(+)
> > >
> > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index
> > > b9ac07d..10d314d 100644
> > > --- a/hw/i386/intel_iommu.c
> > > +++ b/hw/i386/intel_iommu.c
> > > @@ -3134,15 +3134,154 @@ static bool
> > vtd_process_pasid_desc(IntelIOMMUState *s,
> > > return (ret == 0) ? true : false; }
> > >
> > > +/**
> > > + * Caller of this function should hold iommu_lock.
> > > + */
> > > +static void vtd_invalidate_piotlb(IntelIOMMUState *s,
> > > + VTDBus *vtd_bus,
> > > + int devfn,
> > > + DualIOMMUStage1Cache
> > > +*stage1_cache) {
> > > +VTDHostIOMMUContext *vtd_dev_icx;
> > > +HostIOMMUContext *host_icx;
> > > +
> > > +vtd_dev_icx = vtd_bus->dev_icx[devfn];
> > > +if (!vtd_dev_icx) {
> > > +goto out;
> > > +}
> > > +host_icx = vtd_dev_icx->host_icx;
> > > +if (!host_icx) {
> > > +goto out;
> > > +}
> > > +if (host_iommu_ctx_flush_stage1_cache(host_icx, stage1_cache)) {
> > > +error_report("Cache flush failed");
> >
> > I think this should not easily be triggered by the guest, but just in
> > case... Let's use
> > error_report_once() to be safe.
>
> Agreed.
>
> > > +}
> > > +out:
> > > +return;
> > > +}
> > > +
> > > +static inline bool vtd_pasid_cache_valid(
> > > + VTDPASIDAddressSpace *vtd_pasid_as) {
> > > +return vtd_pasid_as->iommu_state &&
> >
> > This check can be dropped because always true?
> >
> > If you agree with both the changes, please add:
> >
> > Reviewed-by: Peter Xu
>
> I think the code should ensure all the pasid_as in hash table is valid. And
> we can
> since all the operations are under protection of iommu_lock.
>
Peter,
I think my reply was wrong. pasid_as in has table may be stale since
the per pasid_as cache_gen may be not identical with the cache_gen
in iommu_state. e.g. vtd_pasid_cache_reset() only increases the
cache_gen in iommu_state. So there will be pasid_as in hash table
which has cached pasid entry but its cache_gen is not equal to the
one in iommu_state. For such pasid_as, we should treat it as stale.
So I guess the vtd_pasid_cache_valid() is still necessary.
Regards,
Yi Liu
RE: [PATCH v1 20/22] intel_iommu: propagate PASID-based iotlb invalidation to host
> From: Peter Xu
> Sent: Wednesday, March 25, 2020 2:34 AM
> To: Liu, Yi L
> Subject: Re: [PATCH v1 20/22] intel_iommu: propagate PASID-based iotlb
> invalidation to host
>
> On Sun, Mar 22, 2020 at 05:36:17AM -0700, Liu Yi L wrote:
> > This patch propagates PASID-based iotlb invalidation to host.
> >
> > Intel VT-d 3.0 supports nested translation in PASID granular.
> > Guest SVA support could be implemented by configuring nested
> > translation on specific PASID. This is also known as dual stage DMA
> > translation.
> >
> > Under such configuration, guest owns the GVA->GPA translation which is
> > configured as first level page table in host side for a specific
> > pasid, and host owns GPA->HPA translation. As guest owns first level
> > translation table, piotlb invalidation should be propagated to host
> > since host IOMMU will cache first level page table related mappings
> > during DMA address translation.
> >
> > This patch traps the guest PASID-based iotlb flush and propagate it to
> > host.
> >
> > Cc: Kevin Tian
> > Cc: Jacob Pan
> > Cc: Peter Xu
> > Cc: Yi Sun
> > Cc: Paolo Bonzini
> > Cc: Richard Henderson
> > Cc: Eduardo Habkost
> > Signed-off-by: Liu Yi L
> > ---
> > hw/i386/intel_iommu.c | 139
> +
> > hw/i386/intel_iommu_internal.h | 7 +++
> > 2 files changed, 146 insertions(+)
> >
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index
> > b9ac07d..10d314d 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -3134,15 +3134,154 @@ static bool
> vtd_process_pasid_desc(IntelIOMMUState *s,
> > return (ret == 0) ? true : false; }
> >
> > +/**
> > + * Caller of this function should hold iommu_lock.
> > + */
> > +static void vtd_invalidate_piotlb(IntelIOMMUState *s,
> > + VTDBus *vtd_bus,
> > + int devfn,
> > + DualIOMMUStage1Cache *stage1_cache)
> > +{
> > +VTDHostIOMMUContext *vtd_dev_icx;
> > +HostIOMMUContext *host_icx;
> > +
> > +vtd_dev_icx = vtd_bus->dev_icx[devfn];
> > +if (!vtd_dev_icx) {
> > +goto out;
> > +}
> > +host_icx = vtd_dev_icx->host_icx;
> > +if (!host_icx) {
> > +goto out;
> > +}
> > +if (host_iommu_ctx_flush_stage1_cache(host_icx, stage1_cache)) {
> > +error_report("Cache flush failed");
>
> I think this should not easily be triggered by the guest, but just in case...
> Let's use
> error_report_once() to be safe.
Agreed.
> > +}
> > +out:
> > +return;
> > +}
> > +
> > +static inline bool vtd_pasid_cache_valid(
> > + VTDPASIDAddressSpace *vtd_pasid_as) {
> > +return vtd_pasid_as->iommu_state &&
>
> This check can be dropped because always true?
>
> If you agree with both the changes, please add:
>
> Reviewed-by: Peter Xu
I think the code should ensure all the pasid_as in hash table is valid. And
we can since all the operations are under protection of iommu_lock.
Thanks,
Yi Liu
> > + (vtd_pasid_as->iommu_state->pasid_cache_gen
> > + == vtd_pasid_as->pasid_cache_entry.pasid_cache_gen);
> > +}
> > +
> > +/**
> > + * This function is a loop function for the s->vtd_pasid_as
> > + * list with VTDPIOTLBInvInfo as execution filter. It propagates
> > + * the piotlb invalidation to host. Caller of this function
> > + * should hold iommu_lock.
> > + */
> > +static void vtd_flush_pasid_iotlb(gpointer key, gpointer value,
> > + gpointer user_data) {
> > +VTDPIOTLBInvInfo *piotlb_info = user_data;
> > +VTDPASIDAddressSpace *vtd_pasid_as = value;
> > +uint16_t did;
> > +
> > +/*
> > + * Needs to check whether the pasid entry cache stored in
> > + * vtd_pasid_as is valid or not. "invalid" means the pasid
> > + * cache has been flushed, thus host should have done piotlb
> > + * invalidation together with a pasid cache invalidation, so
> > + * no need to pass down piotlb invalidation to host for better
> > + * performance. Only when pasid entry cache is "valid", should
> > + * a piotlb invalidation be propagated to host since it means
> > + * guest just modified a mapping in its page table.
> > + */
> > +
Re: [PATCH v1 20/22] intel_iommu: propagate PASID-based iotlb invalidation to host
On Sun, Mar 22, 2020 at 05:36:17AM -0700, Liu Yi L wrote:
> This patch propagates PASID-based iotlb invalidation to host.
>
> Intel VT-d 3.0 supports nested translation in PASID granular.
> Guest SVA support could be implemented by configuring nested
> translation on specific PASID. This is also known as dual stage
> DMA translation.
>
> Under such configuration, guest owns the GVA->GPA translation
> which is configured as first level page table in host side for
> a specific pasid, and host owns GPA->HPA translation. As guest
> owns first level translation table, piotlb invalidation should
> be propagated to host since host IOMMU will cache first level
> page table related mappings during DMA address translation.
>
> This patch traps the guest PASID-based iotlb flush and propagate
> it to host.
>
> Cc: Kevin Tian
> Cc: Jacob Pan
> Cc: Peter Xu
> Cc: Yi Sun
> Cc: Paolo Bonzini
> Cc: Richard Henderson
> Cc: Eduardo Habkost
> Signed-off-by: Liu Yi L
> ---
> hw/i386/intel_iommu.c | 139
> +
> hw/i386/intel_iommu_internal.h | 7 +++
> 2 files changed, 146 insertions(+)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index b9ac07d..10d314d 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3134,15 +3134,154 @@ static bool vtd_process_pasid_desc(IntelIOMMUState
> *s,
> return (ret == 0) ? true : false;
> }
>
> +/**
> + * Caller of this function should hold iommu_lock.
> + */
> +static void vtd_invalidate_piotlb(IntelIOMMUState *s,
> + VTDBus *vtd_bus,
> + int devfn,
> + DualIOMMUStage1Cache *stage1_cache)
> +{
> +VTDHostIOMMUContext *vtd_dev_icx;
> +HostIOMMUContext *host_icx;
> +
> +vtd_dev_icx = vtd_bus->dev_icx[devfn];
> +if (!vtd_dev_icx) {
> +goto out;
> +}
> +host_icx = vtd_dev_icx->host_icx;
> +if (!host_icx) {
> +goto out;
> +}
> +if (host_iommu_ctx_flush_stage1_cache(host_icx, stage1_cache)) {
> +error_report("Cache flush failed");
I think this should not easily be triggered by the guest, but just in
case... Let's use error_report_once() to be safe.
> +}
> +out:
> +return;
> +}
> +
> +static inline bool vtd_pasid_cache_valid(
> + VTDPASIDAddressSpace *vtd_pasid_as)
> +{
> +return vtd_pasid_as->iommu_state &&
This check can be dropped because always true?
If you agree with both the changes, please add:
Reviewed-by: Peter Xu
> + (vtd_pasid_as->iommu_state->pasid_cache_gen
> + == vtd_pasid_as->pasid_cache_entry.pasid_cache_gen);
> +}
> +
> +/**
> + * This function is a loop function for the s->vtd_pasid_as
> + * list with VTDPIOTLBInvInfo as execution filter. It propagates
> + * the piotlb invalidation to host. Caller of this function
> + * should hold iommu_lock.
> + */
> +static void vtd_flush_pasid_iotlb(gpointer key, gpointer value,
> + gpointer user_data)
> +{
> +VTDPIOTLBInvInfo *piotlb_info = user_data;
> +VTDPASIDAddressSpace *vtd_pasid_as = value;
> +uint16_t did;
> +
> +/*
> + * Needs to check whether the pasid entry cache stored in
> + * vtd_pasid_as is valid or not. "invalid" means the pasid
> + * cache has been flushed, thus host should have done piotlb
> + * invalidation together with a pasid cache invalidation, so
> + * no need to pass down piotlb invalidation to host for better
> + * performance. Only when pasid entry cache is "valid", should
> + * a piotlb invalidation be propagated to host since it means
> + * guest just modified a mapping in its page table.
> + */
> +if (!vtd_pasid_cache_valid(vtd_pasid_as)) {
> +return;
> +}
> +
> +did = vtd_pe_get_domain_id(
> +&(vtd_pasid_as->pasid_cache_entry.pasid_entry));
> +
> +if ((piotlb_info->domain_id == did) &&
> +(piotlb_info->pasid == vtd_pasid_as->pasid)) {
> +vtd_invalidate_piotlb(vtd_pasid_as->iommu_state,
> + vtd_pasid_as->vtd_bus,
> + vtd_pasid_as->devfn,
> + piotlb_info->stage1_cache);
> +}
> +
> +/*
> + * TODO: needs to add QEMU piotlb flush when QEMU piotlb
> + * infrastructure is ready. For now, it is enough for passthru
> + * devices.
> + */
> +}
> +
> static void vtd_piotlb_pasid_invalidate(IntelIOMMUState *s,
> uint16_t domain_id,
> uint32_t pasid)
> {
> +VTDPIOTLBInvInfo piotlb_info;
> +DualIOMMUStage1Cache *stage1_cache;
> +struct iommu_cache_invalidate_info *cache_info;
> +
> +stage1_cache = g_malloc0(sizeof(*stage1_cache));
> +stage1_cache->pasid = pasid;
> +
> +cache_info = &stage1_cache->cache_info;
> +cache_info->version = IOMMU_UAPI
