Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1406664 > unrolled thread

[Patch v4 3/9] iommu/amd: Detect pre enabled translation

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [Patch v4 3/9] iommu/amd: Detect pre enabled translation Baoquan He <bhe@redhat.com> - 2016-05-25 08:30 +0200
    Re: [Patch v4 3/9] iommu/amd: Detect pre enabled translation Wan Zongshun <vw@iommu.org> - 2016-05-28 15:10 +0200
      Re: [Patch v4 3/9] iommu/amd: Detect pre enabled translation Baoquan He <bhe@redhat.com> - 2016-05-30 05:30 +0200
        Re: [Patch v4 3/9] iommu/amd: Detect pre enabled translation Baoquan He <bhe@redhat.com> - 2016-05-30 05:30 +0200

#1406664 — [Patch v4 3/9] iommu/amd: Detect pre enabled translation

Add functions to check whether translation is already enabled in IOMMU.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 drivers/iommu/amd_iommu_init.c  | 25 +++++++++++++++++++++++++
 drivers/iommu/amd_iommu_types.h |  4 ++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 8361367d..9e1dfcb 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -229,6 +229,26 @@ static int amd_iommu_enable_interrupts(void);
 static int __init iommu_go_to_state(enum iommu_init_state state);
 static void init_device_table_dma(void);
 
+
+static bool translation_pre_enabled(struct amd_iommu *iommu)
+{
+	return (iommu->flags & AMD_IOMMU_FLAG_TRANS_PRE_ENABLED);
+}
+
+static void clear_translation_pre_enabled(struct amd_iommu *iommu)
+{
+        iommu->flags &= ~AMD_IOMMU_FLAG_TRANS_PRE_ENABLED;
+}
+
+static void init_translation_status(struct amd_iommu *iommu)
+{
+	u32 ctrl;
+
+	ctrl = readl(iommu->mmio_base + MMIO_CONTROL_OFFSET);
+	if (ctrl & (1<<CONTROL_IOMMU_EN))
+		iommu->flags |= AMD_IOMMU_FLAG_TRANS_PRE_ENABLED;
+}
+
 static int iommu_pc_get_set_reg_val(struct amd_iommu *iommu,
 				    u8 bank, u8 cntr, u8 fxn,
 				    u64 *value, bool is_write);
@@ -1101,6 +1121,11 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 
 	iommu->int_enabled = false;
 
+	init_translation_status(iommu);
+
+	if (translation_pre_enabled())
+		pr_warn("Translation is already enabled - trying to copy translation structures\n");
+
 	ret = init_iommu_from_acpi(iommu, h);
 	if (ret)
 		return ret;
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 9d32b20..01783cc 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -384,6 +384,7 @@ extern struct kmem_cache *amd_iommu_irq_cache;
 #define APERTURE_PAGE_INDEX(a)	(((a) >> 21) & 0x3fULL)
 
 
+
 /*
  * This struct is used to pass information about
  * incoming PPR faults around.
@@ -401,6 +402,8 @@ struct amd_iommu_fault {
 struct iommu_domain;
 struct irq_domain;
 
+#define AMD_IOMMU_FLAG_TRANS_PRE_ENABLED      (1 << 0)
+
 /*
  * This structure contains generic data for  IOMMU protection domains
  * independent of their use.
@@ -525,6 +528,7 @@ struct amd_iommu {
 	struct irq_domain *ir_domain;
 	struct irq_domain *msi_domain;
 #endif
+	u32 flags;
 };
 
 struct devid_map {
-- 
2.5.5

[toc] | [next] | [standalone]

#1408466

-------- Original Message --------
> Add functions to check whether translation is already enabled in IOMMU.
>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>  drivers/iommu/amd_iommu_init.c  | 25 +++++++++++++++++++++++++
>  drivers/iommu/amd_iommu_types.h |  4 ++++
>  2 files changed, 29 insertions(+)
>
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index 8361367d..9e1dfcb 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -229,6 +229,26 @@ static int amd_iommu_enable_interrupts(void);
>  static int __init iommu_go_to_state(enum iommu_init_state state);
>  static void init_device_table_dma(void);
>
> +
> +static bool translation_pre_enabled(struct amd_iommu *iommu)
> +{
> +	return (iommu->flags & AMD_IOMMU_FLAG_TRANS_PRE_ENABLED);
> +}
> +
> +static void clear_translation_pre_enabled(struct amd_iommu *iommu)
> +{
> +        iommu->flags &= ~AMD_IOMMU_FLAG_TRANS_PRE_ENABLED;
> +}
> +
> +static void init_translation_status(struct amd_iommu *iommu)
> +{
> +	u32 ctrl;
> +
> +	ctrl = readl(iommu->mmio_base + MMIO_CONTROL_OFFSET);
> +	if (ctrl & (1<<CONTROL_IOMMU_EN))
> +		iommu->flags |= AMD_IOMMU_FLAG_TRANS_PRE_ENABLED;
> +}
> +
>  static int iommu_pc_get_set_reg_val(struct amd_iommu *iommu,
>  				    u8 bank, u8 cntr, u8 fxn,
>  				    u64 *value, bool is_write);
> @@ -1101,6 +1121,11 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
>
>  	iommu->int_enabled = false;
>
> +	init_translation_status(iommu);
> +
> +	if (translation_pre_enabled())
> +		pr_warn("Translation is already enabled - trying to copy translation structures\n");
> +

You missed this 'iommu' parameter here, even I saw you fixed it in 
another patch, but please keep each patch to be meaningful.

>  	ret = init_iommu_from_acpi(iommu, h);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
> index 9d32b20..01783cc 100644
> --- a/drivers/iommu/amd_iommu_types.h
> +++ b/drivers/iommu/amd_iommu_types.h
> @@ -384,6 +384,7 @@ extern struct kmem_cache *amd_iommu_irq_cache;
>  #define APERTURE_PAGE_INDEX(a)	(((a) >> 21) & 0x3fULL)
>
>
> +
>  /*
>   * This struct is used to pass information about
>   * incoming PPR faults around.
> @@ -401,6 +402,8 @@ struct amd_iommu_fault {
>  struct iommu_domain;
>  struct irq_domain;
>
> +#define AMD_IOMMU_FLAG_TRANS_PRE_ENABLED      (1 << 0)
> +
>  /*
>   * This structure contains generic data for  IOMMU protection domains
>   * independent of their use.
> @@ -525,6 +528,7 @@ struct amd_iommu {
>  	struct irq_domain *ir_domain;
>  	struct irq_domain *msi_domain;
>  #endif
> +	u32 flags;
>  };
>
>  struct devid_map {
>

[toc] | [prev] | [next] | [standalone]

#1408736

On 05/28/16 at 08:49pm, Wan Zongshun wrote:
> 
> 
> -------- Original Message --------
> >@@ -1101,6 +1121,11 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
> >
> > 	iommu->int_enabled = false;
> >
> >+	init_translation_status(iommu);
> >+
> >+	if (translation_pre_enabled())
> >+		pr_warn("Translation is already enabled - trying to copy translation structures\n");
> >+
> 
> You missed this 'iommu' parameter here, even I saw you fixed it in
> another patch, but please keep each patch to be meaningful.

Hi Zongshun,

Thanks for reviewing this patchset and great comments, will remember and
update with change.

Yes, translation_pre_enabled() in this patchset need a parameter "struct
amd_iommu*". 

In fact I am still debugging and trying to figure out what need be done
further to stop the IO_PAGE_FAULT happened on ethernet network card. I
kept changing code and adjust the patches. Up to now seems I still
didn't figure out why. There must be something I didn't notice and
everything will be fine as soon as I close that valve. With my
understand pci bug scanning will detect each pci device and do the
initialization job like setting configuration space registers and
control registers. After that we can safely re-init the pci device and
re-install the new io-page tables, and this is how Joerg has done for
vt-d fix for kdump if my understanding is correct. And I tried to do
like that in this patchset, don't know why it doesn't work.

As you know in previous post I thought the final initialization of
device should be done when its related driver probe and do the mapping
job, I tried re-install io-page tables at this time. Seems it didn't
work too. So I left that way.

Sorry for this rough post, will pay attention and make a formal post if
there's new update.

Thanks
Baoquan
> 
> > 	ret = init_iommu_from_acpi(iommu, h);
> > 	if (ret)
> > 		return ret;
> >diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
> >index 9d32b20..01783cc 100644
> >--- a/drivers/iommu/amd_iommu_types.h
> >+++ b/drivers/iommu/amd_iommu_types.h
> >@@ -384,6 +384,7 @@ extern struct kmem_cache *amd_iommu_irq_cache;
> > #define APERTURE_PAGE_INDEX(a)	(((a) >> 21) & 0x3fULL)
> >
> >
> >+
> > /*
> >  * This struct is used to pass information about
> >  * incoming PPR faults around.
> >@@ -401,6 +402,8 @@ struct amd_iommu_fault {
> > struct iommu_domain;
> > struct irq_domain;
> >
> >+#define AMD_IOMMU_FLAG_TRANS_PRE_ENABLED      (1 << 0)
> >+
> > /*
> >  * This structure contains generic data for  IOMMU protection domains
> >  * independent of their use.
> >@@ -525,6 +528,7 @@ struct amd_iommu {
> > 	struct irq_domain *ir_domain;
> > 	struct irq_domain *msi_domain;
> > #endif
> >+	u32 flags;
> > };
> >
> > struct devid_map {
> >

[toc] | [prev] | [next] | [standalone]

#1408741

On 05/30/16 at 11:24am, Baoquan He wrote:
> On 05/28/16 at 08:49pm, Wan Zongshun wrote:
> In fact I am still debugging and trying to figure out what need be done
> further to stop the IO_PAGE_FAULT happened on ethernet network card. I
> kept changing code and adjust the patches. Up to now seems I still
> didn't figure out why. There must be something I didn't notice and
> everything will be fine as soon as I close that valve. With my
> understand pci bug scanning will detect each pci device and do the
 		 |_ bus
> initialization job like setting configuration space registers and
> control registers. After that we can safely re-init the pci device and
> re-install the new io-page tables, and this is how Joerg has done for
> vt-d fix for kdump if my understanding is correct. And I tried to do
> like that in this patchset, don't know why it doesn't work.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web