[PATCH] xen: passthrough/amd: Remove unused function guest_iommu_set_base

[PATCH] xen: passthrough/amd: Remove unused function guest_iommu_set_base

[Julien Grall]

The function is unused and could potentially lead a to trigger the
BUG_ON() in p2m_change_type_one if misused as the p2m type is not
sanitized.

So remove it.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/drivers/passthrough/amd/iommu_guest.c     | 23 -----------------------
 xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |  1 -
 2 files changed, 24 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu_guest.c b/xen/drivers/passthrough/amd/iommu_guest.c
index 96175bb9ac..dbb7526025 100644
--- a/xen/drivers/passthrough/amd/iommu_guest.c
+++ b/xen/drivers/passthrough/amd/iommu_guest.c
@@ -805,29 +805,6 @@ static int guest_iommu_mmio_write(struct vcpu *v, unsigned long addr,
     return X86EMUL_OKAY;
 }
 
-int guest_iommu_set_base(struct domain *d, uint64_t base)
-{
-    p2m_type_t t;
-    struct guest_iommu *iommu = domain_iommu(d);
-
-    if ( !iommu )
-        return -EACCES;
-
-    iommu->mmio_base = base;
-    base >>= PAGE_SHIFT;
-
-    for ( int i = 0; i < IOMMU_MMIO_PAGE_NR; i++ )
-    {
-        unsigned long gfn = base + i;
-
-        get_gfn_query(d, gfn, &t);
-        p2m_change_type_one(d, gfn, t, p2m_mmio_dm);
-        put_gfn(d, gfn);
-    }
-
-    return 0;
-}
-
 /* Initialize mmio read only bits */
 static void guest_iommu_reg_init(struct guest_iommu *iommu)
 {
diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
index c5697565d6..0129ffe5a9 100644
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
@@ -142,7 +142,6 @@ void guest_iommu_add_ppr_log(struct domain *d, u32 entry[]);
 void guest_iommu_add_event_log(struct domain *d, u32 entry[]);
 int guest_iommu_init(struct domain* d);
 void guest_iommu_destroy(struct domain *d);
-int guest_iommu_set_base(struct domain *d, uint64_t base);
 
 static inline u32 get_field_from_reg_u32(u32 reg_value, u32 mask, u32 shift)
 {
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] xen: passthrough/amd: Remove unused function guest_iommu_set_base

[Jan Beulich]

>>> Julien Grall <julien.grall@arm.com> 03/20/19 12:21 AM >>>
>The function is unused and could potentially lead a to trigger the
>BUG_ON() in p2m_change_type_one if misused as the p2m type is not
>sanitized.
>
>So remove it.

I don't think I agree - the entire file is effectively unused, and hence removal should
perhaps be an all-or-nothing action. But let's see what the maintainers say.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] xen: passthrough/amd: Remove unused function guest_iommu_set_base

[Andrew Cooper]

On 19/03/2019 23:20, Julien Grall wrote:
> The function is unused and could potentially lead a to trigger the
> BUG_ON() in p2m_change_type_one if misused as the p2m type is not
> sanitized.
>
> So remove it.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Either the entire file goes, or this function stays.  It definitely
isn't production ready yet, as noted in my post-commit notes in
https://lists.xenproject.org/archives/html/xen-devel/2018-11/msg02356.html

However, this functionality is going to be necessary in the future.  It
is probably easier just to leave it as-is.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] xen: passthrough/amd: Remove unused function guest_iommu_set_base

[Julien Grall]

Hi Andrew,

On 3/20/19 11:13 AM, Andrew Cooper wrote:
> On 19/03/2019 23:20, Julien Grall wrote:
>> The function is unused and could potentially lead a to trigger the
>> BUG_ON() in p2m_change_type_one if misused as the p2m type is not
>> sanitized.
>>
>> So remove it.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> Either the entire file goes, or this function stays.  It definitely
> isn't production ready yet, as noted in my post-commit notes in
> https://lists.xenproject.org/archives/html/xen-devel/2018-11/msg02356.html

I should have probably looked at the commit logs first :). I found it 
while looking whether we can re-use p2m_change_type_one on Arm too.

> 
> However, this functionality is going to be necessary in the future.  It
> is probably easier just to leave it as-is.

I will drop the patch then.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] xen: passthrough/amd: Remove unused function guest_iommu_set_base

[Julien Grall]

Hi,

On 3/20/19 7:05 AM, Jan Beulich wrote:
>>>> Julien Grall <julien.grall@arm.com> 03/20/19 12:21 AM >>>
>> The function is unused and could potentially lead a to trigger the
>> BUG_ON() in p2m_change_type_one if misused as the p2m type is not
>> sanitized.
>>
>> So remove it.
> 
> I don't think I agree - the entire file is effectively unused, and hence removal should
> perhaps be an all-or-nothing action. But let's see what the maintainers say.

Based on Andrew comments, I will dropped the patch for now.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] xen: passthrough/amd: Remove unused function guest_iommu_set_base

[Julien Grall]

Hi,

Hmmm, I mistakenly resent this patch. Please ignore this version and 
review <20190319232055.23993-1-julien.grall@arm.com>

Sorry for the noise.

Cheers,

On 3/19/19 11:21 PM, Julien Grall wrote:
> The function is unused and could potentially lead a to trigger the
> BUG_ON() in p2m_change_type_one if misused as the p2m type is not
> sanitized.
> 
> So remove it.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>   xen/drivers/passthrough/amd/iommu_guest.c     | 23 -----------------------
>   xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |  1 -
>   2 files changed, 24 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/amd/iommu_guest.c b/xen/drivers/passthrough/amd/iommu_guest.c
> index 96175bb9ac..dbb7526025 100644
> --- a/xen/drivers/passthrough/amd/iommu_guest.c
> +++ b/xen/drivers/passthrough/amd/iommu_guest.c
> @@ -805,29 +805,6 @@ static int guest_iommu_mmio_write(struct vcpu *v, unsigned long addr,
>       return X86EMUL_OKAY;
>   }
>   
> -int guest_iommu_set_base(struct domain *d, uint64_t base)
> -{
> -    p2m_type_t t;
> -    struct guest_iommu *iommu = domain_iommu(d);
> -
> -    if ( !iommu )
> -        return -EACCES;
> -
> -    iommu->mmio_base = base;
> -    base >>= PAGE_SHIFT;
> -
> -    for ( int i = 0; i < IOMMU_MMIO_PAGE_NR; i++ )
> -    {
> -        unsigned long gfn = base + i;
> -
> -        get_gfn_query(d, gfn, &t);
> -        p2m_change_type_one(d, gfn, t, p2m_mmio_dm);
> -        put_gfn(d, gfn);
> -    }
> -
> -    return 0;
> -}
> -
>   /* Initialize mmio read only bits */
>   static void guest_iommu_reg_init(struct guest_iommu *iommu)
>   {
> diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> index c5697565d6..0129ffe5a9 100644
> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> @@ -142,7 +142,6 @@ void guest_iommu_add_ppr_log(struct domain *d, u32 entry[]);
>   void guest_iommu_add_event_log(struct domain *d, u32 entry[]);
>   int guest_iommu_init(struct domain* d);
>   void guest_iommu_destroy(struct domain *d);
> -int guest_iommu_set_base(struct domain *d, uint64_t base);
>   
>   static inline u32 get_field_from_reg_u32(u32 reg_value, u32 mask, u32 shift)
>   {
> 

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH] xen: passthrough/amd: Remove unused function guest_iommu_set_base

[Julien Grall]

The function is unused and could potentially lead a to trigger the
BUG_ON() in p2m_change_type_one if misused as the p2m type is not
sanitized.

So remove it.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/drivers/passthrough/amd/iommu_guest.c     | 23 -----------------------
 xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |  1 -
 2 files changed, 24 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu_guest.c b/xen/drivers/passthrough/amd/iommu_guest.c
index 96175bb9ac..dbb7526025 100644
--- a/xen/drivers/passthrough/amd/iommu_guest.c
+++ b/xen/drivers/passthrough/amd/iommu_guest.c
@@ -805,29 +805,6 @@ static int guest_iommu_mmio_write(struct vcpu *v, unsigned long addr,
     return X86EMUL_OKAY;
 }
 
-int guest_iommu_set_base(struct domain *d, uint64_t base)
-{
-    p2m_type_t t;
-    struct guest_iommu *iommu = domain_iommu(d);
-
-    if ( !iommu )
-        return -EACCES;
-
-    iommu->mmio_base = base;
-    base >>= PAGE_SHIFT;
-
-    for ( int i = 0; i < IOMMU_MMIO_PAGE_NR; i++ )
-    {
-        unsigned long gfn = base + i;
-
-        get_gfn_query(d, gfn, &t);
-        p2m_change_type_one(d, gfn, t, p2m_mmio_dm);
-        put_gfn(d, gfn);
-    }
-
-    return 0;
-}
-
 /* Initialize mmio read only bits */
 static void guest_iommu_reg_init(struct guest_iommu *iommu)
 {
diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
index c5697565d6..0129ffe5a9 100644
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
@@ -142,7 +142,6 @@ void guest_iommu_add_ppr_log(struct domain *d, u32 entry[]);
 void guest_iommu_add_event_log(struct domain *d, u32 entry[]);
 int guest_iommu_init(struct domain* d);
 void guest_iommu_destroy(struct domain *d);
-int guest_iommu_set_base(struct domain *d, uint64_t base);
 
 static inline u32 get_field_from_reg_u32(u32 reg_value, u32 mask, u32 shift)
 {
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel