diff mbox series

[2/2] intel_iommu: Only allow interrupt remapping to be enabled if it's supported

Message ID 20211201205113.57299-2-dwmw2@infradead.org
State New
Headers show
Series [1/2] intel_iommu: Support IR-only mode without DMA translation | expand

Commit Message

David Woodhouse Dec. 1, 2021, 8:51 p.m. UTC 
From: David Woodhouse <dwmw@amazon.co.uk>

We should probably check if we were meant to be exposing IR, before
letting the guest turn the IRE bit on.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 hw/i386/intel_iommu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Jason Wang Dec. 2, 2021, 3:53 a.m. UTC | #1 
On Thu, Dec 2, 2021 at 4:58 AM David Woodhouse <dwmw2@infradead.org> wrote:
>
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> We should probably check if we were meant to be exposing IR, before
> letting the guest turn the IRE bit on.

This looks correct, but it's a change of guest noticeable behaviour.
It's probably fine since we don't expect a guest that enable IR
without checking ecap.

So

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

>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  hw/i386/intel_iommu.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index ffc852d110..297e1972f8 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2197,6 +2197,7 @@ static void vtd_handle_gcmd_ire(IntelIOMMUState *s, bool en)
>  /* Handle write to Global Command Register */
>  static void vtd_handle_gcmd_write(IntelIOMMUState *s)
>  {
> +    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
>      uint32_t status = vtd_get_long_raw(s, DMAR_GSTS_REG);
>      uint32_t val = vtd_get_long_raw(s, DMAR_GCMD_REG);
>      uint32_t changed = status ^ val;
> @@ -2218,7 +2219,8 @@ static void vtd_handle_gcmd_write(IntelIOMMUState *s)
>          /* Set/update the interrupt remapping root-table pointer */
>          vtd_handle_gcmd_sirtp(s);
>      }
> -    if (changed & VTD_GCMD_IRE) {
> +    if ((changed & VTD_GCMD_IRE) &&
> +        x86_iommu_ir_supported(x86_iommu)) {
>          /* Interrupt remap enable/disable */
>          vtd_handle_gcmd_ire(s, val & VTD_GCMD_IRE);
>      }
> --
> 2.31.1
>
>
Peter Xu Dec. 3, 2021, 7:42 a.m. UTC | #2 
On Thu, Dec 02, 2021 at 11:53:25AM +0800, Jason Wang wrote:
> On Thu, Dec 2, 2021 at 4:58 AM David Woodhouse <dwmw2@infradead.org> wrote:
> >
> > From: David Woodhouse <dwmw@amazon.co.uk>
> >
> > We should probably check if we were meant to be exposing IR, before
> > letting the guest turn the IRE bit on.
> 
> This looks correct, but it's a change of guest noticeable behaviour.
> It's probably fine since we don't expect a guest that enable IR
> without checking ecap.

Agreed.

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

Reviewed-by: Peter Xu <peterx@redhat.com>
diff mbox series

Patch

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index ffc852d110..297e1972f8 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2197,6 +2197,7 @@  static void vtd_handle_gcmd_ire(IntelIOMMUState *s, bool en)
 /* Handle write to Global Command Register */
 static void vtd_handle_gcmd_write(IntelIOMMUState *s)
 {
+    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
     uint32_t status = vtd_get_long_raw(s, DMAR_GSTS_REG);
     uint32_t val = vtd_get_long_raw(s, DMAR_GCMD_REG);
     uint32_t changed = status ^ val;
@@ -2218,7 +2219,8 @@  static void vtd_handle_gcmd_write(IntelIOMMUState *s)
         /* Set/update the interrupt remapping root-table pointer */
         vtd_handle_gcmd_sirtp(s);
     }
-    if (changed & VTD_GCMD_IRE) {
+    if ((changed & VTD_GCMD_IRE) &&
+        x86_iommu_ir_supported(x86_iommu)) {
         /* Interrupt remap enable/disable */
         vtd_handle_gcmd_ire(s, val & VTD_GCMD_IRE);
     }