qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] intel_iommu: Support IR-only mode without DMA translation
@ 2020-10-15 18:34 David Woodhouse
  2020-10-15 18:34 ` [PATCH 2/2] intel_iommu: Only allow interrupt remapping to be enabled if it's supported David Woodhouse
  2021-01-22 15:14 ` [PATCH 1/2] intel_iommu: Support IR-only mode without DMA translation David Woodhouse
  0 siblings, 2 replies; 6+ messages in thread
From: David Woodhouse @ 2020-10-15 18:34 UTC (permalink / raw)
  To: qemu-devel

From: David Woodhouse <dwmw@amazon.co.uk>

By setting none of the SAGAW bits we can indicate to a guest that DMA
translation isn't supported. Tested by booting Windows 10, as well as
Linux guests with the fix at https://git.kernel.org/torvalds/c/c40aaaac10

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 hw/i386/intel_iommu.c         | 14 ++++++++++----
 include/hw/i386/intel_iommu.h |  1 +
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 749eb6ad63..f49b4af931 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2189,7 +2189,7 @@ static void vtd_handle_gcmd_write(IntelIOMMUState *s)
     uint32_t changed = status ^ val;
 
     trace_vtd_reg_write_gcmd(status, val);
-    if (changed & VTD_GCMD_TE) {
+    if ((changed & VTD_GCMD_TE) && s->dma_translation) {
         /* Translation enable/disable */
         vtd_handle_gcmd_te(s, val & VTD_GCMD_TE);
     }
@@ -3086,6 +3086,7 @@ static Property vtd_properties[] = {
     DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode, FALSE),
     DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, scalable_mode, FALSE),
     DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
+    DEFINE_PROP_BOOL("dma-translation", IntelIOMMUState, dma_translation, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -3607,12 +3608,17 @@ static void vtd_init(IntelIOMMUState *s)
     s->next_frcd_reg = 0;
     s->cap = VTD_CAP_FRO | VTD_CAP_NFR | VTD_CAP_ND |
              VTD_CAP_MAMV | VTD_CAP_PSI | VTD_CAP_SLLPS |
-             VTD_CAP_SAGAW_39bit | VTD_CAP_MGAW(s->aw_bits);
+             VTD_CAP_MGAW(s->aw_bits);
     if (s->dma_drain) {
         s->cap |= VTD_CAP_DRAIN;
     }
-    if (s->aw_bits == VTD_HOST_AW_48BIT) {
-        s->cap |= VTD_CAP_SAGAW_48bit;
+    if (s->dma_translation) {
+            if (s->aw_bits >= VTD_HOST_AW_39BIT) {
+                    s->cap |= VTD_CAP_SAGAW_39bit;
+            }
+            if (s->aw_bits >= VTD_HOST_AW_48BIT) {
+                    s->cap |= VTD_CAP_SAGAW_48bit;
+            }
     }
     s->ecap = VTD_ECAP_QI | VTD_ECAP_IRO;
 
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index 41783ee46d..42d6a6a636 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -266,6 +266,7 @@ struct IntelIOMMUState {
     bool buggy_eim;                 /* Force buggy EIM unless eim=off */
     uint8_t aw_bits;                /* Host/IOVA address width (in bits) */
     bool dma_drain;                 /* Whether DMA r/w draining enabled */
+    bool dma_translation;           /* Whether DMA translation supported */
 
     /*
      * Protects IOMMU states in general.  Currently it protects the
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] intel_iommu: Only allow interrupt remapping to be enabled if it's supported
  2020-10-15 18:34 [PATCH 1/2] intel_iommu: Support IR-only mode without DMA translation David Woodhouse
@ 2020-10-15 18:34 ` David Woodhouse
  2021-01-22 15:14 ` [PATCH 1/2] intel_iommu: Support IR-only mode without DMA translation David Woodhouse
  1 sibling, 0 replies; 6+ messages in thread
From: David Woodhouse @ 2020-10-15 18:34 UTC (permalink / raw)
  To: qemu-devel

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(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index f49b4af931..7e3570e875 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2184,6 +2184,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;
@@ -2205,7 +2206,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.26.2



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] intel_iommu: Support IR-only mode without DMA translation
  2020-10-15 18:34 [PATCH 1/2] intel_iommu: Support IR-only mode without DMA translation David Woodhouse
  2020-10-15 18:34 ` [PATCH 2/2] intel_iommu: Only allow interrupt remapping to be enabled if it's supported David Woodhouse
@ 2021-01-22 15:14 ` David Woodhouse
  1 sibling, 0 replies; 6+ messages in thread
From: David Woodhouse @ 2021-01-22 15:14 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 3074 bytes --]

On Thu, 2020-10-15 at 19:34 +0100, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> By setting none of the SAGAW bits we can indicate to a guest that DMA
> translation isn't supported. Tested by booting Windows 10, as well as
> Linux guests with the fix at https://git.kernel.org/torvalds/c/c40aaaac10
> 
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>

Shall I resend these? They still seem to apply cleanly.

> ---
>  hw/i386/intel_iommu.c         | 14 ++++++++++----
>  include/hw/i386/intel_iommu.h |  1 +
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 749eb6ad63..f49b4af931 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2189,7 +2189,7 @@ static void vtd_handle_gcmd_write(IntelIOMMUState *s)
>      uint32_t changed = status ^ val;
>  
>      trace_vtd_reg_write_gcmd(status, val);
> -    if (changed & VTD_GCMD_TE) {
> +    if ((changed & VTD_GCMD_TE) && s->dma_translation) {
>          /* Translation enable/disable */
>          vtd_handle_gcmd_te(s, val & VTD_GCMD_TE);
>      }
> @@ -3086,6 +3086,7 @@ static Property vtd_properties[] = {
>      DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode, FALSE),
>      DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, scalable_mode, FALSE),
>      DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
> +    DEFINE_PROP_BOOL("dma-translation", IntelIOMMUState, dma_translation, true),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -3607,12 +3608,17 @@ static void vtd_init(IntelIOMMUState *s)
>      s->next_frcd_reg = 0;
>      s->cap = VTD_CAP_FRO | VTD_CAP_NFR | VTD_CAP_ND |
>               VTD_CAP_MAMV | VTD_CAP_PSI | VTD_CAP_SLLPS |
> -             VTD_CAP_SAGAW_39bit | VTD_CAP_MGAW(s->aw_bits);
> +             VTD_CAP_MGAW(s->aw_bits);
>      if (s->dma_drain) {
>          s->cap |= VTD_CAP_DRAIN;
>      }
> -    if (s->aw_bits == VTD_HOST_AW_48BIT) {
> -        s->cap |= VTD_CAP_SAGAW_48bit;
> +    if (s->dma_translation) {
> +            if (s->aw_bits >= VTD_HOST_AW_39BIT) {
> +                    s->cap |= VTD_CAP_SAGAW_39bit;
> +            }
> +            if (s->aw_bits >= VTD_HOST_AW_48BIT) {
> +                    s->cap |= VTD_CAP_SAGAW_48bit;
> +            }
>      }
>      s->ecap = VTD_ECAP_QI | VTD_ECAP_IRO;
>  
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index 41783ee46d..42d6a6a636 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -266,6 +266,7 @@ struct IntelIOMMUState {
>      bool buggy_eim;                 /* Force buggy EIM unless eim=off */
>      uint8_t aw_bits;                /* Host/IOVA address width (in bits) */
>      bool dma_drain;                 /* Whether DMA r/w draining enabled */
> +    bool dma_translation;           /* Whether DMA translation supported */
>  
>      /*
>       * Protects IOMMU states in general.  Currently it protects the


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] intel_iommu: Only allow interrupt remapping to be enabled if it's supported
  2021-12-01 20:51 David Woodhouse
@ 2021-12-01 20:51 ` David Woodhouse
  2021-12-02  3:53   ` Jason Wang
  0 siblings, 1 reply; 6+ messages in thread
From: David Woodhouse @ 2021-12-01 20:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eduardo Habkost, Michael S. Tsirkin, Jason Wang,
	Richard Henderson, Peter Xu, Paolo Bonzini

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(-)

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



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] intel_iommu: Only allow interrupt remapping to be enabled if it's supported
  2021-12-01 20:51 ` [PATCH 2/2] intel_iommu: Only allow interrupt remapping to be enabled if it's supported David Woodhouse
@ 2021-12-02  3:53   ` Jason Wang
  2021-12-03  7:42     ` Peter Xu
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Wang @ 2021-12-02  3:53 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Eduardo Habkost, Michael S. Tsirkin, Richard Henderson,
	qemu-devel, Peter Xu, Paolo Bonzini

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
>
>



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] intel_iommu: Only allow interrupt remapping to be enabled if it's supported
  2021-12-02  3:53   ` Jason Wang
@ 2021-12-03  7:42     ` Peter Xu
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Xu @ 2021-12-03  7:42 UTC (permalink / raw)
  To: Jason Wang
  Cc: Eduardo Habkost, Michael S. Tsirkin, Richard Henderson,
	qemu-devel, Paolo Bonzini, David Woodhouse

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>

-- 
Peter Xu



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-12-03  7:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-15 18:34 [PATCH 1/2] intel_iommu: Support IR-only mode without DMA translation David Woodhouse
2020-10-15 18:34 ` [PATCH 2/2] intel_iommu: Only allow interrupt remapping to be enabled if it's supported David Woodhouse
2021-01-22 15:14 ` [PATCH 1/2] intel_iommu: Support IR-only mode without DMA translation David Woodhouse
  -- strict thread matches above, loose matches on Subject: below --
2021-12-01 20:51 David Woodhouse
2021-12-01 20:51 ` [PATCH 2/2] intel_iommu: Only allow interrupt remapping to be enabled if it's supported David Woodhouse
2021-12-02  3:53   ` Jason Wang
2021-12-03  7:42     ` Peter Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).