From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Reference-count the micro-TLBs as several bus masters can be
connected to the same micro-TLB (and drop TODO comment).
This wasn't an issue so far, since the platform devices
(this driver deals with) get assigned/deassigned together during
domain creation/destruction. But, in order to support PCI devices
(which are hot-pluggable) in the near future we will need to
take care of.
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
xen/drivers/passthrough/arm/ipmmu-vmsa.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
index 22dd84e..32609f8 100644
--- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
+++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
@@ -134,6 +134,7 @@ struct ipmmu_vmsa_device {
spinlock_t lock; /* Protects ctx and domains[] */
DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
+ unsigned int utlb_refcount[IPMMU_UTLB_MAX];
const struct ipmmu_features *features;
};
@@ -477,13 +478,12 @@ static int ipmmu_utlb_enable(struct ipmmu_vmsa_domain *domain,
}
}
- /*
- * TODO: Reference-count the micro-TLB as several bus masters can be
- * connected to the same micro-TLB.
- */
- ipmmu_imuasid_write(mmu, utlb, 0);
- ipmmu_imuctr_write(mmu, utlb, imuctr |
- IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_MMUEN);
+ if ( mmu->utlb_refcount[utlb]++ == 0 )
+ {
+ ipmmu_imuasid_write(mmu, utlb, 0);
+ ipmmu_imuctr_write(mmu, utlb, imuctr |
+ IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_MMUEN);
+ }
return 0;
}
@@ -494,7 +494,8 @@ static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain,
{
struct ipmmu_vmsa_device *mmu = domain->mmu;
- ipmmu_imuctr_write(mmu, utlb, 0);
+ if ( --mmu->utlb_refcount[utlb] == 0 )
+ ipmmu_imuctr_write(mmu, utlb, 0);
}
/* Domain/Context Management */
--
2.7.4
Hi Oleksandr, Oleksandr Tyshchenko <olekstysh@gmail.com> writes: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > Reference-count the micro-TLBs as several bus masters can be > connected to the same micro-TLB (and drop TODO comment). > This wasn't an issue so far, since the platform devices > (this driver deals with) get assigned/deassigned together during > domain creation/destruction. But, in order to support PCI devices > (which are hot-pluggable) in the near future we will need to > take care of. > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > --- > xen/drivers/passthrough/arm/ipmmu-vmsa.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c > index 22dd84e..32609f8 100644 > --- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c > +++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c > @@ -134,6 +134,7 @@ struct ipmmu_vmsa_device { > spinlock_t lock; /* Protects ctx and domains[] */ > DECLARE_BITMAP(ctx, IPMMU_CTX_MAX); > struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX]; > + unsigned int utlb_refcount[IPMMU_UTLB_MAX]; > const struct ipmmu_features *features; > }; > > @@ -477,13 +478,12 @@ static int ipmmu_utlb_enable(struct ipmmu_vmsa_domain *domain, > } > } > > - /* > - * TODO: Reference-count the micro-TLB as several bus masters can be > - * connected to the same micro-TLB. > - */ > - ipmmu_imuasid_write(mmu, utlb, 0); > - ipmmu_imuctr_write(mmu, utlb, imuctr | > - IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_MMUEN); > + if ( mmu->utlb_refcount[utlb]++ == 0 ) > + { > + ipmmu_imuasid_write(mmu, utlb, 0); > + ipmmu_imuctr_write(mmu, utlb, imuctr | > + IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_MMUEN); > + } > > return 0; > } > @@ -494,7 +494,8 @@ static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain, > { > struct ipmmu_vmsa_device *mmu = domain->mmu; > > - ipmmu_imuctr_write(mmu, utlb, 0); It would be great to have + ASSERT(mmu->utlb_refcount[utlb] > 0); there. Just in case. > + if ( --mmu->utlb_refcount[utlb] == 0 ) > + ipmmu_imuctr_write(mmu, utlb, 0); > } > > /* Domain/Context Management */ -- Volodymyr Babchuk at EPAM
On 15.12.21 04:58, Volodymyr Babchuk wrote: > Hi Oleksandr, Hi Volodymyr > > > Oleksandr Tyshchenko <olekstysh@gmail.com> writes: > >> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> >> Reference-count the micro-TLBs as several bus masters can be >> connected to the same micro-TLB (and drop TODO comment). >> This wasn't an issue so far, since the platform devices >> (this driver deals with) get assigned/deassigned together during >> domain creation/destruction. But, in order to support PCI devices >> (which are hot-pluggable) in the near future we will need to >> take care of. >> >> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> --- >> xen/drivers/passthrough/arm/ipmmu-vmsa.c | 17 +++++++++-------- >> 1 file changed, 9 insertions(+), 8 deletions(-) >> >> diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c >> index 22dd84e..32609f8 100644 >> --- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c >> +++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c >> @@ -134,6 +134,7 @@ struct ipmmu_vmsa_device { >> spinlock_t lock; /* Protects ctx and domains[] */ >> DECLARE_BITMAP(ctx, IPMMU_CTX_MAX); >> struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX]; >> + unsigned int utlb_refcount[IPMMU_UTLB_MAX]; >> const struct ipmmu_features *features; >> }; >> >> @@ -477,13 +478,12 @@ static int ipmmu_utlb_enable(struct ipmmu_vmsa_domain *domain, >> } >> } >> >> - /* >> - * TODO: Reference-count the micro-TLB as several bus masters can be >> - * connected to the same micro-TLB. >> - */ >> - ipmmu_imuasid_write(mmu, utlb, 0); >> - ipmmu_imuctr_write(mmu, utlb, imuctr | >> - IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_MMUEN); >> + if ( mmu->utlb_refcount[utlb]++ == 0 ) >> + { >> + ipmmu_imuasid_write(mmu, utlb, 0); >> + ipmmu_imuctr_write(mmu, utlb, imuctr | >> + IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_MMUEN); >> + } >> >> return 0; >> } >> @@ -494,7 +494,8 @@ static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain, >> { >> struct ipmmu_vmsa_device *mmu = domain->mmu; >> >> - ipmmu_imuctr_write(mmu, utlb, 0); > It would be great to have > > + ASSERT(mmu->utlb_refcount[utlb] > 0); > > there. Just in case. ok, will add. Thank you. > >> + if ( --mmu->utlb_refcount[utlb] == 0 ) >> + ipmmu_imuctr_write(mmu, utlb, 0); >> } >> >> /* Domain/Context Management */ > -- Regards, Oleksandr Tyshchenko
Hello Oleksandr-san, Thank you for the patch! > From: Oleksandr Tyshchenko, Sent: Sunday, November 28, 2021 2:52 AM > > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > Reference-count the micro-TLBs as several bus masters can be > connected to the same micro-TLB (and drop TODO comment). > This wasn't an issue so far, since the platform devices > (this driver deals with) get assigned/deassigned together during > domain creation/destruction. But, in order to support PCI devices > (which are hot-pluggable) in the near future we will need to > take care of. > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > --- > xen/drivers/passthrough/arm/ipmmu-vmsa.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c > index 22dd84e..32609f8 100644 > --- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c > +++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c > @@ -134,6 +134,7 @@ struct ipmmu_vmsa_device { > spinlock_t lock; /* Protects ctx and domains[] */ > DECLARE_BITMAP(ctx, IPMMU_CTX_MAX); > struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX]; > + unsigned int utlb_refcount[IPMMU_UTLB_MAX]; > const struct ipmmu_features *features; > }; > > @@ -477,13 +478,12 @@ static int ipmmu_utlb_enable(struct ipmmu_vmsa_domain *domain, > } > } > > - /* > - * TODO: Reference-count the micro-TLB as several bus masters can be > - * connected to the same micro-TLB. > - */ > - ipmmu_imuasid_write(mmu, utlb, 0); > - ipmmu_imuctr_write(mmu, utlb, imuctr | > - IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_MMUEN); > + if ( mmu->utlb_refcount[utlb]++ == 0 ) > + { > + ipmmu_imuasid_write(mmu, utlb, 0); > + ipmmu_imuctr_write(mmu, utlb, imuctr | > + IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_MMUEN); > + } > > return 0; > } > @@ -494,7 +494,8 @@ static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain, > { > struct ipmmu_vmsa_device *mmu = domain->mmu; > > - ipmmu_imuctr_write(mmu, utlb, 0); As Volodymyr-san mentioned before, after we added ASSERT(), Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Best regards, Yoshihiro Shimoda
© 2016 - 2024 Red Hat, Inc.