[v6] VT-d: vfio enablement and misc enhances

[Qemu-devel] [PATCH v6 09/18] intel_iommu: vtd_slpt_level_shift check level

Posted by Peter Xu 9 years ago

This helps in debugging incorrect level passed in.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/i386/intel_iommu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index c672621..d74aa27 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -168,6 +168,7 @@ static gboolean vtd_hash_remove_by_domain(gpointer key, gpointer value,
 /* The shift of an addr for a certain level of paging structure */
 static inline uint32_t vtd_slpt_level_shift(uint32_t level)
 {
+    assert(level != 0);
     return VTD_PAGE_SHIFT_4K + (level - 1) * VTD_SL_LEVEL_BITS;
 }
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH v6 09/18] intel_iommu: vtd_slpt_level_shift check level

Posted by Jason Wang 9 years ago


On 2017年02月03日 16:22, Peter Xu wrote:
> This helps in debugging incorrect level passed in.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   hw/i386/intel_iommu.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index c672621..d74aa27 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -168,6 +168,7 @@ static gboolean vtd_hash_remove_by_domain(gpointer key, gpointer value,
>   /* The shift of an addr for a certain level of paging structure */
>   static inline uint32_t vtd_slpt_level_shift(uint32_t level)
>   {
> +    assert(level != 0);
>       return VTD_PAGE_SHIFT_4K + (level - 1) * VTD_SL_LEVEL_BITS;
>   }
>   

No harm but not sure we really need this.

Reviewed-by: Jason Wang <jasowang@redhat.com>

Re: [Qemu-devel] [PATCH v6 09/18] intel_iommu: vtd_slpt_level_shift check level

Posted by Peter Xu 9 years ago

On Tue, Feb 07, 2017 at 01:42:55PM +0800, Jason Wang wrote:
> 
> 
> On 2017年02月03日 16:22, Peter Xu wrote:
> >This helps in debugging incorrect level passed in.
> >
> >Signed-off-by: Peter Xu <peterx@redhat.com>
> >---
> >  hw/i386/intel_iommu.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> >diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> >index c672621..d74aa27 100644
> >--- a/hw/i386/intel_iommu.c
> >+++ b/hw/i386/intel_iommu.c
> >@@ -168,6 +168,7 @@ static gboolean vtd_hash_remove_by_domain(gpointer key, gpointer value,
> >  /* The shift of an addr for a certain level of paging structure */
> >  static inline uint32_t vtd_slpt_level_shift(uint32_t level)
> >  {
> >+    assert(level != 0);
> >      return VTD_PAGE_SHIFT_4K + (level - 1) * VTD_SL_LEVEL_BITS;
> >  }
> 
> No harm but not sure we really need this.
> 
> Reviewed-by: Jason Wang <jasowang@redhat.com>

Yes this is trivial. Michael, please either keep/drop it as you like
when merge. I'll just let it be there. Thanks,

-- peterx