[PATCH v2] hw/i386/intel_iommu: Block CFI when necessary

Yuke Peng posted 1 patch 4 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240708100816.1916346-1-pykfirst@gmail.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>
hw/i386/intel_iommu.c         | 53 +++++++++++++++++++++++++++++++++--
hw/i386/trace-events          |  1 +
include/hw/i386/intel_iommu.h |  1 +
3 files changed, 53 insertions(+), 2 deletions(-)
[PATCH v2] hw/i386/intel_iommu: Block CFI when necessary
Posted by Yuke Peng 4 months, 2 weeks ago
According to Intel VT-d specification 5.1.4, CFI must be blocked when
Extended Interrupt Mode is enabled or Compatibility format interrupts
are disabled.

Signed-off-by: Yuke Peng <pykfirst@gmail.com>
---
Changes in v2:
- Use subsections for the cfi_enabled field.
- Link to v1: https://lore.kernel.org/qemu-devel/20240625112819.862282-1-pykfirst@gmail.com/

---
 hw/i386/intel_iommu.c         | 53 +++++++++++++++++++++++++++++++++--
 hw/i386/trace-events          |  1 +
 include/hw/i386/intel_iommu.h |  1 +
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 5085a6fee3..af9c864bde 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2410,6 +2410,22 @@ static void vtd_handle_gcmd_ire(IntelIOMMUState *s, bool en)
     }
 }
 
+/* Handle Compatibility Format Interrupts Enable/Disable */
+static void vtd_handle_gcmd_cfi(IntelIOMMUState *s, bool en)
+{
+    trace_vtd_cfi_enable(en);
+
+    if (en) {
+        s->cfi_enabled = true;
+        /* Ok - report back to driver */
+        vtd_set_clear_mask_long(s, DMAR_GSTS_REG, 0, VTD_GSTS_CFIS);
+    } else {
+        s->cfi_enabled = false;
+        /* Ok - report back to driver */
+        vtd_set_clear_mask_long(s, DMAR_GSTS_REG, VTD_GSTS_CFIS, 0);
+    }
+}
+
 /* Handle write to Global Command Register */
 static void vtd_handle_gcmd_write(IntelIOMMUState *s)
 {
@@ -2440,6 +2456,10 @@ static void vtd_handle_gcmd_write(IntelIOMMUState *s)
         /* Interrupt remap enable/disable */
         vtd_handle_gcmd_ire(s, val & VTD_GCMD_IRE);
     }
+    if (changed & VTD_GCMD_CFI) {
+        /* Compatibility format interrupts enable/disable */
+        vtd_handle_gcmd_cfi(s, val & VTD_GCMD_CFI);
+    }
 }
 
 /* Handle write to Context Command Register */
@@ -3283,7 +3303,25 @@ static int vtd_post_load(void *opaque, int version_id)
     return 0;
 }
 
-static const VMStateDescription vtd_vmstate = {
+static bool vtd_cfi_needed(void *opaque)
+{
+    IntelIOMMUState *iommu = opaque;
+
+    return iommu->intr_enabled && !iommu->intr_eime;
+}
+
+static const VMStateDescription vmstate_vtd_cfi = {
+    .name = "iommu-intel/cfi",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = vtd_cfi_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_BOOL(cfi_enabled, IntelIOMMUState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_vtd = {
     .name = "iommu-intel",
     .version_id = 1,
     .minimum_version_id = 1,
@@ -3306,6 +3344,10 @@ static const VMStateDescription vtd_vmstate = {
         VMSTATE_BOOL(intr_enabled, IntelIOMMUState),
         VMSTATE_BOOL(intr_eime, IntelIOMMUState),
         VMSTATE_END_OF_LIST()
+    },
+    .subsections = (const VMStateDescription * []) {
+        &vmstate_vtd_cfi,
+        NULL
     }
 };
 
@@ -3525,6 +3567,12 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState *iommu,
 
     /* This is compatible mode. */
     if (addr.addr.int_mode != VTD_IR_INT_FORMAT_REMAP) {
+        if (iommu->intr_eime || !iommu->cfi_enabled) {
+            if (do_fault) {
+                vtd_report_ir_fault(iommu, sid, VTD_FR_IR_REQ_COMPAT, 0);
+            }
+            return -EINVAL;
+        }
         memcpy(translated, origin, sizeof(*origin));
         goto out;
     }
@@ -3950,6 +3998,7 @@ static void vtd_init(IntelIOMMUState *s)
     s->root_scalable = false;
     s->dmar_enabled = false;
     s->intr_enabled = false;
+    s->cfi_enabled = false;
     s->iq_head = 0;
     s->iq_tail = 0;
     s->iq = 0;
@@ -4243,7 +4292,7 @@ static void vtd_class_init(ObjectClass *klass, void *data)
     X86IOMMUClass *x86_class = X86_IOMMU_DEVICE_CLASS(klass);
 
     dc->reset = vtd_reset;
-    dc->vmsd = &vtd_vmstate;
+    dc->vmsd = &vmstate_vtd;
     device_class_set_props(dc, vtd_properties);
     dc->hotpluggable = false;
     x86_class->realize = vtd_realize;
diff --git a/hw/i386/trace-events b/hw/i386/trace-events
index 53c02d7ac8..ffd87db65f 100644
--- a/hw/i386/trace-events
+++ b/hw/i386/trace-events
@@ -57,6 +57,7 @@ vtd_dmar_translate(uint8_t bus, uint8_t slot, uint8_t func, uint64_t iova, uint6
 vtd_dmar_enable(bool en) "enable %d"
 vtd_dmar_fault(uint16_t sid, int fault, uint64_t addr, bool is_write) "sid 0x%"PRIx16" fault %d addr 0x%"PRIx64" write %d"
 vtd_ir_enable(bool en) "enable %d"
+vtd_cfi_enable(bool en) "enable %d"
 vtd_ir_irte_get(int index, uint64_t lo, uint64_t hi) "index %d low 0x%"PRIx64" high 0x%"PRIx64
 vtd_ir_remap(int index, int tri, int vec, int deliver, uint32_t dest, int dest_mode) "index %d trigger %d vector %d deliver %d dest 0x%"PRIx32" mode %d"
 vtd_ir_remap_type(const char *type) "%s"
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index 7fa0a695c8..38e20d0f2c 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -294,6 +294,7 @@ struct IntelIOMMUState {
 
     /* interrupt remapping */
     bool intr_enabled;              /* Whether guest enabled IR */
+    bool cfi_enabled;               /* Whether CFI is enabled */
     dma_addr_t intr_root;           /* Interrupt remapping table pointer */
     uint32_t intr_size;             /* Number of IR table entries */
     bool intr_eime;                 /* Extended interrupt mode enabled */
-- 
2.34.1
Re: [PATCH v2] hw/i386/intel_iommu: Block CFI when necessary
Posted by Michael S. Tsirkin 2 months, 2 weeks ago
On Mon, Jul 08, 2024 at 06:08:16PM +0800, Yuke Peng wrote:
> According to Intel VT-d specification 5.1.4, CFI must be blocked when
> Extended Interrupt Mode is enabled or Compatibility format interrupts
> are disabled.
> 
> Signed-off-by: Yuke Peng <pykfirst@gmail.com>

The rename is fine.
The issue with the patch is the extra section.
We need to avoid adding it for compat machine types.

Do you have the time to address this?

> ---
> Changes in v2:
> - Use subsections for the cfi_enabled field.
> - Link to v1: https://lore.kernel.org/qemu-devel/20240625112819.862282-1-pykfirst@gmail.com/
> 
> ---
>  hw/i386/intel_iommu.c         | 53 +++++++++++++++++++++++++++++++++--
>  hw/i386/trace-events          |  1 +
>  include/hw/i386/intel_iommu.h |  1 +
>  3 files changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 5085a6fee3..af9c864bde 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2410,6 +2410,22 @@ static void vtd_handle_gcmd_ire(IntelIOMMUState *s, bool en)
>      }
>  }
>  
> +/* Handle Compatibility Format Interrupts Enable/Disable */
> +static void vtd_handle_gcmd_cfi(IntelIOMMUState *s, bool en)
> +{
> +    trace_vtd_cfi_enable(en);
> +
> +    if (en) {
> +        s->cfi_enabled = true;
> +        /* Ok - report back to driver */
> +        vtd_set_clear_mask_long(s, DMAR_GSTS_REG, 0, VTD_GSTS_CFIS);
> +    } else {
> +        s->cfi_enabled = false;
> +        /* Ok - report back to driver */
> +        vtd_set_clear_mask_long(s, DMAR_GSTS_REG, VTD_GSTS_CFIS, 0);
> +    }
> +}
> +
>  /* Handle write to Global Command Register */
>  static void vtd_handle_gcmd_write(IntelIOMMUState *s)
>  {
> @@ -2440,6 +2456,10 @@ static void vtd_handle_gcmd_write(IntelIOMMUState *s)
>          /* Interrupt remap enable/disable */
>          vtd_handle_gcmd_ire(s, val & VTD_GCMD_IRE);
>      }
> +    if (changed & VTD_GCMD_CFI) {
> +        /* Compatibility format interrupts enable/disable */
> +        vtd_handle_gcmd_cfi(s, val & VTD_GCMD_CFI);
> +    }
>  }
>  
>  /* Handle write to Context Command Register */
> @@ -3283,7 +3303,25 @@ static int vtd_post_load(void *opaque, int version_id)
>      return 0;
>  }
>  
> -static const VMStateDescription vtd_vmstate = {
> +static bool vtd_cfi_needed(void *opaque)
> +{
> +    IntelIOMMUState *iommu = opaque;
> +
> +    return iommu->intr_enabled && !iommu->intr_eime;
> +}
> +
> +static const VMStateDescription vmstate_vtd_cfi = {
> +    .name = "iommu-intel/cfi",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = vtd_cfi_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_BOOL(cfi_enabled, IntelIOMMUState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static const VMStateDescription vmstate_vtd = {
>      .name = "iommu-intel",
>      .version_id = 1,
>      .minimum_version_id = 1,
> @@ -3306,6 +3344,10 @@ static const VMStateDescription vtd_vmstate = {
>          VMSTATE_BOOL(intr_enabled, IntelIOMMUState),
>          VMSTATE_BOOL(intr_eime, IntelIOMMUState),
>          VMSTATE_END_OF_LIST()
> +    },
> +    .subsections = (const VMStateDescription * []) {
> +        &vmstate_vtd_cfi,
> +        NULL
>      }
>  };
>  
> @@ -3525,6 +3567,12 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState *iommu,
>  
>      /* This is compatible mode. */
>      if (addr.addr.int_mode != VTD_IR_INT_FORMAT_REMAP) {
> +        if (iommu->intr_eime || !iommu->cfi_enabled) {
> +            if (do_fault) {
> +                vtd_report_ir_fault(iommu, sid, VTD_FR_IR_REQ_COMPAT, 0);
> +            }
> +            return -EINVAL;
> +        }
>          memcpy(translated, origin, sizeof(*origin));
>          goto out;
>      }
> @@ -3950,6 +3998,7 @@ static void vtd_init(IntelIOMMUState *s)
>      s->root_scalable = false;
>      s->dmar_enabled = false;
>      s->intr_enabled = false;
> +    s->cfi_enabled = false;
>      s->iq_head = 0;
>      s->iq_tail = 0;
>      s->iq = 0;
> @@ -4243,7 +4292,7 @@ static void vtd_class_init(ObjectClass *klass, void *data)
>      X86IOMMUClass *x86_class = X86_IOMMU_DEVICE_CLASS(klass);
>  
>      dc->reset = vtd_reset;
> -    dc->vmsd = &vtd_vmstate;
> +    dc->vmsd = &vmstate_vtd;
>      device_class_set_props(dc, vtd_properties);
>      dc->hotpluggable = false;
>      x86_class->realize = vtd_realize;
> diff --git a/hw/i386/trace-events b/hw/i386/trace-events
> index 53c02d7ac8..ffd87db65f 100644
> --- a/hw/i386/trace-events
> +++ b/hw/i386/trace-events
> @@ -57,6 +57,7 @@ vtd_dmar_translate(uint8_t bus, uint8_t slot, uint8_t func, uint64_t iova, uint6
>  vtd_dmar_enable(bool en) "enable %d"
>  vtd_dmar_fault(uint16_t sid, int fault, uint64_t addr, bool is_write) "sid 0x%"PRIx16" fault %d addr 0x%"PRIx64" write %d"
>  vtd_ir_enable(bool en) "enable %d"
> +vtd_cfi_enable(bool en) "enable %d"
>  vtd_ir_irte_get(int index, uint64_t lo, uint64_t hi) "index %d low 0x%"PRIx64" high 0x%"PRIx64
>  vtd_ir_remap(int index, int tri, int vec, int deliver, uint32_t dest, int dest_mode) "index %d trigger %d vector %d deliver %d dest 0x%"PRIx32" mode %d"
>  vtd_ir_remap_type(const char *type) "%s"
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index 7fa0a695c8..38e20d0f2c 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -294,6 +294,7 @@ struct IntelIOMMUState {
>  
>      /* interrupt remapping */
>      bool intr_enabled;              /* Whether guest enabled IR */
> +    bool cfi_enabled;               /* Whether CFI is enabled */
>      dma_addr_t intr_root;           /* Interrupt remapping table pointer */
>      uint32_t intr_size;             /* Number of IR table entries */
>      bool intr_eime;                 /* Extended interrupt mode enabled */
> -- 
> 2.34.1
Re: [PATCH v2] hw/i386/intel_iommu: Block CFI when necessary
Posted by CLEMENT MATHIEU--DRIF 4 months, 2 weeks ago
Hi

On 08/07/2024 12:08, Yuke Peng wrote:
> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>
>
> According to Intel VT-d specification 5.1.4, CFI must be blocked when
> Extended Interrupt Mode is enabled or Compatibility format interrupts
> are disabled.
>
> Signed-off-by: Yuke Peng <pykfirst@gmail.com>
> ---
> Changes in v2:
> - Use subsections for the cfi_enabled field.
> - Link to v1: https://lore.kernel.org/qemu-devel/20240625112819.862282-1-pykfirst@gmail.com/
>
> ---
>   hw/i386/intel_iommu.c         | 53 +++++++++++++++++++++++++++++++++--
>   hw/i386/trace-events          |  1 +
>   include/hw/i386/intel_iommu.h |  1 +
>   3 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 5085a6fee3..af9c864bde 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2410,6 +2410,22 @@ static void vtd_handle_gcmd_ire(IntelIOMMUState *s, bool en)
>       }
>   }
>
> +/* Handle Compatibility Format Interrupts Enable/Disable */
> +static void vtd_handle_gcmd_cfi(IntelIOMMUState *s, bool en)
> +{
> +    trace_vtd_cfi_enable(en);
> +
> +    if (en) {
> +        s->cfi_enabled = true;
> +        /* Ok - report back to driver */
> +        vtd_set_clear_mask_long(s, DMAR_GSTS_REG, 0, VTD_GSTS_CFIS);
> +    } else {
> +        s->cfi_enabled = false;
> +        /* Ok - report back to driver */
> +        vtd_set_clear_mask_long(s, DMAR_GSTS_REG, VTD_GSTS_CFIS, 0);
> +    }
> +}
> +
>   /* Handle write to Global Command Register */
>   static void vtd_handle_gcmd_write(IntelIOMMUState *s)
>   {
> @@ -2440,6 +2456,10 @@ static void vtd_handle_gcmd_write(IntelIOMMUState *s)
>           /* Interrupt remap enable/disable */
>           vtd_handle_gcmd_ire(s, val & VTD_GCMD_IRE);
>       }
> +    if (changed & VTD_GCMD_CFI) {
> +        /* Compatibility format interrupts enable/disable */
> +        vtd_handle_gcmd_cfi(s, val & VTD_GCMD_CFI);
> +    }
>   }
>
>   /* Handle write to Context Command Register */
> @@ -3283,7 +3303,25 @@ static int vtd_post_load(void *opaque, int version_id)
>       return 0;
>   }
>
> -static const VMStateDescription vtd_vmstate = {
> +static bool vtd_cfi_needed(void *opaque)
> +{
> +    IntelIOMMUState *iommu = opaque;
> +
> +    return iommu->intr_enabled && !iommu->intr_eime;
> +}
> +
> +static const VMStateDescription vmstate_vtd_cfi = {
> +    .name = "iommu-intel/cfi",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = vtd_cfi_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_BOOL(cfi_enabled, IntelIOMMUState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static const VMStateDescription vmstate_vtd = {
Why is vtd_vmstate renamed to vmstate_vtd? is it necessary?
>       .name = "iommu-intel",
>       .version_id = 1,
>       .minimum_version_id = 1,
> @@ -3306,6 +3344,10 @@ static const VMStateDescription vtd_vmstate = {
>           VMSTATE_BOOL(intr_enabled, IntelIOMMUState),
>           VMSTATE_BOOL(intr_eime, IntelIOMMUState),
>           VMSTATE_END_OF_LIST()
> +    },
> +    .subsections = (const VMStateDescription * []) {
> +        &vmstate_vtd_cfi,
> +        NULL
>       }
>   };
>
> @@ -3525,6 +3567,12 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState *iommu,
>
>       /* This is compatible mode. */
>       if (addr.addr.int_mode != VTD_IR_INT_FORMAT_REMAP) {
> +        if (iommu->intr_eime || !iommu->cfi_enabled) {
> +            if (do_fault) {
> +                vtd_report_ir_fault(iommu, sid, VTD_FR_IR_REQ_COMPAT, 0);
> +            }
> +            return -EINVAL;
> +        }
>           memcpy(translated, origin, sizeof(*origin));
>           goto out;
>       }
> @@ -3950,6 +3998,7 @@ static void vtd_init(IntelIOMMUState *s)
>       s->root_scalable = false;
>       s->dmar_enabled = false;
>       s->intr_enabled = false;
> +    s->cfi_enabled = false;
>       s->iq_head = 0;
>       s->iq_tail = 0;
>       s->iq = 0;
> @@ -4243,7 +4292,7 @@ static void vtd_class_init(ObjectClass *klass, void *data)
>       X86IOMMUClass *x86_class = X86_IOMMU_DEVICE_CLASS(klass);
>
>       dc->reset = vtd_reset;
> -    dc->vmsd = &vtd_vmstate;
> +    dc->vmsd = &vmstate_vtd;
>       device_class_set_props(dc, vtd_properties);
>       dc->hotpluggable = false;
>       x86_class->realize = vtd_realize;
> diff --git a/hw/i386/trace-events b/hw/i386/trace-events
> index 53c02d7ac8..ffd87db65f 100644
> --- a/hw/i386/trace-events
> +++ b/hw/i386/trace-events
> @@ -57,6 +57,7 @@ vtd_dmar_translate(uint8_t bus, uint8_t slot, uint8_t func, uint64_t iova, uint6
>   vtd_dmar_enable(bool en) "enable %d"
>   vtd_dmar_fault(uint16_t sid, int fault, uint64_t addr, bool is_write) "sid 0x%"PRIx16" fault %d addr 0x%"PRIx64" write %d"
>   vtd_ir_enable(bool en) "enable %d"
> +vtd_cfi_enable(bool en) "enable %d"
>   vtd_ir_irte_get(int index, uint64_t lo, uint64_t hi) "index %d low 0x%"PRIx64" high 0x%"PRIx64
>   vtd_ir_remap(int index, int tri, int vec, int deliver, uint32_t dest, int dest_mode) "index %d trigger %d vector %d deliver %d dest 0x%"PRIx32" mode %d"
>   vtd_ir_remap_type(const char *type) "%s"
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index 7fa0a695c8..38e20d0f2c 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -294,6 +294,7 @@ struct IntelIOMMUState {
>
>       /* interrupt remapping */
>       bool intr_enabled;              /* Whether guest enabled IR */
> +    bool cfi_enabled;               /* Whether CFI is enabled */
>       dma_addr_t intr_root;           /* Interrupt remapping table pointer */
>       uint32_t intr_size;             /* Number of IR table entries */
>       bool intr_eime;                 /* Extended interrupt mode enabled */
> --
> 2.34.1
>
>
Re: [PATCH v2] hw/i386/intel_iommu: Block CFI when necessary
Posted by Michael S. Tsirkin 4 months ago
On Tue, Jul 09, 2024 at 04:43:25AM +0000, CLEMENT MATHIEU--DRIF wrote:
> Hi
> 
> On 08/07/2024 12:08, Yuke Peng wrote:
> > Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
> >
> >
> > According to Intel VT-d specification 5.1.4, CFI must be blocked when
> > Extended Interrupt Mode is enabled or Compatibility format interrupts
> > are disabled.
> >
> > Signed-off-by: Yuke Peng <pykfirst@gmail.com>
> > ---
> > Changes in v2:
> > - Use subsections for the cfi_enabled field.
> > - Link to v1: https://lore.kernel.org/qemu-devel/20240625112819.862282-1-pykfirst@gmail.com/
> >
> > ---
> >   hw/i386/intel_iommu.c         | 53 +++++++++++++++++++++++++++++++++--
> >   hw/i386/trace-events          |  1 +
> >   include/hw/i386/intel_iommu.h |  1 +
> >   3 files changed, 53 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index 5085a6fee3..af9c864bde 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -2410,6 +2410,22 @@ static void vtd_handle_gcmd_ire(IntelIOMMUState *s, bool en)
> >       }
> >   }
> >
> > +/* Handle Compatibility Format Interrupts Enable/Disable */
> > +static void vtd_handle_gcmd_cfi(IntelIOMMUState *s, bool en)
> > +{
> > +    trace_vtd_cfi_enable(en);
> > +
> > +    if (en) {
> > +        s->cfi_enabled = true;
> > +        /* Ok - report back to driver */
> > +        vtd_set_clear_mask_long(s, DMAR_GSTS_REG, 0, VTD_GSTS_CFIS);
> > +    } else {
> > +        s->cfi_enabled = false;
> > +        /* Ok - report back to driver */
> > +        vtd_set_clear_mask_long(s, DMAR_GSTS_REG, VTD_GSTS_CFIS, 0);
> > +    }
> > +}
> > +
> >   /* Handle write to Global Command Register */
> >   static void vtd_handle_gcmd_write(IntelIOMMUState *s)
> >   {
> > @@ -2440,6 +2456,10 @@ static void vtd_handle_gcmd_write(IntelIOMMUState *s)
> >           /* Interrupt remap enable/disable */
> >           vtd_handle_gcmd_ire(s, val & VTD_GCMD_IRE);
> >       }
> > +    if (changed & VTD_GCMD_CFI) {
> > +        /* Compatibility format interrupts enable/disable */
> > +        vtd_handle_gcmd_cfi(s, val & VTD_GCMD_CFI);
> > +    }
> >   }
> >
> >   /* Handle write to Context Command Register */
> > @@ -3283,7 +3303,25 @@ static int vtd_post_load(void *opaque, int version_id)
> >       return 0;
> >   }
> >
> > -static const VMStateDescription vtd_vmstate = {
> > +static bool vtd_cfi_needed(void *opaque)
> > +{
> > +    IntelIOMMUState *iommu = opaque;
> > +
> > +    return iommu->intr_enabled && !iommu->intr_eime;
> > +}
> > +
> > +static const VMStateDescription vmstate_vtd_cfi = {
> > +    .name = "iommu-intel/cfi",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .needed = vtd_cfi_needed,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_BOOL(cfi_enabled, IntelIOMMUState),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> > +static const VMStateDescription vmstate_vtd = {
> Why is vtd_vmstate renamed to vmstate_vtd? is it necessary?



Yuke Peng, do you plan to answer this?

> >       .name = "iommu-intel",
> >       .version_id = 1,
> >       .minimum_version_id = 1,
> > @@ -3306,6 +3344,10 @@ static const VMStateDescription vtd_vmstate = {
> >           VMSTATE_BOOL(intr_enabled, IntelIOMMUState),
> >           VMSTATE_BOOL(intr_eime, IntelIOMMUState),
> >           VMSTATE_END_OF_LIST()
> > +    },
> > +    .subsections = (const VMStateDescription * []) {
> > +        &vmstate_vtd_cfi,
> > +        NULL
> >       }
> >   };
> >
> > @@ -3525,6 +3567,12 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState *iommu,
> >
> >       /* This is compatible mode. */
> >       if (addr.addr.int_mode != VTD_IR_INT_FORMAT_REMAP) {
> > +        if (iommu->intr_eime || !iommu->cfi_enabled) {
> > +            if (do_fault) {
> > +                vtd_report_ir_fault(iommu, sid, VTD_FR_IR_REQ_COMPAT, 0);
> > +            }
> > +            return -EINVAL;
> > +        }
> >           memcpy(translated, origin, sizeof(*origin));
> >           goto out;
> >       }
> > @@ -3950,6 +3998,7 @@ static void vtd_init(IntelIOMMUState *s)
> >       s->root_scalable = false;
> >       s->dmar_enabled = false;
> >       s->intr_enabled = false;
> > +    s->cfi_enabled = false;
> >       s->iq_head = 0;
> >       s->iq_tail = 0;
> >       s->iq = 0;
> > @@ -4243,7 +4292,7 @@ static void vtd_class_init(ObjectClass *klass, void *data)
> >       X86IOMMUClass *x86_class = X86_IOMMU_DEVICE_CLASS(klass);
> >
> >       dc->reset = vtd_reset;
> > -    dc->vmsd = &vtd_vmstate;
> > +    dc->vmsd = &vmstate_vtd;
> >       device_class_set_props(dc, vtd_properties);
> >       dc->hotpluggable = false;
> >       x86_class->realize = vtd_realize;
> > diff --git a/hw/i386/trace-events b/hw/i386/trace-events
> > index 53c02d7ac8..ffd87db65f 100644
> > --- a/hw/i386/trace-events
> > +++ b/hw/i386/trace-events
> > @@ -57,6 +57,7 @@ vtd_dmar_translate(uint8_t bus, uint8_t slot, uint8_t func, uint64_t iova, uint6
> >   vtd_dmar_enable(bool en) "enable %d"
> >   vtd_dmar_fault(uint16_t sid, int fault, uint64_t addr, bool is_write) "sid 0x%"PRIx16" fault %d addr 0x%"PRIx64" write %d"
> >   vtd_ir_enable(bool en) "enable %d"
> > +vtd_cfi_enable(bool en) "enable %d"
> >   vtd_ir_irte_get(int index, uint64_t lo, uint64_t hi) "index %d low 0x%"PRIx64" high 0x%"PRIx64
> >   vtd_ir_remap(int index, int tri, int vec, int deliver, uint32_t dest, int dest_mode) "index %d trigger %d vector %d deliver %d dest 0x%"PRIx32" mode %d"
> >   vtd_ir_remap_type(const char *type) "%s"
> > diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> > index 7fa0a695c8..38e20d0f2c 100644
> > --- a/include/hw/i386/intel_iommu.h
> > +++ b/include/hw/i386/intel_iommu.h
> > @@ -294,6 +294,7 @@ struct IntelIOMMUState {
> >
> >       /* interrupt remapping */
> >       bool intr_enabled;              /* Whether guest enabled IR */
> > +    bool cfi_enabled;               /* Whether CFI is enabled */
> >       dma_addr_t intr_root;           /* Interrupt remapping table pointer */
> >       uint32_t intr_size;             /* Number of IR table entries */
> >       bool intr_eime;                 /* Extended interrupt mode enabled */
> > --
> > 2.34.1
> >
> >