RE: [RFC v3 16/25] intel_iommu: add PASID cache management infrastructure
> -Original Message- > From: Peter Xu > Sent: Thursday, February 13, 2020 11:14 PM > To: Liu, Yi L > Subject: Re: [RFC v3 16/25] intel_iommu: add PASID cache management > infrastructure > > On Thu, Feb 13, 2020 at 02:59:37AM +, Liu, Yi L wrote: > > > - Remove the vtd_pasid_as check right below because it's not needed. > > > > > > > > > > > > > > > > > +if (vtd_pasid_as && > > > > > > > yes, it is. In current series vtd_add_find_pasid_as() doesn’t check the > > result of vtd_pasid_as mem allocation, so no need to check vtd_pasid_as > > here either. However, it might be better to check the allocation result > > or it will result in issue if allocation failed. What's your preference > > here? > > That should not be needed, because IIRC g_malloc0() will directly > coredump if allocation fails. Even if not, it'll coredump in > vtd_add_find_pasid_as() soon when accessing the NULL pointer. Cool, thanks for this message. Then I'll follow your suggestion to remove the vtd_pasid_as check. Regards, Yi Liu
Re: [RFC v3 16/25] intel_iommu: add PASID cache management infrastructure
On Thu, Feb 13, 2020 at 02:59:37AM +, Liu, Yi L wrote: > > - Remove the vtd_pasid_as check right below because it's not needed. > > > > > > > > > > > > > +if (vtd_pasid_as && > > > > yes, it is. In current series vtd_add_find_pasid_as() doesn’t check the > result of vtd_pasid_as mem allocation, so no need to check vtd_pasid_as > here either. However, it might be better to check the allocation result > or it will result in issue if allocation failed. What's your preference > here? That should not be needed, because IIRC g_malloc0() will directly coredump if allocation fails. Even if not, it'll coredump in vtd_add_find_pasid_as() soon when accessing the NULL pointer. -- Peter Xu
RE: [RFC v3 16/25] intel_iommu: add PASID cache management infrastructure
> From: Peter Xu
> Sent: Wednesday, February 12, 2020 11:26 PM
> To: Liu, Yi L
> Subject: Re: [RFC v3 16/25] intel_iommu: add PASID cache management
> infrastructure
>
> On Wed, Feb 12, 2020 at 08:37:30AM +, Liu, Yi L wrote:
> > > From: Peter Xu
> > > Sent: Wednesday, February 12, 2020 7:36 AM
> > > To: Liu, Yi L
> > > Subject: Re: [RFC v3 16/25] intel_iommu: add PASID cache management
> > > infrastructure
> > >
> > > On Wed, Jan 29, 2020 at 04:16:47AM -0800, Liu, Yi L wrote:
> > > > From: Liu Yi L
> > > >
> > > > This patch adds a PASID cache management infrastructure based on
> > > > new added structure VTDPASIDAddressSpace, which is used to track
> > > > the PASID usage and future PASID tagged DMA address translation
> > > > support in vIOMMU.
> > > >
> > > > struct VTDPASIDAddressSpace {
> > > > VTDBus *vtd_bus;
> > > > uint8_t devfn;
> > > > AddressSpace as;
> > > > uint32_t pasid;
> > > > IntelIOMMUState *iommu_state;
> > > > VTDContextCacheEntry context_cache_entry;
> > > > QLIST_ENTRY(VTDPASIDAddressSpace) next;
> > > > VTDPASIDCacheEntry pasid_cache_entry;
> > > > };
> > > >
> > > > Ideally, a VTDPASIDAddressSpace instance is created when a PASID
> > > > is bound with a DMA AddressSpace. Intel VT-d spec requires guest
> > > > software to issue pasid cache invalidation when bind or unbind a
> > > > pasid with an address space under caching-mode. However, as
> > > > VTDPASIDAddressSpace instances also act as pasid cache in this
> > > > implementation, its creation also happens during vIOMMU PASID
> > > > tagged DMA translation. The creation in this path will not be
> > > > added in this patch since no PASID-capable emulated devices for
> > > > now.
> > > >
> > > > The implementation in this patch manages VTDPASIDAddressSpace
> > > > instances per PASID+BDF (lookup and insert will use PASID and
> > > > BDF) since Intel VT-d spec allows per-BDF PASID Table. When a
> > > > guest bind a PASID with an AddressSpace, QEMU will capture the
> > > > guest pasid selective pasid cache invalidation, and allocate
> > > > remove a VTDPASIDAddressSpace instance per the invalidation
> > > > reasons:
> > > >
> > > > *) a present pasid entry moved to non-present
> > > > *) a present pasid entry to be a present entry
> > > > *) a non-present pasid entry moved to present
> > > >
> > > > vIOMMU emulator could figure out the reason by fetching latest
> > > > guest pasid entry.
> > > >
> > > > 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 | 367
> > > +
> > > > hw/i386/intel_iommu_internal.h | 14 ++
> > > > hw/i386/trace-events | 1 +
> > > > include/hw/i386/intel_iommu.h | 36 +++-
> > > > 4 files changed, 417 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index
> > > > 58e7213..c75cb7b 100644
> > > > --- a/hw/i386/intel_iommu.c
> > > > +++ b/hw/i386/intel_iommu.c
> > > > @@ -40,6 +40,7 @@
> > > > #include "kvm_i386.h"
> > > > #include "migration/vmstate.h"
> > > > #include "trace.h"
> > > > +#include "qemu/jhash.h"
> > > >
> > > > /* context entry operations */
> > > > #define VTD_CE_GET_RID2PASID(ce) \ @@ -65,6 +66,8 @@ static void
> > > > vtd_address_space_refresh_all(IntelIOMMUState *s); static void
> > > > vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier
> > > *n);
> > > >
> > > > +static void vtd_pasid_cache_reset(IntelIOMMUState *s);
[...]
> > > > +
> > > > +/**
> > > > + * This function is used to clear pasid_cache_gen of cached pasid
> > > > + * entry in vtd_pasid_as instances. Caller of this function
>
Re: [RFC v3 16/25] intel_iommu: add PASID cache management infrastructure
On Wed, Feb 12, 2020 at 08:37:30AM +, Liu, Yi L wrote:
> > From: Peter Xu
> > Sent: Wednesday, February 12, 2020 7:36 AM
> > To: Liu, Yi L
> > Subject: Re: [RFC v3 16/25] intel_iommu: add PASID cache management
> > infrastructure
> >
> > On Wed, Jan 29, 2020 at 04:16:47AM -0800, Liu, Yi L wrote:
> > > From: Liu Yi L
> > >
> > > This patch adds a PASID cache management infrastructure based on
> > > new added structure VTDPASIDAddressSpace, which is used to track
> > > the PASID usage and future PASID tagged DMA address translation
> > > support in vIOMMU.
> > >
> > > struct VTDPASIDAddressSpace {
> > > VTDBus *vtd_bus;
> > > uint8_t devfn;
> > > AddressSpace as;
> > > uint32_t pasid;
> > > IntelIOMMUState *iommu_state;
> > > VTDContextCacheEntry context_cache_entry;
> > > QLIST_ENTRY(VTDPASIDAddressSpace) next;
> > > VTDPASIDCacheEntry pasid_cache_entry;
> > > };
> > >
> > > Ideally, a VTDPASIDAddressSpace instance is created when a PASID
> > > is bound with a DMA AddressSpace. Intel VT-d spec requires guest
> > > software to issue pasid cache invalidation when bind or unbind a
> > > pasid with an address space under caching-mode. However, as
> > > VTDPASIDAddressSpace instances also act as pasid cache in this
> > > implementation, its creation also happens during vIOMMU PASID
> > > tagged DMA translation. The creation in this path will not be
> > > added in this patch since no PASID-capable emulated devices for
> > > now.
> > >
> > > The implementation in this patch manages VTDPASIDAddressSpace
> > > instances per PASID+BDF (lookup and insert will use PASID and
> > > BDF) since Intel VT-d spec allows per-BDF PASID Table. When a
> > > guest bind a PASID with an AddressSpace, QEMU will capture the
> > > guest pasid selective pasid cache invalidation, and allocate
> > > remove a VTDPASIDAddressSpace instance per the invalidation
> > > reasons:
> > >
> > > *) a present pasid entry moved to non-present
> > > *) a present pasid entry to be a present entry
> > > *) a non-present pasid entry moved to present
> > >
> > > vIOMMU emulator could figure out the reason by fetching latest
> > > guest pasid entry.
> > >
> > > 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 | 367
> > +
> > > hw/i386/intel_iommu_internal.h | 14 ++
> > > hw/i386/trace-events | 1 +
> > > include/hw/i386/intel_iommu.h | 36 +++-
> > > 4 files changed, 417 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > > index 58e7213..c75cb7b 100644
> > > --- a/hw/i386/intel_iommu.c
> > > +++ b/hw/i386/intel_iommu.c
> > > @@ -40,6 +40,7 @@
> > > #include "kvm_i386.h"
> > > #include "migration/vmstate.h"
> > > #include "trace.h"
> > > +#include "qemu/jhash.h"
> > >
> > > /* context entry operations */
> > > #define VTD_CE_GET_RID2PASID(ce) \
> > > @@ -65,6 +66,8 @@
> > > static void vtd_address_space_refresh_all(IntelIOMMUState *s);
> > > static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier
> > *n);
> > >
> > > +static void vtd_pasid_cache_reset(IntelIOMMUState *s);
> > > +
> > > static void vtd_panic_require_caching_mode(void)
> > > {
> > > error_report("We need to set caching-mode=on for intel-iommu to
> > > enable "
> > > @@ -276,6 +279,7 @@ static void vtd_reset_caches(IntelIOMMUState *s)
> > > vtd_iommu_lock(s);
> > > vtd_reset_iotlb_locked(s);
> > > vtd_reset_context_cache_locked(s);
> > > +vtd_pasid_cache_reset(s);
> > > vtd_iommu_unlock(s);
> > > }
> > >
> > > @@ -686,6 +690,11 @@ static inline bool vtd_pe_type_check(X86IOMMUState
> > *x86_iommu,
> > > return true;
> > > }
> > >
> > > +static inline uint16_t vtd_pe_get_domain_id(VTDPASIDEntry *pe)
> > > +{
> >
RE: [RFC v3 16/25] intel_iommu: add PASID cache management infrastructure
> From: Peter Xu
> Sent: Wednesday, February 12, 2020 7:36 AM
> To: Liu, Yi L
> Subject: Re: [RFC v3 16/25] intel_iommu: add PASID cache management
> infrastructure
>
> On Wed, Jan 29, 2020 at 04:16:47AM -0800, Liu, Yi L wrote:
> > From: Liu Yi L
> >
> > This patch adds a PASID cache management infrastructure based on
> > new added structure VTDPASIDAddressSpace, which is used to track
> > the PASID usage and future PASID tagged DMA address translation
> > support in vIOMMU.
> >
> > struct VTDPASIDAddressSpace {
> > VTDBus *vtd_bus;
> > uint8_t devfn;
> > AddressSpace as;
> > uint32_t pasid;
> > IntelIOMMUState *iommu_state;
> > VTDContextCacheEntry context_cache_entry;
> > QLIST_ENTRY(VTDPASIDAddressSpace) next;
> > VTDPASIDCacheEntry pasid_cache_entry;
> > };
> >
> > Ideally, a VTDPASIDAddressSpace instance is created when a PASID
> > is bound with a DMA AddressSpace. Intel VT-d spec requires guest
> > software to issue pasid cache invalidation when bind or unbind a
> > pasid with an address space under caching-mode. However, as
> > VTDPASIDAddressSpace instances also act as pasid cache in this
> > implementation, its creation also happens during vIOMMU PASID
> > tagged DMA translation. The creation in this path will not be
> > added in this patch since no PASID-capable emulated devices for
> > now.
> >
> > The implementation in this patch manages VTDPASIDAddressSpace
> > instances per PASID+BDF (lookup and insert will use PASID and
> > BDF) since Intel VT-d spec allows per-BDF PASID Table. When a
> > guest bind a PASID with an AddressSpace, QEMU will capture the
> > guest pasid selective pasid cache invalidation, and allocate
> > remove a VTDPASIDAddressSpace instance per the invalidation
> > reasons:
> >
> > *) a present pasid entry moved to non-present
> > *) a present pasid entry to be a present entry
> > *) a non-present pasid entry moved to present
> >
> > vIOMMU emulator could figure out the reason by fetching latest
> > guest pasid entry.
> >
> > 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 | 367
> +
> > hw/i386/intel_iommu_internal.h | 14 ++
> > hw/i386/trace-events | 1 +
> > include/hw/i386/intel_iommu.h | 36 +++-
> > 4 files changed, 417 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index 58e7213..c75cb7b 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -40,6 +40,7 @@
> > #include "kvm_i386.h"
> > #include "migration/vmstate.h"
> > #include "trace.h"
> > +#include "qemu/jhash.h"
> >
> > /* context entry operations */
> > #define VTD_CE_GET_RID2PASID(ce) \
> > @@ -65,6 +66,8 @@
> > static void vtd_address_space_refresh_all(IntelIOMMUState *s);
> > static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier
> *n);
> >
> > +static void vtd_pasid_cache_reset(IntelIOMMUState *s);
> > +
> > static void vtd_panic_require_caching_mode(void)
> > {
> > error_report("We need to set caching-mode=on for intel-iommu to enable
> > "
> > @@ -276,6 +279,7 @@ static void vtd_reset_caches(IntelIOMMUState *s)
> > vtd_iommu_lock(s);
> > vtd_reset_iotlb_locked(s);
> > vtd_reset_context_cache_locked(s);
> > +vtd_pasid_cache_reset(s);
> > vtd_iommu_unlock(s);
> > }
> >
> > @@ -686,6 +690,11 @@ static inline bool vtd_pe_type_check(X86IOMMUState
> *x86_iommu,
> > return true;
> > }
> >
> > +static inline uint16_t vtd_pe_get_domain_id(VTDPASIDEntry *pe)
> > +{
> > +return VTD_SM_PASID_ENTRY_DID((pe)->val[1]);
> > +}
> > +
> > static inline bool vtd_pdire_present(VTDPASIDDirEntry *pdire)
> > {
> > return pdire->val & 1;
> > @@ -2393,19 +2402,370 @@ static bool
> vtd_process_iotlb_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
> > return true;
> > }
> >
> > +static inline void vtd_init_pasid_key(uint32_t pasid,
> > + uint16_t sid,
> > + struct pasid_key *key)
&
Re: [RFC v3 16/25] intel_iommu: add PASID cache management infrastructure
On Wed, Jan 29, 2020 at 04:16:47AM -0800, Liu, Yi L wrote:
> From: Liu Yi L
>
> This patch adds a PASID cache management infrastructure based on
> new added structure VTDPASIDAddressSpace, which is used to track
> the PASID usage and future PASID tagged DMA address translation
> support in vIOMMU.
>
> struct VTDPASIDAddressSpace {
> VTDBus *vtd_bus;
> uint8_t devfn;
> AddressSpace as;
> uint32_t pasid;
> IntelIOMMUState *iommu_state;
> VTDContextCacheEntry context_cache_entry;
> QLIST_ENTRY(VTDPASIDAddressSpace) next;
> VTDPASIDCacheEntry pasid_cache_entry;
> };
>
> Ideally, a VTDPASIDAddressSpace instance is created when a PASID
> is bound with a DMA AddressSpace. Intel VT-d spec requires guest
> software to issue pasid cache invalidation when bind or unbind a
> pasid with an address space under caching-mode. However, as
> VTDPASIDAddressSpace instances also act as pasid cache in this
> implementation, its creation also happens during vIOMMU PASID
> tagged DMA translation. The creation in this path will not be
> added in this patch since no PASID-capable emulated devices for
> now.
>
> The implementation in this patch manages VTDPASIDAddressSpace
> instances per PASID+BDF (lookup and insert will use PASID and
> BDF) since Intel VT-d spec allows per-BDF PASID Table. When a
> guest bind a PASID with an AddressSpace, QEMU will capture the
> guest pasid selective pasid cache invalidation, and allocate
> remove a VTDPASIDAddressSpace instance per the invalidation
> reasons:
>
> *) a present pasid entry moved to non-present
> *) a present pasid entry to be a present entry
> *) a non-present pasid entry moved to present
>
> vIOMMU emulator could figure out the reason by fetching latest
> guest pasid entry.
>
> 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 | 367
> +
> hw/i386/intel_iommu_internal.h | 14 ++
> hw/i386/trace-events | 1 +
> include/hw/i386/intel_iommu.h | 36 +++-
> 4 files changed, 417 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 58e7213..c75cb7b 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -40,6 +40,7 @@
> #include "kvm_i386.h"
> #include "migration/vmstate.h"
> #include "trace.h"
> +#include "qemu/jhash.h"
>
> /* context entry operations */
> #define VTD_CE_GET_RID2PASID(ce) \
> @@ -65,6 +66,8 @@
> static void vtd_address_space_refresh_all(IntelIOMMUState *s);
> static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n);
>
> +static void vtd_pasid_cache_reset(IntelIOMMUState *s);
> +
> static void vtd_panic_require_caching_mode(void)
> {
> error_report("We need to set caching-mode=on for intel-iommu to enable "
> @@ -276,6 +279,7 @@ static void vtd_reset_caches(IntelIOMMUState *s)
> vtd_iommu_lock(s);
> vtd_reset_iotlb_locked(s);
> vtd_reset_context_cache_locked(s);
> +vtd_pasid_cache_reset(s);
> vtd_iommu_unlock(s);
> }
>
> @@ -686,6 +690,11 @@ static inline bool vtd_pe_type_check(X86IOMMUState
> *x86_iommu,
> return true;
> }
>
> +static inline uint16_t vtd_pe_get_domain_id(VTDPASIDEntry *pe)
> +{
> +return VTD_SM_PASID_ENTRY_DID((pe)->val[1]);
> +}
> +
> static inline bool vtd_pdire_present(VTDPASIDDirEntry *pdire)
> {
> return pdire->val & 1;
> @@ -2393,19 +2402,370 @@ static bool vtd_process_iotlb_desc(IntelIOMMUState
> *s, VTDInvDesc *inv_desc)
> return true;
> }
>
> +static inline void vtd_init_pasid_key(uint32_t pasid,
> + uint16_t sid,
> + struct pasid_key *key)
> +{
> +key->pasid = pasid;
> +key->sid = sid;
> +}
> +
> +static guint vtd_pasid_as_key_hash(gconstpointer v)
> +{
> +struct pasid_key *key = (struct pasid_key *)v;
> +uint32_t a, b, c;
> +
> +/* Jenkins hash */
> +a = b = c = JHASH_INITVAL + sizeof(*key);
> +a += key->sid;
> +b += extract32(key->pasid, 0, 16);
> +c += extract32(key->pasid, 16, 16);
> +
> +__jhash_mix(a, b, c);
> +__jhash_final(a, b, c);
> +
> +return c;
> +}
> +
> +static gboolean vtd_pasid_as_key_equal(gconstpointer v1, gconstpointer v2)
> +{
> +const struct pasid_key *k1 = v1;
> +const struct pasid_key *k2 = v2;
> +
> +return (k1->pasid == k2->pasid) && (k1->sid == k2->sid);
> +}
> +
> +static inline int vtd_dev_get_pe_from_pasid(IntelIOMMUState *s,
> +uint8_t bus_num,
> +uint8_t devfn,
> +uint32_t pasid,
> +VTDPASIDEntry *pe)
> +{
> +VTDContextEntry ce;
> +int ret;
> +dma_addr_
