Re: [Xen-devel] [PATCH for-4.12] amd/iommu: fix present bit checking when clearing PTE

2019-01-24 Thread Woods, Brian
On 1/23/19 3:47 AM, Roger Pau Monne wrote:
> The current check for the present bit is wrong, since the present bit
> is located in the low part of the entry.
> 
> Fixes: e8afe1124cc1 ("iommu: elide flushing for higher order map/unmap 
> operations")
> Signed-off-by: Roger Pau Monné 

Reviewed-by: Brian Woods 

> ---
> Cc: Suravee Suthikulpanit 
> Cc: Brian Woods 
> Cc: Juergen Gross 
> Cc: Paul Durrant 
> ---
>   xen/drivers/passthrough/amd/iommu_map.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/amd/iommu_map.c 
> b/xen/drivers/passthrough/amd/iommu_map.c
> index 99ac0a6862..67329b0c95 100644
> --- a/xen/drivers/passthrough/amd/iommu_map.c
> +++ b/xen/drivers/passthrough/amd/iommu_map.c
> @@ -39,15 +39,13 @@ static unsigned int clear_iommu_pte_present(unsigned long 
> l1_mfn,
>   unsigned long dfn)
>   {
>   uint64_t *table, *pte;
> -uint32_t entry;
>   unsigned int flush_flags;
>   
>   table = map_domain_page(_mfn(l1_mfn));
>   
>   pte = (table + pfn_to_pde_idx(dfn, 1));
> -entry = *pte >> 32;
>   
> -flush_flags = get_field_from_reg_u32(entry, IOMMU_PTE_PRESENT_MASK,
> +flush_flags = get_field_from_reg_u32(*pte, IOMMU_PTE_PRESENT_MASK,
>IOMMU_PTE_PRESENT_SHIFT) ?
>IOMMU_FLUSHF_modified : 0;
>   
> 

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

Re: [Xen-devel] [PATCH for-4.12] amd/iommu: fix present bit checking when clearing PTE

2019-01-23 Thread Juergen Gross
On 23/01/2019 10:47, Roger Pau Monne wrote:
> The current check for the present bit is wrong, since the present bit
> is located in the low part of the entry.
> 
> Fixes: e8afe1124cc1 ("iommu: elide flushing for higher order map/unmap 
> operations")
> Signed-off-by: Roger Pau Monné 

Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH for-4.12] amd/iommu: fix present bit checking when clearing PTE

2019-01-23 Thread Paul Durrant
> -Original Message-
> From: Roger Pau Monne [mailto:roger@citrix.com]
> Sent: 23 January 2019 09:48
> To: xen-devel@lists.xenproject.org
> Cc: Roger Pau Monne ; Suravee Suthikulpanit
> ; Brian Woods ;
> Juergen Gross ; Paul Durrant 
> Subject: [PATCH for-4.12] amd/iommu: fix present bit checking when
> clearing PTE
> 
> The current check for the present bit is wrong, since the present bit
> is located in the low part of the entry.
> 
> Fixes: e8afe1124cc1 ("iommu: elide flushing for higher order map/unmap
> operations")
> Signed-off-by: Roger Pau Monné 

No idea how I managed to get that wrong. I had a bad feeling at the time about 
not accessing the pte as a uint32_t[2] array.

Reviewed-by: Paul Durrant 

> ---
> Cc: Suravee Suthikulpanit 
> Cc: Brian Woods 
> Cc: Juergen Gross 
> Cc: Paul Durrant 
> ---
>  xen/drivers/passthrough/amd/iommu_map.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/amd/iommu_map.c
> b/xen/drivers/passthrough/amd/iommu_map.c
> index 99ac0a6862..67329b0c95 100644
> --- a/xen/drivers/passthrough/amd/iommu_map.c
> +++ b/xen/drivers/passthrough/amd/iommu_map.c
> @@ -39,15 +39,13 @@ static unsigned int clear_iommu_pte_present(unsigned
> long l1_mfn,
>  unsigned long dfn)
>  {
>  uint64_t *table, *pte;
> -uint32_t entry;
>  unsigned int flush_flags;
> 
>  table = map_domain_page(_mfn(l1_mfn));
> 
>  pte = (table + pfn_to_pde_idx(dfn, 1));
> -entry = *pte >> 32;
> 
> -flush_flags = get_field_from_reg_u32(entry, IOMMU_PTE_PRESENT_MASK,
> +flush_flags = get_field_from_reg_u32(*pte, IOMMU_PTE_PRESENT_MASK,
>   IOMMU_PTE_PRESENT_SHIFT) ?
>   IOMMU_FLUSHF_modified : 0;
> 
> --
> 2.20.1

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

[Xen-devel] [PATCH for-4.12] amd/iommu: fix present bit checking when clearing PTE

2019-01-23 Thread Roger Pau Monne
The current check for the present bit is wrong, since the present bit
is located in the low part of the entry.

Fixes: e8afe1124cc1 ("iommu: elide flushing for higher order map/unmap 
operations")
Signed-off-by: Roger Pau Monné 
---
Cc: Suravee Suthikulpanit 
Cc: Brian Woods 
Cc: Juergen Gross 
Cc: Paul Durrant 
---
 xen/drivers/passthrough/amd/iommu_map.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu_map.c 
b/xen/drivers/passthrough/amd/iommu_map.c
index 99ac0a6862..67329b0c95 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -39,15 +39,13 @@ static unsigned int clear_iommu_pte_present(unsigned long 
l1_mfn,
 unsigned long dfn)
 {
 uint64_t *table, *pte;
-uint32_t entry;
 unsigned int flush_flags;
 
 table = map_domain_page(_mfn(l1_mfn));
 
 pte = (table + pfn_to_pde_idx(dfn, 1));
-entry = *pte >> 32;
 
-flush_flags = get_field_from_reg_u32(entry, IOMMU_PTE_PRESENT_MASK,
+flush_flags = get_field_from_reg_u32(*pte, IOMMU_PTE_PRESENT_MASK,
  IOMMU_PTE_PRESENT_SHIFT) ?
  IOMMU_FLUSHF_modified : 0;
 
-- 
2.20.1


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