diff mbox

[v3,09/12] intel_iommu: allow dev-iotlb context entry conditionally

Message ID 1494403315-12760-10-git-send-email-peterx@redhat.com
State New
Headers show

Commit Message

Peter Xu May 10, 2017, 8:01 a.m. UTC
When device-iotlb is not specified, we should fail this check.

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

Comments

Jason Wang May 11, 2017, 2:56 a.m. UTC | #1
On 2017年05月10日 16:01, Peter Xu wrote:
> When device-iotlb is not specified, we should fail this check.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   hw/i386/intel_iommu.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 05ae631..1a7eba2 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -836,6 +836,8 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num,
>   {
>       VTDRootEntry re;
>       int ret_fr;
> +    bool type_fail = false;
> +    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
>   
>       ret_fr = vtd_get_root_entry(s, bus_num, &re);
>       if (ret_fr) {
> @@ -872,10 +874,19 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num,
>       } else {
>           switch (vtd_ce_get_type(ce)) {
>           case VTD_CONTEXT_TT_MULTI_LEVEL:
> -            /* fall through */
> +            /* Always supported */
> +            break;
>           case VTD_CONTEXT_TT_DEV_IOTLB:
> +            if (!x86_iommu->dt_supported) {
> +                type_fail = true;
> +            }
>               break;
>           default:
> +            /* Unknwon type */
> +            type_fail = true;
> +            break;
> +        }
> +        if (type_fail) {
>               trace_vtd_ce_invalid(ce->hi, ce->lo);
>               return -VTD_FR_CONTEXT_ENTRY_INV;
>           }

How about e.g exclude the type if not supported in vtd_ce_get_type()? 
This looks better than using something like type_fail.

Thanks
Peter Xu May 11, 2017, 3:51 a.m. UTC | #2
On Thu, May 11, 2017 at 10:56:42AM +0800, Jason Wang wrote:
> 
> 
> On 2017年05月10日 16:01, Peter Xu wrote:
> >When device-iotlb is not specified, we should fail this check.
> >
> >Signed-off-by: Peter Xu <peterx@redhat.com>
> >---
> >  hw/i386/intel_iommu.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> >diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> >index 05ae631..1a7eba2 100644
> >--- a/hw/i386/intel_iommu.c
> >+++ b/hw/i386/intel_iommu.c
> >@@ -836,6 +836,8 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num,
> >  {
> >      VTDRootEntry re;
> >      int ret_fr;
> >+    bool type_fail = false;
> >+    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
> >      ret_fr = vtd_get_root_entry(s, bus_num, &re);
> >      if (ret_fr) {
> >@@ -872,10 +874,19 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num,
> >      } else {
> >          switch (vtd_ce_get_type(ce)) {
> >          case VTD_CONTEXT_TT_MULTI_LEVEL:
> >-            /* fall through */
> >+            /* Always supported */
> >+            break;
> >          case VTD_CONTEXT_TT_DEV_IOTLB:
> >+            if (!x86_iommu->dt_supported) {
> >+                type_fail = true;
> >+            }
> >              break;
> >          default:
> >+            /* Unknwon type */
> >+            type_fail = true;
> >+            break;
> >+        }
> >+        if (type_fail) {
> >              trace_vtd_ce_invalid(ce->hi, ce->lo);
> >              return -VTD_FR_CONTEXT_ENTRY_INV;
> >          }
> 
> How about e.g exclude the type if not supported in vtd_ce_get_type()? This
> looks better than using something like type_fail.

(after a quick discussion with Jason offlist)

I'll keep current vtd_ce_get_type() since there are other places that
used it, and introduce another vtd_ce_type_check() to make codes more
elegant (and get rid of type_fail variable). Thanks.
diff mbox

Patch

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 05ae631..1a7eba2 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -836,6 +836,8 @@  static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num,
 {
     VTDRootEntry re;
     int ret_fr;
+    bool type_fail = false;
+    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
 
     ret_fr = vtd_get_root_entry(s, bus_num, &re);
     if (ret_fr) {
@@ -872,10 +874,19 @@  static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num,
     } else {
         switch (vtd_ce_get_type(ce)) {
         case VTD_CONTEXT_TT_MULTI_LEVEL:
-            /* fall through */
+            /* Always supported */
+            break;
         case VTD_CONTEXT_TT_DEV_IOTLB:
+            if (!x86_iommu->dt_supported) {
+                type_fail = true;
+            }
             break;
         default:
+            /* Unknwon type */
+            type_fail = true;
+            break;
+        }
+        if (type_fail) {
             trace_vtd_ce_invalid(ce->hi, ce->lo);
             return -VTD_FR_CONTEXT_ENTRY_INV;
         }