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 - 2026 Red Hat, Inc.