| Message ID | 20160929112329.2408-6-rkrcmar@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Sep 29, 2016 at 01:23:27PM +0200, Radim Krčmář wrote: > The default (auto) emulates the current behavior. > > Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> > --- > hw/i386/intel_iommu.c | 20 +++++++++++++++++++- > include/hw/i386/intel_iommu.h | 1 + > 2 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index eb488c14625d..47141cea64f4 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -2011,6 +2011,8 @@ static const MemoryRegionOps vtd_mem_ops = { > > static Property vtd_properties[] = { > DEFINE_PROP_UINT32("version", IntelIOMMUState, version, 0), > + DEFINE_PROP_ON_OFF_AUTO("eim", IntelIOMMUState, intr_eim, > + ON_OFF_AUTO_AUTO), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -2367,7 +2369,11 @@ static void vtd_init(IntelIOMMUState *s) > s->ecap = VTD_ECAP_QI | VTD_ECAP_IRO; > > if (x86_iommu->intr_supported) { > - s->ecap |= VTD_ECAP_IR | VTD_ECAP_EIM | VTD_ECAP_MHMV; > + s->ecap |= VTD_ECAP_IR | VTD_ECAP_MHMV; > + if (s->intr_eim == ON_OFF_AUTO_ON) { > + s->ecap |= VTD_ECAP_EIM; > + } > + assert(s->intr_eim != ON_OFF_AUTO_AUTO); > } > > vtd_reset_context_cache(s); > @@ -2466,6 +2472,18 @@ static void vtd_realize(DeviceState *dev, Error **errp) > exit(1); > } > > + if (s->intr_eim == ON_OFF_AUTO_ON && !x86_iommu->intr_supported) { > + error_report("intel-iommu,eim=on cannot be selected without " > + "intremap=on."); > + exit(1); > + } > + if (s->intr_eim == ON_OFF_AUTO_AUTO && !x86_iommu->intr_supported) { > + s->intr_eim = ON_OFF_AUTO_OFF; > + } > + if (s->intr_eim == ON_OFF_AUTO_AUTO) { > + s->intr_eim = ON_OFF_AUTO_ON; > + } A single if() instead of above two might be nicer: if (s->intr_eim == ON_OFF_AUTO_AUTO) { e->intr_eim = x86_iommu->intr_supported ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; } Otherwise good to me. Thanks, -- peterx
2016-09-30 13:13+0800, Peter Xu: > On Thu, Sep 29, 2016 at 01:23:27PM +0200, Radim Krčmář wrote: >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >> @@ -2466,6 +2472,18 @@ static void vtd_realize(DeviceState *dev, Error **errp) >> exit(1); >> } >> >> + if (s->intr_eim == ON_OFF_AUTO_ON && !x86_iommu->intr_supported) { >> + error_report("intel-iommu,eim=on cannot be selected without " >> + "intremap=on."); >> + exit(1); >> + } >> + if (s->intr_eim == ON_OFF_AUTO_AUTO && !x86_iommu->intr_supported) { >> + s->intr_eim = ON_OFF_AUTO_OFF; >> + } >> + if (s->intr_eim == ON_OFF_AUTO_AUTO) { >> + s->intr_eim = ON_OFF_AUTO_ON; >> + } > > A single if() instead of above two might be nicer: > > if (s->intr_eim == ON_OFF_AUTO_AUTO) { > e->intr_eim = x86_iommu->intr_supported ? > ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; > } Hm, it'll end up looking like if (s->intr_eim == ON_OFF_AUTO_AUTO) { e->intr_eim = (x86_iommu->intr_supported && kvm_irqchip_in_kernel()) || pcmc->buggy_intel_iommu_eim ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; } It's harder to read, but less LOC ... I'll use it, thanks.
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index eb488c14625d..47141cea64f4 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -2011,6 +2011,8 @@ static const MemoryRegionOps vtd_mem_ops = { static Property vtd_properties[] = { DEFINE_PROP_UINT32("version", IntelIOMMUState, version, 0), + DEFINE_PROP_ON_OFF_AUTO("eim", IntelIOMMUState, intr_eim, + ON_OFF_AUTO_AUTO), DEFINE_PROP_END_OF_LIST(), }; @@ -2367,7 +2369,11 @@ static void vtd_init(IntelIOMMUState *s) s->ecap = VTD_ECAP_QI | VTD_ECAP_IRO; if (x86_iommu->intr_supported) { - s->ecap |= VTD_ECAP_IR | VTD_ECAP_EIM | VTD_ECAP_MHMV; + s->ecap |= VTD_ECAP_IR | VTD_ECAP_MHMV; + if (s->intr_eim == ON_OFF_AUTO_ON) { + s->ecap |= VTD_ECAP_EIM; + } + assert(s->intr_eim != ON_OFF_AUTO_AUTO); } vtd_reset_context_cache(s); @@ -2466,6 +2472,18 @@ static void vtd_realize(DeviceState *dev, Error **errp) exit(1); } + if (s->intr_eim == ON_OFF_AUTO_ON && !x86_iommu->intr_supported) { + error_report("intel-iommu,eim=on cannot be selected without " + "intremap=on."); + exit(1); + } + if (s->intr_eim == ON_OFF_AUTO_AUTO && !x86_iommu->intr_supported) { + s->intr_eim = ON_OFF_AUTO_OFF; + } + if (s->intr_eim == ON_OFF_AUTO_AUTO) { + s->intr_eim = ON_OFF_AUTO_ON; + } + memset(s->vtd_as_by_bus_num, 0, sizeof(s->vtd_as_by_bus_num)); memory_region_init_io(&s->csrmem, OBJECT(s), &vtd_mem_ops, s, "intel_iommu", DMAR_REG_SIZE); diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h index a42dbd745a70..b5ac60927b1f 100644 --- a/include/hw/i386/intel_iommu.h +++ b/include/hw/i386/intel_iommu.h @@ -289,6 +289,7 @@ struct IntelIOMMUState { 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 */ + OnOffAuto intr_eim; /* Toggle for EIM cabability */ }; /* Find the VTD Address space associated with the given bus pointer,
The default (auto) emulates the current behavior. Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> --- hw/i386/intel_iommu.c | 20 +++++++++++++++++++- include/hw/i386/intel_iommu.h | 1 + 2 files changed, 20 insertions(+), 1 deletion(-)