diff mbox series

[5/5] intel_iommu: turn on IR by default

Message ID 20181217073114.11984-6-peterx@redhat.com
State New
Headers show
Series intel_iommu: fix windows svvp verification, and trivial cleanups | expand

Commit Message

Peter Xu Dec. 17, 2018, 7:31 a.m. UTC
IR has been there for a long time and long time no bug reported.
Let's turn it on by default to match general hardwares.  Providing
compatibility bit for QEMU<=3.1.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/i386/x86-iommu.c  | 2 +-
 include/hw/i386/pc.h | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Peter Xu Dec. 18, 2018, 3:07 a.m. UTC | #1
On Mon, Dec 17, 2018 at 03:31:14PM +0800, Peter Xu wrote:
> IR has been there for a long time and long time no bug reported.
> Let's turn it on by default to match general hardwares.  Providing
> compatibility bit for QEMU<=3.1.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

I forgot that IR will depend on split kernel irqchip and by default
that's still "on" so "-M q35 -device intel-iommu" may not be able to
boot correctly with all the default values and instead QEMU will ask
user to turn on split irqchip.

Paolo/Michael, do you think it would make any sense to turn the
default kernel-irqchip machine parameter to split starting from qemu
4.0?  Since AFAIU it should have little degradation to performance but
at the same time it reduces kvm attack serface, which seems good.

Thanks,

> ---
>  hw/i386/x86-iommu.c  | 2 +-
>  include/hw/i386/pc.h | 4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
> index abc3c03158..0150ceda14 100644
> --- a/hw/i386/x86-iommu.c
> +++ b/hw/i386/x86-iommu.c
> @@ -135,7 +135,7 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
>  }
>  
>  static Property x86_iommu_properties[] = {
> -    DEFINE_PROP_BOOL("intremap", X86IOMMUState, intr_supported, false),
> +    DEFINE_PROP_BOOL("intremap", X86IOMMUState, intr_supported, true),
>      DEFINE_PROP_BOOL("device-iotlb", X86IOMMUState, dt_supported, false),
>      DEFINE_PROP_BOOL("pt", X86IOMMUState, pt_supported, true),
>      DEFINE_PROP_END_OF_LIST(),
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index c7c0c944e8..ed958b9af1 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -300,6 +300,10 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>          .driver   = "intel-iommu",\
>          .property = "dma-drain",\
>          .value    = "off",\
> +    },{\
> +        .driver   = "x86-iommu",\
> +        .property = "intremap",\
> +        .value    = "off",\
>      },
>  
>  #define PC_COMPAT_3_0 \
> -- 
> 2.17.1
> 

Regards,
Paolo Bonzini Dec. 18, 2018, 12:30 p.m. UTC | #2
On 18/12/18 04:07, Peter Xu wrote:
> On Mon, Dec 17, 2018 at 03:31:14PM +0800, Peter Xu wrote:
>> IR has been there for a long time and long time no bug reported.
>> Let's turn it on by default to match general hardwares.  Providing
>> compatibility bit for QEMU<=3.1.
>>
>> Signed-off-by: Peter Xu <peterx@redhat.com>
> I forgot that IR will depend on split kernel irqchip and by default
> that's still "on" so "-M q35 -device intel-iommu" may not be able to
> boot correctly with all the default values and instead QEMU will ask
> user to turn on split irqchip.
> 
> Paolo/Michael, do you think it would make any sense to turn the
> default kernel-irqchip machine parameter to split starting from qemu
> 4.0?  Since AFAIU it should have little degradation to performance but
> at the same time it reduces kvm attack serface, which seems good.

The main problem with that would be the minimal required kernel version,
which is 4.4 for split irqchip.  But, we're already planning to make it
4.5 so it's not an issue.  Go for it. :)

Paolo
Peter Xu Dec. 19, 2018, 5:21 a.m. UTC | #3
On Tue, Dec 18, 2018 at 01:30:23PM +0100, Paolo Bonzini wrote:
> On 18/12/18 04:07, Peter Xu wrote:
> > On Mon, Dec 17, 2018 at 03:31:14PM +0800, Peter Xu wrote:
> >> IR has been there for a long time and long time no bug reported.
> >> Let's turn it on by default to match general hardwares.  Providing
> >> compatibility bit for QEMU<=3.1.
> >>
> >> Signed-off-by: Peter Xu <peterx@redhat.com>
> > I forgot that IR will depend on split kernel irqchip and by default
> > that's still "on" so "-M q35 -device intel-iommu" may not be able to
> > boot correctly with all the default values and instead QEMU will ask
> > user to turn on split irqchip.
> > 
> > Paolo/Michael, do you think it would make any sense to turn the
> > default kernel-irqchip machine parameter to split starting from qemu
> > 4.0?  Since AFAIU it should have little degradation to performance but
> > at the same time it reduces kvm attack serface, which seems good.
> 
> The main problem with that would be the minimal required kernel version,
> which is 4.4 for split irqchip.  But, we're already planning to make it
> 4.5 so it's not an issue.  Go for it. :)

Thanks Paolo. :)

I'll try to prepare a patch for it.

Regards,
diff mbox series

Patch

diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index abc3c03158..0150ceda14 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -135,7 +135,7 @@  static void x86_iommu_realize(DeviceState *dev, Error **errp)
 }
 
 static Property x86_iommu_properties[] = {
-    DEFINE_PROP_BOOL("intremap", X86IOMMUState, intr_supported, false),
+    DEFINE_PROP_BOOL("intremap", X86IOMMUState, intr_supported, true),
     DEFINE_PROP_BOOL("device-iotlb", X86IOMMUState, dt_supported, false),
     DEFINE_PROP_BOOL("pt", X86IOMMUState, pt_supported, true),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index c7c0c944e8..ed958b9af1 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -300,6 +300,10 @@  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
         .driver   = "intel-iommu",\
         .property = "dma-drain",\
         .value    = "off",\
+    },{\
+        .driver   = "x86-iommu",\
+        .property = "intremap",\
+        .value    = "off",\
     },
 
 #define PC_COMPAT_3_0 \